Skip to content

Conversation

@barry-scott
Copy link
Contributor

@barry-scott barry-scott commented Apr 12, 2020

bpo-40260: Allow compile() to handle the module's source decoding
by passing in an fp that is opened in "rb".

This fixes problems seen on Windows where the source code is not
decoded as utf-8 but as the code page default for the user, cp1252
for example.

Add three new tests to test_modulefinder.py to excercise
the decoding of source code.

Always write the test source as bytes to avoid using
locale.getpreferredencoding() as the encoding.

test_coding_default_utf8 - designed to fail if decoded with cp1252
test_coding_explicit_utf8 - designed to fail if decoded with cp1252
test_coding_explicit_cp1252 - designed to fail if decoded with utf-8

https://bugs.python.org/issue40260

by passing in an fp that is opened in "rb".

This fixes problems seen on Windows where the source code is not
decoded as utf-8 but as the code page default for the user, cp1252
for example.

Add three new tests to test_modulefinder.py to excercise
the decoding of source code.

Always write the test source as bytes to avoid using
locale.getpreferredencoding() as the encoding.

test_coding_default_utf8 - designed to fail if decoded with cp1252
test_coding_explicit_utf8 - designed to fail if decoded with cp1252
test_coding_explicit_cp1252 - designed to fail if decoded with utf-8
@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

Recognized GitHub username

We couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames:

@barry-scott

This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@merwok
Copy link
Member

merwok commented Apr 12, 2020

To add a NEWS item, you can install and run blurb on your machine, or use a web app (the link is Details next to the bedevere/news check line).

Please refer to the devguide for more information about the process!

Remove the mode in file_info that is usused.
@barry-scott
Copy link
Contributor Author

Change made. Tests passed on macOS 3.8 and 3.9a5, Windows 3.8 and 3.9a5.

@zooba
Copy link
Member

zooba commented Apr 14, 2020

Thanks! I updated the NEWS entry to help people find this later on, so once the retest passes I'll merge.

@zooba zooba added needs backport to 3.8 type-bug An unexpected behavior, bug, or error labels Apr 14, 2020
@merwok
Copy link
Member

merwok commented Apr 14, 2020

I would suggest to edit the PR title (and thus the future commit message) to mention io.open_code, so that this can be found by a search later with all the other commits related to that function.

@zooba
Copy link
Member

zooba commented Apr 14, 2020

I was going to update the commit message when merging. Updating the issue title is probably more important for future searches - there's far more useful information on the tracker than here.

@zooba zooba merged commit d42e582 into python:master Apr 14, 2020
@miss-islington
Copy link
Contributor

Thanks @barry-scott for the PR, and @zooba for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 14, 2020
…ng comments (pythonGH-19488)

(cherry picked from commit d42e582)

Co-authored-by: Barry <barry@barrys-emacs.org>
@bedevere-bot
Copy link

GH-19522 is a backport of this pull request to the 3.8 branch.

miss-islington added a commit that referenced this pull request Apr 14, 2020
…ng comments (GH-19488)

(cherry picked from commit d42e582)

Co-authored-by: Barry <barry@barrys-emacs.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-bug An unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants