Bazel incompatible changes #5383
Comments
|
@laurentlb, what version of Bazel should we test with? |
|
In general, the latest version of Bazel. This would be 0.22 (or 0.23, released a few minutes ago, the announcement is pending). If you use bazelisk, it will automatically fetch the latest version. |
|
I asked because 1) there was no statement as to what is broken and 2) I had run with 0.22 already. In fact, it still is working. Please in the future provide at least a subset of the errors you see. |
If you run Bazel with any of these 4 flags, the build fails. We plan to enable these flags by default in a future version of Bazel, so fixing the issue know will prevent a future breakage. Some issues can be fixed automatically with Buildifier. Actually, they have already been fixed inside Google (cl/235192309). The issue |
|
@laurentlb, so Bazel isn't giving warnings any more for features that will be removed? Is there somewhere "thou must use bazelisk in order to prevent being broken" is written down? I do know that I purposefully did not fix incompatible_disable_legacy_proto_provider because it was too new of a change. I'd have to look at the others. |
|
bazelisk was released less than two weeks ago and it isn't "official" (although it was written by a member of bazel team), so I don't really trust what it tells me. But I looked and it appears to figure out the flags to use based on labels like https://github.com/bazelbuild/bazel/labels/migration-0.22 . Okay, so now I trust it more. And looking more, strict and migrate operate on the same flags, except strict passes them all at once and migrate runs multiple times. Before many of these can be fixed we'll have to swap to some different building with kokoro, because kokoro team will no longer update the Bazel version (even though it was unannounced). sigh... Bazel 0.23 broke the build and it was unrelated to all the incompatible flags. The error message is also unclear what the conflicting actions were (I only see one action listed)... |
|
Sorry about that. This conflict is a mistake on our side (see bazelbuild/bazel#7551). You've received a PR about it. Bazelisk should now pick it correctly and I've added the flag to the release announcement. The release announcement about 0.23 is now published here: https://blog.bazel.build/2019/02/26/bazel-0.23.html It mentions Bazelisk. The tool is still new, we're iterating on it and we should make it more visible on our website. At the moment, I think running |
|
I see #5395, and it makes sense why 1) it would be an ugly error and 2) why it failed. That was something that was done internally that I hadn't been able to externalize due to bazel versions. Ah, okay, so bazelisk is an official unofficial thing. Yeah, I don't trust |
Also pass in the binary correctly as a tool. This is required for grpc#5383. I have tested the build with older Bazel versions and this doesn't appear to affect compatibility, ie grpc-java continues to be compatible with Bazel >=0.19.
|
EDIT: the flip has been postponed. |
|
Supporting Bazel 0.22.0+ would be acceptable, but Kokoro is not updating Bazel any more, so we are apparently stuck at Bazel 0.20.0. We will have to swap how we get/select Bazel for the CI. |
|
@ejona86 Have you considered using Kokoro's |
This is part of grpc#5383. As Bazel 0.22.0 is the only supported version now, use this in the Kokoro build.
|
@drigz, no, I wasn't aware of that. That looks excellent. I do wish it wasn't Kokoro-specific, because that will cause some issues given how our scripts are currently designed, but it seems like it will work. |
|
nm. Oh, silly me. It is easy to do for our bazel build because we aren't using unix.sh (I thought we were, but obviously we would be.) |
Also pass in the binary correctly as a tool. This is required for #5383. I have tested the build with older Bazel versions and this doesn't appear to affect compatibility, ie grpc-java continues to be compatible with Bazel >=0.19.
This is part of #5383. As Bazel 0.22.0 is the only supported version now, use this in the Kokoro build.
|
Thanks for merging the changes! I think the next step is to upgrade to protobuf 3.7.0, which will introduce a dependency on @bazel_skylib, but I haven't tried this out yet. |
|
I've started work on upgrading Protobuf. I've gotten the Bazel pieces in place (it was a bit of a PITA though). I will update the rest of the build and then send out a PR. |
|
We can add grpc-java to the Bazel CI, see: https://github.com/bazelbuild/continuous-integration/issues/new/choose When it's done, we'll test the project every night with Bazel at head and we'll make sure we're not breaking you. It will also be tested with every flag and show up here: https://buildkite.com/bazel/bazelisk-plus-incompatible-flags/builds/26 |
|
Can't upgrade protobuf to 3.7.0 yet because the Java release process looks incomplete; there aren't any artifacts on Maven Central. protocolbuffers/protobuf#5845 |
|
@cushon, I can't figure out how to make diff --git a/java_grpc_library.bzl b/java_grpc_library.bzl
index 4601d4d78..ff8abd422 100644
--- a/java_grpc_library.bzl
+++ b/java_grpc_library.bzl
@@ -1,3 +1,5 @@
+load("@bazel_tools//tools/jdk:toolchain_utils.bzl", "find_java_runtime_toolchain", "find_java_toolchain")
+
# "repository" here is for Bazel builds that span multiple WORKSPACES.
def _path_ignoring_repository(f):
if len(f.owner.workspace_root) == 0:
@@ -40,8 +42,8 @@ def _java_rpc_library_impl(ctx):
java_info = java_common.compile(
ctx,
- java_toolchain = ctx.attr._java_toolchain,
- host_javabase = ctx.attr._host_javabase,
+ java_toolchain = find_java_toolchain(ctx, ctx.attr._java_toolchain),
+ host_javabase = find_java_runtime_toolchain(ctx, ctx.attr._host_javabase),
source_jars = [srcjar],
output_source_jar = ctx.outputs.srcjar,
output = ctx.outputs.jar,But building still fails: |
|
@ejona86 it looks like the final implementation of It's still safe to start using the macros with 0.23, and they'll ensure things keep working once the flag flips in 0.25. |
|
Protobuf 3.7.0 doesn't compile with Bazel because it is missing a dependency on error-prone-annotations. Specifically |
…in_providers_in_java_common This doesn't actually yet work with --incompatible_use_toolchain_providers_in_java_common, as Bazel 0.23 didn't include enough pieces. But this will work in 0.24 with the flag flipped. In both cases it will continue working if the flag is not specified. See grpc#5383 (comment) and bazelbuild/bazel#7186
…in_providers_in_java_common This doesn't actually yet work with --incompatible_use_toolchain_providers_in_java_common, as Bazel 0.23 didn't include enough pieces. But this will work in 0.24 with the flag flipped. In both cases it will continue working if the flag is not specified. See #5383 (comment) and bazelbuild/bazel#7186
This version of Protobuf works with Bazel's --incompatible_disable_deprecated_attr_params. It also avoids a noisy reflective access warning in JDK 9+ (nothing was broken before; the warning was overzealous). Fixes grpc#5383
This version of Protobuf works with Bazel's --incompatible_disable_deprecated_attr_params. It also avoids a noisy reflective access warning in JDK 9+ (nothing was broken before; the warning was overzealous). Fixes grpc#5383
This repository doesn't build with Bazel future incompatible changes, and will break with future releases of Bazel.
In order to detect the issues in advance, you can try Bazelisk:
bazelisk --migrate test //...Some of the issues can be fixed automatically using Buildifier:
buildifier --lint=fix path/to/file.bzlThe text was updated successfully, but these errors were encountered: