Skip to content

Commit

Permalink
Fix Bazel 7 related protobuf build failures
Browse files Browse the repository at this point in the history
Related to bazelbuild#1482, bazelbuild#1618, and bazelbuild#1619. Results from the investigation
documented at:

- bazelbuild#1619 (comment)

Updates `_import_paths()` in `scala_proto_aspect.bzl` to handle
differences `ProtoInfo.proto_source_root` and `ProtoInfo.direct_sources`
values between Bazel 6 and Bazel 7.

Without this change, `_import_paths()` emits incorrect values under
Bazel 7, causing targets containing generated `.proto` inputs to fail,
e.g.  `//test/proto3:test_generated_proto`.

See also:

- Fix paths for sibling repository setup and generated .proto files
  bazelbuild/bazel@6c6c196

- The docstring for `ProtoInfo.proto_source_root` in the Bazel sources:
  https://github.com/bazelbuild/bazel/blob/7.3.2/src/main/starlark/builtins_bzl/common/proto/proto_info.bzl#L155-L172

- Remove incompatible_generated_protos_in_virtual_imports
  bazelbuild/bazel@3efaa32

- Comment from: Generated Protos are no longer considered as
  virtual_imports in Bazel 7
  bazelbuild/bazel#21075 (comment)

---

I cherrypicked this commit into bazelbuild#1618. While it fixed the
`//test/proto3` build failure, it does _not_ fix the hanging
scalapb_workers from the ProtoScalaPBRule aspect.

I'll have to investiate further whether than hang is related to Bazel,
rules_proto, com_google_protobuf, or some mixture thereof. Still,
progress!
  • Loading branch information
mbland committed Oct 5, 2024
1 parent 3da60a8 commit 463509d
Showing 1 changed file with 14 additions and 7 deletions.
21 changes: 14 additions & 7 deletions scala_proto/private/scala_proto_aspect.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,20 @@ load(
)
load("@bazel_skylib//lib:dicts.bzl", "dicts")

def _import_paths(proto):
def _import_paths(proto, ctx):
# Under Bazel 7.x, direct_sources from generated protos may still contain
# ctx.bin_dir.path, even when proto_source_root does not. proto_source_root
# may also be relative to ctx.bin_dir.path, or it may contain it. So we try
# removing ctx.bin_dir.path from everything.
bin_dir = ctx.bin_dir.path + "/"
source_root = proto.proto_source_root
if "." == source_root:
return [src.path for src in proto.direct_sources]
else:
offset = len(source_root) + 1 # + '/'
return [src.path[offset:] for src in proto.direct_sources]
source_root += "/" if source_root != "." else ""
source_root = source_root.removeprefix(bin_dir)

return [
ds.path.removeprefix(bin_dir).removeprefix(source_root)
for ds in proto.direct_sources
]

def _code_should_be_generated(ctx, toolchain):
# This feels rather hacky and odd, but we can't compare the labels to ignore a target easily
Expand Down Expand Up @@ -56,7 +63,7 @@ def _pack_sources(ctx, src_jars):
)

def _generate_sources(ctx, toolchain, proto):
sources = _import_paths(proto)
sources = _import_paths(proto, ctx)
descriptors = proto.transitive_descriptor_sets
outputs = {
k: ctx.actions.declare_file("%s_%s_scalapb.srcjar" % (ctx.label.name, k))
Expand Down

0 comments on commit 463509d

Please sign in to comment.