Skip to content

Commit

Permalink
fix: add missing python version flag attr to pex (#399)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Matt Mackay authored Oct 2, 2024
1 parent 28486e5 commit c355f6b
Showing 1 changed file with 27 additions and 23 deletions.
50 changes: 27 additions & 23 deletions py/private/py_pex_binary.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<PEX_UNPACK_ROOT>/.deps/<name_of_the_package>`
# --distinfo: is only used for determining package metadata
# --source: into `<PEX_UNPACK_ROOT>/<relative_path_to_workspace_root>/<file_name>`
Expand All @@ -37,23 +37,24 @@ 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:
if f.path.find("dist-info", site_packages_i) != -1:
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)]

Expand All @@ -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")
Expand All @@ -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")

Expand All @@ -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(
Expand All @@ -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
Expand All @@ -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,
)
)

0 comments on commit c355f6b

Please sign in to comment.