Skip to content

Commit 5e31d8f

Browse files
committed
fix: py_proto_library: external runfiles
Previously, the import path within the runfiles was only correct for the case --legacy_external_runfiles=True (which copied the runfiles into `$RUNFILES/<main repo>/external/<external repo>/<path>` in addition to `$RUNFILES/<external repo>/<path>`. This flag was flipped to False in Bazel 8.0.0. Fixes #2515. Added a regression test, and tested locally against the minimal reproducer in that issue.
1 parent 66a8b5b commit 5e31d8f

File tree

10 files changed

+67
-2
lines changed

10 files changed

+67
-2
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ Unreleased changes template.
7070
* (pypi) Using {bzl:obj}`pip_parse.experimental_requirement_cycles` and
7171
{bzl:obj}`pip_parse.use_hub_alias_dependencies` together now works when
7272
using WORKSPACE files.
73+
* (py_proto_library) Fix import paths in Bazel 8.
7374

7475
[pep-695]: https://peps.python.org/pep-0695/
7576

examples/bzlmod/.bazelignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
other_module
2+
py_proto_library/foo_external

examples/bzlmod/MODULE.bazel

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,5 +272,11 @@ local_path_override(
272272
path = "other_module",
273273
)
274274

275+
bazel_dep(name = "foo_external", version = "")
276+
local_path_override(
277+
module_name = "foo_external",
278+
path = "py_proto_library/foo_external",
279+
)
280+
275281
# example test dependencies
276282
bazel_dep(name = "rules_shell", version = "0.3.0", dev_dependency = True)

examples/bzlmod/py_proto_library/BUILD.bazel

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
load("@bazel_skylib//rules:native_binary.bzl", "native_test")
12
load("@rules_python//python:py_test.bzl", "py_test")
23

34
py_test(
@@ -16,3 +17,12 @@ py_test(
1617
"//py_proto_library/example.com/another_proto:message_proto_py_pb2",
1718
],
1819
)
20+
21+
# Regression test for https://github.com/bazelbuild/rules_python/issues/2515
22+
#
23+
# This test failed before https://github.com/bazelbuild/rules_python/pull/2516
24+
# when ran with --legacy_external_runfiles=False (default in Bazel 8.0.0).
25+
native_test(
26+
name = "external_import_test",
27+
src = "@foo_external//:py_binary_with_proto",
28+
)
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
2+
load("@rules_python//python:proto.bzl", "py_proto_library")
3+
load("@rules_python//python:py_binary.bzl", "py_binary")
4+
5+
package(default_visibility = ["//visibility:public"])
6+
7+
proto_library(
8+
name = "proto_lib",
9+
srcs = ["nested/foo/my_proto.proto"],
10+
strip_import_prefix = "/nested/foo",
11+
)
12+
13+
py_proto_library(
14+
name = "a_proto",
15+
deps = [":proto_lib"],
16+
)
17+
18+
py_binary(
19+
name = "py_binary_with_proto",
20+
srcs = ["py_binary_with_proto.py"],
21+
deps = [":a_proto"],
22+
)
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
module(
2+
name = "foo_external",
3+
version = "0.0.1",
4+
)
5+
6+
bazel_dep(name = "rules_python", version = "1.0.0")
7+
bazel_dep(name = "protobuf", version = "28.2", repo_name = "com_google_protobuf")
8+
bazel_dep(name = "rules_proto", version = "7.0.2")

examples/bzlmod/py_proto_library/foo_external/WORKSPACE

Whitespace-only changes.
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
syntax = "proto3";
2+
3+
package my_proto;
4+
5+
message MyMessage {
6+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import sys
2+
3+
if __name__ == "__main__":
4+
import my_proto_pb2
5+
sys.exit(0)

python/private/proto/py_proto_library.bzl

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,13 @@ def _py_proto_aspect_impl(target, ctx):
9898
proto_root = proto_root[len(ctx.bin_dir.path) + 1:]
9999

100100
plugin_output = ctx.bin_dir.path + "/" + proto_root
101-
proto_root = ctx.workspace_name + "/" + proto_root
101+
102+
# Import path within the runfiles tree
103+
proto_import_path = ""
104+
if proto_root.startswith("external/"):
105+
proto_import_path = proto_root[len("external") + 1:]
106+
else:
107+
proto_import_path = ctx.workspace_name + "/" + proto_root
102108

103109
proto_common.compile(
104110
actions = ctx.actions,
@@ -130,7 +136,7 @@ def _py_proto_aspect_impl(target, ctx):
130136
# _virtual_imports. But it's undesirable otherwise, because it
131137
# will put the repo root at the top of the PYTHONPATH, ahead of
132138
# directories added through `imports` attributes.
133-
[proto_root] if "_virtual_imports" in proto_root else [],
139+
[proto_import_path] if "_virtual_imports" in proto_import_path else [],
134140
transitive = [dep[PyInfo].imports for dep in api_deps] + [dep.imports for dep in deps],
135141
),
136142
runfiles_from_proto_deps = runfiles_from_proto_deps,

0 commit comments

Comments
 (0)