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

Updating MANIFEST.in does not correctly update the package sdist creates #436

Open
bb-migration opened this Issue Sep 14, 2015 · 10 comments

Comments

Projects
None yet
5 participants
@bb-migration

bb-migration commented Sep 14, 2015

Originally reported by: spookylukey (Bitbucket: spookylukey, GitHub: spookylukey)


The behaviour of sdist depends on previous contents of MANIFEST.in, not just the current. This is not fixed even by running setup.py clean or setup.py clean --all (although this should not be necessary).

This is very surprising behaviour, and potentially dangerous too - if someone accidentally adds a MANIFEST.in rule that includes a file that must not be distributed and notice the problem, they would expect that removing the rule will remove the file, but it does not.

I've attached a bash script that demonstrates the problem.


@miccoli

This comment has been minimized.

Contributor

miccoli commented Feb 27, 2017

I have been hit by this same bug. My current remedy is to delete the *.egg-info directory before running setup.py sdist.

@miccoli

This comment has been minimized.

Contributor

miccoli commented Feb 27, 2017

The problema appears to be in the logic with which the SOURCES.txt file is updated by python setup.py egg_info:

$ python setup.py egg_info
running egg_info
writing sample.egg-info/PKG-INFO
writing dependency_links to sample.egg-info/dependency_links.txt
writing entry points to sample.egg-info/entry_points.txt
writing requirements to sample.egg-info/requires.txt
writing top-level names to sample.egg-info/top_level.txt
reading manifest file 'sample.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
writing manifest file 'sample.egg-info/SOURCES.txt'

Files already in SOURCES.txt are preserved across subsequent runs of python setup.py egg_info. Could please one of the developers clarify if this is intended behaviour and why?

@miccoli

This comment has been minimized.

Contributor

miccoli commented Feb 28, 2017

See also

def add_defaults(self):
sdist.add_defaults(self)
self.filelist.append(self.template)
self.filelist.append(self.manifest)
rcfiles = list(walk_revctrl())
if rcfiles:
self.filelist.extend(rcfiles)
elif os.path.exists(self.manifest):
self.read_manifest()
ei_cmd = self.get_finalized_command('egg_info')
self.filelist.graft(ei_cmd.egg_info)

where the actual reading of SOURCES.txt takes place:

        rcfiles = list(walk_revctrl())
        if rcfiles:
            self.filelist.extend(rcfiles)
        elif os.path.exists(self.manifest):
            self.read_manifest()

I cannot understand the reason for this logic.

@miccoli

This comment has been minimized.

Contributor

miccoli commented Apr 12, 2017

@jaraco I think I nailed down this old (annoying bug), but nobody cares of these old bugs. Should I reopen or provide a pull request?

@jaraco

This comment has been minimized.

Member

jaraco commented Apr 12, 2017

@miccoli: If this ticket describes the issue, it's still open. A PR would be most appreciated.

@miccoli

This comment has been minimized.

Contributor

miccoli commented Apr 13, 2017

@jaraco yes: this ticket is accurate. Since there is no discussion active, I assume that I should provide my own PR.

jaraco added a commit that referenced this issue Apr 15, 2017

Merge pull request #1014 from miccoli/issue-436
addresses #436: Updating MANIFEST.in does not correctly update the package sdist creates

@jaraco jaraco closed this Apr 15, 2017

@pjdelport

This comment has been minimized.

Contributor

pjdelport commented Apr 18, 2017

I think this change may have broken sdist installs with package data: #1016

@miccoli

This comment has been minimized.

Contributor

miccoli commented Apr 18, 2017

The reason for

        rcfiles = list(walk_revctrl())
        if rcfiles:
            self.filelist.extend(rcfiles)
        elif os.path.exists(self.manifest):
            self.read_manifest()

is now clear: in the develop tree (under SCM control) the files to be installed are determined from the SCM system (walk_revctrl()); in an sdist install the files are to be determined from the existing manifest file, which was previously generated in the develop tree + setuptools_scm

This logic is broken for packages that do not use setuptools_scm, for which there is no guarantee that the current manifest file (in the development tree) is correct.

Unfortunately my PR #1014 badly brokes this intended behaviour, so that pip install of sdist packages under SCM control and include_package_data=True is not more possible, see #1016 .

miccoli added a commit to miccoli/setuptools that referenced this issue Apr 18, 2017

jaraco added a commit that referenced this issue Apr 18, 2017

@jaraco jaraco reopened this Apr 18, 2017

@Firenze11

This comment has been minimized.

Firenze11 commented Apr 17, 2018

Is there a fix to this now? Even if I delete <package>.egg_info, w hen I update MANIFEST.in and run python setup.py sdist, a new <package>.egg_info was created and the content of <package>.egg_info/SOURCES.txt was still not updated.

@miccoli

This comment has been minimized.

Contributor

miccoli commented Apr 19, 2018

@Firenze11 unfortunately my previous attempt at solving this bug (PR #1014) was catastrophic. (I'm still a little ashamed of the mess I made.)

But from my analysis the only place where previous content of MANIFEST.in is persisted is <package>.egg_info/SOURCES.txt. In my use cases, if you delete <package>.egg_info/SOURCES.txt and run python setup.py egg_info you get a brand-new SOURCES.txt file which is correctly populated, following the usual setuptools logic and the current MANIFEST.in.

Can you please provide an example in which SOURCES.txt is first deleted and when recreated still contains files included in a previous MANIFEST.in?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment