This issue tracker has been migrated to GitHub ,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2011年02月03日 00:05 by jdennis, last changed 2022年04月11日 14:57 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| sdist.patch | jdennis, 2011年02月03日 00:44 | Suggested patch | review | |
| sdist_new | jdennis, 2011年02月03日 00:45 | result of appying suggested patch | ||
| distutils_bug.tar.gz | jdennis, 2011年02月04日 01:51 | reproducer | ||
| manifest-respect.patch | jerub, 2011年06月04日 10:19 | python2.7 patch | ||
| manifest-respect-3.patch | jerub, 2011年06月04日 11:22 | python3.1+default patch | review | |
| manifest-respect-3 | jerub, 2011年06月24日 13:14 | python3.1+default patch | review | |
| manifest-respect-3 | jerub, 2011年06月24日 23:30 | Updated python3.1+default patch | review | |
| manifest-respect-3 | jerub, 2011年06月25日 13:54 | python3.1 patch with documentation changes | review | |
| Messages (23) | |||
|---|---|---|---|
| msg127777 - (view) | Author: John Dennis (jdennis) | Date: 2011年02月03日 00:05 | |
The behaviour of sdist has changed dramatically in Python 2.7. Some projects prefer to maintain their own manifest file instead of utilizing automatic manifest generation from a template. The defined behaviour of sdist is to check for the presence of both a template and a manifest, if the template is absent but a manifest exists sdist is supposed to read the existing manifest, it no longer does this. Instead it creates a default file list with the catastrophic result of omitting the bulk of a projects files. It appears this bug was introduced in r81255 and is discussed in #8688. Unfortunately #8688 contained a number of different issues and was closed addressing only a subset of the problems. Changeset r83996 was introduced to prevent sdist from overwriting a project maintained manifest by testing for a comment at the head of the manifest file. It's not clear to me this was necessary because the write_manifest() should never have been called if the template was absent but a manifest existed. Even after the application of changeset r83996 one of the fundamental problems in #8688 remained, the manifest is not read. The solution is to check for both the manifest and template (as was formerly done) and if the template is absent but the manifest exists then the manifest should be read. I have made modifications to get_file_list() to reintroduce the defined behaviour. With the introduction of r83996 it is now legal syntax to have comments in the manifest however read_manifest() was not enhanced to account for the possible presence of comments. I also modified read_manifest() to handle comments. These suggested fixes are attached as a patch against the current 2.7 maintenance branch. I've also attached a file with the two modified methods because sometimes it's difficult to comprehend a patch. |
|||
| msg127847 - (view) | Author: Éric Araujo (eric.araujo) * (Python committer) | Date: 2011年02月04日 01:04 | |
This behavior change is actually present in 2.7.1, 3.1.3 and 3.2. I’m concerned that you’ve found a bug with it (thanks for the report), and that we may have to change behavior again. First, could you open another report about comment handling in read_manifest? I overlooked that, and it should be fixed as quickly as possible, although there’s very little change this will make it into 3.2.0 (we’ll have 3.2.1 though). Second, can you give us a test script (shell or Python) or a set of instructions we can reproduce to observe the behavior you say is defective? Thanks in advance. |
|||
| msg127857 - (view) | Author: John Dennis (jdennis) | Date: 2011年02月04日 01:51 | |
$ tar xzf distutils_bug.tar.gz $ cd distutils_bug $ ./setup.py sdist $ ./setup.py sdist running sdist running check warning: sdist: manifest template 'MANIFEST.in' does not exist (using default file list) not writing to manually maintained manifest file 'MANIFEST' creating foobar-1.0 making hard links in foobar-1.0... hard linking README -> foobar-1.0 hard linking setup.py -> foobar-1.0 creating dist Creating tar archive removing 'foobar-1.0' (and everything under it) $ cat MANIFEST README setup.py my_special_file Note, the MANIFEST should have been read and the resulting tar file should have had my_special_file in it. sdist complains about a missing MANIFEST.in template which it shouldn't, it should use the existing MANIFEST, it also emits a warning about not overwriting a manually maintained MANIFEST which it shouldn't. It should just use the existing MANIFEST. |
|||
| msg127867 - (view) | Author: Éric Araujo (eric.araujo) * (Python committer) | Date: 2011年02月04日 03:51 | |
Thank you, the bug is clear now. To fix this regression, the first step is to turn your tarball and instructions into a unit test and then fix the logic in the code. If you want to do it, there are some process guidelines at http://wiki.python.org/moin/Distutils/FixingBugs |
|||
| msg127895 - (view) | Author: John Dennis (jdennis) | Date: 2011年02月04日 14:47 | |
No, I don't think I'm going to turn the tarball into a unit test, etc. I didn't break the code but I did report the problem, researched the history, provided a clear explanation, a reproducer and a patch to fix the problem. I think I've done my part as a community member. |
|||
| msg127937 - (view) | Author: Éric Araujo (eric.araujo) * (Python committer) | Date: 2011年02月04日 20:21 | |
I didn’t mean to imply you had to, I just asked if you wanted to. Reporting the bug is a valuable contribution indeed, I’ll take over for the rest. |
|||
| msg137632 - (view) | Author: Stephen Thorne (jerub) * | Date: 2011年06月04日 10:19 | |
I've taken the sdist.patch and wrote some tests for it. The resulting patch is attached as 'manifest-respect.patch'. |
|||
| msg137633 - (view) | Author: Stephen Thorne (jerub) * | Date: 2011年06月04日 11:22 | |
This patch is tested against the 3.1 and default branches, the previous patch attached was against the 2.7 branch. |
|||
| msg138891 - (view) | Author: Stephen Thorne (jerub) * | Date: 2011年06月24日 08:20 | |
I have 2 patches, with tests, that applies on python2.7 and the python3 series of branches, attached this ticket. I have also got a signed contributor agreement lodged with the PSF. Can I please have someone either apply my patches or tell me what I need to do in order to change them if they are being rejected. |
|||
| msg138901 - (view) | Author: Éric Araujo (eric.araujo) * (Python committer) | Date: 2011年06月24日 10:44 | |
I have outstanding comments and questions on the review page (follow the review link on the right of the 3.x patch). |
|||
| msg138903 - (view) | Author: Stephen Thorne (jerub) * | Date: 2011年06月24日 10:50 | |
Oh! I didn't see any notification that there was a review done. Thanks, I'll attend to that. |
|||
| msg138941 - (view) | Author: Stephen Thorne (jerub) * | Date: 2011年06月24日 13:14 | |
This patch is an updated patch that fixes the things noted in the review from eric.araujo. |
|||
| msg139002 - (view) | Author: Stephen Thorne (jerub) * | Date: 2011年06月24日 23:30 | |
Updated the patch to address the 'why not use .strip()' question. I used .rstrip('\r\n') on the basis that filenames may have leading or trailing spaces, and if you need that, you need to be able to specify that in a MANIFEST, but it is perfectly logical to disallow them, so here's a patch that doesn't support them.
It also reduces the line count by 2 because I'm composing the 'comment' and 'blank line' cases.
|
|||
| msg139013 - (view) | Author: Éric Araujo (eric.araujo) * (Python committer) | Date: 2011年06月25日 00:52 | |
Thanks for your work, the code can now be fixed; I’ll have time to commit in a few days. Can you check if the documentation still accurately describes the behavior? (search for MANIFEST in Doc/distutils/*) |
|||
| msg139065 - (view) | Author: Stephen Thorne (jerub) * | Date: 2011年06月25日 13:54 | |
Éric mentioned that i should check that this behaviour matches the documentation. I have gone and looked for all instances of MANIFEST in the documentation and found one place which was inconsistent. I've added the doc patch to the patch. Please review this new version. |
|||
| msg139114 - (view) | Author: Éric Araujo (eric.araujo) * (Python committer) | Date: 2011年06月25日 20:13 | |
Thanks for your work. I will make a final test and review and commit it in the coming days. I’ll adapt the versionchanged text, since I won’t commit this in 3.1 (in security mode now, not bugfix mode). |
|||
| msg141495 - (view) | Author: Roundup Robot (python-dev) (Python triager) | Date: 2011年08月01日 12:45 | |
New changeset 5d3e22d69d4f by Éric Araujo in branch '3.2': Fix regression with distutils MANIFEST handing (#11104, #8688). http://hg.python.org/cpython/rev/5d3e22d69d4f |
|||
| msg141499 - (view) | Author: Roundup Robot (python-dev) (Python triager) | Date: 2011年08月01日 12:49 | |
New changeset 21feea7f35e5 by Éric Araujo in branch '2.7': Fix regression with distutils MANIFEST handing (#11104, #8688). http://hg.python.org/cpython/rev/21feea7f35e5 |
|||
| msg141502 - (view) | Author: Éric Araujo (eric.araujo) * (Python committer) | Date: 2011年08月01日 12:57 | |
I did some more work on the patch and committed. Thanks again to both of you. I also thought again about this remark: > Changeset r83996 was introduced to prevent sdist from overwriting a > project maintained manifest by testing for a comment at the head of > the manifest file. It's not clear to me this was necessary because the > write_manifest() should never have been called if the template was > absent but a manifest existed. I also wonder if that was necessary; I did not fully understand the bug at the time. Unfortunately, now that we have a handful of releases out there with this change, we have to support the magic comment. For distutils2 however (a.k.a. "packaging" module in 3.3), I’ll open another bug report and if the same bugs apply (MANIFEST handling is a bit different there), I’ll try to fix the bug without using a magic comment. |
|||
| msg152534 - (view) | Author: John Dennis (jdennis) | Date: 2012年02月03日 16:21 | |
The changesets are not in the release27-maint branch. sdist still does not generate a correct archive for release, this is a very serious regression. |
|||
| msg152535 - (view) | Author: Éric Araujo (eric.araujo) * (Python committer) | Date: 2012年02月03日 16:25 | |
> The changesets are not in the release27-maint branch. Where did you look? This looks like a Subversion branch name, but now we’re using Mercurial. If you look a few messages up, you’ll see that a changeset was committed to the 2.7 branch and will be included in 2.7.3. |
|||
| msg152536 - (view) | Author: Stephen Thorne (jerub) * | Date: 2012年02月03日 16:26 | |
Yep - 2.7.2 was released 11th June 2011, the fix was committed Aug 1st 2011. So it won't be in the current 2.7 release. |
|||
| msg152537 - (view) | Author: Éric Araujo (eric.araujo) * (Python committer) | Date: 2012年02月03日 16:30 | |
Obviously I can’t fix past releases. Sometimes the time machine is not available :) |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022年04月11日 14:57:12 | admin | set | github: 55313 |
| 2012年02月03日 16:30:52 | eric.araujo | set | status: open -> closed resolution: fixed messages: + msg152537 |
| 2012年02月03日 16:26:22 | jerub | set | messages: + msg152536 |
| 2012年02月03日 16:25:31 | eric.araujo | set | messages: + msg152535 |
| 2012年02月03日 16:21:01 | jdennis | set | status: closed -> open resolution: fixed -> (no value) messages: + msg152534 |
| 2011年08月01日 12:57:21 | eric.araujo | set | status: open -> closed messages: + msg141502 components: - Distutils2 resolution: fixed stage: commit review -> resolved |
| 2011年08月01日 12:49:57 | python-dev | set | messages: + msg141499 |
| 2011年08月01日 12:45:24 | python-dev | set | nosy:
+ python-dev messages: + msg141495 |
| 2011年06月25日 20:13:07 | eric.araujo | set | priority: normal -> high messages: + msg139114 |
| 2011年06月25日 13:54:23 | jerub | set | files:
+ manifest-respect-3 messages: + msg139065 |
| 2011年06月25日 00:52:55 | eric.araujo | set | stage: commit review messages: + msg139013 versions: + Python 3.3, - Python 3.1 |
| 2011年06月24日 23:30:57 | jerub | set | files:
+ manifest-respect-3 messages: + msg139002 |
| 2011年06月24日 13:14:25 | jerub | set | files:
+ manifest-respect-3 messages: + msg138941 |
| 2011年06月24日 10:50:56 | jerub | set | messages: + msg138903 |
| 2011年06月24日 10:44:17 | eric.araujo | set | messages: + msg138901 |
| 2011年06月24日 08:20:32 | jerub | set | messages: + msg138891 |
| 2011年06月04日 11:22:39 | jerub | set | files:
+ manifest-respect-3.patch messages: + msg137633 |
| 2011年06月04日 10:19:24 | jerub | set | files:
+ manifest-respect.patch nosy: + jerub messages: + msg137632 |
| 2011年02月07日 16:59:39 | jdennis | set | nosy:
+ dmalcolm |
| 2011年02月04日 20:21:15 | eric.araujo | set | nosy:
tarek, eric.araujo, jdennis messages: + msg127937 |
| 2011年02月04日 14:47:01 | jdennis | set | nosy:
tarek, eric.araujo, jdennis messages: + msg127895 |
| 2011年02月04日 03:51:48 | eric.araujo | set | assignee: tarek -> eric.araujo messages: + msg127867 nosy: tarek, eric.araujo, jdennis |
| 2011年02月04日 01:51:15 | jdennis | set | files:
+ distutils_bug.tar.gz nosy: tarek, eric.araujo, jdennis messages: + msg127857 |
| 2011年02月04日 01:04:09 | eric.araujo | set | nosy:
tarek, eric.araujo, jdennis messages: + msg127847 components: + Distutils2 versions: + 3rd party, Python 3.1, Python 3.2 |
| 2011年02月03日 00:45:17 | jdennis | set | files:
+ sdist_new nosy: tarek, eric.araujo, jdennis |
| 2011年02月03日 00:44:40 | jdennis | set | files:
+ sdist.patch nosy: tarek, eric.araujo, jdennis |
| 2011年02月03日 00:44:03 | jdennis | set | files:
- sdist_new nosy: tarek, eric.araujo, jdennis |
| 2011年02月03日 00:43:57 | jdennis | set | files:
- sdist.patch nosy: tarek, eric.araujo, jdennis |
| 2011年02月03日 00:08:20 | jdennis | set | files:
+ sdist_new nosy: tarek, eric.araujo, jdennis |
| 2011年02月03日 00:06:49 | jdennis | set | files:
+ sdist.patch nosy: tarek, eric.araujo, jdennis keywords: + patch |
| 2011年02月03日 00:05:43 | jdennis | create | |