classification
Title: packaging uses wrong line endings in RECORD files on Windows
Type: behavior Stage: resolved
Components: Distutils2 Versions: Python 3.3, 3rd party
process
Status: closed Resolution: out of date
Dependencies: 13198 Superseder:
Assigned To: eric.araujo Nosy List: alexis, eric.araujo, paul.moore, tarek, vinay.sajip
Priority: normal Keywords: patch

Created on 2011-10-13 22:47 by paul.moore, last changed 2014-03-12 09:45 by eric.araujo. This issue is now closed.

Files
File name Uploaded Description Edit
recordfix.patch paul.moore, 2011-10-16 14:23 Patch for this issue
Messages (17)
msg145488 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2011-10-13 22:47
On Windows, packaging seems to create RECORD files with an additional CR at the end of the line. (So the line end is CR CR LF). This does not seem to be consistent, but it is likely to be because a file is being opened in text mode rather than binary.

I am trying to develop a reproducible test case, but am having difficulty at the moment. I have opened this bug in any case as a place holder and in case someone else can reproduce the issue.
msg145533 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-10-14 15:27
packaging.util creates a RECORD file with OS-specific line endings and csv.writerow; packaging.command.install_distinfo uses LF everywhere and csv.writerow.  I’m not sure this is related to binary vs. text I/O; maybe it’s some other code like the RESOURCES system that’s giving lines with endings to install_distinfo, which then adds CRLF.  Do you have a RECORD file to upload for inspection?
msg145553 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2011-10-14 17:54
Unfortunately, no. I have been unable to get this in a reproducible form - but I have seen it a few times now. I will keep trying to reproduce.

The worst thing is that packaging fails to recognise the data in RECORD and won't uninstall the package. It would be helpful if packaging were a little more permissive in how it handles this sort of problem in RECORD.
msg145595 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2011-10-15 15:41
I'm trying to reproduce this too, but have failed so far. Which version(s) of Windows did the problems appear on?
msg145597 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2011-10-15 15:44
Windows 7, 32 bit.
msg145619 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2011-10-16 13:50
I found the problem - it's in packaging.util.write_record_file. The
file passed to csv.writer should be opened with newline=''.
msg145620 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2011-10-16 14:23
Here's a patch. It includes a test, but I don't expect the test will catch the issue except on Windows...
msg145688 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-10-17 13:15
> I found the problem - it's in packaging.util.write_record_file.
We have two functions that write RECORD files; I’ve opened a report to kill one.

> The file passed to csv.writer should be opened with newline=''.
How will we port this to 2.x?

> I don't expect the test will catch the issue except on Windows...
Do you mean that the test will fail or be a no-op on other OSes?  We can mark it as Windows-specific (@unittest.skipIf(sys.platform != 'win32', 'test only relevant on win32')) or just let it run if it’s harmless.  The important point is: does it fail before the fix, does it pass after?
msg145700 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2011-10-17 14:28
On 17 October 2011 14:15, Éric Araujo <report@bugs.python.org> wrote:
>> The file passed to csv.writer should be opened with newline=''.
> How will we port this to 2.x?

No idea :-( The 2.7 documentation says use the 'b' flag, but that
probably doesn't allow an encoding parameter (it doesn't on 3.x).

>> I don't expect the test will catch the issue except on Windows...
> Do you mean that the test will fail or be a no-op on other OSes?  We can mark it as Windows-specific (@unittest.skipIf(sys.platform != 'win32', 'test only relevant on win32')) or just let it run if it’s harmless.  The important point is: does it fail before the fix, does it pass after?

The test fails before the fix, passes after. It's a no-op on platforms
where text and binary files are the same, (i.e., non-Windows systems).
So it's harmless.
msg145723 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-10-17 16:12
>>> The file passed to csv.writer should be opened with newline=''.
>> How will we port this to 2.x?
> No idea :-( The 2.7 documentation says use the 'b' flag, but that
> probably doesn't allow an encoding parameter (it doesn't on 3.x).

Ah, I see that newline controls newline translation.  We use codecs.open in distutils2; I think it uses universal newlines mode.  We’ll figure it out, the important part is to have good tests.

