Skip to content

Fix #1890: set sys.argv[0] to the setup.py path in the setuptools shim #5936

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.

Sign up for GitHub

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
Merged
merged 1 commit into from
Jul 8, 2019
Merged

Fix #1890: set sys.argv[0] to the setup.py path in the setuptools shim #5936

merged 1 commit into from
Jul 8, 2019

Conversation

@bertilhatt
Copy link

@bertilhatt bertilhatt commented Oct 27, 2018

Why

Addressing #1890

How

Switch % pattern to .format()

@bertilhatt bertilhatt changed the title Subprocess invocation out setuptools args WIP: Subprocess invocation out setuptools args Oct 27, 2018
@bertilhatt
Copy link
Author

bertilhatt commented Oct 27, 2018

Wasn’t able to test locally because of too many errors after cloning the repository; see #5937.

@benoit-pierre
Copy link
Member

benoit-pierre commented Oct 27, 2018

You forgot a number or references to SETUPTOOLS_SHIM.

@pradyunsg
Copy link
Member

pradyunsg commented Oct 27, 2018 

./src/pip/_internal/download.py:from pip._internal.utils.setuptools_build import SETUPTOOLS_SHIM
./src/pip/_internal/download.py:    sdist_args.append(SETUPTOOLS_SHIM % setup_py)
./src/pip/_internal/utils/setuptools_build.py:SETUPTOOLS_SHIM = (
./src/pip/_internal/req/req_install.py:from pip._internal.utils.setuptools_build import SETUPTOOLS_SHIM
./src/pip/_internal/req/req_install.py:            script = SETUPTOOLS_SHIM % self.setup_py
./src/pip/_internal/req/req_install.py:                        SETUPTOOLS_SHIM % self.setup_py
./src/pip/_internal/req/req_install.py:        install_args.append(SETUPTOOLS_SHIM % self.setup_py)
./src/pip/_internal/wheel.py:from pip._internal.utils.setuptools_build import SETUPTOOLS_SHIM
./src/pip/_internal/wheel.py:            SETUPTOOLS_SHIM % req.setup_py

@cjerdonek cjerdonek added the type: enhancement Improvements to functionality label Oct 27, 2018
@bertilhatt bertilhatt changed the title WIP: Subprocess invocation out setuptools args Subprocess invocation out setuptools args Oct 27, 2018
@bertilhatt bertilhatt mentioned this pull request Oct 27, 2018
Closed
pradyunsg
pradyunsg reviewed Oct 28, 2018
View reviewed changes
Copy link
Member

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@BrownTruck
Copy link
Contributor

BrownTruck commented Nov 16, 2018

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will be eligible for code review and hopefully merging!

@BrownTruck BrownTruck added the needs rebase or merge PR has conflicts with current master label Nov 16, 2018
@pganssle
Copy link
Member

pganssle commented Mar 16, 2019

@bertilhatt @pypa/pip-committers I have rebased (and did some squashing along the way) this PR against the current master in this branch on my fork

Could a maintainer force-push my changes to the original PR branch so that this can be merged? Here's how to do that:

git remote add bertilhatt [email protected]:bertilhatt/pip.git
git fetch bertilhatt
git checkout subprocess_invocation_out_setuptools_args
git remote add pganssle [email protected]:pganssle/pip.git
git fetch pganssle
git reset --hard pganssle/subprocess_invocation_out_setuptools_args
git push --force-with-lease

@bertilhatt If you want to do it yourself, you can skip the first two lines and start at git checkout subprocess_invocation_out_setuptools_args.

@cjerdonek cjerdonek force-pushed the subprocess_invocation_out_setuptools_args branch from 0b3c811 to 1ddafa5 Compare March 17, 2019 07:40
@pypa-bot pypa-bot removed the needs rebase or merge PR has conflicts with current master label Mar 17, 2019
@cjerdonek
Copy link
Member

cjerdonek commented Mar 17, 2019

@pganssle I did what you requested. Thanks!

@pganssle
Copy link
Member

pganssle commented Mar 17, 2019

Ugh, I seem to have broken things in my rebase because I accidentally converted setup_py to setup.py.

@cjerdonek I've force-pushed a fixed version to my branch that hopefully fixes this. Sorry about that, you should be able to fetch my latest version and reset @bertilhatt's to my current head, which is commit 844f4f697f74575213ed786635c17de4fb66f350

@cjerdonek cjerdonek force-pushed the subprocess_invocation_out_setuptools_args branch from 1ddafa5 to 844f4f6 Compare March 17, 2019 20:02
@cjerdonek
Copy link
Member

cjerdonek commented Mar 17, 2019

Done.

@BrownTruck
Copy link
Contributor

BrownTruck commented Jun 30, 2019

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will be eligible for code review and hopefully merging!

@BrownTruck BrownTruck added the needs rebase or merge PR has conflicts with current master label Jun 30, 2019
@cjerdonek cjerdonek force-pushed the subprocess_invocation_out_setuptools_args branch from 844f4f6 to 3e93e56 Compare July 4, 2019 09:49
@pypa-bot pypa-bot removed the needs rebase or merge PR has conflicts with current master label Jul 4, 2019
@cjerdonek
Copy link
Member

cjerdonek commented Jul 4, 2019

@pradyunsg @pganssle Could either of you take a look at this again before it's merged? It's been several months and a couple rebases before either of you gave the first okay.

@pradyunsg
Copy link
Member

pradyunsg commented Jul 5, 2019

LGTM. @cjerdonek Let's merge if you think this is good to go. :)

@cjerdonek cjerdonek force-pushed the subprocess_invocation_out_setuptools_args branch 2 times, most recently from c4218c7 to 8133616 Compare July 8, 2019 00:06
@cjerdonek cjerdonek added the project: setuptools Related to setuptools label Jul 8, 2019
@cjerdonek cjerdonek force-pushed the subprocess_invocation_out_setuptools_args branch from 8133616 to 82aaf94 Compare July 8, 2019 01:38
@cjerdonek cjerdonek force-pushed the subprocess_invocation_out_setuptools_args branch from 82aaf94 to b47da27 Compare July 8, 2019 01:41
@cjerdonek cjerdonek changed the title Subprocess invocation out setuptools args Fix #1890: set sys.argv[0] to the setup.py path in the setuptools shim Jul 8, 2019
@cjerdonek cjerdonek merged commit ca017ca into pypa:master Jul 8, 2019
@cjerdonek
Copy link
Member

cjerdonek commented Jul 8, 2019

Thanks for your work on this, @bertilhatt, and for your patience!

@cjerdonek cjerdonek mentioned this pull request Jul 8, 2019
Closed
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Aug 7, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Aug 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@pradyunsg pradyunsg pradyunsg approved these changes

Assignees

No one assigned

Labels

auto-locked Outdated issues that have been locked by automation project: setuptools Related to setuptools type: enhancement Improvements to functionality

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@bertilhatt @benoit-pierre @pradyunsg @BrownTruck @pganssle @cjerdonek @pypa-bot