Skip to content

bpo-34537: Fix test_gdb:test_strings with LC_ALL=C #9483

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 22, 2018

Conversation

elprans
Copy link
Contributor

@elprans elprans commented Sep 21, 2018

We cannot simply call locale.getpreferredencoding() here,
as GDB might have been linked against a different version
of Python with a different encoding and coercion policy
with respect to PEP 538 and PEP 540.

Thanks to Victor Stinner for a hint on how to fix this.

https://bugs.python.org/issue34537

We cannot simply call locale.getpreferredencoding() here,
as GDB might have been linked against a different version
of Python with a different encoding and coercion policy
with respect to PEP 538 and PEP 540.

Thanks to Victor Stinner for a hint on how to fix this.
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I have a few minor requests.

@@ -0,0 +1,2 @@
Fix test_gdb:test_strings when LC_ALL=C and GDB was compiled with Python 3.6
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest "test_gdb.test_strings()" or "test_strings() of test_gdb". You might use `` around function and file names.

if err:
raise RuntimeError(
'unable to determine the preferred encoding '
'of embedded Python in GDB.')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind to include err in the error message? You can use an f-string.

You might also raise the error if encoding is an empty string (if err or not encofing: ...)

'unable to determine the preferred encoding '
'of embedded Python in GDB.')

encoding = out.strip()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.rstrip() should be enough ;-)

if not encoding:
raise RuntimeError(
f'unable to determine the preferred encoding '
f'of embedded Python in GDB: the command returned no output')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I don't think that we need two code path, just reuse the first if.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, the first error message is fine if encoding is an empty string.

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for update!

I tested manually the PR on Linux using LC_ALL=C: it works as expected.

@miss-islington
Copy link
Contributor

Thanks @elprans for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 22, 2018
We cannot simply call locale.getpreferredencoding() here,
as GDB might have been linked against a different version
of Python with a different encoding and coercion policy
with respect to PEP 538 and PEP 540.

Thanks to Victor Stinner for a hint on how to fix this.
(cherry picked from commit 7279b51)

Co-authored-by: Elvis Pranskevichus <elvis@magic.io>
@bedevere-bot
Copy link

GH-9485 is a backport of this pull request to the 3.7 branch.

miss-islington added a commit that referenced this pull request Sep 22, 2018
We cannot simply call locale.getpreferredencoding() here,
as GDB might have been linked against a different version
of Python with a different encoding and coercion policy
with respect to PEP 538 and PEP 540.

Thanks to Victor Stinner for a hint on how to fix this.
(cherry picked from commit 7279b51)

Co-authored-by: Elvis Pranskevichus <elvis@magic.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Tests in the Lib/test dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants