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

chore: support removal of builtin providers #2274

Merged
merged 17 commits into from
Oct 8, 2024
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
4 changes: 2 additions & 2 deletions python/config_settings/transition.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,12 @@ def _transition_py_impl(ctx):
]
if PyInfo in target:
providers.append(target[PyInfo])
if BuiltinPyInfo in target and PyInfo != BuiltinPyInfo:
if BuiltinPyInfo != None and BuiltinPyInfo in target and PyInfo != BuiltinPyInfo:
providers.append(target[BuiltinPyInfo])

if PyRuntimeInfo in target:
providers.append(target[PyRuntimeInfo])
if BuiltinPyRuntimeInfo in target and PyRuntimeInfo != BuiltinPyRuntimeInfo:
if BuiltinPyRuntimeInfo != None and BuiltinPyRuntimeInfo in target and PyRuntimeInfo != BuiltinPyRuntimeInfo:
providers.append(target[BuiltinPyRuntimeInfo])
return providers

Expand Down
5 changes: 4 additions & 1 deletion python/private/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,10 @@ bzl_library(
visibility = [
"//:__subpackages__",
],
deps = [":bazel_tools_bzl"],
deps = [
":bazel_tools_bzl",
"@rules_python_internal//:rules_python_config_bzl",
],
)

bzl_library(
Expand Down
5 changes: 3 additions & 2 deletions python/private/common/attributes.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -253,15 +253,16 @@ COMMON_ATTRS = union_attrs(
allow_none = True,
)

_MaybeBuiltinPyInfo = [[BuiltinPyInfo]] if BuiltinPyInfo != None else []

# Attributes common to rules accepting Python sources and deps.
PY_SRCS_ATTRS = union_attrs(
{
"deps": attr.label_list(
providers = [
[PyInfo],
[CcInfo],
[BuiltinPyInfo],
],
] + _MaybeBuiltinPyInfo,
# TODO(b/228692666): Google-specific; remove these allowances once
# the depot is cleaned up.
allow_rules = DEPS_ATTR_ALLOW_RULES,
Expand Down
8 changes: 4 additions & 4 deletions python/private/common/common.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ def collect_imports(ctx, semantics):
dep[BuiltinPyInfo].imports
for dep in ctx.attr.deps
if BuiltinPyInfo in dep
])
] if BuiltinPyInfo != None else [])

def collect_runfiles(ctx, files = depset()):
"""Collects the necessary files from the rule's context.
Expand Down Expand Up @@ -374,7 +374,7 @@ def create_py_info(ctx, *, direct_sources, direct_pyc_files, imports):

for target in ctx.attr.deps:
# PyInfo may not be present e.g. cc_library rules.
if PyInfo in target or BuiltinPyInfo in target:
if PyInfo in target or (BuiltinPyInfo != None and BuiltinPyInfo in target):
py_info.merge(_get_py_info(target))
else:
# TODO(b/228692666): Remove this once non-PyInfo targets are no
Expand All @@ -395,7 +395,7 @@ def create_py_info(ctx, *, direct_sources, direct_pyc_files, imports):
for target in ctx.attr.data:
# TODO(b/234730058): Remove checking for PyInfo in data once depot
# cleaned up.
if PyInfo in target or BuiltinPyInfo in target:
if PyInfo in target or (BuiltinPyInfo != None and BuiltinPyInfo in target):
info = _get_py_info(target)
py_info.merge_uses_shared_libraries(info.uses_shared_libraries)
else:
Expand All @@ -410,7 +410,7 @@ def create_py_info(ctx, *, direct_sources, direct_pyc_files, imports):
return py_info.build(), deps_transitive_sources, py_info.build_builtin_py_info()

def _get_py_info(target):
return target[PyInfo] if PyInfo in target else target[BuiltinPyInfo]
return target[PyInfo] if PyInfo in target or BuiltinPyInfo == None else target[BuiltinPyInfo]

def create_instrumented_files_info(ctx):
return _coverage_common.instrumented_files_info(
Expand Down
5 changes: 3 additions & 2 deletions python/private/common/py_executable.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -855,7 +855,7 @@ def _create_providers(
# builtin py_runtime rule or defined their own. We can't directly detect
# the type of the provider object, but the rules_python PyRuntimeInfo
# object has an extra attribute that the builtin one doesn't.
if hasattr(py_runtime_info, "interpreter_version_info"):
if hasattr(py_runtime_info, "interpreter_version_info") and BuiltinPyRuntimeInfo != None:
providers.append(BuiltinPyRuntimeInfo(
interpreter_path = py_runtime_info.interpreter_path,
interpreter = py_runtime_info.interpreter,
Expand Down Expand Up @@ -890,7 +890,8 @@ def _create_providers(
)

providers.append(py_info)
providers.append(builtin_py_info)
if builtin_py_info:
providers.append(builtin_py_info)
providers.append(create_output_group_info(py_info.transitive_sources, output_groups))

extra_providers = semantics.get_extra_providers(
Expand Down
6 changes: 4 additions & 2 deletions python/private/common/py_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -98,14 +98,16 @@ def py_library_impl(ctx, *, semantics):
dependency_transitive_python_sources = deps_transitive_sources,
)

return [
providers = [
DefaultInfo(files = default_outputs, runfiles = runfiles),
py_info,
builtins_py_info,
create_instrumented_files_info(ctx),
PyCcLinkParamsProvider(cc_info = cc_info),
create_output_group_info(py_info.transitive_sources, extra_groups = {}),
]
if builtins_py_info:
providers.append(builtins_py_info)
return providers

_DEFAULT_PY_LIBRARY_DOC = """
A library of Python code that can be depended upon.
Expand Down
14 changes: 8 additions & 6 deletions python/private/common/py_runtime_rule.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -125,18 +125,20 @@ def _py_runtime_impl(ctx):
if not IS_BAZEL_7_OR_HIGHER:
builtin_py_runtime_info_kwargs.pop("bootstrap_template")

