Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: use rules_python implemented py_runtime, py_runtime_pair, PyRuntimeInfo #1574

Merged
merged 1 commit into from
Nov 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ A brief description of the categories of changes:

## Unreleased

### Changed

* (toolchains) `py_runtime`, `py_runtime_pair`, and `PyRuntimeInfo` now use the
rules_python Starlark implementation, not the one built into Bazel. NOTE: This
only applies to Bazel 6+; Bazel 5 still uses the builtin implementation.

[0.XX.0]: https://github.com/bazelbuild/rules_python/releases/tag/0.XX.0

## [0.27.0] - 2023-11-16
Expand Down
5 changes: 2 additions & 3 deletions python/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,6 @@ bzl_library(
deps = [
"//python/private:util_bzl",
"//python/private/common:py_runtime_macro_bzl",
"@rules_python_internal//:rules_python_config_bzl",
],
)

Expand All @@ -167,7 +166,7 @@ bzl_library(
deps = [
"//python/private:bazel_tools_bzl",
"//python/private:py_runtime_pair_macro_bzl",
"@rules_python_internal//:rules_python_config_bzl",
"//python/private:util_bzl",
],
)

Expand All @@ -176,8 +175,8 @@ bzl_library(
srcs = ["py_runtime_info.bzl"],
deps = [
"//python/private:reexports_bzl",
"//python/private:util_bzl",
"//python/private/common:providers_bzl",
"@rules_python_internal//:rules_python_config_bzl",
],
)

Expand Down
5 changes: 3 additions & 2 deletions python/private/common/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ bzl_library(
srcs = ["providers.bzl"],
deps = [
":semantics_bzl",
"@rules_python_internal//:rules_python_config_bzl",
"//python/private:util_bzl",
],
)

