Skip to content

Commit

Permalink
fold per-target python version into base rule
Browse files Browse the repository at this point in the history
  • Loading branch information
@rickeylev
rickeylev committed Dec 29, 2024
1 parent 2136215 commit 41e1574
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 271 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
265 changes: 7 additions & 258 deletions python/config_settings/transition.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
53 changes: 44 additions & 9 deletions python/private/py_executable.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand All @@ -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),
Expand Down
Loading

0 comments on commit 41e1574

Please sign in to comment.