Skip to content

Commit 0b69149

Browse files
committed
revert(pypi): use Python for marker eval and METADATA parsing
Summary: - Revert to using Python for marker evaluation during parsing of requirements (partial revert of bazel-contrib#2692). - Use Python to parse whl METADATA. Fixes bazel-contrib#2830
1 parent 9e613d5 commit 0b69149

File tree

6 files changed

+132
-70
lines changed

6 files changed

+132
-70
lines changed

python/private/pypi/evaluate_markers.bzl

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,17 @@ load(":pep508_env.bzl", "env")
1818
load(":pep508_evaluate.bzl", "evaluate")
1919
load(":pep508_platform.bzl", "platform_from_str")
2020
load(":pep508_requirement.bzl", "requirement")
21+
load(":deps.bzl", "record_files")
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

Lines changed: 32 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")
@@ -172,7 +172,28 @@ def _create_whl_repos(
172172
),
173173
extra_pip_args = pip_attr.extra_pip_args,
174174
get_index_urls = get_index_urls,
175-
evaluate_markers = evaluate_markers,
175+
# NOTE @aignas 2024-08-02: , we will execute any interpreter that we find either
176+
# in the PATH or if specified as a label. We will configure the env
177+
# markers when evaluating the requirement lines based on the output
178+
# from the `requirements_files_by_platform` which should have something
179+
# similar to:
180+
# {
181+
# "//:requirements.txt": ["cp311_linux_x86_64", ...]
182+
# }
183+
#
184+
# We know the target python versions that we need to evaluate the
185+
# markers for and thus we don't need to use multiple python interpreter
186+
# instances to perform this manipulation. This function should be executed
187+
# only once by the underlying code to minimize the overhead needed to
188+
# spin up a Python interpreter.
189+
evaluate_markers = lambda module_ctx, requirements: evaluate_markers_py(
190+
module_ctx,
191+
requirements = requirements,
192+
python_interpreter = pip_attr.python_interpreter,
193+
python_interpreter_target = python_interpreter_target,
194+
srcs = pip_attr._evaluate_markers_srcs,
195+
logger = logger,
196+
),
176197
logger = logger,
177198
)
178199

@@ -193,6 +214,7 @@ def _create_whl_repos(
193214
enable_implicit_namespace_pkgs = pip_attr.enable_implicit_namespace_pkgs,
194215
environment = pip_attr.environment,
195216
envsubst = pip_attr.envsubst,
217+
experimental_target_platforms = pip_attr.experimental_target_platforms,
196218
group_deps = group_deps,
197219
group_name = group_name,
198220
pip_data_exclude = pip_attr.pip_data_exclude,
@@ -281,6 +303,7 @@ def _whl_repos(*, requirement, whl_library_args, download_only, netrc, auth_patt
281303
args["urls"] = [distribution.url]
282304
args["sha256"] = distribution.sha256
283305
args["filename"] = distribution.filename
306+
args["experimental_target_platforms"] = requirement.target_platforms
284307

285308
# Pure python wheels or sdists may need to have a platform here
286309
target_platforms = None
@@ -775,6 +798,13 @@ EXPERIMENTAL: this may be removed without notice.
775798
doc = """\
776799
A dict of labels to wheel names that is typically generated by the whl_modifications.
777800
The labels are JSON config files describing the modifications.
801+
""",
802+
),
803+
"_evaluate_markers_srcs": attr.label_list(
804+
default = EVALUATE_MARKERS_SRCS,
805+
doc = """\
806+
The list of labels to use as SRCS for the marker evaluation code. This ensures that the
807+
code will be re-evaluated when any of files in the default changes.
778808
""",
779809
),
780810
}, **ATTRS)

