Skip to content

Commit

Permalink
fix: py_proto_library: external runfiles
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tpudlik committed Dec 20, 2024
1 parent 66a8b5b commit 037c389
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ Unreleased changes template.
* (pypi) Using {bzl:obj}`pip_parse.experimental_requirement_cycles` and
{bzl:obj}`pip_parse.use_hub_alias_dependencies` together now works when
using WORKSPACE files.
* (py_proto_library) Fix import paths in Bazel 8.

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

Expand Down
6 changes: 6 additions & 0 deletions examples/bzlmod/MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -272,5 +272,11 @@ local_path_override(
path = "other_module",
)

bazel_dep(name = "foo_external", version = "")
local_path_override(
module_name = "foo_external",
path = "py_proto_library/foo_external",
)

# example test dependencies
bazel_dep(name = "rules_shell", version = "0.3.0", dev_dependency = True)
10 changes: 10 additions & 0 deletions examples/bzlmod/py_proto_library/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
load("@bazel_skylib//rules:native_binary.bzl", "native_test")
load("@rules_python//python:py_test.bzl", "py_test")

py_test(
Expand All @@ -16,3 +17,12 @@ py_test(
"//py_proto_library/example.com/another_proto:message_proto_py_pb2",
],
)

# Regression test for https://github.com/bazelbuild/rules_python/issues/2515
#
# This test failed before https://github.com/bazelbuild/rules_python/pull/2516
# when ran with --legacy_external_runfiles=False (default in Bazel 8.0.0).
native_test(
name = "external_import_test",
src = "@foo_external//:py_binary_with_proto",
)
22 changes: 22 additions & 0 deletions examples/bzlmod/py_proto_library/foo_external/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@

load("@rules_python//python:proto.bzl", "py_proto_library")
load("@rules_python//python:py_binary.bzl", "py_binary")

package(default_visibility = ["//visibility:public"])

proto_library(
name = "proto_lib",
srcs = ["nested/foo/my_proto.proto"],
strip_import_prefix = "/nested/foo",
)

py_proto_library(
name = "a_proto",
deps = [":proto_lib"],
)

py_binary(
name = "py_binary_with_proto",
srcs = ["py_binary_with_proto.py"],
deps = [":a_proto"],
)
8 changes: 8 additions & 0 deletions examples/bzlmod/py_proto_library/foo_external/MODULE.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
module(
name = "foo_external",
version = "0.0.1",
)

bazel_dep(name = "rules_python", version = "1.0.0")
bazel_dep(name = "protobuf", version = "28.2", repo_name = "com_google_protobuf")
bazel_dep(name = "rules_proto", version = "7.0.2")
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
syntax = "proto3";

package my_proto;

message MyMessage {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import sys

if __name__ == "__main__":
import my_proto_pb2
sys.exit(0)
10 changes: 8 additions & 2 deletions python/private/proto/py_proto_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,13 @@ def _py_proto_aspect_impl(target, ctx):
proto_root = proto_root[len(ctx.bin_dir.path) + 1:]

plugin_output = ctx.bin_dir.path + "/" + proto_root
proto_root = ctx.workspace_name + "/" + proto_root

# Import path within the runfiles tree
proto_import_path = ""
if proto_root.startswith("external/"):
proto_import_path = proto_root[len("external") + 1:]
else:
proto_import_path = ctx.workspace_name + "/" + proto_root

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

0 comments on commit 037c389

Please sign in to comment.