Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 5b9d545

Browse filesBrowse files
aignasaignas
authored
revert(pypi): use Python for marker eval and METADATA parsing (#2834)
Summary: - Revert to using Python for marker evaluation during parsing of requirements (partial revert of #2692). - Use Python to parse whl METADATA. - Bugfix the new simpler algorithm and add a new unit test. Fixes #2830
1 parent 9e613d5 commit 5b9d545
Copy full SHA for 5b9d545

File tree

12 files changed

+304
-96
lines changed
Filter options

12 files changed

+304
-96
lines changed

‎CHANGELOG.md

Copy file name to clipboardExpand all lines: CHANGELOG.md
-9Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,6 @@ END_UNRELEASED_TEMPLATE
103103
* 3.12.9
104104
* 3.13.2
105105
* (pypi) Use `xcrun xcodebuild --showsdks` to find XCode root.
106-
* (pypi) The `bzlmod` extension will now generate smaller lock files for when
107-
using `experimental_index_url`.
108106
* (toolchains) Remove all but `3.8.20` versions of the Python `3.8` interpreter who has
109107
reached EOL. If users still need other versions of the `3.8` interpreter, please supply
110108
the URLs manually {bzl:obj}`python.toolchain` or {bzl:obj}`python_register_toolchains` calls.
@@ -120,13 +118,6 @@ END_UNRELEASED_TEMPLATE
120118
[PR #2746](https://github.com/bazel-contrib/rules_python/pull/2746).
121119
* (rules) {attr}`py_binary.srcs` and {attr}`py_test.srcs` is no longer mandatory when
122120
`main_module` is specified (for `--bootstrap_impl=script`)
123-
* (pypi) From now on the `Requires-Dist` from the wheel metadata is analysed in
124-
the loading phase instead of repository rule phase giving better caching
125-
performance when the target platforms are changed (e.g. target python
126-
versions). This is preparatory work for stabilizing the cross-platform wheel
127-
support. From now on the usage of `experimental_target_platforms` should be
128-
avoided and the `requirements_by_platform` values should be instead used to
129-
specify the target platforms for the given dependencies.
130121

131122
[20250317]: https://github.com/astral-sh/python-build-standalone/releases/tag/20250317
132123

‎python/private/pypi/evaluate_markers.bzl

Copy file name to clipboardExpand all lines: python/private/pypi/evaluate_markers.bzl
+62Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,21 @@
1414

1515
"""A simple function that evaluates markers using a python interpreter."""
1616

17+
load(":deps.bzl", "record_files")
1718
load(":pep508_env.bzl", "env")
1819
load(":pep508_evaluate.bzl", "evaluate")
1920
load(":pep508_platform.bzl", "platform_from_str")
2021
load(":pep508_requirement.bzl", "requirement")
22+
load(":pypi_repo_utils.bzl", "pypi_repo_utils")
23+
24+
# Used as a default value in a rule to ensure we fetch the dependencies.
25+
SRCS = [
26+
# When the version, or any of the files in `packaging` package changes,
27+
# this file will change as well.
28+
record_files["pypi__packaging"],
29+
Label("//python/private/pypi/requirements_parser:resolve_target_platforms.py"),
30+
Label("//python/private/pypi/whl_installer:platform.py"),
31+
]
2132

2233
def evaluate_markers(requirements, python_version = None):
2334
"""Return the list of supported platforms per requirements line.
@@ -37,3 +48,54 @@ def evaluate_markers(requirements, python_version = None):
3748
ret.setdefault(req_string, []).append(platform)
3849

3950
return ret
51+
52+
def evaluate_markers_py(mrctx, *, requirements, python_interpreter, python_interpreter_target, srcs, logger = None):
53+
"""Return the list of supported platforms per requirements line.
54+
55+
Args:
56+
mrctx: repository_ctx or module_ctx.
57+
requirements: list[str] of the requirement file lines to evaluate.
58+
python_interpreter: str, path to the python_interpreter to use to
59+
evaluate the env markers in the given requirements files. It will
60+
be only called if the requirements files have env markers. This
61+
should be something that is in your PATH or an absolute path.
62+
python_interpreter_target: Label, same as python_interpreter, but in a
63+
label format.
64+
srcs: list[Label], the value of SRCS passed from the `rctx` or `mctx` to this function.
65+
logger: repo_utils.logger or None, a simple struct to log diagnostic
66+
messages. Defaults to None.
67+
68+
Returns:
69+
dict of string lists with target platforms
70+
"""
71+
if not requirements:
72+
return {}
73+
74+
in_file = mrctx.path("requirements_with_markers.in.json")
75+
out_file = mrctx.path("requirements_with_markers.out.json")
76+
mrctx.file(in_file, json.encode(requirements))
77+
78+
pypi_repo_utils.execute_checked(
79+
mrctx,
80+
op = "ResolveRequirementEnvMarkers({})".format(in_file),
81+
python = pypi_repo_utils.resolve_python_interpreter(
82+
mrctx,
83+
python_interpreter = python_interpreter,
84+
python_interpreter_target = python_interpreter_target,
85+
),
86+
arguments = [
87+
"-m",
88+
"python.private.pypi.requirements_parser.resolve_target_platforms",
89+
in_file,
90+
out_file,
91+
],
92+
srcs = srcs,
93+
environment = {
94+
"PYTHONPATH": [
95+
Label("@pypi__packaging//:BUILD.bazel"),
96+
Label("//:BUILD.bazel"),
97+
],
98+
},
99+
logger = logger,
100+
)
101+
return json.decode(mrctx.read(out_file))

‎python/private/pypi/extension.bzl

Copy file name to clipboardExpand all lines: python/private/pypi/extension.bzl
+40-2Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ load("//python/private:repo_utils.bzl", "repo_utils")
2424
load("//python/private:semver.bzl", "semver")
2525
load("//python/private:version_label.bzl", "version_label")
2626
load(":attrs.bzl", "use_isolated")
27-
load(":evaluate_markers.bzl", "evaluate_markers")
27+
load(":evaluate_markers.bzl", "evaluate_markers_py", EVALUATE_MARKERS_SRCS = "SRCS")
2828
load(":hub_repository.bzl", "hub_repository", "whl_config_settings_to_json")
2929
load(":parse_requirements.bzl", "parse_requirements")
3030
load(":parse_whl_name.bzl", "parse_whl_name")
@@ -71,6 +71,7 @@ def _create_whl_repos(
7171
whl_overrides,
7272
available_interpreters = INTERPRETER_LABELS,
7373
minor_mapping = MINOR_MAPPING,
74+
evaluate_markers = evaluate_markers_py,
7475
get_index_urls = None):
7576
"""create all of the whl repositories
7677
@@ -85,6 +86,7 @@ def _create_whl_repos(
8586
used during the `repository_rule` and must be always compatible with the host.
8687
minor_mapping: {type}`dict[str, str]` The dictionary needed to resolve the full
8788
python version used to parse package METADATA files.
89+
evaluate_markers: the function used to evaluate the markers.
8890
8991
Returns a {type}`struct` with the following attributes:
9092
whl_map: {type}`dict[str, list[struct]]` the output is keyed by the
@@ -172,7 +174,28 @@ def _create_whl_repos(
172174
),
173175
extra_pip_args = pip_attr.extra_pip_args,
174176
get_index_urls = get_index_urls,
175-
evaluate_markers = evaluate_markers,
177+
# NOTE @aignas 2024-08-02: , we will execute any interpreter that we find either
178+
# in the PATH or if specified as a label. We will configure the env
179+
# markers when evaluating the requirement lines based on the output
180+
# from the `requirements_files_by_platform` which should have something
181+
# similar to:
182+
# {
183+
# "//:requirements.txt": ["cp311_linux_x86_64", ...]
184+
# }
185+
#
186+
# We know the target python versions that we need to evaluate the
187+
# markers for and thus we don't need to use multiple python interpreter
188+
# instances to perform this manipulation. This function should be executed
189+
# only once by the underlying code to minimize the overhead needed to
190+
# spin up a Python interpreter.
191+
evaluate_markers = lambda module_ctx, requirements: evaluate_markers(
192+
module_ctx,
193+
requirements = requirements,
194+
python_interpreter = pip_attr.python_interpreter,
195+
python_interpreter_target = python_interpreter_target,
196+
srcs = pip_attr._evaluate_markers_srcs,
197+
logger = logger,
198+
),
176199
logger = logger,
177200
)
178201

@@ -193,6 +216,7 @@ def _create_whl_repos(
193216
enable_implicit_namespace_pkgs = pip_attr.enable_implicit_namespace_pkgs,
194217
environment = pip_attr.environment,
195218
envsubst = pip_attr.envsubst,
219+
experimental_target_platforms = pip_attr.experimental_target_platforms,
196220
group_deps = group_deps,
197221
group_name = group_name,
198222
pip_data_exclude = pip_attr.pip_data_exclude,
@@ -281,6 +305,13 @@ def _whl_repos(*, requirement, whl_library_args, download_only, netrc, auth_patt
281305
args["urls"] = [distribution.url]
282306
args["sha256"] = distribution.sha256
283307
args["filename"] = distribution.filename
308+
args["experimental_target_platforms"] = [
309+
# Get rid of the version fot the target platforms because we are
310+
# passing the interpreter any way. Ideally we should search of ways
311+
# how to pass the target platforms through the hub repo.
312+
p.partition("_")[2]
313+
for p in requirement.target_platforms
314+
]
284315

285316
# Pure python wheels or sdists may need to have a platform here
286317
target_platforms = None
@@ -775,6 +806,13 @@ EXPERIMENTAL: this may be removed without notice.
775806
doc = """\
776807
A dict of labels to wheel names that is typically generated by the whl_modifications.
777808
The labels are JSON config files describing the modifications.
809+
""",
810+
),
811+
"_evaluate_markers_srcs": attr.label_list(
812+
default = EVALUATE_MARKERS_SRCS,
813+
doc = """\
814+
The list of labels to use as SRCS for the marker evaluation code. This ensures that the
815+
code will be re-evaluated when any of files in the default changes.
778816
""",
779817
),
780818
}, **ATTRS)

‎python/private/pypi/generate_whl_library_build_bazel.bzl

Copy file name to clipboardExpand all lines: python/private/pypi/generate_whl_library_build_bazel.bzl
+25-10Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,14 @@ _RENDER = {
2121
"copy_files": render.dict,
2222
"data": render.list,
2323
"data_exclude": render.list,
24+
"dependencies": render.list,
25+
"dependencies_by_platform": lambda x: render.dict(x, value_repr = render.list),
2426
"entry_points": render.dict,
2527
"extras": render.list,
2628
"group_deps": render.list,
2729
"requires_dist": render.list,
2830
"srcs_exclude": render.list,
31+
"tags": render.list,
2932
"target_platforms": lambda x: render.list(x) if x else "target_platforms",
3033
}
3134

@@ -37,7 +40,7 @@ _TEMPLATE = """\
3740
3841
package(default_visibility = ["//visibility:public"])
3942
40-
whl_library_targets_from_requires(
43+
{fn}(
4144
{kwargs}
4245
)
4346
"""
@@ -59,17 +62,28 @@ def generate_whl_library_build_bazel(
5962
A complete BUILD file as a string
6063
"""
6164

65+
fn = "whl_library_targets"
66+
if kwargs.get("tags"):
67+
# legacy path
68+
unsupported_args = [
69+
"requires",
70+
"metadata_name",
71+
"metadata_version",
72+
]
73+
else:
74+
fn = "{}_from_requires".format(fn)
75+
unsupported_args = [
76+
"dependencies",
77+
"dependencies_by_platform",
78+
]
79+
80+
for arg in unsupported_args:
81+
if kwargs.get(arg):
82+
fail("BUG, unsupported arg: '{}'".format(arg))
83+
6284
loads = [
63-
"""load("@rules_python//python/private/pypi:whl_library_targets.bzl", "whl_library_targets_from_requires")""",
85+
"""load("@rules_python//python/private/pypi:whl_library_targets.bzl", "{}")""".format(fn),
6486
]
65-
if not kwargs.setdefault("target_platforms", None):
66-
dep_template = kwargs["dep_template"]
67-
loads.append(
68-
"load(\"{}\", \"{}\")".format(
69-
dep_template.format(name = "", target = "config.bzl"),
70-
"target_platforms",
71-
),
72-
)
7387

7488
additional_content = []
7589
if annotation:
@@ -87,6 +101,7 @@ def generate_whl_library_build_bazel(
87101
[
88102
_TEMPLATE.format(
89103
loads = "\n".join(loads),
104+
fn = fn,
90105
kwargs = render.indent("\n".join([
91106
"{} = {},".format(k, _RENDER.get(k, repr)(v))
92107
for k, v in sorted(kwargs.items())

‎python/private/pypi/parse_requirements.bzl

Copy file name to clipboardExpand all lines: python/private/pypi/parse_requirements.bzl
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ def parse_requirements(
8080
8181
The second element is extra_pip_args should be passed to `whl_library`.
8282
"""
83-
evaluate_markers = evaluate_markers or (lambda _: {})
83+
evaluate_markers = evaluate_markers or (lambda _ctx, _requirements: {})
8484
options = {}
8585
requirements = {}
8686
for file, plats in requirements_by_platform.items():
@@ -156,7 +156,7 @@ def parse_requirements(
156156
# to do, we could use Python to parse the requirement lines and infer the
157157
# URL of the files to download things from. This should be important for
158158
# VCS package references.
159-
env_marker_target_platforms = evaluate_markers(reqs_with_env_markers)
159+
env_marker_target_platforms = evaluate_markers(ctx, reqs_with_env_markers)
160160
if logger:
161161
logger.debug(lambda: "Evaluated env markers from:\n{}\n\nTo:\n{}".format(
162162
reqs_with_env_markers,

‎python/private/pypi/pip_repository.bzl

Copy file name to clipboardExpand all lines: python/private/pypi/pip_repository.bzl
+16-24Lines changed: 16 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,11 @@
1616

1717
load("@bazel_skylib//lib:sets.bzl", "sets")
1818
load("//python/private:normalize_name.bzl", "normalize_name")
19-
load("//python/private:repo_utils.bzl", "REPO_DEBUG_ENV_VAR", "repo_utils")
19+
load("//python/private:repo_utils.bzl", "REPO_DEBUG_ENV_VAR")
2020
load("//python/private:text_util.bzl", "render")
21-
load(":evaluate_markers.bzl", "evaluate_markers")
21+
load(":evaluate_markers.bzl", "evaluate_markers_py", EVALUATE_MARKERS_SRCS = "SRCS")
2222
load(":parse_requirements.bzl", "host_platform", "parse_requirements", "select_requirement")
2323
load(":pip_repository_attrs.bzl", "ATTRS")
24-
load(":pypi_repo_utils.bzl", "pypi_repo_utils")
2524
load(":render_pkg_aliases.bzl", "render_pkg_aliases")
2625
load(":requirements_files_by_platform.bzl", "requirements_files_by_platform")
2726

@@ -71,27 +70,7 @@ package(default_visibility = ["//visibility:public"])
7170
exports_files(["requirements.bzl"])
7271
"""
7372

74-
def _evaluate_markers(rctx, requirements, logger = None):
75-
python_interpreter = _get_python_interpreter_attr(rctx)
76-
stdout = pypi_repo_utils.execute_checked_stdout(
77-
rctx,
78-
op = "GetPythonVersionForMarkerEval",
79-
python = python_interpreter,
80-
arguments = [
81-
# Run the interpreter in isolated mode, this options implies -E, -P and -s.
82-
# Ensures environment variables are ignored that are set in userspace, such as PYTHONPATH,
83-
# which may interfere with this invocation.
84-
"-I",
85-
"-c",
86-
"import sys; print(f'{sys.version_info[0]}.{sys.version_info[1]}.{sys.version_info[2]}', end='')",
87-
],
88-
srcs = [],
89-
logger = logger,
90-
)
91-
return evaluate_markers(requirements, python_version = stdout)
92-
9373
def _pip_repository_impl(rctx):
94-
logger = repo_utils.logger(rctx)
9574
requirements_by_platform = parse_requirements(
9675
rctx,
9776
requirements_by_platform = requirements_files_by_platform(
@@ -103,7 +82,13 @@ def _pip_repository_impl(rctx):
10382
extra_pip_args = rctx.attr.extra_pip_args,
10483
),
10584
extra_pip_args = rctx.attr.extra_pip_args,
106-
evaluate_markers = lambda requirements: _evaluate_markers(rctx, requirements, logger),
85+
evaluate_markers = lambda rctx, requirements: evaluate_markers_py(
86+
rctx,
87+
requirements = requirements,
88+
python_interpreter = rctx.attr.python_interpreter,
89+
python_interpreter_target = rctx.attr.python_interpreter_target,
90+
srcs = rctx.attr._evaluate_markers_srcs,
91+
),
10792
)
10893
selected_requirements = {}
10994
options = None
@@ -249,6 +234,13 @@ file](https://github.com/bazel-contrib/rules_python/blob/main/examples/pip_repos
249234
_template = attr.label(
250235
default = ":requirements.bzl.tmpl.workspace",
251236
),
237+
_evaluate_markers_srcs = attr.label_list(
238+
default = EVALUATE_MARKERS_SRCS,
239+
doc = """\
240+
The list of labels to use as SRCS for the marker evaluation code. This ensures that the
241+
code will be re-evaluated when any of files in the default changes.
242+
""",
243+
),
252244
**ATTRS
253245
),
254246
doc = """Accepts a locked/compiled requirements file and installs the dependencies listed within.

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.