classification
Title: distutils sdist is too lazy w.r.t. recalculating MANIFEST
Type: behavior Stage: commit review
Components: Distutils, Distutils2 Versions: Python 3.2, Python 3.1, Python 2.7, Python 2.6, Python 2.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: eric.araujo Nosy List: barry, djc, eric.araujo, jdavid.ibp@gmail.com, meatballhat, mpm, ronaldoussoren, tarek
Priority: high Keywords: needs review, patch

Created on 2010-05-11 12:32 by ronaldoussoren, last changed 2011-02-04 03:47 by eric.araujo. This issue is now closed.

Files
File name Uploaded Description Edit
always-regen-manifest.patch ronaldoussoren, 2010-05-16 10:34
sdist-manifest-marker.diff eric.araujo, 2010-08-14 00:18
Messages (18)
msg105505 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2010-05-11 12:32
The sdist command in distutils calculates the MANIFEST file from a template (by default MANIFEST.in). 

The code in sdist assumes that the contents of MANIFEST only depends on MANIFEST.in and setup.py, which can cause files to be missed when building an sdist.

In particular: given a MANIFEST.in file that includes all documentation in a 'Doc' subdirectory, using"recursive-include Doc *.txt" in the template, the MANIFEST won't get regenerated when a new file is added to the documentation subdirectory and hence that new file is not included in sdist distributions.
msg105856 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2010-05-16 10:34
One way to fix this: is to always recreate MANIFEST when an explicit MANIFEST.in file exists, as in the attached patch.

