diff --git a/CHANGELOG.md b/CHANGELOG.md index fd5d455147..caad8b35db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,14 @@ Unreleased changes template. tested by CI, so functionality cannot be guaranteed. * ({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 a7646dcda3..f664adddba 100644 --- a/python/config_settings/transition.bzl +++ b/python/config_settings/transition.bzl @@ -14,266 +14,15 @@ """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) -def _strip_suffix(s, suffix): - if s.endswith(suffix): - return s[:-len(suffix)] - else: - return s +py_binary = _py_binary +py_test = _py_test diff --git a/python/private/py_executable.bzl b/python/private/py_executable.bzl index 40c74100f2..982040feaf 100644 --- a/python/private/py_executable.bzl +++ b/python/private/py_executable.bzl @@ -76,6 +76,7 @@ load( _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,32 @@ Valid values are: 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` format. If empty or unspecified, the +incoming configuration's {obj}`--python_version` flag is inherited. + +:::{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", @@ -1008,7 +1025,7 @@ def _get_build_info(ctx, cc_toolchain): 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): @@ -1691,9 +1708,26 @@ def _create_run_environment_info(ctx, inherited_environment): 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 @@ -1715,6 +1749,7 @@ def create_base_executable_rule(*, attrs, fragments = [], **kwargs): 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 5805bab32d..50b4402fce 100644 --- a/tests/config_settings/transition/multi_version_tests.bzl +++ b/tests/config_settings/transition/multi_version_tests.bzl @@ -109,8 +109,7 @@ def _test_py_binary_windows_build_python_zip_false_impl(env, target): # 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 @@ def _test_py_binary_windows_build_python_zip_true_impl(env, target): # 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", ])