Skip to content
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

Read README.rst and HISTORY.rst in UTF-8 #14

Merged
merged 1 commit into from
Jun 8, 2015
Merged

Read README.rst and HISTORY.rst in UTF-8 #14

merged 1 commit into from
Jun 8, 2015

Conversation

vstinner
Copy link
Contributor

@vstinner vstinner commented Jun 8, 2015

Since README.rst now contains a snowman character, running setup.py on
Python 3 with a locale encoding different than UTF-8 doesn't work
anymore.

Specify the UTF-8 encoding when reading README.rst and HISTORY.rst in
setup.py to fix this issue (issue #13).

Since README.rst now contains a snowman character, running setup.py on
Python 3 with a locale encoding different than UTF-8 doesn't work
anymore.

Specify the UTF-8 encoding when reading README.rst and HISTORY.rst in
setup.py to fix this issue.
@vstinner
Copy link
Contributor Author

vstinner commented Jun 8, 2015

This change is not compatible with Python 2.5, but it looks like rfc3986 doesn't support older than Python 2.6, so it's fine.

@vstinner
Copy link
Contributor Author

vstinner commented Jun 8, 2015

@sigmavirus24
Copy link
Collaborator

I'm not opposed to this change but your description of the reasoning is wrong. This only happens with tox 2.0. If you look at the history of README.rst, that character has been there since the first release.

sigmavirus24 added a commit that referenced this pull request Jun 8, 2015
Read README.rst and HISTORY.rst in UTF-8
@sigmavirus24 sigmavirus24 merged commit c3a8c9b into python-hyper:master Jun 8, 2015
@vstinner
Copy link
Contributor Author

vstinner commented Jun 8, 2015

"I'm not opposed to this change but your description of the reasoning is wrong. This only happens with tox 2.0. If you look at the history of README.rst, that character has been there since the first release."

I don't know what changed in tox 2.0. Maybe tox 2.0 uses Python 3 instead of Python 2?

This issue is specific to Python 3. Anyway, it's now fixed. Thanks :-)

@vstinner vstinner deleted the setup_utf8 branch June 8, 2015 12:45
@sigmavirus24
Copy link
Collaborator

@Haypo please notice that the bug you linked to in Nova was fixed with https://review.openstack.org/186922. Specifically, note that that has to force LC_ALL in tox.ini for Nova. That's because v2.0 of tox has prevented environment variables from being passed through to the virtual environment it's running. And yes, this is only a problem on Python 3 because Python 3 pays extra attention to the locale settings of the environment. If they're not set (which they won't be under tox 2.0), then Python 3 assumes it's the C locale, which means that unicode is not valid unless you explicitly set it. That's why I'm not opposed to this change. Explicit is better than implicit. Regardless, this is not a bug here.

@vstinner
Copy link
Contributor Author

vstinner commented Jun 8, 2015

Regardless, this is not a bug here.

Try to run setup.py without my change in an environment configured with the C locale, or any locale with an encoding different than UTF-8, you will get the same error. setup.py must work with any locale including the POSIX locale ("C").

I wasn't aware that tox doesn't copy LC_* environment variables anymore. I'm not sure that it's a good idea :-(

@sigmavirus24
Copy link
Collaborator

Try to run setup.py without my change in an environment configured with the C locale, or any locale with an encoding different than UTF-8, you will get the same error.

True, but I can't think of a modern operating system that sets the locale to C by default. Even Fedora sets it correctly (if I recall correctly).

That aside, I didn't notice it on my phone, but why did you choose the io module over the codecs module?

@vstinner
Copy link
Contributor Author

vstinner commented Jun 8, 2015

That aside, I didn't notice it on my phone, but why did you choose the io module over the codecs module?

I don't like the codecs module :-) https://www.python.org/dev/peps/pep-0400/#motivation

@sigmavirus24
Copy link
Collaborator

Good to know. Perhaps someone should publicize the idea of deprecating that since I thought it was a BCP

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants