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

Use a single setuptools shim #3265

Merged
merged 4 commits into from
Nov 26, 2015
Merged

Conversation

xavfernandez
Copy link
Member

for all setup.py invocations

Review on Reviewable

@@ -460,11 +458,9 @@ def run_egg_info(self):
# FIXME: This is a lame hack, entirely for PasteScript which has
# a self-provided entry point that causes this awkwardness
_run_setup_py = """
Copy link
Member Author

Choose a reason for hiding this comment

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

I think this "lame hack" present since c2000d7 (7 years now) could now be removed ?

Copy link
Member Author

Choose a reason for hiding this comment

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

cc @dstufft (I'm talking of the egg_info.egg_info.run = replacement_run bit)

@rbtcollins
Copy link

Review status: 0 of 4 files reviewed at latest revision, 3 unresolved discussions.


pip/req/req_install.py, line 460 [r1] (raw file):
Have you checked to see if PasteScript has been fixed?


pip/req/req_install.py, line 473 [r2] (raw file):
The ; here is gross - why is it needed?


pip/utils/init.py, line 825 [r2] (raw file):
Could we keep this in req_install? Its closer to the users there. Or maybe in a dedicated setuptools_build.py file?


Comments from the review on Reviewable.io

@xavfernandez
Copy link
Member Author

pip/req/req_install.py, line 460 [r1] (raw file):
Well I'm not sure what this hack was supposed to fix :-/

But pip 7.1.2 is currently enable to install PasteScript 0.3/0.3.1 and seems to work with most other versions including the last ones (I didn't try all of them).

Removing the hack leads to the same result: crashes with 0.3/0.3.1 and seems to work with other versions including the last ones.


Comments from the review on Reviewable.io

@xavfernandez
Copy link
Member Author

pip/req/req_install.py, line 460 [r1] (raw file):
err I meant "pip 7.1.2 is currently unable"


Comments from the review on Reviewable.io

@xavfernandez
Copy link
Member Author

Updated, cc @rbtcollins @dstufft

@rbtcollins
Copy link

Review status: 0 of 4 files reviewed at latest revision, 2 unresolved discussions.


pip/req/req_install.py, line 460 [r1] (raw file):
Well if current pip can't do it, being no worse is ok; still, lets do that as a separate change.


Comments from the review on Reviewable.io

for all setup.py invocations
bdist_wheel will now use tokenize in Python 3 just like for install
fixes pypa#2042
xavfernandez added a commit that referenced this pull request Nov 26, 2015
@xavfernandez xavfernandez merged commit 57db684 into pypa:develop Nov 26, 2015
@xavfernandez xavfernandez deleted the setuptool_shim branch November 26, 2015 23:00
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 3, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants