Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix Bazel 7 related protobuf build failures #1620

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 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!
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 bazelbuild: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:

- bazelbuild#1619
- bazelbuild#1620

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

- bazelbuild#1619
- bazelbuild#1620

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

Successfully merging this pull request may close these issues.

3 participants