-
-
Notifications
You must be signed in to change notification settings - Fork 11k
Fix setuptools sdist #7131
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
Fix setuptools sdist #7131
Conversation
This was seriously broken. Setuptools does unwanted 'smart' things and ignores MANIFEST.in. Closes numpygh-7127.
Warn in numpy/distutils/command/egg_info.py if using setuptools.sdist. See numpygh-7127 for details.
Checked the build log of the |
Do we know why that test didn't catch this in the first place? On Wed, Jan 27, 2016 at 2:12 PM, Ralf Gommers notifications@github.com
Nathaniel J. Smith -- https://vorpus.org http://vorpus.org |
@njsmith The test installing from the sdist created archive is new. Don't know if that answers your question. |
Ah, right, sorry :-) I guess I misremembered us adding something like that On Wed, Jan 27, 2016 at 2:35 PM, Charles Harris notifications@github.com
Nathaniel J. Smith -- https://vorpus.org http://vorpus.org |
The wheel tests were added fairly recently, but those don't exercise |
from setuptools.command.egg_info import egg_info as _egg_info | ||
|
||
class egg_info(_egg_info): | ||
def run(self): | ||
if 'sdist' in sys.argv: | ||
import warnings | ||
warnings.warn("`build_src` is being run, this may lead to missing " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@charris @rgommers I ran into this issue with one of the skimage tools, which checks to see whether our MANIFEST file is complete. It says to look at gh-7127, but I'm not sure what to look at! Could we point to a likely fix in this message instead? I am happy to make the patch, but was hoping you could point me in the right direction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's the tool executing python setup.py sdist
, for what it's worth: https://github.com/stefanv/scikit-image/blob/check_sdist/tools/check_sdist.py#L12
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fix is in this PR: 6770f98
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the skimage
setup.py
you don't have any sdist
handling, so you also have to add something like cmdclass={'sdist': sdist}
or (better) the sdist_checked
version that numpy and scipy have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @rgommers, that's very helpful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're welcome. I'll try to get around to tweaking the warning message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It took a while, but addressed in gh-8761
Addresses comment of @stefanv on numpygh-7131.
Numpy warns if you use the setuptools sdist - see numpy/numpy#7131 - so revert this change for now.
Numpy warns if you use the setuptools sdist - see numpy/numpy#7131 - so revert this change for now.
Fix for #7127: Setuptools sdist doesn't include all the files in the extension sources list.
Also add test installing from sdist archive to TravisCI.