feat: add support for passing extra pip args by platform #2745
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,65 @@ | ||||||||||
| # Copyright 2023 The Bazel Authors. All rights reserved. | ||||||||||
| # | ||||||||||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||||||||||
| # you may not use this file except in compliance with the License. | ||||||||||
| # You may obtain a copy of the License at | ||||||||||
| # | ||||||||||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||||||||||
| # | ||||||||||
| # Unless required by applicable law or agreed to in writing, software | ||||||||||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||||||||||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||||||
| # See the License for the specific language governing permissions and | ||||||||||
| # limitations under the License. | ||||||||||
|
|
||||||||||
| "" | ||||||||||
|
|
||||||||||
| def _get_extra_pip_args_for_platform(ctx, extra_pip_args): | ||||||||||
| """Returns the pip args for the given platform from the dict. | ||||||||||
|
|
||||||||||
| Args: | ||||||||||
| ctx: Current repository context. | ||||||||||
| extra_pip_args: (string keyed string list dict): Extra pip arguments. | ||||||||||
|
|
||||||||||
| Returns: | ||||||||||
| List of arguments for the current platform. | ||||||||||
| """ | ||||||||||
|
|
||||||||||
| os = ctx.os.name.replace(" ", "") | ||||||||||
| arch = ctx.os.arch.replace(" ", "") | ||||||||||
|
Comment on lines
+28
to
+29
There was a problem hiding this comment.
Suggested change
This is duplicating code from |
||||||||||
|
|
||||||||||
| # Check if there's an exact match. | ||||||||||
| full_match = "{}_{}".format(os, arch) | ||||||||||
| if full_match in extra_pip_args: | ||||||||||
| return extra_pip_args[full_match] | ||||||||||
|
|
||||||||||
| # Match on the os. | ||||||||||
| os_match = "{}_*".format(os) | ||||||||||
| if os_match in extra_pip_args: | ||||||||||
| return extra_pip_args[os_match] | ||||||||||
|
|
||||||||||
| # Match on the arch. | ||||||||||
| arch_match = "*_{}".format(arch) | ||||||||||
| if arch_match in extra_pip_args: | ||||||||||
| return extra_pip_args[arch_match] | ||||||||||
|
|
||||||||||
| # Wildcard match last to allow for a more specific match. | ||||||||||
| if "*" in extra_pip_args: | ||||||||||
| return extra_pip_args["*"] | ||||||||||
|
|
||||||||||
| return [] | ||||||||||
|
|
||||||||||
| def resolve_extra_pip_args(ctx, extra_pip_args, extra_pip_args_by_platform): | ||||||||||
| """Resolves the given set of extra pip args to the list that should be used on the current platform. | ||||||||||
|
|
||||||||||
| Args: | ||||||||||
| ctx: Current repository or module context. | ||||||||||
| extra_pip_args: (string list): Extra pip arguments. | ||||||||||
| extra_pip_args_by_platform: (string keyed string list dict): Extra pip arguments keyed by platform. | ||||||||||
|
|
||||||||||
| Returns: | ||||||||||
| List of arguments for the current platform. | ||||||||||
| """ | ||||||||||
| if len(extra_pip_args_by_platform): | ||||||||||
| return _get_extra_pip_args_for_platform(ctx, extra_pip_args_by_platform) | ||||||||||
| return extra_pip_args | ||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,7 @@ load("//python/private:repo_utils.bzl", "REPO_DEBUG_ENV_VAR") | |
| load("//python/private:text_util.bzl", "render") | ||
| load(":evaluate_markers.bzl", "evaluate_markers") | ||
| load(":parse_requirements.bzl", "host_platform", "parse_requirements", "select_requirement") | ||
| load(":pip_args.bzl", "resolve_extra_pip_args") | ||
| load(":pip_repository_attrs.bzl", "ATTRS") | ||
| load(":render_pkg_aliases.bzl", "render_pkg_aliases") | ||
| load(":requirements_files_by_platform.bzl", "requirements_files_by_platform") | ||
|
|
@@ -71,6 +72,8 @@ exports_files(["requirements.bzl"]) | |
| """ | ||
|
|
||
| def _pip_repository_impl(rctx): | ||
| extra_pip_args = resolve_extra_pip_args(rctx, rctx.attr.extra_pip_args, rctx.attr.extra_pip_args_by_platform) | ||
|
There was a problem hiding this comment. Do I understand correctly that this will resolve the pip args depending on the host platform? What about resolving based on the target platform? I'd rather pass the The current API is somewhat not great at doing the overrides per platform because we need to have these dictionaries for the overrides and if we add this, then we'll have to maintain this until v2. There was a problem hiding this comment. Can you get the target platform in this context? The way The use case we have here is that we need to force binary only fetching on certain platforms, where as on others we can't do that and allow the sdist to be built. There are some other flags we pass only on Linux too, but I don't recall them, can check tomorrow. There was a problem hiding this comment. I created #2747 to create a better API for this, but in the mean time this is how I can see things fit:
|
||
|
|
||
| requirements_by_platform = parse_requirements( | ||
| rctx, | ||
| requirements_by_platform = requirements_files_by_platform( | ||
|
|
@@ -79,9 +82,9 @@ def _pip_repository_impl(rctx): | |
| requirements_lock = rctx.attr.requirements_lock, | ||
| requirements_osx = rctx.attr.requirements_darwin, | ||
| requirements_windows = rctx.attr.requirements_windows, | ||
| extra_pip_args = extra_pip_args, | ||
| ), | ||
| extra_pip_args = extra_pip_args, | ||
| evaluate_markers = evaluate_markers, | ||
| ) | ||
| selected_requirements = {} | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -21,6 +21,21 @@ repositories.""" | |||||||||||
| load(":attrs.bzl", COMMON_ATTRS = "ATTRS") | ||||||||||||
|
|
||||||||||||
| ATTRS = { | ||||||||||||
| "extra_pip_args_by_platform": attr.string_list_dict( | ||||||||||||
| doc = """Similar to `extra_pip_args`. | ||||||||||||
|
|
||||||||||||
| Extra arguments to pass on to pip on the assoicated platform specifiers. Args must not contain spaces. | ||||||||||||
|
There was a problem hiding this comment. Could you please update https://github.com/bazel-contrib/rules_python/blob/main/docs/pypi-dependencies.md?plain=1#L320 section on how this feature could be used? |
||||||||||||
|
|
||||||||||||
| The keys in the dict take the form `<os>_<cpu>`, wildcards in the form `<os>_*` are supported. | ||||||||||||
| Key values are as those returned in Bazels `repository_os` struct, with spaces removed. | ||||||||||||
|
|
||||||||||||
| Supports environment variables using the syntax `$VARNAME` or | ||||||||||||
| `${VARNAME}` (expanding to empty string if unset) or | ||||||||||||
| `${VARNAME:-default}` (expanding to default if the variable is unset | ||||||||||||
| or empty in the environment), if `"VARNAME"` is listed in the | ||||||||||||
| `envsubst` attribute. See also `envsubst`. | ||||||||||||
|
There was a problem hiding this comment. Please add a note when this has been added and we can link to the attributes in our docs.
Suggested change
|
||||||||||||
| """, | ||||||||||||
| ), | ||||||||||||
| "requirements_by_platform": attr.label_keyed_string_dict( | ||||||||||||
| doc = """\ | ||||||||||||
| The requirements files and the comma delimited list of target platforms as values. | ||||||||||||
|
|
||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
The pre-commit hook updated this 🤷