return [
providers = [
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 = runfiles,
),
]
if BuiltinPyRuntimeInfo != None and BuiltinPyRuntimeInfo != PyRuntimeInfo:
# 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
providers.append(BuiltinPyRuntimeInfo(**builtin_py_runtime_info_kwargs))
return providers

# Bind to the name "py_runtime" to preserve the kind/rule_class it shows up
# as elsewhere.
Expand Down
15 changes: 15 additions & 0 deletions python/private/internal_config_repo.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ _ENABLE_PYSTAR_DEFAULT = "1"
_CONFIG_TEMPLATE = """\
config = struct(
enable_pystar = {enable_pystar},
BuiltinPyInfo = getattr(getattr(native, "legacy_globals", None), "PyInfo", {builtin_py_info_symbol}),
BuiltinPyRuntimeInfo = getattr(getattr(native, "legacy_globals", None), "PyRuntimeInfo", {builtin_py_runtime_info_symbol}),
BuiltinPyCcLinkParamsProvider = getattr(getattr(native, "legacy_globals", None), "PyCcLinkParamsProvider", {builtin_py_cc_link_params_provider}),
)
"""

Expand Down Expand Up @@ -65,8 +68,20 @@ def _internal_config_repo_impl(rctx):
else:
enable_pystar = False

if native.bazel_version.startswith("8."):
builtin_py_info_symbol = "None"
builtin_py_runtime_info_symbol = "None"
builtin_py_cc_link_params_provider = "None"
else:
builtin_py_info_symbol = "PyInfo"
builtin_py_runtime_info_symbol = "PyRuntimeInfo"
builtin_py_cc_link_params_provider = "PyCcLinkParamsProvider"

rctx.file("rules_python_config.bzl", _CONFIG_TEMPLATE.format(
enable_pystar = enable_pystar,
builtin_py_info_symbol = builtin_py_info_symbol,
builtin_py_runtime_info_symbol = builtin_py_runtime_info_symbol,
builtin_py_cc_link_params_provider = builtin_py_cc_link_params_provider,
))

if enable_pystar:
Expand Down
7 changes: 5 additions & 2 deletions python/private/py_info.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ This field is currently unused in Bazel and may go away in the future.
)

# The "effective" PyInfo is what the canonical //python:py_info.bzl%PyInfo symbol refers to
_EffectivePyInfo = PyInfo if config.enable_pystar else BuiltinPyInfo
_EffectivePyInfo = PyInfo if (config.enable_pystar or BuiltinPyInfo == None) else BuiltinPyInfo

def PyInfoBuilder():
# buildifier: disable=uninitialized
Expand Down Expand Up @@ -206,7 +206,7 @@ def _PyInfoBuilder_merge_all(self, transitive, *, direct = []):
def _PyInfoBuilder_merge_target(self, target):
if PyInfo in target:
self.merge(target[PyInfo])
elif BuiltinPyInfo in target:
elif BuiltinPyInfo != None and BuiltinPyInfo in target:
self.merge(target[BuiltinPyInfo])
return self

Expand Down Expand Up @@ -234,6 +234,9 @@ def _PyInfoBuilder_build(self):
)

def _PyInfoBuilder_build_builtin_py_info(self):
if BuiltinPyInfo == None:
return None

