Skip to content

Commit

Permalink
Use rules_python implemented py_runtime
Browse files Browse the repository at this point in the history
  • Loading branch information
rickeylev committed Nov 22, 2023
1 parent f676656 commit 63ab062
Show file tree
Hide file tree
Showing 11 changed files with 126 additions and 63 deletions.
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
3 changes: 1 addition & 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,7 +171,6 @@ bzl_library(
srcs = ["py_runtime_rule.bzl"],
deps = [
":attributes_bzl",
":common_bzl",
":providers_bzl",
":py_internal_bzl",
"@bazel_skylib//lib:dicts",
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")

# 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")

_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
25 changes: 10 additions & 15 deletions python/private/common/py_runtime_rule.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,12 @@
load("@bazel_skylib//lib:dicts.bzl", "dicts")
load("@bazel_skylib//lib:paths.bzl", "paths")
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(":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 +42,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,16 +58,6 @@ 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":
Expand All @@ -93,6 +81,13 @@ def _py_runtime_impl(ctx):
),
]

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 +184,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"],
doc = """
Whether this runtime is for Python major version 2 or 3. Valid values are `"PY2"`
and `"PY3"`.
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")

# 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
121 changes: 93 additions & 28 deletions tests/py_runtime/py_runtime_tests.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,20 @@ _SKIP_TEST = {
"target_compatible_with": ["@platforms//:incompatible"],
}

def _simple_binary_impl(ctx):
output = ctx.actions.declare_file(ctx.label.name)
ctx.actions.write(output, "", is_executable = True)
return [DefaultInfo(
executable = output,
runfiles = ctx.runfiles(ctx.files.data),
)]

_simple_binary = rule(
implementation = _simple_binary_impl,
attrs = {"data": attr.label_list(allow_files = True)},
executable = True,
)

def _test_bootstrap_template(name):
# The bootstrap_template arg isn't present in older Bazel versions, so
# we have to conditionally pass the arg and mark the test incompatible.
Expand Down Expand Up @@ -123,86 +137,137 @@ def _test_cannot_specify_files_for_system_interpreter_impl(env, target):

_tests.append(_test_cannot_specify_files_for_system_interpreter)

def _test_in_build_interpreter(name):
def _test_coverage_tool_executable(name):
if br_util.is_bazel_6_or_higher():
py_runtime_kwargs = {
"coverage_tool": name + "_coverage_tool",
}
attr_values = {}
else:
py_runtime_kwargs = {}
attr_values = _SKIP_TEST

rt_util.helper_target(
py_runtime,
name = name + "_subject",
interpreter = "fake_interpreter",
python_version = "PY3",
files = ["file1.txt"],
interpreter_path = "/bogus",
**py_runtime_kwargs
)
rt_util.helper_target(
_simple_binary,
name = name + "_coverage_tool",
data = ["coverage_file1.txt", "coverage_file2.txt"],
)
analysis_test(
name = name,
target = name + "_subject",
impl = _test_in_build_interpreter_impl,
impl = _test_coverage_tool_executable_impl,
attr_values = attr_values,
)

def _test_in_build_interpreter_impl(env, target):
def _test_coverage_tool_executable_impl(env, target):
info = env.expect.that_target(target).provider(PyRuntimeInfo, factory = py_runtime_info_subject)
info.python_version().equals("PY3")
info.files().contains_predicate(matching.file_basename_equals("file1.txt"))
info.interpreter().path().contains("fake_interpreter")
info.coverage_tool().short_path_equals("{package}/{test_name}_coverage_tool")
info.coverage_files().contains_exactly([
"{package}/{test_name}_coverage_tool",
"{package}/coverage_file1.txt",
"{package}/coverage_file2.txt",
])

_tests.append(_test_in_build_interpreter)
_tests.append(_test_coverage_tool_executable)

def _test_must_have_either_inbuild_or_system_interpreter(name):
def _test_coverage_tool_plain_files(name):
if br_util.is_bazel_6_or_higher():
py_runtime_kwargs = {}
attr_values = {}
else:
py_runtime_kwargs = {
"interpreter_path": "/some/path",
"coverage_tool": name + "_coverage_tool",
}
attr_values = {}
else:
py_runtime_kwargs = {}
attr_values = _SKIP_TEST
rt_util.helper_target(
py_runtime,
name = name + "_subject",
python_version = "PY3",
interpreter_path = "/bogus",
**py_runtime_kwargs
)
rt_util.helper_target(
native.filegroup,
name = name + "_coverage_tool",
srcs = ["coverage_tool.py"],
data = ["coverage_file1.txt", "coverage_file2.txt"],
)
analysis_test(
name = name,
target = name + "_subject",
impl = _test_must_have_either_inbuild_or_system_interpreter_impl,
expect_failure = True,
impl = _test_coverage_tool_plain_files_impl,
attr_values = attr_values,
)

def _test_must_have_either_inbuild_or_system_interpreter_impl(env, target):
env.expect.that_target(target).failures().contains_predicate(
matching.str_matches("one of*interpreter*interpreter_path"),
def _test_coverage_tool_plain_files_impl(env, target):
info = env.expect.that_target(target).provider(PyRuntimeInfo, factory = py_runtime_info_subject)
info.coverage_tool().short_path_equals("{package}/coverage_tool.py")
info.coverage_files().contains_exactly([
"{package}/coverage_tool.py",
"{package}/coverage_file1.txt",
"{package}/coverage_file2.txt",
])

_tests.append(_test_coverage_tool_plain_files)

def _test_in_build_interpreter(name):
rt_util.helper_target(
py_runtime,
name = name + "_subject",
interpreter = "fake_interpreter",
python_version = "PY3",
files = ["file1.txt"],
)
analysis_test(
name = name,
target = name + "_subject",
impl = _test_in_build_interpreter_impl,
)

_tests.append(_test_must_have_either_inbuild_or_system_interpreter)
def _test_in_build_interpreter_impl(env, target):
info = env.expect.that_target(target).provider(PyRuntimeInfo, factory = py_runtime_info_subject)
info.python_version().equals("PY3")
info.files().contains_predicate(matching.file_basename_equals("file1.txt"))
info.interpreter().path().contains("fake_interpreter")

_tests.append(_test_in_build_interpreter)

def _test_python_version_required(name):
# Bazel 5.4 will entirely crash when python_version is missing.
def _test_must_have_either_inbuild_or_system_interpreter(name):
if br_util.is_bazel_6_or_higher():
py_runtime_kwargs = {}
attr_values = {}
else:
py_runtime_kwargs = {"python_version": "PY3"}
py_runtime_kwargs = {
"interpreter_path": "/some/path",
}
attr_values = _SKIP_TEST
rt_util.helper_target(
py_runtime,
name = name + "_subject",
interpreter_path = "/math/pi",
python_version = "PY3",
**py_runtime_kwargs
)
analysis_test(
name = name,
target = name + "_subject",
impl = _test_python_version_required_impl,
impl = _test_must_have_either_inbuild_or_system_interpreter_impl,
expect_failure = True,
attr_values = attr_values,
)

def _test_python_version_required_impl(env, target):
def _test_must_have_either_inbuild_or_system_interpreter_impl(env, target):
env.expect.that_target(target).failures().contains_predicate(
matching.str_matches("must be set*PY2*PY3"),
matching.str_matches("one of*interpreter*interpreter_path"),
)

_tests.append(_test_python_version_required)
_tests.append(_test_must_have_either_inbuild_or_system_interpreter)

def _test_system_interpreter(name):
rt_util.helper_target(
Expand Down

0 comments on commit 63ab062

Please sign in to comment.