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

Don't use deprecated 'U' flag to read manifest #623

Merged
merged 1 commit into from Jul 1, 2016

Conversation

Projects
None yet
2 participants
@vstinner
Contributor

vstinner commented Jun 29, 2016

The universal newlines mode ('U' flag) is deprecated since Python
3.4. It only replaces "\r\n" with "\n", but it doesn't split lines at
"\r" (Mac newline). In practice, the flag was useless, the
sdist.read_manifest() method already uses line.strip() and so removes
newline characters.

Example of warning:

.../setuptools/command/sdist.py:182: DeprecationWarning: 'U' mode is deprecated
manifest = open(self.manifest, 'rbU')

Don't use deprecated 'U' flag to read manifest
The universal newlines mode ('U' flag) is deprecated since Python
3.4. It only replaces "\r\n" with "\n", but it doesn't split lines at
"\r" (Mac newline). In practice, the flag was useless, the
sdist.read_manifest() method already uses line.strip() and so removes
newline characters.
@jaraco

This comment has been minimized.

Show comment
Hide comment
@jaraco

jaraco Jun 29, 2016

Member

I vaguely recall experimenting with this before and finding we couldn't drop it for some reason. Can we be sure the U doesn't affect other aspects such as text encoding?

Member

jaraco commented Jun 29, 2016

I vaguely recall experimenting with this before and finding we couldn't drop it for some reason. Can we be sure the U doesn't affect other aspects such as text encoding?

@vstinner

This comment has been minimized.

Show comment
Hide comment
@vstinner

vstinner Jun 29, 2016

Contributor

New test on Python 2:

$ python2
Python 2.7.11 (default, Jun 13 2016, 12:33:53) 
[GCC 6.1.1 20160510 (Red Hat 6.1.1-2)] on linux2

>>> open("x", "wb").write("\n".join(["a", "b", ""]))
>>> open("x", "rb").readlines()
['a\n', 'b\n']
>>> open("x", "rbU").readlines()
['a\n', 'b\n']

>>> open("x", "wb").write("\r\n".join(["a", "b", ""]))
>>> open("x", "rb").readlines()
['a\r\n', 'b\r\n']
>>> open("x", "rbU").readlines()
['a\n', 'b\n']

>>> open("x", "wb").write("\r".join(["a", "b", ""]))
>>> open("x", "rb").readlines()
['a\rb\r']
>>> open("x", "rbU").readlines()
['a\n', 'b\n']

Oh wait: replacing "rbU" with "rb" doesn't work for Mac newline (\r) :-/ I made a mistake in my previous test.

Test on Python 3:

$ python3
Python 3.5.1 (default, Jun 13 2016, 16:14:50) 
[GCC 6.1.1 20160510 (Red Hat 6.1.1-2)] on linux

>>> open("x", "wb").write(b"\n".join([b"a", b"b", b""]))
>>> open("x", "rb").readlines()
[b'a\n', b'b\n']
>>> open("x", "rbU").readlines()
[b'a\n', b'b\n']

>>> open("x", "wb").write(b"\r\n".join([b"a", b"b", b""]))
>>> open("x", "rb").readlines()
[b'a\r\n', b'b\r\n']
>>> open("x", "rbU").readlines()
[b'a\r\n', b'b\r\n']

>>> open("x", "wb").write(b"\r".join([b"a", b"b", b""]))
>>> open("x", "rb").readlines()
[b'a\rb\r']
>>> open("x", "rbU").readlines()
[b'a\rb\r']

On Python 3, the "U" flag is simply ignored for binary mode and Mac newlines don't work as expected.

Contributor

vstinner commented Jun 29, 2016

New test on Python 2:

$ python2
Python 2.7.11 (default, Jun 13 2016, 12:33:53) 
[GCC 6.1.1 20160510 (Red Hat 6.1.1-2)] on linux2

>>> open("x", "wb").write("\n".join(["a", "b", ""]))
>>> open("x", "rb").readlines()
['a\n', 'b\n']
>>> open("x", "rbU").readlines()
['a\n', 'b\n']

>>> open("x", "wb").write("\r\n".join(["a", "b", ""]))
>>> open("x", "rb").readlines()
['a\r\n', 'b\r\n']
>>> open("x", "rbU").readlines()
['a\n', 'b\n']

>>> open("x", "wb").write("\r".join(["a", "b", ""]))
>>> open("x", "rb").readlines()
['a\rb\r']
>>> open("x", "rbU").readlines()
['a\n', 'b\n']

Oh wait: replacing "rbU" with "rb" doesn't work for Mac newline (\r) :-/ I made a mistake in my previous test.

Test on Python 3:

$ python3
Python 3.5.1 (default, Jun 13 2016, 16:14:50) 
[GCC 6.1.1 20160510 (Red Hat 6.1.1-2)] on linux

>>> open("x", "wb").write(b"\n".join([b"a", b"b", b""]))
>>> open("x", "rb").readlines()
[b'a\n', b'b\n']
>>> open("x", "rbU").readlines()
[b'a\n', b'b\n']

>>> open("x", "wb").write(b"\r\n".join([b"a", b"b", b""]))
>>> open("x", "rb").readlines()
[b'a\r\n', b'b\r\n']
>>> open("x", "rbU").readlines()
[b'a\r\n', b'b\r\n']

>>> open("x", "wb").write(b"\r".join([b"a", b"b", b""]))
>>> open("x", "rb").readlines()
[b'a\rb\r']
>>> open("x", "rbU").readlines()
[b'a\rb\r']

On Python 3, the "U" flag is simply ignored for binary mode and Mac newlines don't work as expected.

@vstinner

This comment has been minimized.

Show comment
Hide comment
@vstinner

vstinner Jun 29, 2016

Contributor

"All checks have passed"

If setuptools must support Mac newlines, at least one unit test must be added, no?

Contributor

vstinner commented Jun 29, 2016

"All checks have passed"

If setuptools must support Mac newlines, at least one unit test must be added, no?

@jaraco

This comment has been minimized.

Show comment
Hide comment
@jaraco

jaraco Jul 1, 2016

Member

I'm all but certain we don't need to support Mac newlines any longer. I agree about the tests, with the caveat that setuptools has never had suitable use-case coverage, so we have to be extra considerate. In this case, let's make the change and if someone complains, we'll add the necessary test.

Member

jaraco commented Jul 1, 2016

I'm all but certain we don't need to support Mac newlines any longer. I agree about the tests, with the caveat that setuptools has never had suitable use-case coverage, so we have to be extra considerate. In this case, let's make the change and if someone complains, we'll add the necessary test.

@jaraco jaraco merged commit 929b72a into pypa:master Jul 1, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment