-
-
Notifications
You must be signed in to change notification settings - Fork 30.8k
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
bpo-38488: Upgrade bundled versions of pip & setuptools #16782
Conversation
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA). Recognized GitHub usernameWe couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames: This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. You can check yourself to see if the CLA has been received. Thanks again for the contribution, we look forward to reviewing it! |
5a43995
to
bba9f56
Compare
bba9f56
to
9b24d00
Compare
This would need a bpo and a news entry similar to https://bugs.python.org/issue35807 . |
Lib/ensurepip/__init__.py
Outdated
import pip._internal | ||
return pip._internal.main(args) | ||
import pip._internal.main | ||
return pip._internal.main.main(args) |
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.
Are we OK to change this to use runpy? That way we'd basically be doing python -m pip, which is directly supported.
See https://github.com/pradyunsg/pip-cli/blob/master/src/_pip_cli.py#L75 for how we can do that. You can skip setting the env there, since that's for passing information that's not relevant here.
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.
I wouldn't say that it's directly supported (as in, part of the public API) since running in the same process space is still not considered supported, but it would avoid any issues with changes to the location of the entrypoint function.
It may also be helpful to consider backporting that change to bugfix Python versions, which can help people that build and patch in new versions of pip (as mentioned in 09fd200).
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.
Why is this not simply run in a sub-process, then?
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.
We don't use runpy (or a subprocess) here because of the path manipulation just above to import pip from the bundled wheel archives.
It's useful to think of ensurepip as being part of pip, as we're free to make whatever changes are needed to sync up when updating to new versions of pip.
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.
Let's discuss in https://bugs.python.org/issue38662 or #17029
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.
https://bugs.python.org/issue38662 is now fixed in master. You can rebased your PR on master.
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.
Done.
44071d4
to
821012e
Compare
Moving content out of `__init__` is preferred in general because it avoids conflicts with module names and unnecessary imports.
Also see this Discourse discussion: https://discuss.python.org/t/can-we-finally-add-a-minimal-api-to-pip/2833 |
The best (though still not supported) way to run pip in-process is with Could you update the pull request to use that? Please comment if you have any questions, or if you'd prefer to leave this to someone else. |
A subprocess.run might be a more robust solution here. |
As I mentioned in the inline comment, I wonder why this isn't run in a sub-process, and suggest doing so. Regardless, that could be a separate issue. This simple update should likely go in. |
@xavfernandez, have you any intention to continue working on this? |
Sounds fair to me -- I filed a bpo issue for this a while back -- maybe someone/me can find it and link to it from here. None of my concerns here are blocking this version update, which should definitely go in. |
@pradyunsg Is it this https://bugs.python.org/issue38662? |
Yes, however it's unclear what needs to be done in this PR ? |
@tirkarthi Yes, indeed. Many thanks for finding that. :) @xavfernandez IMO nothing else is needed here -- we can tackle those in a follow up. /cc @ncoghlan @dstufft @pfmoore in case one of them wants to review + click merge here. :) |
By now the latest versions of setuptools and pip have moved along:
So we should bump those and then IMO this would be ready to go. EDIT: P.S. Apologies for the delay so far, I'll make sure this moves along quickly now. |
821012e
to
6b92720
Compare
7b22480
to
75a8678
Compare
75a8678
to
f77f057
Compare
f77f057
to
9297a25
Compare
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.
Sorry about the delay, I suspect this was a case of all of us assuming one of the others would approve & merge it.
@ncoghlan: Please replace |
|
|
Various buildbots which run the tests with an installed python, fail with: chmod: target/lib/python3.10/site-packages/pip-20.1.dist-info/RECORD: new permissions are r-xrw-rwx, not r-xr--r-x |
It's the "chmod" step which fails. I guess that only "Install" buildbot workers are affected. I failed to reproduce the issue with commands:
I get these permissions:
|
I reverted the change: see https://bugs.python.org/issue38488 |
The permission issue is likely due to pypa/pip#8164 that was fixed in 20.1.1. |
Following the release of pip 19.3.
It's unclear to me if a bpo ticket is needed ?
https://bugs.python.org/issue38488