-
Notifications
You must be signed in to change notification settings - Fork 139
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
Actually test Gerrit on ubuntu1804_java11 #333
Conversation
@philwo Can you take a look? I assume that the [1] https://gerrit-review.googlesource.com/c/gerrit/+/193832 |
Hi @davido, sorry for the late reply. We're using Azul Zulu on the machines, so the correct path would be: For reference:
Otherwise LGTM! |
Done. |
@philwo Note, that on Bazel 0.19.0, we could simplify the integration with recent Java versions, see: [1]. That's because Also, do you have already Java 11 on Buildkite CI environment up and running? In that case we could jump directly to Java 11 and skip Java 10 altogether. |
Hi @davido, sorry for not getting back to this earlier. I just came back from parental leave and am catching up with stuff. Is this fine to merge?
We still don't have support for Java 11 yet, but @meisterT might know what our plans are for that. Maybe Bazel already supports it and we just have to add it to our CI, in which case I'll happily do that. |
Let's assume it will take quite some time (years?) to make Bazel natively support Java 11, see bazelbuild/bazel#6245. In that light, wouldn't it make sense to independently install Java 11 on Bazel CI environment and ensure that at least 2 projects: Gerrit and Bazel itself (or just Gerrit, if you don't care that Bazel can be built with Java 11, we do care, though) can be built with Java 11 using
All I need from you to make it happen two inputs:
|
I'll upgrade the embedded JDK to JDK11 soonish; and @iirina is trying to shape the general Java/JDK roadmap. |
By this you mean on Bazel CI or in Bazel itself (native support for JDK 11, that is tracked in bazelbuild/bazel#6245)? If in Bazel itself, do you have time frame estimation for "soonish"? |
I mean the embedded JDK in Bazel, see #6592 |
I'll get that done ASAP. |
Thanks @philwo. That's awesome! Let me adjust this PR to use Azul Zulu 11 JDK with |
Merged and it's green on CI: https://buildkite.com/bazel/gerrit/builds/2010 - yay, Gerrit! @cushon @meisterT @iirina Our first build with a Java 11 host_javabase succeeded :) FWIW, I spotted this new warning during the build with Java 11, which doesn't seem to cause problems, but at least confirms that we're actually building with Java 11 (because that dropped support for targeting Java 1.7, I guess):
|
Great news, thank you! I have just realized, that Bazel can also be built with Java 11, so that something like this worked:
Then to make use of it, I added this line to
And then I tested to build gerrit with Bazel-built-with-JDK11 and compared it building gerrit Gerrit with vanilla bazel 0.22rc2. The results are below:
then I clean everything, including caches and re-run the build with vanilla Bazel 0.22-rc2:
Do we have an explanation, why it takes 3m58.359s to build gerrit (with cold cache) with Bazel@JDK11 and 4m48.769s to build gerrit (with cold cache) with vanilla Bazel? Is it only because of Error Prone integration and other neat features that vanilla Bazel offers? |
I think the VanillaJavaBuilder is faster than the full-featured one with Error Prone etc., but I don't think anyone ever benchmarked how much the difference is. It's possible that this is the reason, yes. |
* Actually test Gerrit on ubuntu1804_java10 Closes bazelbuild#325. * Fix path to Azul Zulu jvm * Update path to JDK 11
Closes #325.