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.

Author: rickeylev

CHANGELOG.md

@@ -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

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)}
-
-_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)