-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add requirement.constraint
key to support package-wise constraints
#97
Conversation
for more information, see https://pre-commit.ci
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.
Thanks it looks very useful. It might be nice to add automation to override rust constraints later too.
pyodide_build/recipe/builder.py
Outdated
@@ -337,6 +338,33 @@ def _download_and_extract(self) -> None: | |||
shutil.move(self.build_dir / extract_dir_name, self.src_extract_dir) | |||
self.src_dist_dir.mkdir(parents=True, exist_ok=True) | |||
|
|||
def _override_constraints(self) -> str: |
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.
Maybe let's just call it _get_constraints()
?
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.
Thanks for the suggestion. I renamed it to _create_constraints_file
as I thought that using the word get
is confusing regarding that it creates a file internally.
version: "1.0.0" | ||
requirements: | ||
constraint: | ||
- numpy < 2.0 |
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.
Maybe numpy and scipy are bad examples though since realistically this is broken I'd assume?
I mostly agree with the functionality here, but I would like to suggest that we have simpler syntax – instead of having a requirements:
constraint:
- numpy <2
executable:
- rustup
host:
- numpy
run:
- numpy
- scipy and requirements:
executable:
- rustup
host:
- numpy <2
run:
- numpy >= 1.24
- scipy >= 1.12.1, <1.14 The second option seems better, as it's closer to the conda-forge format. Also, this would allow adding constraints separately to the host and runtime dependencies (useful for build-time dependencies such as NumPy in this example), while |
@agriyakhetarpal Thanks for the suggestion! I generally agree that it would be better to use the But the problem is that currently, the |
Resolve: #87
This PR adds a new key in meta.yaml file:
requirement.constraint
.In the build time, pyodide-build will generate a new constraints.txt file in the build directory, and update the PIP_CONSTRAINT env variable to point that file. The new constarints.txt file contains
host_constraints_file + constraints defined in the recipe
.