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

Egg include setup py #1554

Merged
merged 4 commits into from Nov 4, 2018

Conversation

3 participants
@shashanksingh28
Contributor

shashanksingh28 commented Oct 28, 2018

Summary of changes

While creating sdists, we want the source dist .tar.gz to contain setup.py because this is what pip would use to install the source distribution. python setup.py sdist ensured setup.py was included because distutils by default adds the script_name that was executing the build process. However, for someone calling build_sdist('.') programatically, the script name is the script callind build_sdist function and not setup.py.

The problem is that sdist command uses the egg-info command to extract file list to be tar-ziped, which did not include setup.py as default (unless it was called via 'setup.py' as the script name). This fix adds an explicit check in the egg-info command for setup.py.

An alternative solution is solving this one level deeper at distutils add_defaults method which egg-info command uses.

Closes #1506

Will update changelog once we agree on the approach

Pull Request Checklist

  • Changes have tests
  • News fragment added in changelog.d. See documentation for details
@pganssle

This comment has been minimized.

Member

pganssle commented Oct 28, 2018

@shashanksingh28 Can you add a changelog here? changelog.d/1554.change.rst

@pganssle

This comment has been minimized.

Member

pganssle commented Oct 30, 2018

@gaborbernat Do you mind taking a look at this? Are there tests that cover your use case?

@gaborbernat

It does solve my problem, thanks a lot !

Show resolved Hide resolved changelog.d/1554.change.rst Outdated
Show resolved Hide resolved setuptools/command/egg_info.py Outdated
Show resolved Hide resolved setuptools/tests/test_build_meta.py Outdated
Show resolved Hide resolved setuptools/tests/test_sdist.py

@pganssle pganssle moved this from Submitted PRs to Approved PRs in PyPA Sprint Weekend at Bloomberg (2018) Nov 1, 2018

@pganssle pganssle force-pushed the shashanksingh28:egg_include_setup_py branch from 1e8cb17 to 421f49d Nov 2, 2018

pganssle added a commit to shashanksingh28/setuptools that referenced this pull request Nov 2, 2018

shashanksingh28 added some commits Oct 28, 2018

Add tests for setup.py inclusion
This tests that `setup.py` is included by default in the distribution
with the egg_info command and when an sdist is built with
build_meta.build_sdist

@pganssle pganssle force-pushed the shashanksingh28:egg_include_setup_py branch from 421f49d to 932c72d Nov 3, 2018

pganssle added a commit to shashanksingh28/setuptools that referenced this pull request Nov 3, 2018

shashanksingh28 and others added some commits Oct 28, 2018

@pganssle pganssle force-pushed the shashanksingh28:egg_include_setup_py branch from 932c72d to bff67db Nov 3, 2018

@pganssle pganssle merged commit 46af765 into pypa:master Nov 4, 2018

5 checks passed

codecov/patch 100% of diff hit (target 81.65%)
Details
codecov/project 81.74% (+0.08%) compared to 6ed6fe8
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details

PyPA Sprint Weekend at Bloomberg (2018) automation moved this from Approved PRs to Merged PRs Nov 4, 2018

@pganssle

This comment has been minimized.

Member

pganssle commented Nov 4, 2018

@shashanksingh28 Thanks for your PR, and thanks for following up with all the changes!

@shashanksingh28

This comment has been minimized.

Contributor

shashanksingh28 commented Nov 4, 2018

Thanks @pganssle and @gaborbernat for your help :) This was my first open source PR, will try to be more involved

@pganssle pganssle referenced this pull request Nov 11, 2018

Closed

Cutting a new release #1585

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