Skip to content

Fix Bazel 7 related protobuf build failures #1620

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Conversation

mbland
Copy link
Contributor

@mbland mbland commented Oct 5, 2024

Description

Fixes protobuf related build failures under Bazel 7, while remaining compatible with Bazel 6.

Related to #1482, #1618, and #1619. Results from the investigation documented at:

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:

Motivation

This helps unblock the resolution of #1482 to add Bzlmod support, by removing an obstacle to updating to Bazel 7. I decided not to include this in #1619 to give this subtle, substantial issue the focused attention it deserves. Either PR can go in first, but both must go in before updating to Bazel 7.

cc: @BillyAutrey @jayconrod @benjaminp @TheGrizzlyDev

Related to bazel-contrib#1482, bazel-contrib#1618, and bazel-contrib#1619. Results from the investigation
documented at:

- bazel-contrib#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 bazel-contrib#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!
Copy link
Collaborator

@simuons simuons left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mbland

@liucijus liucijus merged commit 7d3f022 into bazel-contrib:master Oct 8, 2024
2 checks passed
@mbland mbland deleted the bzlmod-fix-bazel-7-generated-proto-breakage branch October 8, 2024 13:15
mbland added a commit to mbland/rules_scala that referenced this pull request Oct 8, 2024
mbland added a commit to mbland/rules_scala that referenced this pull request Oct 8, 2024
Incorporates:

- bazel-contrib#1619
- bazel-contrib#1620

Removes --noenable_bzlmod from .bazelrc
mbland added a commit to mbland/rules_scala that referenced this pull request Oct 8, 2024
Incorporates:

- bazel-contrib#1619
- bazel-contrib#1620

Removes --noenable_bzlmod from .bazelrc
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants