From c355f6b4a2ab355fadc55103eeb7ddc6c61958b0 Mon Sep 17 00:00:00 2001 From: Matt Mackay Date: Wed, 2 Oct 2024 09:10:00 -0400 Subject: [PATCH] fix: add missing python version flag attr to pex (#399) Adds the missing `_interpreter_version_flag` attr to the pex binary rule that allows it to resolve the correct Python toolchain in older versions of bazel. --- ### Changes are visible to end-users: yes - Searched for relevant documentation and updated as needed: yes - Breaking change (forces users to change their own code or config): no - Suggested release notes appear below: Ensure `py_pex_binary` can locate the Python version on the toolchain correctly on older bazel versions. ### Test plan - Manual testing; please provide instructions so we can reproduce: Run test suite with bazel 6.5.0 --- py/private/py_pex_binary.bzl | 50 +++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/py/private/py_pex_binary.bzl b/py/private/py_pex_binary.bzl index faf80814..92af316f 100644 --- a/py/private/py_pex_binary.bzl +++ b/py/private/py_pex_binary.bzl @@ -19,12 +19,12 @@ exclude_paths = [ "aspect_rules_py~/py/tools/", # these will match in bzlmod setup with --incompatible_use_plus_in_repo_names flag flipped. "rules_python++python+", - "aspect_rules_py+/py/tools/" + "aspect_rules_py+/py/tools/", ] # determines if the given file is a `distinfo`, `dep` or a `source` # this required to allow PEX to put files into different places. -# +# # --dep: into `/.deps/` # --distinfo: is only used for determining package metadata # --source: into `//` @@ -37,14 +37,14 @@ def _map_srcs(f, workspace): return [] site_packages_i = f.path.find("site-packages") - + # if path contains `site-packages` and there is only two path segments # after it, it will be treated as third party dep. # Here are some examples of path we expect and use and ones we ignore. - # + # # Match: `external/rules_python~~pip~pypi_39_rtoml/site-packages/rtoml-0.11.0.dist-info/INSTALLER` # Reason: It has two `/` after first `site-packages` substring. - # + # # No Match: `external/rules_python~~pip~pypi_39_rtoml/site-packages/rtoml-0.11.0/src/mod/parse.py` # Reason: It has three `/` after first `site-packages` substring. if site_packages_i != -1 and f.path.count("/", site_packages_i) == 2: @@ -52,8 +52,9 @@ def _map_srcs(f, workspace): return ["--distinfo={}".format(f.dirname)] return ["--dep={}".format(f.dirname)] - # If the path does not have a `site-packages` in it, then put it into - # the standard runfiles tree. + # If the path does not have a `site-packages` in it, then put it into + # the standard runfiles tree. + elif site_packages_i == -1: return ["--source={}={}".format(f.path, dest_path)] @@ -74,22 +75,22 @@ def _py_python_pex_impl(ctx): workspace_name = str(ctx.workspace_name) args.add_all( - ctx.attr.inject_env.items(), + ctx.attr.inject_env.items(), map_each = lambda e: "--inject-env={}={}".format(e[0], e[1]), - # this is needed to allow passing a lambda to map_each + # this is needed to allow passing a lambda to map_each allow_closure = True, ) args.add_all( - binary[PyInfo].imports, - format_each = "--sys-path=%s" + binary[PyInfo].imports, + format_each = "--sys-path=%s", ) args.add_all( runfiles.files, map_each = lambda f: _map_srcs(f, workspace_name), uniquify = True, - # this is needed to allow passing a lambda (with workspace_name) to map_each + # this is needed to allow passing a lambda (with workspace_name) to map_each allow_closure = True, ) args.add(binary[DefaultInfo].files_to_run.executable, format = "--executable=%s") @@ -99,10 +100,10 @@ def _py_python_pex_impl(ctx): py_version = py_toolchain.interpreter_version_info args.add_all( [ - constraint.format(major = py_version.major, minor = py_version.minor, patch = py_version.micro) + constraint.format(major = py_version.major, minor = py_version.minor, patch = py_version.micro) for constraint in ctx.attr.python_interpreter_constraints - ], - format_each = "--python-version-constraint=%s" + ], + format_each = "--python-version-constraint=%s", ) args.add(output, format = "--output-file=%s") @@ -116,10 +117,9 @@ def _py_python_pex_impl(ctx): ) return [ - DefaultInfo(files = depset([output]), executable = output) + DefaultInfo(files = depset([output]), executable = output), ] - _attrs = dict({ "binary": attr.label(executable = True, cfg = "target", mandatory = True, doc = "A py_binary target"), "inject_env": attr.string_dict( @@ -128,7 +128,7 @@ _attrs = dict({ ), "python_shebang": attr.string(default = "#!/usr/bin/env python3"), "python_interpreter_constraints": attr.string_list( - default = ["CPython=={major}.{minor}.*"], + default = ["CPython=={major}.{minor}.*"], doc = """\ Python interpreter versions this PEX binary is compatible with. A list of semver strings. The placeholder strings `{major}`, `{minor}`, `{patch}` can be used for gathering version @@ -143,17 +143,21 @@ py_pex_binary ] ) ``` -"""), - "_pex": attr.label(executable = True, cfg = "exec", default = "//py/tools/pex") +""", + ), + # NB: this is read by _resolve_toolchain in py_semantics. + "_interpreter_version_flag": attr.label( + default = "//py:interpreter_version", + ), + "_pex": attr.label(executable = True, cfg = "exec", default = "//py/tools/pex"), }) - py_pex_binary = rule( doc = "Build a pex executable from a py_binary", implementation = _py_python_pex_impl, attrs = _attrs, toolchains = [ - PY_TOOLCHAIN + PY_TOOLCHAIN, ], executable = True, -) \ No newline at end of file +)