Expand Down Expand Up @@ -171,9 +171,10 @@ bzl_library(
srcs = ["py_runtime_rule.bzl"],
deps = [
":attributes_bzl",
":common_bzl",
":providers_bzl",
":py_internal_bzl",
"//python/private:reexports_bzl",
"//python/private:util_bzl",
"@bazel_skylib//lib:dicts",
"@bazel_skylib//lib:paths",
],
Expand Down
7 changes: 4 additions & 3 deletions python/private/common/providers.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,15 @@
# limitations under the License.
"""Providers for Python rules."""

load("@rules_python_internal//:rules_python_config.bzl", "config")
load("//python/private:util.bzl", "IS_BAZEL_6_OR_HIGHER")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit, what about using bazel_features package for this? I know that this adds an extra dep, but we are already using that on with bzlmod setups.


# TODO: load CcInfo from rules_cc
_CcInfo = CcInfo

DEFAULT_STUB_SHEBANG = "#!/usr/bin/env python3"

DEFAULT_BOOTSTRAP_TEMPLATE = "@bazel_tools//tools/python:python_bootstrap_template.txt"
DEFAULT_BOOTSTRAP_TEMPLATE = Label("//python/private:python_bootstrap_template.txt")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean we can have a custom template on bazel 6? I think that no, but I am not sure if this was just a fixup of previously broken code due to migration.


_PYTHON_VERSION_VALUES = ["PY2", "PY3"]

# Helper to make the provider definitions not crash under Bazel 5.4:
Expand All @@ -31,7 +32,7 @@ _PYTHON_VERSION_VALUES = ["PY2", "PY3"]
# This isn't actually used under Bazel 5.4, so just stub out the values
# to get past the loading phase.
def _define_provider(doc, fields, **kwargs):
if not config.enable_pystar:
if not IS_BAZEL_6_OR_HIGHER:
return provider("Stub, not used", fields = []), None
return provider(doc = doc, fields = fields, **kwargs)

Expand Down
58 changes: 32 additions & 26 deletions python/private/common/py_runtime_rule.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@

load("@bazel_skylib//lib:dicts.bzl", "dicts")
load("@bazel_skylib//lib:paths.bzl", "paths")
load("//python/private:reexports.bzl", "BuiltinPyRuntimeInfo")
load("//python/private:util.bzl", "IS_BAZEL_7_OR_HIGHER")
load(":attributes.bzl", "NATIVE_RULES_ALLOWLIST_ATTRS")
load(":common.bzl", "check_native_allowed")
load(":providers.bzl", "DEFAULT_BOOTSTRAP_TEMPLATE", "DEFAULT_STUB_SHEBANG", _PyRuntimeInfo = "PyRuntimeInfo")
load(":providers.bzl", "DEFAULT_BOOTSTRAP_TEMPLATE", "DEFAULT_STUB_SHEBANG", "PyRuntimeInfo")
load(":py_internal.bzl", "py_internal")

_py_builtins = py_internal

def _py_runtime_impl(ctx):
check_native_allowed(ctx)
interpreter_path = ctx.attr.interpreter_path or None # Convert empty string to None
interpreter = ctx.file.interpreter
if (interpreter_path and interpreter) or (not interpreter_path and not interpreter):
Expand All @@ -44,7 +44,7 @@ def _py_runtime_impl(ctx):
if ctx.attr.coverage_tool:
coverage_di = ctx.attr.coverage_tool[DefaultInfo]

if _py_builtins.is_singleton_depset(coverage_di.files):
if _is_singleton_depset(coverage_di.files):
coverage_tool = coverage_di.files.to_list()[0]
elif coverage_di.files_to_run and coverage_di.files_to_run.executable:
coverage_tool = coverage_di.files_to_run.executable
Expand All @@ -60,39 +60,45 @@ def _py_runtime_impl(ctx):
coverage_files = None

python_version = ctx.attr.python_version
if python_version == "_INTERNAL_SENTINEL":
if ctx.fragments.py.use_toolchains:
fail(
"When using Python toolchains, this attribute must be set explicitly to either 'PY2' " +
"or 'PY3'. See https://github.com/bazelbuild/bazel/issues/7899 for more " +
"information. You can temporarily avoid this error by reverting to the legacy " +
"Python runtime mechanism (`--incompatible_use_python_toolchains=false`).",
)
else:
python_version = ctx.fragments.py.default_python_version

# TODO: Uncomment this after --incompatible_python_disable_py2 defaults to true
# if ctx.fragments.py.disable_py2 and python_version == "PY2":
# fail("Using Python 2 is not supported and disabled; see " +
# "https://github.com/bazelbuild/bazel/issues/15684")

py_runtime_info_kwargs = dict(
interpreter_path = interpreter_path or None,
interpreter = interpreter,
files = runtime_files if hermetic else None,
coverage_tool = coverage_tool,
coverage_files = coverage_files,
python_version = python_version,
stub_shebang = ctx.attr.stub_shebang,
bootstrap_template = ctx.file.bootstrap_template,
)
builtin_py_runtime_info_kwargs = dict(py_runtime_info_kwargs)
if not IS_BAZEL_7_OR_HIGHER:
builtin_py_runtime_info_kwargs.pop("bootstrap_template")
return [
_PyRuntimeInfo(
interpreter_path = interpreter_path or None,
interpreter = interpreter,
files = runtime_files if hermetic else None,
coverage_tool = coverage_tool,
coverage_files = coverage_files,
python_version = python_version,
stub_shebang = ctx.attr.stub_shebang,
bootstrap_template = ctx.file.bootstrap_template,
),
PyRuntimeInfo(**py_runtime_info_kwargs),
# Return the builtin provider for better compatibility.
# 1. There is a legacy code path in py_binary that
# checks for the provider when toolchains aren't used
# 2. It makes it easier to transition from builtins to rules_python
BuiltinPyRuntimeInfo(**builtin_py_runtime_info_kwargs),
DefaultInfo(
files = runtime_files,
runfiles = ctx.runfiles(),
),
]

def _is_singleton_depset(files):
# Bazel 6 doesn't have this helper to optimize detecting singleton depsets.
if _py_builtins:
return _py_builtins.is_singleton_depset(files)
else:
return len(files.to_list()) == 1

# Bind to the name "py_runtime" to preserve the kind/rule_class it shows up
# as elsewhere.
py_runtime = rule(
Expand Down Expand Up @@ -189,8 +195,8 @@ For a platform runtime, this is the absolute path of a Python interpreter on
the target platform. For an in-build runtime this attribute must not be set.
"""),
"python_version": attr.string(
default = "_INTERNAL_SENTINEL",
values = ["PY2", "PY3", "_INTERNAL_SENTINEL"],
default = "PY3",
values = ["PY2", "PY3"],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I thought we are not supporting PY2?

doc = """
Whether this runtime is for Python major version 2 or 3. Valid values are `"PY2"`
and `"PY3"`.
Expand Down
15 changes: 11 additions & 4 deletions python/private/py_runtime_pair_rule.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,19 @@
"""Implementation of py_runtime_pair."""

load("//python:py_runtime_info.bzl", "PyRuntimeInfo")
load("//python/private:reexports.bzl", "BuiltinPyRuntimeInfo")

def _py_runtime_pair_impl(ctx):
if ctx.attr.py2_runtime != None:
py2_runtime = ctx.attr.py2_runtime[PyRuntimeInfo]
py2_runtime = _get_py_runtime_info(ctx.attr.py2_runtime)
if py2_runtime.python_version != "PY2":
fail("The Python runtime in the 'py2_runtime' attribute did not have " +
"version 'PY2'")
else:
py2_runtime = None

if ctx.attr.py3_runtime != None:
py3_runtime = ctx.attr.py3_runtime[PyRuntimeInfo]
py3_runtime = _get_py_runtime_info(ctx.attr.py3_runtime)
if py3_runtime.python_version != "PY3":
fail("The Python runtime in the 'py3_runtime' attribute did not have " +
"version 'PY3'")
Expand All @@ -43,6 +44,12 @@ def _py_runtime_pair_impl(ctx):
py3_runtime = py3_runtime,
)]

