-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Switch from get-pip.py to ensurepip #955
Conversation
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.
Excellent, thank you! Just a few really minor stylistic notes, but overall this looks really good. ❤️
Thank you for the feedback! I've made the those changes - I wouldn't normally force push for review changes, but since this repo uses merge commits rather than squash and merge I wanted to avoid the additional commits ending up on Happy to make further changes if you spot anything else :-) (And also no rush to merge this; we can wait until the other changes are released first etc) |
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.
Last thing I see -- my mental model here is that the closing ;
on a long multi-line is similar to the closing )
or }
so I outdent it to make it clear this is the end of the extra long command+args. 👍
Thanks for your work on this! ❤️
I was going to rebase this on |
Since: * All versions of Python that are actively built by this repo now include the `ensurepip` module. * The policy of these images is now to use the same pip version as the one bundled with `ensurepip` (rather than always upgrading as pip releases occur) to avoid breaking changes, and for parity with the `venv` module. * As such, we might as well actually use `ensurepip` to install pip (since it installs the exact pip version we want) rather than manually doing the same using `get-pip.py`. Now that the pip/setuptools versions track (or mostly track, in the case of setuptools) the ensurepip versions, the concerns over frequent invalidation of the Python layer no longer apply, and so the pip/setuptools install can now be part of the Python layer, reducing layer count by one. This change is a no-op in terms of pip/setuptools/wheel versions, since the pip versions being used already exactly matched the `ensurepip` version of pip. Closes docker-library#951.
I've rebased on |
Also, to be really explicit, thank you so much for bringing up the original thread, being so thoughtful and thorough, seeing it all the way through including the extra cleanup of |
That's very kind of you to say, it's been no trouble! :-) |
Changes: - docker-library/python@31bbb37: Merge pull request docker-library/python#955 from edmorley/rm-get-pip - docker-library/python@9cd3243: Switch from get-pip.py to ensurepip
Since:
ensurepip
module.ensurepip
(rather than always upgrading as pip releases occur) to avoid breaking changes, and for parity with thevenv
module.ensurepip
to install pip (since it installs the exact pip version we want) rather than manually doing the same usingget-pip.py
.Now that the pip/setuptools versions track (or mostly track, in the case of setuptools) the ensurepip versions, the concerns over frequent invalidation of the Python layer no longer apply, and so the pip/setuptools install can now be part of the Python layer, reducing layer count by one.
This change is a no-op in terms of pip/setuptools/wheel versions, since the pip versions being used already exactly matched the
ensurepip
version of pip.Closes #951.