diff --git a/python/defs.bzl b/python/defs.bzl index 3fb6b5bb65..bd89f5b1f2 100644 --- a/python/defs.bzl +++ b/python/defs.bzl @@ -15,7 +15,7 @@ load("@bazel_tools//tools/python:srcs_version.bzl", _find_requirements = "find_requirements") load("//python:py_binary.bzl", _py_binary = "py_binary") -load("//python:py_info.bzl", internal_PyInfo = "PyInfo") +load("//python:py_info.bzl", _PyInfo = "PyInfo") load("//python:py_library.bzl", _py_library = "py_library") load("//python:py_runtime.bzl", _py_runtime = "py_runtime") load("//python:py_runtime_info.bzl", internal_PyRuntimeInfo = "PyRuntimeInfo") @@ -26,9 +26,7 @@ load(":py_import.bzl", _py_import = "py_import") # Patching placeholder: end of loads -# Exports of native-defined providers. - -PyInfo = internal_PyInfo +PyInfo = _PyInfo PyRuntimeInfo = internal_PyRuntimeInfo diff --git a/python/private/common/attributes.bzl b/python/private/common/attributes.bzl index b1c54a0973..36f99fb9b4 100644 --- a/python/private/common/attributes.bzl +++ b/python/private/common/attributes.bzl @@ -15,6 +15,7 @@ load(":common.bzl", "union_attrs") load(":providers.bzl", "PyInfo") +load("//python/private:reexports.bzl", "BuiltinPyInfo") load(":py_internal.bzl", "py_internal") load( ":semantics.bzl", @@ -127,7 +128,11 @@ COMMON_ATTRS = union_attrs( PY_SRCS_ATTRS = union_attrs( { "deps": attr.label_list( - providers = [[PyInfo], [_CcInfo]], + providers = [ + [PyInfo], + [_CcInfo], + [BuiltinPyInfo], + ], # TODO(b/228692666): Google-specific; remove these allowances once # the depot is cleaned up. allow_rules = DEPS_ATTR_ALLOW_RULES, diff --git a/python/private/common/common.bzl b/python/private/common/common.bzl index 84b2aa5388..4e3b2e2ba4 100644 --- a/python/private/common/common.bzl +++ b/python/private/common/common.bzl @@ -15,6 +15,7 @@ load(":cc_helper.bzl", "cc_helper") load(":providers.bzl", "PyInfo") +load("//python/private:reexports.bzl", "BuiltinPyInfo") load(":py_internal.bzl", "py_internal") load( ":semantics.bzl", @@ -265,6 +266,10 @@ def collect_imports(ctx, semantics): dep[PyInfo].imports for dep in ctx.attr.deps if PyInfo in dep + ] + [ + dep[BuiltinPyInfo].imports + for dep in ctx.attr.deps + if BuiltinPyInfo in dep ]) def collect_runfiles(ctx, files): @@ -355,8 +360,8 @@ def create_py_info(ctx, *, direct_sources, imports): transitive_sources_files = [] # list of Files for target in ctx.attr.deps: # PyInfo may not be present e.g. cc_library rules. - if PyInfo in target: - info = target[PyInfo] + if PyInfo in target or BuiltinPyInfo in target: + info = _get_py_info(target) transitive_sources_depsets.append(info.transitive_sources) uses_shared_libraries = uses_shared_libraries or info.uses_shared_libraries has_py2_only_sources = has_py2_only_sources or info.has_py2_only_sources @@ -384,8 +389,8 @@ def create_py_info(ctx, *, direct_sources, imports): for target in ctx.attr.data: # TODO(b/234730058): Remove checking for PyInfo in data once depot # cleaned up. - if PyInfo in target: - info = target[PyInfo] + if PyInfo in target or BuiltinPyInfo in target: + info = _get_py_info(target) uses_shared_libraries = info.uses_shared_libraries else: files = target.files.to_list() @@ -396,9 +401,7 @@ def create_py_info(ctx, *, direct_sources, imports): if uses_shared_libraries: break - # TODO(b/203567235): Set `uses_shared_libraries` field, though the Bazel - # docs indicate it's unused in Bazel and may be removed. - py_info = PyInfo( + py_info_kwargs = dict( transitive_sources = depset( transitive = [deps_transitive_sources, direct_sources], ), @@ -410,7 +413,16 @@ def create_py_info(ctx, *, direct_sources, imports): has_py3_only_sources = has_py3_only_sources, uses_shared_libraries = uses_shared_libraries, ) - return py_info, deps_transitive_sources + + # TODO(b/203567235): Set `uses_shared_libraries` field, though the Bazel + # docs indicate it's unused in Bazel and may be removed. + py_info = PyInfo(**py_info_kwargs) + builtin_py_info = BuiltinPyInfo(**py_info_kwargs) + + return py_info, deps_transitive_sources, builtin_py_info + +def _get_py_info(target): + return target[PyInfo] if PyInfo in target else target[BuiltinPyInfo] def create_instrumented_files_info(ctx): return _coverage_common.instrumented_files_info( diff --git a/python/private/common/py_executable.bzl b/python/private/common/py_executable.bzl index d188b3ad98..a24bad6788 100644 --- a/python/private/common/py_executable.bzl +++ b/python/private/common/py_executable.bzl @@ -765,7 +765,7 @@ def _create_providers( PyCcLinkParamsProvider(cc_info = cc_info), ) - py_info, deps_transitive_sources = create_py_info( + py_info, deps_transitive_sources, builtin_py_info = create_py_info( ctx, direct_sources = depset(direct_sources), imports = imports, @@ -780,6 +780,7 @@ def _create_providers( ) providers.append(py_info) + providers.append(builtin_py_info) providers.append(create_output_group_info(py_info.transitive_sources, output_groups)) extra_legacy_providers, extra_providers = semantics.get_extra_providers( diff --git a/python/private/reexports.bzl b/python/private/reexports.bzl index a300a20365..af5b394275 100644 --- a/python/private/reexports.bzl +++ b/python/private/reexports.bzl @@ -12,20 +12,14 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Internal re-exports of built-in symbols. +"""Internal re-exports of builtin symbols. -Currently the definitions here are re-exports of the native rules, "blessed" to -work under `--incompatible_load_python_rules_from_bzl`. As the native rules get -migrated to Starlark, their implementations will be removed from here. +We want to use both the PyInfo defined by builtins and the one defined by +rules_python. Because the builtin symbol is going away, the rules_python +PyInfo symbol is given preference. Unfortunately, that masks the builtin, +so we have to rebind it to another name and load it to make it available again. -We want to re-export a built-in symbol as if it were defined in a Starlark -file, so that users can for instance do: - -``` -load("@rules_python//python:defs.bzl", "PyInfo") -``` - -Unfortunately, we can't just write in defs.bzl +Unfortunately, we can't just write: ``` PyInfo = PyInfo @@ -33,15 +27,14 @@ PyInfo = PyInfo because the declaration of module-level symbol `PyInfo` makes the builtin inaccessible. So instead we access the builtin here and export it under a -different name. Then we can load it from defs.bzl and export it there under -the original name. +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. # buildifier: disable=name-conventions -internal_PyInfo = PyInfo +BuiltinPyInfo = PyInfo # buildifier: disable=name-conventions internal_PyRuntimeInfo = PyRuntimeInfo diff --git a/python/py_info.bzl b/python/py_info.bzl index cbf145d07d..0af35ac320 100644 --- a/python/py_info.bzl +++ b/python/py_info.bzl @@ -15,7 +15,7 @@ """Public entry point for PyInfo.""" load("@rules_python_internal//:rules_python_config.bzl", "config") -load("//python/private:reexports.bzl", "internal_PyInfo") +load("//python/private:reexports.bzl", "BuiltinPyInfo") load("//python/private/common:providers.bzl", _starlark_PyInfo = "PyInfo") -PyInfo = _starlark_PyInfo if config.enable_pystar else internal_PyInfo +PyInfo = _starlark_PyInfo if config.enable_pystar else BuiltinPyInfo diff --git a/tests/base_rules/base_tests.bzl b/tests/base_rules/base_tests.bzl index 99a35f9e5e..0306bfe533 100644 --- a/tests/base_rules/base_tests.bzl +++ b/tests/base_rules/base_tests.bzl @@ -17,17 +17,37 @@ load("@rules_testing//lib:analysis_test.bzl", "analysis_test") load("@rules_testing//lib:truth.bzl", "matching") load("@rules_testing//lib:util.bzl", "PREVENT_IMPLICIT_BUILDING_TAGS", rt_util = "util") load("//python:defs.bzl", "PyInfo") +load("//python/private:reexports.bzl", "BuiltinPyInfo") load("//tests/base_rules:py_info_subject.bzl", "py_info_subject") load("//tests/base_rules:util.bzl", pt_util = "util") _tests = [] +_PRODUCES_PY_INFO_ATTRS = { + "srcs": attr.label_list(allow_files = True), + "imports": attr.string_list(), +} + +def _create_py_info(ctx, provider_type): + return [provider_type( + transitive_sources = depset(ctx.files.srcs), + imports = depset(ctx.attr.imports), + )] + +def _produces_builtin_py_info_impl(ctx): + return _create_py_info(ctx, BuiltinPyInfo) + +_produces_builtin_py_info = rule( + implementation = _produces_builtin_py_info_impl, + attrs = _PRODUCES_PY_INFO_ATTRS, +) + def _produces_py_info_impl(ctx): - return [PyInfo(transitive_sources = depset(ctx.files.srcs))] + return _create_py_info(ctx, BuiltinPyInfo) _produces_py_info = rule( implementation = _produces_py_info_impl, - attrs = {"srcs": attr.label_list(allow_files = True)}, + attrs = _PRODUCES_PY_INFO_ATTRS, ) def _not_produces_py_info_impl(ctx): @@ -38,30 +58,58 @@ _not_produces_py_info = rule( implementation = _not_produces_py_info_impl, ) -def _test_consumes_provider(name, config): +def _py_info_propagation_setup(name, config, produce_py_info_rule, test_impl): rt_util.helper_target( config.base_test_rule, name = name + "_subject", - deps = [name + "_produces_py_info"], + deps = [name + "_produces_builtin_py_info"], ) rt_util.helper_target( - _produces_py_info, - name = name + "_produces_py_info", + produce_py_info_rule, + name = name + "_produces_builtin_py_info", srcs = [rt_util.empty_file(name + "_produce.py")], + imports = ["custom-import"], ) analysis_test( name = name, target = name + "_subject", - impl = _test_consumes_provider_impl, + impl = test_impl, ) -def _test_consumes_provider_impl(env, target): - env.expect.that_target(target).provider( - PyInfo, +def _py_info_propagation_test_impl(env, target, provider_type): + info = env.expect.that_target(target).provider( + provider_type, factory = py_info_subject, - ).transitive_sources().contains("{package}/{test_name}_produce.py") + ) + + info.transitive_sources().contains("{package}/{test_name}_produce.py") + info.imports().contains("custom-import") + +def _test_py_info_propagation_builtin(name, config): + _py_info_propagation_setup( + name, + config, + _produces_builtin_py_info, + _test_py_info_propagation_builtin_impl, + ) + +def _test_py_info_propagation_builtin_impl(env, target): + _py_info_propagation_test_impl(env, target, BuiltinPyInfo) + +_tests.append(_test_py_info_propagation_builtin) + +def _test_py_info_propagation(name, config): + _py_info_propagation_setup( + name, + config, + _produces_py_info, + _test_py_info_propagation_impl, + ) + +def _test_py_info_propagation_impl(env, target): + _py_info_propagation_test_impl(env, target, PyInfo) -_tests.append(_test_consumes_provider) +_tests.append(_test_py_info_propagation) def _test_requires_provider(name, config): rt_util.helper_target( diff --git a/tests/base_rules/py_info_subject.bzl b/tests/base_rules/py_info_subject.bzl index 20185e55e4..b23308c388 100644 --- a/tests/base_rules/py_info_subject.bzl +++ b/tests/base_rules/py_info_subject.bzl @@ -70,7 +70,7 @@ def _py_info_subject_imports(self): Method: PyInfoSubject.imports """ return subjects.collection( - self.actual.imports, + self.actual.imports.to_list(), meta = self.meta.derive("imports()"), )