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

Upgrade everything! #23767

Closed
wants to merge 1 commit into from
Closed

Upgrade everything! #23767

wants to merge 1 commit into from

Conversation

Wyverald
Copy link
Member

@Wyverald Wyverald commented Sep 25, 2024

What started out as an attempt to upgrade gRPC has snowballed into this monster...

  • Upgrades gRPC to the latest version, including the Java bindings from grpc-java
    • grpc-java actually specifies Maven overrides, causing rules_jvm_external's references to io.grpc.* and com.google.protobuf.* to alias to actual Bazel build targets in @grpc-java and @protobuf. So this effectively means we're no longer using the gRPC jars from Maven
    • To support these overrides, our custom patch for rules_jvm_external had to be updated to include the alias targets in the BUILD.vendor file.
  • The upgrade to gRPC means we now use the latest googleapis from BCR
    • This means we can remove the vendored copy of those protos. Updated all references to googleapis protos across the codebase.
  • The upgrade to gRPC means we now use the latest version of protobuf
    • This ran into some trouble with the concurrent work to move Bazel's proto rules into protobuf, which means that we currently have to use the protobuf version from HEAD.
    • We also have to patch out protobuf's dependency on rules_rust, as it introduces a lot of dependency bloat and makes it impossible to build a bootstrap distribution archive
    • The dependency on upb is completely removed.

Fixes #22719

@Wyverald Wyverald force-pushed the wyv-grpc branch 3 times, most recently from 6679c4c to 7038438 Compare September 26, 2024 19:15
@Wyverald
Copy link
Member Author

Currently stuck on making HEAD Bazel build itself.

At the source checkout state of this PR, I can run USE_BAZEL_VERSION=7.3.1 bazelisk build src:bazel and that's all fine. But if I then use the just-built bazel (cp bazel-bin/src/bazel ~/elsewhere/bazel; ~/elsewhere/bazel build src:bazel), then I get errors like this:

ERROR: /Users/wyv/github/bazel2/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/BUILD:23:14: in deps attribute of proto_library rule //src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_proto: '@@protobuf+//src/google/protobuf:timestamp_proto' does not have mandatory providers: 'ProtoInfo'. Since this rule was created by the macro 'proto_library', the error might have been caused by the macro implementation

Using Bazel 7.3.1 and the just-built Bazel to cquery the provider list, the difference is evident:

$ USE_BAZEL_VERSION=7.3.1 bazelisk cquery "'@@protobuf+//src/google/protobuf:duration_proto'" --output=starlark --starlark:expr="providers(target)"
INFO: Analyzed target @@protobuf+//src/google/protobuf:duration_proto (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
{"LicenseInfo": <instance of provider LicenseInfo>, "@@_builtins//:common/proto/proto_info.bzl%ProtoInfo": struct( [...]
$ ~/elsewhere/bazel cquery "'@@protobuf+//src/google/protobuf:duration_proto'" --output=starlark --starlark:expr="providers(target)"
INFO: Analyzed target @@protobuf+//src/google/protobuf:duration_proto (128 packages loaded, 2060 targets configured).
INFO: Found 1 target...
{"LicenseInfo": <instance of provider LicenseInfo>, "@@protobuf+//bazel/private:proto_info.bzl%ProtoInfo": struct( [...]

it looks like on HEAD Bazel, the proto_library targets export the ProtoInfo from protobuf, but the rule itself for some reason expects deps to have the ProtoInfo from builtins.

@Wyverald Wyverald force-pushed the wyv-grpc branch 4 times, most recently from bca80c3 to 0c8f971 Compare September 27, 2024 20:04
@Wyverald Wyverald marked this pull request as ready for review September 27, 2024 20:56
@Wyverald Wyverald requested review from a team and meteorcloudy as code owners September 27, 2024 20:56
@Wyverald Wyverald changed the title [DRAFT] Upgrade everything! Upgrade everything! Sep 27, 2024
@github-actions github-actions bot added team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. team-Rules-Java Issues for Java rules team-Local-Exec Issues and PRs for the Execution (Local) team team-Remote-Exec Issues and PRs for the Execution (Remote) team awaiting-review PR is awaiting review from an assigned reviewer labels Sep 27, 2024
@Wyverald Wyverald removed request for a team September 27, 2024 20:56
@Wyverald Wyverald removed team-Rules-Java Issues for Java rules team-Local-Exec Issues and PRs for the Execution (Local) team team-Remote-Exec Issues and PRs for the Execution (Remote) team labels Sep 27, 2024
@Wyverald
Copy link
Member Author

The issue above has been addressed by 3cb5bea. This is now ready for review.

Copy link
Member

@meteorcloudy meteorcloudy left a comment

Choose a reason for hiding this comment

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

Nice work!!

.bazelrc Show resolved Hide resolved
MODULE.bazel Outdated Show resolved Hide resolved
MODULE.bazel Show resolved Hide resolved
MODULE.bazel Show resolved Hide resolved
@meteorcloudy meteorcloudy added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Oct 1, 2024
@meteorcloudy meteorcloudy removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Oct 1, 2024
@meteorcloudy
Copy link
Member

@Wyverald Can you merge this one since there are many BUILD file changes. We'll also need a separate PR to delete googleapis from third_party.

copybara-service bot pushed a commit that referenced this pull request Oct 1, 2024
Partial commit for third_party/*, see #23767.

Change-Id: I5696cc64436dcec9ea87f72696abad50e4ba3619
Signed-off-by: Xudong Yang <[email protected]>
@copybara-service copybara-service bot closed this in c46b9ce Oct 1, 2024
@Wyverald Wyverald deleted the wyv-grpc branch October 1, 2024 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade grpc and abseil-cpp version
2 participants