-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Pin build dependencies #13113
base: main
Are you sure you want to change the base?
Pin build dependencies #13113
Conversation
e47503b
to
47b74af
Compare
I've created a similiar build step in my work environment, for maximum reproducability I would reccomend |
47b74af
to
8e6c4e1
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.
I think it's a good start if it works for you. Not exactly how I'd structure it, but that's inconsequential.
As a future improvement, it would be good to have a nox session wrapping whatever generation command is used.
I've been playing with comprehensive pip constraint-based "DIY lock files" to the extremes. Here's some things I've got (sharing for the record, it's much more than what's in the scope of this PR):
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.
.txt.in
? The typical convention is just .in
. Dependabot will recognize pairs of files if they have the same base name.
@@ -0,0 +1,3 @@ | |||
build | |||
twine | |||
setuptools |
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.
So this duplicates what's in pyproject.toml
. Instead, perhaps make use of --all-build-deps
to retrieve it from there? https://pip-tools.readthedocs.io/en/stable/#maximizing-reproducibility.
build | ||
twine |
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.
These two are on the front-end side, so I'd keep them separate.
# This file was autogenerated by uv via the following command: | ||
# uv pip compile --only-binary :all: --generate-hashes build-requirements.txt.in |
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.
Does uv
support the same mechanism of retrieving PEP 517 build deps as pip-tools
? Also, can the settings be put into a config? (.pip-tools.toml
, for example)
"--python", | ||
build_python, | ||
"install", | ||
"-r", |
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.
Perhaps, document why it's not a constraint?
"-r", | |
"-r", # can't be constraint dues to regression @ https://github.com/pypa/pip/issues/9243 |
build_python, | ||
"-m", | ||
"twine", | ||
"check", |
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.
(not really related to the refactoring in this PR, but it should always be strict — would be good to put into a separate PR, I suppose)
"check", | |
"check", | |
"--strict", |
build_python, | ||
"-m", | ||
"build", | ||
"--no-isolation", |
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.
Oh, is this because of PIP_CONSTRAINT
not working with hashes and resolvelib?
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.
So if not for #9243, #4582 (comment) could be used instead.
@@ -29,6 +28,7 @@ | |||
"tests": "tests/requirements.txt", | |||
"common-wheels": "tests/requirements-common_wheels.txt", | |||
} | |||
HERE = Path(__file__).parent |
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.
How about a descriptive name?
HERE = Path(__file__).parent | |
GIT_REPOSITORY_ROOT = Path(__file__).parent |
Coming from the suggestions in #13048, this is a little experiment to see how pinning build deps looks like and see what @pypa/pip-committers and others think about this.
Here we pin build dependencies with pip-compile, use that to create a build environment with the build dependencies and nothing else, and build using
--no-isolation
to make sure (hopefully?) that nothing unpinned is downloaded during the build process.Notably, at this point nox is not pinned. I have also not investigated what nox downloads when creating a session.
cc/ @sethmlarson @webknjaz