def _get_py_runtime_info(target):
if PyRuntimeInfo in target:
return target[PyRuntimeInfo]
else:
return target[BuiltinPyRuntimeInfo]

# buildifier: disable=unused-variable
def _is_py2_disabled(ctx):
# Because this file isn't bundled with Bazel, so we have to conditionally
Expand All @@ -58,15 +65,15 @@ py_runtime_pair = rule(
# The two runtimes are used by the py_binary at runtime, and so need to
# be built for the target platform.
"py2_runtime": attr.label(
providers = [PyRuntimeInfo],
providers = [[PyRuntimeInfo], [BuiltinPyRuntimeInfo]],
cfg = "target",
doc = """\
The runtime to use for Python 2 targets. Must have `python_version` set to
`PY2`.
""",
),
"py3_runtime": attr.label(
providers = [PyRuntimeInfo],
providers = [[PyRuntimeInfo], [BuiltinPyRuntimeInfo]],
cfg = "target",
doc = """\
The runtime to use for Python 3 targets. Must have `python_version` set to
Expand Down
2 changes: 1 addition & 1 deletion python/private/reexports.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,4 @@ different name. Then we can load it from elsewhere.
BuiltinPyInfo = PyInfo

# buildifier: disable=name-conventions
internal_PyRuntimeInfo = PyRuntimeInfo
BuiltinPyRuntimeInfo = PyRuntimeInfo
6 changes: 6 additions & 0 deletions python/private/util.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,9 @@ def add_tag(attrs, tag):
attrs["tags"] = tags + [tag]
else:
attrs["tags"] = [tag]

IS_BAZEL_7_OR_HIGHER = hasattr(native, "starlark_doc_extract")

# Bazel 5.4 has a bug where every access of testing.ExecutionInfo is a
# different object that isn't equal to any other. This is fixed in bazel 6+.
IS_BAZEL_6_OR_HIGHER = testing.ExecutionInfo == testing.ExecutionInfo
5 changes: 2 additions & 3 deletions python/py_runtime.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,11 @@

"""Public entry point for py_runtime."""

load("@rules_python_internal//:rules_python_config.bzl", "config")
load("//python/private:util.bzl", "add_migration_tag")
load("//python/private:util.bzl", "IS_BAZEL_6_OR_HIGHER", "add_migration_tag")
load("//python/private/common:py_runtime_macro.bzl", _starlark_py_runtime = "py_runtime")

# buildifier: disable=native-python
_py_runtime_impl = _starlark_py_runtime if config.enable_pystar else native.py_runtime
_py_runtime_impl = _starlark_py_runtime if IS_BAZEL_6_OR_HIGHER else native.py_runtime

def py_runtime(**attrs):
"""See the Bazel core [py_runtime](https://docs.bazel.build/versions/master/be/python.html#py_runtime) documentation.
Expand Down
6 changes: 3 additions & 3 deletions python/py_runtime_info.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@

"""Public entry point for PyRuntimeInfo."""

load("@rules_python_internal//:rules_python_config.bzl", "config")
load("//python/private:reexports.bzl", "internal_PyRuntimeInfo")
load("//python/private:reexports.bzl", "BuiltinPyRuntimeInfo")
load("//python/private:util.bzl", "IS_BAZEL_6_OR_HIGHER")
load("//python/private/common:providers.bzl", _starlark_PyRuntimeInfo = "PyRuntimeInfo")

PyRuntimeInfo = _starlark_PyRuntimeInfo if config.enable_pystar else internal_PyRuntimeInfo
PyRuntimeInfo = _starlark_PyRuntimeInfo if IS_BAZEL_6_OR_HIGHER else BuiltinPyRuntimeInfo
4 changes: 2 additions & 2 deletions python/py_runtime_pair.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@
"""Public entry point for py_runtime_pair."""

load("@bazel_tools//tools/python:toolchain.bzl", _bazel_tools_impl = "py_runtime_pair")
load("@rules_python_internal//:rules_python_config.bzl", "config")
load("//python/private:py_runtime_pair_macro.bzl", _starlark_impl = "py_runtime_pair")
load("//python/private:util.bzl", "IS_BAZEL_6_OR_HIGHER")

_py_runtime_pair = _bazel_tools_impl if not config.enable_pystar else _starlark_impl
_py_runtime_pair = _starlark_impl if IS_BAZEL_6_OR_HIGHER else _bazel_tools_impl

# NOTE: This doc is copy/pasted from the builtin py_runtime_pair rule so our
# doc generator gives useful API docs.
Expand Down
5 changes: 2 additions & 3 deletions tests/base_rules/util.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
"""Helpers and utilities multiple tests re-use."""

load("@bazel_skylib//lib:structs.bzl", "structs")
load("//python/private:util.bzl", "IS_BAZEL_6_OR_HIGHER") # buildifier: disable=bzl-visibility

# Use this with is_windows()
WINDOWS_ATTR = {"windows": attr.label(default = "@platforms//os:windows")}
Expand Down Expand Up @@ -53,9 +54,7 @@ def _struct_with(s, **kwargs):
return struct(**struct_dict)

def _is_bazel_6_or_higher():
# Bazel 5.4 has a bug where every access of testing.ExecutionInfo is a
# different object that isn't equal to any other. This is fixed in bazel 6+.
return testing.ExecutionInfo == testing.ExecutionInfo
return IS_BAZEL_6_OR_HIGHER

def _is_windows(env):
"""Tell if the target platform is windows.
Expand Down
Loading