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

Jump to bottom

distutils sdist is too lazy w.r.t. recalculating MANIFEST #52934

Closed
ronaldoussoren opened this issue May 11, 2010 · 18 comments
Closed

distutils sdist is too lazy w.r.t. recalculating MANIFEST #52934

ronaldoussoren opened this issue May 11, 2010 · 18 comments
Assignees
@merwok
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@ronaldoussoren
Copy link
Contributor

BPO 8688
Nosy @warsaw, @ronaldoussoren, @tarekziade, @djc, @merwok
Files
  • always-regen-manifest.patch
  • sdist-manifest-marker.diff
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/merwok'
closed_at = <Date 2010-08-14.04:22:03.957>
created_at = <Date 2010-05-11.12:32:46.944>
labels = ['type-bug', 'library']
title = 'distutils sdist is too lazy w.r.t. recalculating MANIFEST'
updated_at = <Date 2011-02-04.03:47:35.303>
user = 'https://github.com/ronaldoussoren'

    bugs.python.org fields:

    activity = <Date 2011-02-04.03:47:35.303>
actor = 'eric.araujo'
assignee = 'eric.araujo'
closed = True
closed_date = <Date 2010-08-14.04:22:03.957>
closer = 'eric.araujo'
components = ['Distutils', 'Distutils2']
creation = <Date 2010-05-11.12:32:46.944>
creator = 'ronaldoussoren'
dependencies = []
files = ['17360', '18517']
hgrepos = []
issue_num = 8688
keywords = ['patch', 'needs review']
message_count = 18.0
messages = ['105505', '105856', '105858', '105897', '112371', '112373', '112383', '112387', '113530', '113531', '113534', '113645', '113856', '113860', '113863', '113867', '113868', '127866']
nosy_count = 8.0
nosy_names = ['barry', 'ronaldoussoren', 'tarek', 'djc', 'eric.araujo', 'jdavid.ibp@gmail.com', 'meatballhat', 'mpm']
pr_nums = []
priority = 'high'
resolution = 'fixed'
stage = 'commit review'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue8688'
versions = ['Python 2.6', 'Python 2.5', 'Python 3.1', 'Python 2.7', 'Python 3.2']
    @ronaldoussoren
    Copy link
    Contributor Author

    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.

    @ronaldoussoren ronaldoussoren added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels May 11, 2010
    @ronaldoussoren
    Copy link
    Contributor Author

    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)

    @tarekziade
    Copy link
    Mannequin

    tarekziade mannequin commented May 16, 2010

    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.

    @tarekziade
    Copy link
    Mannequin

    tarekziade mannequin commented May 17, 2010

    done in r81255, r81256 (2.6), r81258 (py3), r81260 (3.1)

    Thanks Ronald

    @tarekziade tarekziade mannequin closed this as completed May 17, 2010
    @mpm
    Copy link
    Mannequin

    mpm mannequin commented Aug 1, 2010

    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.

    @tarekziade
    Copy link
    Mannequin

    tarekziade mannequin commented Aug 1, 2010

    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.

    @mpm
    Copy link
    Mannequin

    mpm mannequin commented Aug 1, 2010

    Ok, we need a change that will work with Python 2.4 through 2.7.

    @tarekziade
    Copy link
    Mannequin

    tarekziade mannequin commented Aug 1, 2010

    Follow-up at http://selenic.com/pipermail/mercurial-packaging/2010-August/000009.html

    @jdavidibpgmailcom
    Copy link
    Mannequin

    jdavidibpgmailcom mannequin commented Aug 10, 2010

    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.

    @ronaldoussoren
    Copy link
    Contributor Author

    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.

    @tarekziade
    Copy link
    Mannequin

    tarekziade mannequin commented Aug 10, 2010

    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

    @merwok
    Copy link
    Member

    merwok commented Aug 12, 2010

    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.)

    @merwok
    Copy link
    Member

    merwok commented Aug 14, 2010

    New version of the patch to address Tarek’s comments and add NEWS and docs entries.

    @warsaw
    Copy link
    Member

    warsaw commented Aug 14, 2010

    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.

    @merwok
    Copy link
    Member

    merwok commented Aug 14, 2010

    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.

    @merwok merwok assigned merwok and unassigned tarekziade Aug 14, 2010
    @merwok
    Copy link
    Member

    merwok commented Aug 14, 2010

    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.

    @merwok
    Copy link
    Member

    merwok commented Aug 14, 2010

    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.

    @merwok merwok closed this as completed Aug 14, 2010
    @merwok merwok changed the title distutils sdist is too laze w.r.t. recalculating MANIFEST distutils sdist is too lazy w.r.t. recalculating MANIFEST Aug 14, 2010
    @merwok
    Copy link
    Member

    merwok commented Feb 4, 2011

    There is a bug in the new code. See bpo-11104.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @merwok merwok

    Labels
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @warsaw @merwok @ronaldoussoren