-
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
Stop installing setuptools in python 3.12+ #954
Stop installing setuptools in python 3.12+ #954
Conversation
Dockerfile-linux.template
Outdated
@@ -307,7 +307,10 @@ RUN set -eux; \ | |||
"pip==$PYTHON_PIP_VERSION" \ | |||
{{ if .setuptools then ( -}} | |||
"setuptools==$PYTHON_SETUPTOOLS_VERSION" \ | |||
{{ ) else "" end -}} | |||
{{ ) else ( -}} | |||
--no-setup-tools \ |
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.
--no-setup-tools \ | |
--no-setuptools \ |
3889aba
to
f7aece1
Compare
Dockerfile-linux.template
Outdated
python -c 'import wheel' | ||
{{ ) else ( -}} | ||
# smoke test to ensure that setuptools/wheel are not included | ||
if python -c 'import setuptools' || python -c 'import wheel'; 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.
+ python -c import setuptools
Traceback (most recent call last):
File "<string>", line 1, in <module>
ModuleNotFoundError: No module named 'setuptools'
+ python -c import wheel
Traceback (most recent call last):
File "<string>", line 1, in <module>
ModuleNotFoundError: No module named 'wheel'
🤔 Maybe these could be 2>/dev/null
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 guess relevant to #954 (comment), if we made these a dedicated script we could probably do a single script that's more complex, like catching the import exception directly in Python code instead?
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.
Thank you for opening this! :-)
Dockerfile-linux.template
Outdated
pip --version | ||
pip --version; \ | ||
{{ if .setuptools then ( -}} | ||
# smoke test to ensure that setuptools/wheel are included |
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.
Longer term should these smoke tests live in docker-library/official-images
where the other tests are? eg:
https://github.com/docker-library/official-images/tree/master/test/tests/python-imports
https://github.com/docker-library/official-images/tree/master/test/tests/python-no-pyc
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.
Yeah, that makes sense. I removed them so they can just be in the test scripts.
@@ -307,10 +307,23 @@ RUN set -eux; \ | |||
"pip==$PYTHON_PIP_VERSION" \ | |||
{{ if .setuptools then ( -}} | |||
"setuptools==$PYTHON_SETUPTOOLS_VERSION" \ |
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.
For completeness, and in case get-pip
were to ever stop installing wheel
by default for older Python versions, should we add wheel
as an explicit dependency here (without a version specfier, to match the get-pip
behaviour)?
ie:
"setuptools==$PYTHON_SETUPTOOLS_VERSION" \ | |
"setuptools==$PYTHON_SETUPTOOLS_VERSION" \ | |
wheel \ |
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.
Ah yeah, that's probably a good idea, even though I don't love it 😂 😭 ❤️
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 think the Windows template will need similar changes too:
python/Dockerfile-windows.template
Lines 74 to 81 in cc2cf19
{{ if .setuptools then ( -}} | |
('setuptools=={0}' -f $env:PYTHON_SETUPTOOLS_VERSION) \ | |
{{ ) else "" end -}} | |
; \ | |
Remove-Item get-pip.py -Force; \ | |
\ | |
Write-Host 'Verifying pip install ...'; \ | |
pip --version; \ |
I think it might also be worth updating the PR description to give an example of how to restore the previous behaviour, for the small subset of users who are are running pip with build isolation disabled, or have broken dependencies (that have run-time dependency on setuptools but that don't correctly list an explicit For example: FROM python:3
WORKDIR /usr/src/app
RUN pip install --no-cache-dir setuptools wheel
COPY requirements.txt ./
RUN pip install --no-cache-dir -r requirements.txt
# ... (this is an adapted version of the |
f7aece1
to
a8ec33a
Compare
docker-library/official-images#17446 👀 (I've verified that works with this PR's images, and fails on the current images) |
(Close and reopen to force trigger new CI so it includes our updated test. 🙇 I've also got https://github.com/docker-library/python/actions/runs/10623880671 running fresh on master so we can see the failing baseline.) |
Hilariously, thanks to pypa/get-pip#218, my "baseline" test on master is succeeding because now |
Now we're being really explicit about it both directions, which feels right. 😄 |
Changes: - docker-library/python@811625e: Merge pull request docker-library/python#954 from infosiftr/really-no-setuptools-or-wheel - docker-library/python@a8ec33a: Stop installing setuptools in python 3.12+ - docker-library/python@5ee49c2: Update 3.9 to get-pip pypa/get-pip@def4aec - docker-library/python@0dabaf3: Update 3.8 to get-pip pypa/get-pip@def4aec - docker-library/python@14e03d8: Update 3.13-rc to get-pip pypa/get-pip@def4aec - docker-library/python@109e83a: Update 3.12 to get-pip pypa/get-pip@def4aec - docker-library/python@625a0a3: Update 3.11 to get-pip pypa/get-pip@def4aec - docker-library/python@e84c3f7: Update 3.10 to get-pip pypa/get-pip@def4aec
* feat: rework stats * feat: decrease docker version for now * fix: lint and format * feat: remove import * feat: unpin docker version and add wheel + setuptools docker-library/python#954 * fix test * use better permissions checks --------- Co-authored-by: Sam Onaisi <[email protected]>
This fixes the intent that was meant to happen in #833 of no longer including setuptools (or wheel).
Fixes #952.
If you are using Python 3.12+ and you have build or runtime dependencies on setuptools or wheel, a simple addition to your Dockerfile is all that is needed. (I think longer term, they might make sense as additions to your
requirements.txt
)