Skip to content
Draft
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
15 changes: 13 additions & 2 deletions py/private/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ bzl_library(
srcs = ["py_image_layer.bzl"],
deps = [
":py_info",
":py_info_interop",
"@bazel_lib//lib:transitions",
"@tar.bzl//tar:mtree",
"@tar.bzl//tar:tar",
Expand All @@ -58,7 +59,7 @@ bzl_library(
name = "pth",
srcs = ["pth.bzl"],
deps = [
":py_info",
":py_info_interop",
"@bazel_skylib//lib:paths",
],
)
Expand All @@ -70,6 +71,7 @@ bzl_library(
":providers",
":pth",
":py_info",
":py_info_interop",
":py_semantics",
":py_wheel",
"@bazel_skylib//lib:new_sets",
Expand Down Expand Up @@ -123,13 +125,22 @@ bzl_library(
],
)

# keep
bzl_library(
name = "py_info",
srcs = ["py_info.bzl"],
# PyInfo is re-exported as public API from //py:defs.bzl, so its bzl_library
# is public too, allowing sibling trees (e.g. //uv) to depend on it.
visibility = ["//visibility:public"],
deps = ["@rules_python//python:defs_bzl"],
)

bzl_library(
name = "py_info_interop",
srcs = ["py_info_interop.bzl"],
deps = [
":py_info",
"@rules_python//python:defs_bzl",
],
)

bzl_library(
Expand Down
8 changes: 0 additions & 8 deletions py/private/providers.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,3 @@ def make_wheel_record(
metadata_top_levels = tuple(metadata_top_levels),
cs_claims = tuple(cs_claims),
)

PyVirtualInfo = provider(
doc = "FIXME",
fields = {
"dependencies": "Depset of required virtual dependencies, independent of their resolution status",
"resolutions": "FIXME",
},
)
16 changes: 10 additions & 6 deletions py/private/pth.bzl
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"""Helper functions for building imports depsets."""

load("@bazel_skylib//lib:paths.bzl", "paths")
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("/"):
Expand Down Expand Up @@ -72,11 +72,15 @@ 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` may carry rules_py's PyInfo or native @rules_python's; both expose
# `imports`. See py_info_interop.bzl.
transitive = [
get_py_info(target).imports
for target in deps
if has_py_info(target)
]

return depset(
direct = import_paths,
transitive = [
target[PyInfo].imports
for target in deps
if PyInfo in target
] + extra_imports_depsets,
transitive = transitive + extra_imports_depsets,
)
9 changes: 7 additions & 2 deletions py/private/py_image_layer.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ Sharing model:

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"
Expand Down Expand Up @@ -373,6 +374,10 @@ def _layer_aspect_impl(target, ctx):
interpreter_layer = None
interpreter_files = None
kind = ctx.rule.kind

# The binary being layered must be a rules_py py_binary (it carries rules_py's
# PyInfo); rules_python's py_binary is not supported. rules_python *library*
# deps within the graph are still handled by the membership checks below.
is_binary = (
PyInfo in target and
target[DefaultInfo].files_to_run.executable != None
Expand All @@ -394,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 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:
if has_py_info(dep):
continue
if DefaultInfo in dep:
own_parts.append(dep[DefaultInfo].files)
Expand Down
25 changes: 18 additions & 7 deletions py/private/py_info.bzl
Original file line number Diff line number Diff line change
@@ -1,11 +1,22 @@
"""The `PyInfo` provider used by rules_py.
"""The `PyInfo` provider produced and consumed by rules_py targets.

This is the single seam through which rules_py imports `PyInfo`. Today it simply
re-exports `@rules_python//python:defs.bzl%PyInfo`; centralising the import here
means a future change to rules_py's own provider only touches this file, not
every load site. Re-exported from `//py:defs.bzl` as public API.
`PyInfo` carries the information rules_py needs to assemble a target's dependency
closure: the transitive set of first-party Python sources, the import roots to
place on `sys.path`, and the virtual-dependency declarations and their
resolutions. Targets in a dependency graph aggregate these fields from their
deps to build the eventual venv or wheel.
"""

load("@rules_python//python:defs.bzl", _PyInfo = "PyInfo")
# First binding wins the exported name, so diagnostics say `RulesPyInfo`.
RulesPyInfo = provider(
doc = "Python source, import-path, and virtual-dependency information for a target's dependency closure.",
fields = {
"transitive_sources": "depset[File] — postorder depset of first-party `.py` sources in the transitive closure.",
"imports": "depset[str] — import roots to place on `sys.path` (rlocation-root-relative).",
"virtual_dependencies": "depset[str] — names of required virtual dependencies, independent of their resolution status.",
"virtual_resolutions": "depset[struct(virtual, target)] — virtual-dependency-name to concrete-target resolutions.",
},
)

PyInfo = _PyInfo
# The name load sites and the public API import; same provider object.
PyInfo = RulesPyInfo
32 changes: 32 additions & 0 deletions py/private/py_info_interop.bzl
Original file line number Diff line number Diff line change
@@ -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
43 changes: 24 additions & 19 deletions py/private/py_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +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")
load("//py/private:providers.bzl", "PyVirtualInfo", "PyWheelsInfo")
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(
Expand All @@ -19,13 +20,15 @@ def _make_instrumented_files_info(ctx):
)

def _make_srcs_depset(ctx):
# `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].transitive_sources
get_py_info(target).transitive_sources
for target in ctx.attr.deps
if PyInfo in target
if has_py_info(target)
],
)

Expand All @@ -34,16 +37,17 @@ def _make_virtual_depset(ctx):
order = "postorder",
direct = getattr(ctx.attr, "virtual_deps", []),
transitive = [
target[PyVirtualInfo].dependencies
target[PyInfo].virtual_dependencies
for target in ctx.attr.deps
if PyVirtualInfo in target
if PyInfo in target
],
)

def _make_resolved_virtual_depset(target):
transitive = [target[DefaultInfo].files]
if PyInfo in target:
transitive.append(target[PyInfo].transitive_sources)
info = get_py_info(target)
if info:
transitive.append(info.transitive_sources)

return depset(
order = "postorder",
Expand All @@ -58,9 +62,9 @@ def _make_virtual_resolutions_depset(ctx):
for k, v in ctx.attr.resolutions.items()
],
transitive = [
target[PyVirtualInfo].resolutions
target[PyInfo].virtual_resolutions
for target in ctx.attr.deps
if PyVirtualInfo in target
if PyInfo in target
],
)

Expand All @@ -84,8 +88,9 @@ 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:
v_imports.append(resolution.target[PyInfo].imports)
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())))
if len(missing) > 0:
Expand Down Expand Up @@ -161,13 +166,8 @@ def _py_library_impl(ctx):
PyInfo(
imports = imports,
transitive_sources = transitive_srcs,
has_py2_only_sources = False,
has_py3_only_sources = True,
uses_shared_libraries = False,
),
PyVirtualInfo(
dependencies = virtuals,
resolutions = resolutions,
virtual_dependencies = virtuals,
virtual_resolutions = resolutions,
),
PyWheelsInfo(
wheels = wheels,
Expand All @@ -182,7 +182,12 @@ _attrs = dict({
),
"deps": attr.label_list(
doc = "Targets that produce Python code, commonly `py_library` rules.",
providers = [[PyInfo], [PyVirtualInfo], [CcInfo]],
# 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). Reads go through py_info_interop.bzl's accessors;
# rules_py never emits RulesPythonPyInfo.
providers = [[PyInfo], [RulesPythonPyInfo], [CcInfo]],
),
"data": attr.label_list(
doc = """Runtime dependencies of the program.
Expand Down
5 changes: 2 additions & 3 deletions py/private/py_unpacked_wheel.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,8 @@ def _py_unpacked_wheel_impl(ctx):
PyInfo(
imports = imports,
transitive_sources = depset([unpack_directory]),
has_py2_only_sources = False,
has_py3_only_sources = True,
uses_shared_libraries = False,
virtual_dependencies = depset(),
virtual_resolutions = depset(),
),
]

Expand Down
5 changes: 2 additions & 3 deletions py/private/py_venv/py_venv_exec.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,8 @@ def _py_venv_exec_impl(ctx):
# sibling venv, not on this rule.
imports = vinfo.imports,
transitive_sources = vinfo.transitive_sources,
has_py2_only_sources = False,
has_py3_only_sources = True,
uses_shared_libraries = False,
virtual_dependencies = depset(),
virtual_resolutions = depset(),
),
instrumented_files_info,
RunEnvironmentInfo(
Expand Down
17 changes: 14 additions & 3 deletions py/tests/import-pathing/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,11 +1,22 @@
load("@rules_python//python:defs.bzl", "py_library")
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")

# This is used in the py_library import pathing tests
# Used by the import-pathing tests to verify a dep's `imports` merge
# transitively through rules_py's own PyInfo.
py_library(
name = "__native_rule_import_list_for_test",
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()
17 changes: 16 additions & 1 deletion py/tests/import-pathing/tests.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,18 @@ def _can_resolve_path_in_workspace_test_impl(ctx):
# Note that under bzlmod the import_dep's repo is _main.
# We can't override or force this because we're using a real label below.
asserts.equals(env, "{}/py/tests/import-pathing/baz".format(ctx.workspace_name), imports[0])

# The dep is a rules_py py_library, which appends the workspace root to its
# own imports, so that entry is carried transitively too.
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)
Expand All @@ -79,7 +91,10 @@ _can_resolve_path_in_workspace_test = unittest.make(
_can_resolve_path_in_workspace_test_impl,
attrs = {
"import_dep": attr.label(
default = "@//py/tests/import-pathing:__native_rule_import_list_for_test",
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",
),
},
)
Expand Down
7 changes: 4 additions & 3 deletions py/tests/internal-deps/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,15 @@ rules_py_py_library(
],
)

# Forward interop: a @rules_python py_library consumed by the rules_py
# py_binary/py_test below. It can't depend on a rules_py py_library (`:init`) —
# rules_py emits its own PyInfo, not @rules_python's, which rules_python's deps
# constraint requires.
py_library(
name = "pi",
srcs = [
"pi.py",
],
deps = [
":init",
],
)

py_binary(
Expand Down
Loading
Loading