refactor: fold per-target python version into base rules (#2541)
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.
diff --git a/CHANGELOG.md b/CHANGELOG.md
index ad3f0a6..e842cf3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -62,6 +62,14 @@
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
diff --git a/python/config_settings/transition.bzl b/python/config_settings/transition.bzl
index a7646dc..14e2a73 100644
--- a/python/config_settings/transition.bzl
+++ b/python/config_settings/transition.bzl
@@ -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)}
+_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}")
+"""
-_transition_python_version = transition(
- implementation = _transition_python_version_impl,
- inputs = [],
- outputs = ["//python/config_settings:python_version"],
-)
+def py_binary(**kwargs):
+ """[DEPRECATED] Deprecated alias for py_binary.
-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
+ Args:
+ **kwargs: keyword args forwarded onto {obj}`py_binary`.
+ """
- # 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
+ deprecation = _DEPRECATION_MESSAGE.format(name = "py_binary")
+ if kwargs.get("deprecation"):
+ deprecation = kwargs.get("deprecation") + "\n\n" + deprecation
+ kwargs["deprecation"] = deprecation
+ _py_binary(**kwargs)
- 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)
+def py_test(**kwargs):
+ """[DEPRECATED] Deprecated alias for py_test.
- 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)
-
-def _strip_suffix(s, suffix):
- if s.endswith(suffix):
- return s[:-len(suffix)]
- else:
- return s
+ 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)
diff --git a/python/private/py_executable.bzl b/python/private/py_executable.bzl
index 20af98d..3b063aa 100644
--- a/python/private/py_executable.bzl
+++ b/python/private/py_executable.bzl
@@ -76,6 +76,7 @@
_py_builtins = py_internal
_EXTERNAL_PATH_PREFIX = "external"
_ZIP_RUNFILES_DIRECTORY_NAME = "runfiles"
+_PYTHON_VERSION_FLAG = str(Label("//python/config_settings:python_version"))
# Bazel 5.4 doesn't have config_common.toolchain_type
_CC_TOOLCHAINS = [config_common.toolchain_type(
@@ -132,16 +133,34 @@
target level.
""",
),
- # TODO(b/203567235): In Google, this attribute is deprecated, and can
- # only effectively be PY3. Externally, with Bazel, this attribute has
- # a separate story.
"python_version": attr.string(
# TODO(b/203567235): In the Java impl, the default comes from
# --python_version. Not clear what the Starlark equivalent is.
- default = "PY3",
- # NOTE: Some tests care about the order of these values.
- values = ["PY2", "PY3"],
- doc = "Defunct, unused, does nothing.",
+ doc = """
+The Python version this target should use.
+
+The value should be in `X.Y` or `X.Y.Z` (or compatible) format. If empty or
+unspecified, the incoming configuration's {obj}`--python_version` flag is
+inherited. For backwards compatibility, the values `PY2` and `PY3` are
+accepted, but treated as an empty/unspecified value.
+
+:::{note}
+In order for the requested version to be used, there must be a
+toolchain configured to match the Python version. If there isn't, then it
+may be silently ignored, or an error may occur, depending on the toolchain
+configuration.
+:::
+
+:::{versionchanged} VERSION_NEXT_PATCH
+
+This attribute was changed from only accepting `PY2` and `PY3` values to
+accepting arbitrary Python versions.
+:::
+""",
+ ),
+ # Required to opt-in to the transition feature.
+ "_allowlist_function_transition": attr.label(
+ default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
),
"_bootstrap_impl_flag": attr.label(
default = "//python/config_settings:bootstrap_impl",
@@ -1009,7 +1028,7 @@
return build_info_files.redacted_build_info_files.to_list()
def _validate_executable(ctx):
- if ctx.attr.python_version != "PY3":
+ if ctx.attr.python_version == "PY2":
fail("It is not allowed to use Python 2")
def _declare_executable_file(ctx):
@@ -1696,9 +1715,26 @@
inherited_environment = inherited_environment,
)
+def _transition_executable_impl(input_settings, attr):
+ settings = {
+ _PYTHON_VERSION_FLAG: input_settings[_PYTHON_VERSION_FLAG],
+ }
+ if attr.python_version and attr.python_version not in ("PY2", "PY3"):
+ settings[_PYTHON_VERSION_FLAG] = attr.python_version
+ return settings
+
+_transition_executable = transition(
+ implementation = _transition_executable_impl,
+ inputs = [
+ _PYTHON_VERSION_FLAG,
+ ],
+ outputs = [
+ _PYTHON_VERSION_FLAG,
+ ],
+)
+
def create_executable_rule(*, attrs, **kwargs):
return create_base_executable_rule(
- ##attrs = dicts.add(EXECUTABLE_ATTRS, attrs),
attrs = attrs,
fragments = ["py", "bazel_py"],
**kwargs
@@ -1720,6 +1756,7 @@
fragments = fragments + ["py"]
kwargs.setdefault("provides", []).append(PyExecutableInfo)
kwargs["exec_groups"] = REQUIRED_EXEC_GROUPS | (kwargs.get("exec_groups") or {})
+ kwargs.setdefault("cfg", _transition_executable)
return rule(
# TODO: add ability to remove attrs, i.e. for imports attr
attrs = dicts.add(EXECUTABLE_ATTRS, attrs),
diff --git a/tests/config_settings/transition/multi_version_tests.bzl b/tests/config_settings/transition/multi_version_tests.bzl
index 5805bab..50b4402 100644
--- a/tests/config_settings/transition/multi_version_tests.bzl
+++ b/tests/config_settings/transition/multi_version_tests.bzl
@@ -109,8 +109,7 @@
# have the "_" prefix on them (those are coming from the underlying
# wrapped binary).
env.expect.that_target(target).default_outputs().contains_exactly([
- "{package}/_{test_name}_subject",
- "{package}/_{test_name}_subject.exe",
+ "{package}/{test_name}_subject.exe",
"{package}/{test_name}_subject",
"{package}/{test_name}_subject.py",
])
@@ -136,8 +135,7 @@
# have the "_" prefix on them (those are coming from the underlying
# wrapped binary).
default_outputs.contains_exactly([
- "{package}/_{test_name}_subject.exe",
- "{package}/_{test_name}_subject.zip",
+ "{package}/{test_name}_subject.exe",
"{package}/{test_name}_subject.py",
"{package}/{test_name}_subject.zip",
])
