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

unpin pycurl #3371

Merged
merged 1 commit into from
Mar 21, 2024
Merged

unpin pycurl #3371

merged 1 commit into from
Mar 21, 2024

Conversation

minrk
Copy link
Member

@minrk minrk commented Mar 21, 2024

and make sure we build it from source with --no-binary pycurl

Also make sure to use build-stage wheels by adding --no-index to pip install.

avoids installing higher-priority wheels from PyPI instead of what we built (i.e. pycurl)

--no-index was the part missing in #3365, because the manylinux_2_28 wheel from PyPI was preferred at install time to the linux_arch wheel that we built. --no-index ensures no contact with PyPI at all at install time.

This is a low priority fix to unpin versions going forward, no need to trigger a new patch release.

make sure to use build-stage wheels by adding --no-index to pip install

avoids installing higher-priority wheels from PyPI instead of what we built (i.e. pycurl)
@@ -21,9 +21,11 @@ FROM python:3.11-bullseye as build-stage
COPY requirements.txt requirements.txt
ARG PIP_CACHE_DIR=/tmp/pip-cache
RUN --mount=type=cache,target=${PIP_CACHE_DIR} \
pip install build \
Copy link
Member

Choose a reason for hiding this comment

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

I'm not confident why we installed build here, so I'm not confident on seeing it removed - can you motivate the change?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was added but never used without comment in #2733.

I believe it was added by mistake, copying what we do in binderhub, which includes building the binderhub wheel itself, which used to be done with python3 -m build --wheel . (not anymore). If you're not building local packages from source via python3 -m build (and not pip wheel, which can also do the same thing), build is never used.

From what I can find, it's here because we used to invoke it here in binderhub, but since https://github.com/jupyterhub/binderhub/pull/1535/files switched to invoking pip wheel, build has not been used in either binderhub or z2jh. It's a tiny unused pure-python package in the build stage, so nobody noticed the very tiny bit of wasted time.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, thank you @minrk!

@consideRatio consideRatio merged commit 3b6df9f into jupyterhub:main Mar 21, 2024
15 checks passed
consideRatio pushed a commit to jupyterhub/helm-chart that referenced this pull request Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants