inc

Author: jbedard

py/private/BUILD.bazel

@@ -48,8 +48,8 @@ bzl_library(
     srcs = ["py_image_layer.bzl"],
     deps = [
         ":py_info",
+        ":py_info_interop",
         "@bazel_lib//lib:transitions",
-        "@rules_python//python:defs_bzl",
         "@tar.bzl//tar:mtree",
         "@tar.bzl//tar:tar",
     ],
@@ -59,9 +59,8 @@ bzl_library(
     name = "pth",
     srcs = ["pth.bzl"],
     deps = [
-        ":py_info",
+        ":py_info_interop",
         "@bazel_skylib//lib:paths",
-        "@rules_python//python:defs_bzl",
     ],
 )
 
@@ -72,11 +71,11 @@ bzl_library(
         ":providers",
         ":pth",
         ":py_info",
+        ":py_info_interop",
         ":py_semantics",
         ":py_wheel",
         "@bazel_skylib//lib:new_sets",
         "@bazel_skylib//lib:types",
-        "@rules_python//python:defs_bzl",
     ],
 )
 
@@ -135,6 +134,15 @@ bzl_library(
     visibility = ["//visibility:public"],
 )
 
+bzl_library(
+    name = "py_info_interop",
+    srcs = ["py_info_interop.bzl"],
+    deps = [
+        ":py_info",
+        "@rules_python//python:defs_bzl",
+    ],
+)
+
 bzl_library(
     name = "providers",
     srcs = ["providers.bzl"],

py/private/pth.bzl

@@ -1,10 +1,7 @@
 """Helper functions for building imports depsets."""
 
 load("@bazel_skylib//lib:paths.bzl", "paths")
-
-# API edge: deps may expose rules_py's PyInfo or native `@rules_python`'s.
-load("@rules_python//python:defs.bzl", RulesPythonPyInfo = "PyInfo")
-load("//py/private:py_info.bzl", "PyInfo")
+load("//py/private:py_info_interop.bzl", "get_py_info", "has_py_info")
 
 def _make_import_path(label, workspace, imp):
     if imp.startswith("/"):
@@ -75,13 +72,12 @@ def make_imports_depset(deps, imports, workspace_name, label = None, extra_impor
     if label and label.workspace_name:
         import_paths.append(label.workspace_name)
 
-    # `deps` come from py_library/py_binary/py_test, whose deps attribute accepts
-    # both rules_py (PyInfo) and native @rules_python (RulesPythonPyInfo) targets;
-    # both expose `imports`.
+    # `deps` may carry rules_py's PyInfo or native @rules_python's; both expose
+    # `imports`. See py_info_interop.bzl.
     transitive = [
-        (target[PyInfo] if PyInfo in target else target[RulesPythonPyInfo]).imports
+        get_py_info(target).imports
         for target in deps
-        if PyInfo in target or RulesPythonPyInfo in target
+        if has_py_info(target)
     ]
 
     return depset(

py/private/py_image_layer.bzl

@@ -32,9 +32,9 @@ Sharing model:
   - Ungrouped pip packages: squashed by the rule into one per-rule tar.
 """
 
-load("@rules_python//python:defs.bzl", RulesPythonPyInfo = "PyInfo")
 load("//py/private:providers.bzl", "PyWheelsInfo")
 load("//py/private:py_info.bzl", "PyInfo")
+load("//py/private:py_info_interop.bzl", "has_py_info")
 load("//py/private/toolchain:types.bzl", "PY_TOOLCHAIN")
 
 _TAR_TOOLCHAIN = "@tar.bzl//tar/toolchain:type"
@@ -399,14 +399,14 @@ def _layer_aspect_impl(target, ctx):
 
     # Skip PyInfo deps (including wheel-leaf targets, which also emit PyInfo) —
     # they self-capture via the aspect.
-    if kind not in _PY_VENV_KINDS and (PyInfo in target or RulesPythonPyInfo in target) and not is_binary:
+    if kind not in _PY_VENV_KINDS and has_py_info(target) and not is_binary:
         own_parts = [target[DefaultInfo].files]
         for attr_name in ("data", "deps"):
             attr_val = getattr(ctx.rule.attr, attr_name, None)
             if not attr_val:
                 continue
             for dep in attr_val:
-                if PyInfo in dep or RulesPythonPyInfo in dep:
+                if has_py_info(dep):
                     continue
                 if DefaultInfo in dep:
                     own_parts.append(dep[DefaultInfo].files)

py/private/py_info_interop.bzl

@@ -0,0 +1,32 @@
+"""Interop between rules_py's `PyInfo` and `@rules_python`'s.
+
+The `deps` attribute shared by py_library/py_binary/py_test (and rules reusing
+their attrs) accepts targets built by either ruleset. rules_py always emits its
+own `PyInfo` (`//py/private:py_info.bzl`); native `@rules_python` targets (e.g.
+a `py_proto_library`) carry `@rules_python`'s. Both expose `transitive_sources`
+and `imports`, which is everything rules_py reads from a foreign dep.
+
+This module is the single place that knows about both providers. Rule code
+calls these accessors at the API edge instead of loading `@rules_python`'s
+provider directly, so a field read added for one provider cannot silently miss
+the other. rules_py never *emits* `@rules_python`'s provider — the reverse
+direction (a `@rules_python` rule consuming a rules_py target) is not supported.
+"""
+
+load("@rules_python//python:defs.bzl", _RulesPythonPyInfo = "PyInfo")
+load("//py/private:py_info.bzl", "PyInfo")
+
+# Re-exported for `providers` constraints on `deps`-style attributes.
+RulesPythonPyInfo = _RulesPythonPyInfo
+
+def has_py_info(target):
+    """Whether the target carries rules_py's or `@rules_python`'s `PyInfo`."""
+    return PyInfo in target or RulesPythonPyInfo in target
+
+def get_py_info(target):
+    """Return the target's `PyInfo` — rules_py's if present, else `@rules_python`'s, else `None`."""
+    if PyInfo in target:
+        return target[PyInfo]
+    if RulesPythonPyInfo in target:
+        return target[RulesPythonPyInfo]
+    return None

py/private/py_library.bzl

@@ -6,15 +6,10 @@ without binding them to a particular version of that package.
 
 load("@bazel_skylib//lib:new_sets.bzl", "sets")
 load("@rules_cc//cc/common:cc_info.bzl", "CcInfo")
-
-# rules_python's PyInfo, referenced in the `deps` provider constraint so native
-# `@rules_python` targets (e.g. py_proto_library) are accepted as deps. rules_py
-# emits its own PyInfo; it reads a dep's sources/imports from either provider via
-# the accessors below.
-load("@rules_python//python:defs.bzl", RulesPythonPyInfo = "PyInfo")
 load("//py/private:providers.bzl", "PyWheelsInfo")
 load("//py/private:pth.bzl", "make_imports_depset")
 load("//py/private:py_info.bzl", "PyInfo")
+load("//py/private:py_info_interop.bzl", "RulesPythonPyInfo", "get_py_info", "has_py_info")
 
 def _make_instrumented_files_info(ctx):
     return coverage_common.instrumented_files_info(
@@ -25,15 +20,15 @@ def _make_instrumented_files_info(ctx):
     )
 
 def _make_srcs_depset(ctx):
-    # `deps` accepts rules_py targets (PyInfo) and native @rules_python targets
-    # (RulesPythonPyInfo); both expose `transitive_sources`.
+    # `deps` may carry rules_py's PyInfo or native @rules_python's; both expose
+    # `transitive_sources`. See py_info_interop.bzl.
     return depset(
         order = "postorder",
         direct = ctx.files.srcs,
         transitive = [
-            (target[PyInfo] if PyInfo in target else target[RulesPythonPyInfo]).transitive_sources
+            get_py_info(target).transitive_sources
             for target in ctx.attr.deps
-            if PyInfo in target or RulesPythonPyInfo in target
+            if has_py_info(target)
         ],
     )
 
@@ -50,8 +45,8 @@ def _make_virtual_depset(ctx):
 
 def _make_resolved_virtual_depset(target):
     transitive = [target[DefaultInfo].files]
-    if PyInfo in target or RulesPythonPyInfo in target:
-        info = target[PyInfo] if PyInfo in target else target[RulesPythonPyInfo]
+    info = get_py_info(target)
+    if info:
         transitive.append(info.transitive_sources)
 
     return depset(
@@ -93,8 +88,8 @@ def _resolve_virtuals(ctx):
         v_srcs.append(_make_resolved_virtual_depset(resolution.target))
         v_runfiles.append(resolution.target[DefaultInfo].default_runfiles.files)
 
-        if PyInfo in resolution.target or RulesPythonPyInfo in resolution.target:
-            info = resolution.target[PyInfo] if PyInfo in resolution.target else resolution.target[RulesPythonPyInfo]
+        info = get_py_info(resolution.target)
+        if info:
             v_imports.append(info.imports)
 
     missing = sets.to_list(sets.difference(sets.make(virtual), sets.make(seen.keys())))
@@ -190,8 +185,8 @@ _attrs = dict({
         # This attribute — shared by py_library, py_binary and py_test — is the
         # public surface that supports rules_python interop: a dep may carry
         # rules_py's PyInfo, or native @rules_python's PyInfo (e.g. a
-        # py_proto_library). RulesPythonPyInfo appears here and at the reads in
-        # this file for exactly that reason; rules_py never emits it.
+        # py_proto_library). Reads go through py_info_interop.bzl's accessors;
+        # rules_py never emits RulesPythonPyInfo.
         providers = [[PyInfo], [RulesPythonPyInfo], [CcInfo]],
     ),
     "data": attr.label_list(

py/tests/import-pathing/BUILD.bazel

@@ -1,13 +1,22 @@
+load("@rules_python//python:defs.bzl", rules_python_py_library = "py_library")
 load("//py:defs.bzl", "py_library")
 load(":tests.bzl", "py_library_import_pathing_test_suite")
 
 # Used by the import-pathing tests to verify a dep's `imports` merge
-# transitively. A rules_py py_library: rules_py reads import roots only from its
-# own PyInfo, so transitive-imports coverage uses a rules_py dep.
+# transitively through rules_py's own PyInfo.
 py_library(
     name = "__import_list_for_test",
     imports = ["baz"],
     tags = ["manual"],
 )
 
+# Same, but a native @rules_python py_library: covers the interop edge where
+# make_imports_depset reads `imports` from @rules_python's PyInfo
+# (see //py/private:py_info_interop.bzl).
+rules_python_py_library(
+    name = "__rules_python_import_list_for_test",
+    imports = ["rp_baz"],
+    tags = ["manual"],
+)
+
 py_library_import_pathing_test_suite()

py/tests/import-pathing/tests.bzl

@@ -77,6 +77,14 @@ def _can_resolve_path_in_workspace_test_impl(ctx):
     asserts.equals(env, ctx.workspace_name, imports[1])
     asserts.equals(env, "aspect_rules_py/foo", imports[2])
 
+    # Transitive imports from a native @rules_python dep are merged too, read
+    # from @rules_python's PyInfo at the API edge (py_info_interop.bzl). Unlike
+    # rules_py, @rules_python does not append the workspace root to imports.
+    fake_ctx = _ctx_with_imports([".."], [ctx.attr.rules_python_import_dep])
+    imports = py_library.make_imports_depset(fake_ctx).to_list()
+    asserts.equals(env, "{}/py/tests/import-pathing/rp_baz".format(ctx.workspace_name), imports[0])
+    asserts.equals(env, "aspect_rules_py/foo", imports[1])
+
     return unittest.end(env)
 
 _can_resolve_path_in_workspace_test = unittest.make(
@@ -85,6 +93,9 @@ _can_resolve_path_in_workspace_test = unittest.make(
         "import_dep": attr.label(
             default = "@//py/tests/import-pathing:__import_list_for_test",
         ),
+        "rules_python_import_dep": attr.label(
+            default = "@//py/tests/import-pathing:__rules_python_import_list_for_test",
+        ),
     },
 )