(The patch is not perfect: I'd rename "template_newer" before committing)
msg105858 - (view) Author: Tarek Ziadé (tarek) * (Python committer) Date: 2010-05-16 10:55
The same problem occurs without a manifest template: you can have options or imports in setup.py that will change the MANIFEST file. 

For example : if setup() has:

  packages=['foo']

And if you add a subpackage "bar" in "foo" with some modules, with an existing MANIFEST, they won't be added.

IOW, the MANIFEST file should be recalculated *everytime*. I am going to remove all the "don't rebuilt MANIFEST" code in Distutils *and* Distutils2.
msg105897 - (view) Author: Tarek Ziadé (tarek) * (Python committer) Date: 2010-05-17 10:49
done in r81255, r81256 (2.6), r81258 (py3), r81260 (3.1)

Thanks Ronald
msg112371 - (view) Author: Matt Mackall (mpm) Date: 2010-08-01 19:21
This change just wrecked Mercurial's release build process.

We've been building Mercurial release tarballs with a Makefile target wrapped around sdist for most of five years, and we've never had or wanted a MANIFEST.in file. We generate an exact, complete, and correct MANIFEST ourselves with:

 hg manifest > MANIFEST

This fix ignores that MANIFEST entirely and attempts to build a new one even though MANIFEST.in doesn't exist.

So not only does this change years of documented behavior in a way that very nearly caused us to ship a tarball missing over a thousand files, it means doing what we want to do with sdist (ship precisely the files listed in our version control manifest) is now significantly harder.

Unfortunately, this fix is now in a long-lived Python release which we support, which means we're stuck with it.
msg112373 - (view) Author: Tarek Ziadé (tarek) * (Python committer) Date: 2010-08-01 19:39
this fix was done to avoid generating broken MANIFEST file with sdist.

IIUC your problem is more about avoiding generating a MANIFEST file at *all* via sdist, so we should add a --no-manifest option to the sdist command.

This can be added today in Mercurial setup.py file as well so you don't have the problem with 2.7. I can contribute this change if you want.
msg112383 - (view) Author: Matt Mackall (mpm) Date: 2010-08-01 20:51
Ok, we need a change that will work with Python 2.4 through 2.7.
msg112387 - (view) Author: Tarek Ziadé (tarek) * (Python committer) Date: 2010-08-01 21:19
Follow-up at http://selenic.com/pipermail/mercurial-packaging/2010-August/000009.html
msg113530 - (view) Author: J. David Ibáñez (jdavid.ibp@gmail.com) Date: 2010-08-10 12:53
This issue has broken our build system too.

A bug has been fixed, but another one is open now.

If the MANIFEST file has been produced by distutils it should be
regenerated each time.  But if it has not been produced by distutils
then it should not regenerated.

The problem is how to make the distinction.  Maybe an special line at
the beginning of the MANIFEST file could identify that it has been
generated by distutils.
msg113531 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2010-08-10 12:59
I agree with msg113530 and msg112371 that the current behavior is a bug: distutils is now too agressive because it recalculates the MANIFEST file even when distutils isn't the one that calculated it in the first place.

This is a functional regression w.r.t. earlier python releases.
msg113534 - (view) Author: Tarek Ziadé (tarek) * (Python committer) Date: 2010-08-10 13:17
I like the idea of a marker in a comment line in the MANIFEST, we can introduce this as a bug fix to avoid this regression
msg113645 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-08-12 00:44
Attaching a patch to implement the marker idea. No doc changes yet.

It should work transparently: you don’t have to give an option to have automatic recalculation, you don’t have to give an option to have it leave your manual MANIFEST alone.

(For the curious, history is at http://beta.intuxication.org/hg/merwok/python-patches. Yay for TDD and Mercurial.)
msg113856 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-08-14 00:19
New version of the patch to address Tarek’s comments and add NEWS and docs entries.
msg113860 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2010-08-14 01:15
This is a regression introduced in 2.6.6rc1.  After discussion in irc with merwok, it is agreed that he will revert to pre-2.6.6rc1 behavior and not apply any fix.  It's way too late to be changing behavior in 2.6.6.  Folks using Python 2.6 will just have to live with the devil they know.
msg113863 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-08-14 02:11
2.6 regression reverted in r83992.

Tarek is okay with my patch for the other branches. I noticed the first fix did not have a versionadded in the docs, so I’ll add that and commit.
msg113867 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-08-14 04:00
Patch committed in r83993 (py3k), r83994 (3.1) and r83996 (2.7).

I got the version wrong in 3.x and fixed it in r83995 and r83998.
msg113868 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-08-14 04:22
Patch ported to distutils2 (http://bitbucket.org/tarek/distutils2/changeset/5abba4a77f5a). Closing.

Please reopen with priority set to “release blocker” if 2.6 behaves badly before Monday. Thanks.
msg127866 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-02-04 03:47
There is a bug in the new code.  See #11104.
History
Date User Action Args
2011-02-04 03:47:35eric.araujosetnosy: barry, ronaldoussoren, tarek, djc, eric.araujo, jdavid.ibp@gmail.com, meatballhat, mpm
messages: + msg127866
2010-08-14 04:22:03eric.araujosetstatus: open -> closed
versions: + Python 2.5
title: distutils sdist is too laze w.r.t. recalculating MANIFEST -> distutils sdist is too lazy w.r.t. recalculating MANIFEST
messages: + msg113868

resolution: fixed
stage: patch review -> commit review
2010-08-14 04:00:50eric.araujosetmessages: + msg113867
2010-08-14 02:11:04eric.araujosetpriority: release blocker -> high
assignee: tarek -> eric.araujo
messages: + msg113863
2010-08-14 01:15:54barrysetpriority: high -> release blocker
nosy: + barry
messages: + msg113860

2010-08-14 00:19:20eric.araujosetmessages: + msg113856
2010-08-14 00:18:33eric.araujosetfiles: + sdist-manifest-marker.diff
2010-08-14 00:17:59eric.araujosetfiles: - sdist-manifest-marker.diff
2010-08-12 00:44:44eric.araujosetfiles: + sdist-manifest-marker.diff

nosy: + eric.araujo
messages: + msg113645

stage: needs patch -> patch review
2010-08-10 13:17:53tareksetmessages: + msg113534
2010-08-10 12:59:46ronaldoussorensetstatus: closed -> open

messages: + msg113531
2010-08-10 12:53:02jdavid.ibp@gmail.comsetnosy: + jdavid.ibp@gmail.com
messages: + msg113530
2010-08-02 07:56:14djcsetnosy: + djc
2010-08-01 21:19:49tareksetmessages: + msg112387
2010-08-01 20:51:27mpmsetmessages: + msg112383
2010-08-01 19:39:03tareksetmessages: + msg112373
2010-08-01 19:21:40mpmsetnosy: + mpm
messages: + msg112371
2010-05-17 10:49:20tareksetstatus: open -> closed

messages: + msg105897
2010-05-16 10:56:31tareksetpriority: normal -> high
2010-05-16 10:55:57tareksetnosy: ronaldoussoren, tarek, meatballhat
messages: + msg105858
components: + Distutils2
2010-05-16 10:34:18ronaldoussorensetkeywords: + patch, needs review
files: + always-regen-manifest.patch
messages: + msg105856
2010-05-12 00:05:12meatballhatsetnosy: + meatballhat
2010-05-11 12:32:47ronaldoussorencreate