bpo-34537: Fix test_gdb:test_strings with LC_ALL=C by elprans · Pull Request #9483 · python/cpython (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation9 Commits3 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
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.
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 |
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.') |
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() |
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') |
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.
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.
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.
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
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
miss-islington added a commit that referenced this pull request
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
Labels
Tests in the Lib/test dir