> The test fails before the fix, passes after. It's a no-op on platforms
> where text and binary files are the same, (i.e., non-Windows systems).
> So it's harmless.

Great.  I’ll look at your patch and try to port it to distutils2.
msg152961 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2012-02-09 16:45
Any progress on this one?

I tried to reproduce using distutils2, but don't know how distutils2 works - there's no pysetup and python -m distutils2.run doesn't work the same as python -m packaging.run.

Can we at least get packaging working in time for 3.3, even if it leaves an issue with distutils2 to sort out?
msg153011 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-10 03:56
> I tried to reproduce using distutils2, but don't know how distutils2
> works - there's no pysetup and python -m distutils2.run doesn't work
> the same as python -m packaging.run.

Maybe you’re using the old PyPI release or Tarek’s outdated Bitbucket repository.  http://hg.python.org/distutils is current, contains pysetup and supports -m distutils2.run.

I’ll port your patch to d2 this week.
msg153028 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2012-02-10 06:58
> Maybe you’re using the old PyPI release or Tarek’s outdated Bitbucket repository.  http://hg.python.org/distutils is current, contains pysetup and supports -m distutils2.run.

I just did pip install distutils2 in a virtualenv. If that doesn't
work then yes, I got the wrong one :-(

Do I need to hg clone then do some magic pip incantation to get it
installed? I'm new to pip/viirtualenv. I'll have a look.
msg153105 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-11 05:48
> I just did pip install distutils2 in a virtualenv. If that doesn't work then yes,
> I got the wrong one :-(
You need to get a development repo to make patches, not a released version, just like for CPython.

> Do I need to hg clone then do some magic pip incantation to get it installed?
Why would you want to install it?  Clone it, hack on it in place, run tests in place.  :)

FTR, “bin/pip install -e .” is the command to “install” a project in develop (or editable) mode with pip in a virtualenv.
msg153118 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2012-02-11 09:57
>> Do I need to hg clone then do some magic pip incantation to get it installed?
> Why would you want to install it?  Clone it, hack on it in place, run tests in place.  :)
>
> FTR, “bin/pip install -e .” is the command to “install” a project in develop (or editable) mode with pip in a virtualenv.

Thanks, that's what I needed. I'm new to pip and know little more than
"pip install" :-)
msg161035 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-05-18 04:50
I should be able to run a VM to test and commit this soon.  Keep faith!
msg213232 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2014-03-12 09:45
This issue is now obsolete for distutils2.  Other projects that write RECORD files are hopefully more robust in their code and tests.
History
Date User Action Args
2014-03-12 09:45:46eric.araujosetstatus: open -> closed
resolution: out of date
messages: + msg213232

stage: resolved
2012-05-18 04:50:51eric.araujosetmessages: + msg161035
2012-02-11 09:57:36paul.mooresetmessages: + msg153118
2012-02-11 05:48:07eric.araujosetmessages: + msg153105
2012-02-10 06:58:30paul.mooresetmessages: + msg153028
2012-02-10 03:56:51eric.araujosetmessages: + msg153011
2012-02-09 16:45:18paul.mooresetmessages: + msg152961
2011-10-17 16:12:12eric.araujosetmessages: + msg145723
2011-10-17 14:28:59paul.mooresetmessages: + msg145700
2011-10-17 13:15:11eric.araujosetassignee: tarek -> eric.araujo
dependencies: + Remove duplicate definition of write_record_file
messages: + msg145688
versions: + 3rd party
2011-10-16 14:23:40paul.mooresetfiles: + recordfix.patch
keywords: + patch
messages: + msg145620
2011-10-16 13:50:30paul.mooresetmessages: + msg145619
2011-10-15 15:44:51paul.mooresetmessages: + msg145597
2011-10-15 15:41:50vinay.sajipsetnosy: + vinay.sajip
messages: + msg145595
2011-10-14 17:54:56paul.mooresetmessages: + msg145553
2011-10-14 15:27:31eric.araujosetmessages: + msg145533
2011-10-13 22:47:01paul.moorecreate