return BuiltinPyInfo(
has_py2_only_sources = self._has_py2_only_sources[0],
has_py3_only_sources = self._has_py3_only_sources[0],
Expand Down
8 changes: 5 additions & 3 deletions python/private/py_runtime_pair_rule.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def _get_py_runtime_info(target):
# py_binary (implemented in Java) performs a type check on the provider
# value to verify it is an instance of the Java-implemented PyRuntimeInfo
# class.
if IS_BAZEL_7_OR_HIGHER and PyRuntimeInfo in target:
if (IS_BAZEL_7_OR_HIGHER and PyRuntimeInfo in target) or BuiltinPyRuntimeInfo == None:
return target[PyRuntimeInfo]
else:
return target[BuiltinPyRuntimeInfo]
Expand All @@ -70,21 +70,23 @@ def _is_py2_disabled(ctx):
return False
return ctx.fragments.py.disable_py2

_MaybeBuiltinPyRuntimeInfo = [[BuiltinPyRuntimeInfo]] if BuiltinPyRuntimeInfo != None else []

py_runtime_pair = rule(
implementation = _py_runtime_pair_impl,
attrs = {
# 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], [BuiltinPyRuntimeInfo]],
providers = [[PyRuntimeInfo]] + _MaybeBuiltinPyRuntimeInfo,
cfg = "target",
doc = """\
The runtime to use for Python 2 targets. Must have `python_version` set to
`PY2`.
""",
),
"py3_runtime": attr.label(
providers = [[PyRuntimeInfo], [BuiltinPyRuntimeInfo]],
providers = [[PyRuntimeInfo]] + _MaybeBuiltinPyRuntimeInfo,
cfg = "target",
doc = """\
The runtime to use for Python 3 targets. Must have `python_version` set to
Expand Down
11 changes: 6 additions & 5 deletions python/private/reexports.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,12 @@ inaccessible. So instead we access the builtin here and export it under a
different name. Then we can load it from elsewhere.
"""

# Don't use underscore prefix, since that would make the symbol local to this
# file only. Use a non-conventional name to emphasize that this is not a public
# symbol.
load("@rules_python_internal//:rules_python_config.bzl", "config")

# NOTE: May be None (Bazel 8 autoloading rules_python)
# buildifier: disable=name-conventions
BuiltinPyInfo = PyInfo
BuiltinPyInfo = config.BuiltinPyInfo

# NOTE: May be None (Bazel 8 autoloading rules_python)
# buildifier: disable=name-conventions
BuiltinPyRuntimeInfo = PyRuntimeInfo
BuiltinPyRuntimeInfo = config.BuiltinPyRuntimeInfo
6 changes: 5 additions & 1 deletion python/py_cc_link_params_info.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,8 @@
load("@rules_python_internal//:rules_python_config.bzl", "config")
load("//python/private/common:providers.bzl", _starlark_PyCcLinkParamsProvider = "PyCcLinkParamsProvider")

PyCcLinkParamsInfo = _starlark_PyCcLinkParamsProvider if config.enable_pystar else PyCcLinkParamsProvider
PyCcLinkParamsInfo = (
_starlark_PyCcLinkParamsProvider if (
config.enable_pystar or config.BuiltinPyCcLinkParamsProvider == None
) else config.BuiltinPyCcLinkParamsProvider
)
2 changes: 1 addition & 1 deletion python/py_info.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@ load("@rules_python_internal//:rules_python_config.bzl", "config")
load("//python/private:py_info.bzl", _starlark_PyInfo = "PyInfo")
load("//python/private:reexports.bzl", "BuiltinPyInfo")

PyInfo = _starlark_PyInfo if config.enable_pystar else BuiltinPyInfo
PyInfo = _starlark_PyInfo if config.enable_pystar or BuiltinPyInfo == None else BuiltinPyInfo
10 changes: 5 additions & 5 deletions tests/base_rules/py_executable_base_tests.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,13 @@ load("@rules_testing//lib:analysis_test.bzl", "analysis_test")
load("@rules_testing//lib:truth.bzl", "matching")
load("@rules_testing//lib:util.bzl", rt_util = "util")
load("//python:py_executable_info.bzl", "PyExecutableInfo")
load("//python/private:reexports.bzl", "BuiltinPyRuntimeInfo") # buildifier: disable=bzl-visibility
load("//python/private:util.bzl", "IS_BAZEL_7_OR_HIGHER") # buildifier: disable=bzl-visibility
load("//tests/base_rules:base_tests.bzl", "create_base_tests")
load("//tests/base_rules:util.bzl", "WINDOWS_ATTR", pt_util = "util")
load("//tests/support:py_executable_info_subject.bzl", "PyExecutableInfoSubject")
load("//tests/support:support.bzl", "CC_TOOLCHAIN", "CROSSTOOL_TOP", "LINUX_X86_64", "WINDOWS_X86_64")

_BuiltinPyRuntimeInfo = PyRuntimeInfo

_tests = []

def _test_basic_windows(name, config):
Expand Down Expand Up @@ -359,9 +358,10 @@ def _test_py_runtime_info_provided_impl(env, target):
# Make sure that the rules_python loaded symbol is provided.
env.expect.that_target(target).has_provider(RulesPythonPyRuntimeInfo)

# For compatibility during the transition, the builtin PyRuntimeInfo should
# also be provided.
env.expect.that_target(target).has_provider(_BuiltinPyRuntimeInfo)
if BuiltinPyRuntimeInfo != None:
# For compatibility during the transition, the builtin PyRuntimeInfo should
# also be provided.
env.expect.that_target(target).has_provider(BuiltinPyRuntimeInfo)

_tests.append(_test_py_runtime_info_provided)

Expand Down
3 changes: 2 additions & 1 deletion tests/base_rules/py_info/py_info_tests.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,8 @@ def _test_py_info_builder_impl(env, targets):
])

check(builder.build())
check(builder.build_builtin_py_info())
if BuiltinPyInfo != None:
check(builder.build_builtin_py_info())

builder.set_has_py2_only_sources(False)
builder.set_has_py3_only_sources(False)
Expand Down