python/private/pypi/generate_whl_library_build_bazel.bzl

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ _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,
@@ -37,7 +39,7 @@ _TEMPLATE = """\
3739
3840
package(default_visibility = ["//visibility:public"])
3941
40-
whl_library_targets_from_requires(
42+
whl_library_targets(
4143
{kwargs}
4244
)
4345
"""
@@ -60,16 +62,8 @@ def generate_whl_library_build_bazel(
6062
"""
6163

6264
loads = [
63-
"""load("@rules_python//python/private/pypi:whl_library_targets.bzl", "whl_library_targets_from_requires")""",
65+
"""load("@rules_python//python/private/pypi:whl_library_targets.bzl", "whl_library_targets")""",
6466
]
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-
)
7367

7468
additional_content = []
7569
if annotation:

python/private/pypi/parse_requirements.bzl

Lines 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

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ load("@bazel_skylib//lib:sets.bzl", "sets")
1818
load("//python/private:normalize_name.bzl", "normalize_name")
1919
load("//python/private:repo_utils.bzl", "REPO_DEBUG_ENV_VAR", "repo_utils")
2020
load("//python/private:text_util.bzl", "render")
21-
load(":evaluate_markers.bzl", "evaluate_markers")
21+
load(":evaluate_markers.bzl", "evaluate_markers_py")
2222
load(":parse_requirements.bzl", "host_platform", "parse_requirements", "select_requirement")
2323
load(":pip_repository_attrs.bzl", "ATTRS")
2424
load(":pypi_repo_utils.bzl", "pypi_repo_utils")
@@ -71,25 +71,6 @@ package(default_visibility = ["//visibility:public"])
7171
exports_files(["requirements.bzl"])
7272
"""
7373

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-
9374
def _pip_repository_impl(rctx):
9475
logger = repo_utils.logger(rctx)
9576
requirements_by_platform = parse_requirements(
@@ -103,7 +84,13 @@ def _pip_repository_impl(rctx):
10384
extra_pip_args = rctx.attr.extra_pip_args,
10485
),
10586
extra_pip_args = rctx.attr.extra_pip_args,
106-
evaluate_markers = lambda requirements: _evaluate_markers(rctx, requirements, logger),
87+
evaluate_markers = lambda rctx, requirements: evaluate_markers_py(
88+
rctx,
89+
requirements = requirements,
90+
python_interpreter = rctx.attr.python_interpreter,
91+
python_interpreter_target = rctx.attr.python_interpreter_target,
92+
srcs = rctx.attr._evaluate_markers_srcs,
93+
),
10794
)
10895
selected_requirements = {}
10996
options = None

python/private/pypi/whl_library.bzl

Lines changed: 24 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,16 @@
1515
""
1616

1717
load("//python/private:auth.bzl", "AUTH_ATTRS", "get_auth")
18-
load("//python/private:bzlmod_enabled.bzl", "BZLMOD_ENABLED")
1918
load("//python/private:envsubst.bzl", "envsubst")
2019
load("//python/private:is_standalone_interpreter.bzl", "is_standalone_interpreter")
2120
load("//python/private:repo_utils.bzl", "REPO_DEBUG_ENV_VAR", "repo_utils")
2221
load(":attrs.bzl", "ATTRS", "use_isolated")
2322
load(":deps.bzl", "all_repo_names", "record_files")
2423
load(":generate_whl_library_build_bazel.bzl", "generate_whl_library_build_bazel")
25-
load(":parse_requirements.bzl", "host_platform")
24+
load(":parse_whl_name.bzl", "parse_whl_name")
2625
load(":patch_whl.bzl", "patch_whl")
27-
load(":pep508_requirement.bzl", "requirement")
2826
load(":pypi_repo_utils.bzl", "pypi_repo_utils")
29-
load(":whl_metadata.bzl", "whl_metadata")
27+
load(":whl_target_platforms.bzl", "whl_target_platforms")
3028

3129
_CPPFLAGS = "CPPFLAGS"
3230
_COMMAND_LINE_TOOLS_PATH_SLUG = "commandlinetools"
@@ -342,14 +340,29 @@ def _whl_library_impl(rctx):
342340
timeout = rctx.attr.timeout,
343341
)
344342

