Skip to content

Commit

Permalink
refactor: fold per-target python version into base rules (#2541)
Browse files Browse the repository at this point in the history 
Today, specifying the Python version for a target requires using the
version-aware
rules in `transition.bzl` (or the generated equivalents bound to a
specific Python
version). With the rules rewritten in Bazel, that functionality can be
moved into
the base rules themselves. Moving the logic into the base rules
simplifies the
implementation and avoids having to re-implement subtle behaviors in the
wrappers
to correctly emulate the wrapped target.

For backwards compatibility, the symbols in `transition.bzl` are left as
aliases
to the underlying rules.
  • Loading branch information
@rickeylev
rickeylev authored Dec 31, 2024
1 parent 475a99e commit 611eda8
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 270 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,14 @@ Unreleased changes template.
not using {bzl:obj}`pip.parse.experimental_index_url_overrides`.
* ({bzl:obj}`pip.parse`) Only query SimpleAPI for packages that have
sha values in the `requirements.txt` file.
* (rules) The version-aware rules have been folded into the base rules and
the version-aware rules are now simply aliases for the base rules. The
`python_version` attribute is still used to specify the Python version.

{#v0-0-0-deprecations}
#### Deprecations
* `//python/config_settings:transitions.bzl` and its `py_binary` and `py_test`
wrappers are deprecated. Use the regular rules instead.

{#v0-0-0-fixed}
### Fixed
Expand Down
291 changes: 34 additions & 257 deletions python/config_settings/transition.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,266 +14,43 @@

"""The transition module contains the rule definitions to wrap py_binary and py_test and transition
them to the desired target platform.
:::{versionchanged} VERSION_NEXT_PATCH
The `py_binary` and `py_test` symbols are aliases to the regular rules. Usages
of them should be changed to load the regular rules directly.
:::
"""

load("@bazel_skylib//lib:dicts.bzl", "dicts")
load("//python:py_binary.bzl", _py_binary = "py_binary")
load("//python:py_info.bzl", "PyInfo")
load("//python:py_runtime_info.bzl", "PyRuntimeInfo")
load("//python:py_test.bzl", _py_test = "py_test")
load("//python/config_settings/private:py_args.bzl", "py_args")
load("//python/private:reexports.bzl", "BuiltinPyInfo", "BuiltinPyRuntimeInfo")

def _transition_python_version_impl(_, attr):
return {"//python/config_settings:python_version": str(attr.python_version)}

_transition_python_version = transition(
implementation = _transition_python_version_impl,
inputs = [],
outputs = ["//python/config_settings:python_version"],
)

def _transition_py_impl(ctx):
target = ctx.attr.target
windows_constraint = ctx.attr._windows_constraint[platform_common.ConstraintValueInfo]
target_is_windows = ctx.target_platform_has_constraint(windows_constraint)
executable = ctx.actions.declare_file(ctx.attr.name + (".exe" if target_is_windows else ""))
ctx.actions.symlink(
is_executable = True,
output = executable,
target_file = target[DefaultInfo].files_to_run.executable,
)
default_outputs = []
if target_is_windows:
# NOTE: Bazel 6 + host=linux + target=windows results in the .exe extension missing
inner_bootstrap_path = _strip_suffix(target[DefaultInfo].files_to_run.executable.short_path, ".exe")
inner_bootstrap = None
inner_zip_file_path = inner_bootstrap_path + ".zip"
inner_zip_file = None
for file in target[DefaultInfo].files.to_list():
if file.short_path == inner_bootstrap_path:
inner_bootstrap = file
elif file.short_path == inner_zip_file_path:
inner_zip_file = file

# TODO: Use `fragments.py.build_python_zip` once Bazel 6 support is dropped.
# Which file the Windows .exe looks for depends on the --build_python_zip file.
# Bazel 7+ has APIs to know the effective value of that flag, but not Bazel 6.
# To work around this, we treat the existence of a .zip in the default outputs
# to mean --build_python_zip=true.
if inner_zip_file:
suffix = ".zip"
underlying_launched_file = inner_zip_file
else:
suffix = ""
underlying_launched_file = inner_bootstrap

if underlying_launched_file:
launched_file_symlink = ctx.actions.declare_file(ctx.attr.name + suffix)
ctx.actions.symlink(
is_executable = True,
output = launched_file_symlink,
target_file = underlying_launched_file,
)
default_outputs.append(launched_file_symlink)

env = {}
for k, v in ctx.attr.env.items():
env[k] = ctx.expand_location(v)

providers = [
DefaultInfo(
executable = executable,
files = depset(default_outputs, transitive = [target[DefaultInfo].files]),
runfiles = ctx.runfiles(default_outputs).merge(target[DefaultInfo].default_runfiles),
),
# Ensure that the binary we're wrapping is included in code coverage.
coverage_common.instrumented_files_info(
ctx,
dependency_attributes = ["target"],
),
target[OutputGroupInfo],
# TODO(f0rmiga): testing.TestEnvironment is deprecated in favour of RunEnvironmentInfo but
# RunEnvironmentInfo is not exposed in Bazel < 5.3.
# https://github.com/bazelbuild/rules_python/issues/901
# https://github.com/bazelbuild/bazel/commit/dbdfa07e92f99497be9c14265611ad2920161483
testing.TestEnvironment(env),
]
if PyInfo in target:
providers.append(target[PyInfo])
if BuiltinPyInfo != None and BuiltinPyInfo in target and PyInfo != BuiltinPyInfo:
providers.append(target[BuiltinPyInfo])

if PyRuntimeInfo in target:
providers.append(target[PyRuntimeInfo])
if BuiltinPyRuntimeInfo != None and BuiltinPyRuntimeInfo in target and PyRuntimeInfo != BuiltinPyRuntimeInfo:
providers.append(target[BuiltinPyRuntimeInfo])
return providers

_COMMON_ATTRS = {
"deps": attr.label_list(
mandatory = False,
),
"env": attr.string_dict(
mandatory = False,
),
"python_version": attr.string(
mandatory = True,
),
"srcs": attr.label_list(
allow_files = True,
mandatory = False,
),
"target": attr.label(
executable = True,
cfg = "target",
mandatory = True,
providers = [PyInfo],
),
# "tools" is a hack here. It should be "data" but "data" is not included by default in the
# location expansion in the same way it is in the native Python rules. The difference on how
# the Bazel deals with those special attributes differ on the LocationExpander, e.g.:
# https://github.com/bazelbuild/bazel/blob/ce611646/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java#L415-L429
#
# Since the default LocationExpander used by ctx.expand_location is not the same as the native
# rules (it doesn't set "allowDataAttributeEntriesInLabel"), we use "tools" temporarily while a
# proper fix in Bazel happens.
#
# A fix for this was proposed in https://github.com/bazelbuild/bazel/pull/16381.
"tools": attr.label_list(
allow_files = True,
mandatory = False,
),
# Required to Opt-in to the transitions feature.
"_allowlist_function_transition": attr.label(
default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
),
"_windows_constraint": attr.label(
default = "@platforms//os:windows",
),
}

_PY_TEST_ATTRS = {
# Magic attribute to help C++ coverage work. There's no
# docs about this; see TestActionBuilder.java
"_collect_cc_coverage": attr.label(
default = "@bazel_tools//tools/test:collect_cc_coverage",
executable = True,
cfg = "exec",
),
# Magic attribute to make coverage work. There's no
# docs about this; see TestActionBuilder.java
"_lcov_merger": attr.label(
default = configuration_field(fragment = "coverage", name = "output_generator"),
executable = True,
cfg = "exec",
),
}

_transition_py_binary = rule(
_transition_py_impl,
attrs = _COMMON_ATTRS | _PY_TEST_ATTRS,
cfg = _transition_python_version,
executable = True,
fragments = ["py"],
)

_transition_py_test = rule(
_transition_py_impl,
attrs = _COMMON_ATTRS | _PY_TEST_ATTRS,
cfg = _transition_python_version,
test = True,
fragments = ["py"],
)

def _py_rule(rule_impl, transition_rule, name, python_version, **kwargs):
pyargs = py_args(name, kwargs)
args = pyargs["args"]
data = pyargs["data"]
env = pyargs["env"]
srcs = pyargs["srcs"]
deps = pyargs["deps"]
main = pyargs["main"]

# Attributes common to all build rules.
# https://bazel.build/reference/be/common-definitions#common-attributes
compatible_with = kwargs.pop("compatible_with", None)
deprecation = kwargs.pop("deprecation", None)
exec_compatible_with = kwargs.pop("exec_compatible_with", None)
exec_properties = kwargs.pop("exec_properties", None)
features = kwargs.pop("features", None)
restricted_to = kwargs.pop("restricted_to", None)
tags = kwargs.pop("tags", None)
target_compatible_with = kwargs.pop("target_compatible_with", None)
testonly = kwargs.pop("testonly", None)
toolchains = kwargs.pop("toolchains", None)
visibility = kwargs.pop("visibility", None)

common_attrs = {
"compatible_with": compatible_with,
"deprecation": deprecation,
"exec_compatible_with": exec_compatible_with,
"exec_properties": exec_properties,
"features": features,
"restricted_to": restricted_to,
"target_compatible_with": target_compatible_with,
"testonly": testonly,
"toolchains": toolchains,
}

# Test-specific extra attributes.
if "env_inherit" in kwargs:
common_attrs["env_inherit"] = kwargs.pop("env_inherit")
if "size" in kwargs:
common_attrs["size"] = kwargs.pop("size")
if "timeout" in kwargs:
common_attrs["timeout"] = kwargs.pop("timeout")
if "flaky" in kwargs:
common_attrs["flaky"] = kwargs.pop("flaky")
if "shard_count" in kwargs:
common_attrs["shard_count"] = kwargs.pop("shard_count")
if "local" in kwargs:
common_attrs["local"] = kwargs.pop("local")

# Binary-specific extra attributes.
if "output_licenses" in kwargs:
common_attrs["output_licenses"] = kwargs.pop("output_licenses")

rule_impl(
name = "_" + name,
args = args,
data = data,
deps = deps,
env = env,
srcs = srcs,
main = main,
tags = ["manual"] + (tags if tags else []),
visibility = ["//visibility:private"],
**dicts.add(common_attrs, kwargs)
)

return transition_rule(
name = name,
args = args,
deps = deps,
env = env,
python_version = python_version,
srcs = srcs,
tags = tags,
target = ":_" + name,
tools = data,
visibility = visibility,
**common_attrs
)

def py_binary(name, python_version, **kwargs):
return _py_rule(_py_binary, _transition_py_binary, name, python_version, **kwargs)

def py_test(name, python_version, **kwargs):
return _py_rule(_py_test, _transition_py_test, name, python_version, **kwargs)
_DEPRECATION_MESSAGE = """
The {name} symbol in @rules_python//python/config_settings:transition.bzl
is deprecated. It is an alias to the regular rule; use it directly instead:
load("@rules_python//python:{name}.bzl", "{name}")
"""

def _strip_suffix(s, suffix):
if s.endswith(suffix):
return s[:-len(suffix)]
else:
return s
def py_binary(**kwargs):
"""[DEPRECATED] Deprecated alias for py_binary.
Args:
**kwargs: keyword args forwarded onto {obj}`py_binary`.
"""

deprecation = _DEPRECATION_MESSAGE.format(name = "py_binary")
if kwargs.get("deprecation"):
deprecation = kwargs.get("deprecation") + "\n\n" + deprecation
kwargs["deprecation"] = deprecation
_py_binary(**kwargs)

def py_test(**kwargs):
"""[DEPRECATED] Deprecated alias for py_test.
Args:
**kwargs: keyword args forwarded onto {obj}`py_binary`.
"""
deprecation = _DEPRECATION_MESSAGE.format(name = "py_test")
if kwargs.get("deprecation"):
deprecation = kwargs.get("deprecation") + "\n\n" + deprecation
kwargs["deprecation"] = deprecation
_py_test(**kwargs)
Loading

0 comments on commit 611eda8

Please sign in to comment.