343+
target_platforms = rctx.attr.experimental_target_platforms or []
344+
if target_platforms:
345+
parsed_whl = parse_whl_name(whl_path.basename)
346+
347+
# NOTE @aignas 2023-12-04: if the wheel is a platform specific wheel, we
348+
# only include deps for that target platform
349+
if parsed_whl.platform_tag != "any":
350+
target_platforms = [
351+
p.target_platform
352+
for p in whl_target_platforms(
353+
platform_tag = parsed_whl.platform_tag,
354+
abi_tag = parsed_whl.abi_tag.strip("tm"),
355+
)
356+
]
357+
345358
pypi_repo_utils.execute_checked(
346359
rctx,
347360
op = "whl_library.ExtractWheel({}, {})".format(rctx.attr.name, whl_path),
348361
python = python_interpreter,
349362
arguments = args + [
350363
"--whl-file",
351364
whl_path,
352-
],
365+
] + ["--platform={}".format(p) for p in target_platforms],
353366
srcs = rctx.attr._python_srcs,
354367
environment = environment,
355368
quiet = rctx.attr.quiet,
@@ -384,45 +397,21 @@ def _whl_library_impl(rctx):
384397
)
385398
entry_points[entry_point_without_py] = entry_point_script_name
386399

387-
if BZLMOD_ENABLED:
388-
# The following attributes are unset on bzlmod and we pass data through
389-
# the hub via load statements.
390-
default_python_version = None
391-
target_platforms = []
392-
else:
393-
# NOTE @aignas 2025-04-16: if BZLMOD_ENABLED, we should use
394-
# DEFAULT_PYTHON_VERSION since platforms always come with the actual
395-
# python version otherwise we should use the version of the interpreter
396-
# here. In WORKSPACE `multi_pip_parse` is using an interpreter for each
397-
# `pip_parse` invocation, so we will have the host target platform
398-
# only. Even if somebody would change the code to support
399-
# `experimental_target_platforms`, they would be for a single python
400-
# version. Hence, using the `default_python_version` that we get from the
401-
# interpreter is correct. Hence, we unset the argument if we are on bzlmod.
402-
default_python_version = metadata["python_version"]
403-
target_platforms = rctx.attr.experimental_target_platforms or [host_platform(rctx)]
404-
405-
metadata = whl_metadata(
406-
install_dir = rctx.path("site-packages"),
407-
read_fn = rctx.read,
408-
logger = logger,
409-
)
410-
411400
build_file_contents = generate_whl_library_build_bazel(
412401
name = whl_path.basename,
413-
metadata_name = metadata.name,
414-
metadata_version = metadata.version,
415-
requires_dist = metadata.requires_dist,
416402
dep_template = rctx.attr.dep_template or "@{}{{name}}//:{{target}}".format(rctx.attr.repo_prefix),
417403
entry_points = entry_points,
418-
target_platforms = target_platforms,
419-
default_python_version = default_python_version,
420404
# TODO @aignas 2025-04-14: load through the hub:
405+
dependencies = metadata["deps"],
406+
dependencies_by_platform = metadata["deps_by_platform"],
421407
annotation = None if not rctx.attr.annotation else struct(**json.decode(rctx.read(rctx.attr.annotation))),
422408
data_exclude = rctx.attr.pip_data_exclude,
423-
extras = requirement(rctx.attr.requirement).extras,
424409
group_deps = rctx.attr.group_deps,
425410
group_name = rctx.attr.group_name,
411+
tags = [
412+
"pypi_name={}".format(metadata["name"]),
413+
"pypi_version={}".format(metadata["version"]),
414+
],
426415
)
427416
rctx.file("BUILD.bazel", build_file_contents)
428417

0 commit comments

Comments
 (0)