bazel: 0.18.0 -> 0.20.0 #51748
bazel: 0.18.0 -> 0.20.0 #51748
Conversation
|
We have a second PR in #51783, which only adds one new line. |
| --host_javabase=@bazel_tools//tools/jdk:absolute_javabase \ | ||
| --define=ABSOLUTE_JAVABASE=$JAVA_HOME \ | ||
| --host_java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla \ | ||
| --java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla \ |
There was a problem hiding this comment.
What do these lines do? It looks similar to what https://github.com/NixOS/nixpkgs/pull/51783/files#diff-4746b81564a01e241a84d455575437e7R191 does in one line.
There was a problem hiding this comment.
--server_javabase set the java version used for the Bazel server, --host_javabase sets the java version used for the worker processes, --*java_toolchain sets the toolchain used to build java code.
I'm surprised setting --server_javabase is enough, maybe the other two default to the value passed for that.
Why I pass it separately here is that in the checkPhase we haven't wrapped the program yet (#51783 deletes checkPhase so it doesn't have that issue).
|
Yeah #51783 avoids a lot of complexity by deleting some stuff, I think that's good assuming we're ok with deleting it. |
|
Oh, hm, not a fan of deleting tests like these. Of course for a tool like bazel that takes half an hour to build, it’s infuriating if some ad-hoc tests fail at the very end. |
|
@mboes What do you think? I'm happy for either one to go through. |
| sed -i -e "421 a --copt=\"$(echo $NIX_CFLAGS_COMPILE | sed -e 's/ /" --copt=\"/g')\" \\\\" scripts/bootstrap/compile.sh | ||
| sed -i -e "421 a --host_copt=\"$(echo $NIX_CFLAGS_COMPILE | sed -e 's/ /" --host_copt=\"/g')\" \\\\" scripts/bootstrap/compile.sh | ||
| sed -i -e "421 a --linkopt=\"-Wl,$(echo $NIX_LDFLAGS | sed -e 's/ /" --linkopt=\"-Wl,/g')\" \\\\" scripts/bootstrap/compile.sh | ||
| sed -i -e "421 a --host_linkopt=\"-Wl,$(echo $NIX_LDFLAGS | sed -e 's/ /" --host_linkopt=\"-Wl,/g')\" \\\\" scripts/bootstrap/compile.sh |
There was a problem hiding this comment.
Can be BAZEL_BOOTSTRAP_STARTUP_OPTIONS on https://github.com/bazelbuild/bazel/blob/be2fd95b29e55eb5b300635b82610e35be7f0040/scripts/bootstrap/compile.sh#L433
used instead sed commands?
There was a problem hiding this comment.
Yeah, we should replace those lines by something sane. Sadly, the bootstrap scripts are a bit wild and hard to patch correctly.
There was a problem hiding this comment.
It seems like we can set that environment variable when calling the script, that's a very good find, not sure how I missed it!
There was a problem hiding this comment.
I've tried it but it doesn't work because this is only for startup options: options that go before the command, and are parsed by the bazel client, and not regular options, which go after the command and are parsed by the bazel server.
|
I merged #51783, because it removes more lines than it introduces (by disabling a superfluous test). |
|
@uri-canva could you make a new pull request that switches to |
Motivation for this change
Upgrading Bazel. Tested
bazel,bazel_jdk11,bazel-watcher,bazel-deps,pythonPackages.tensorflow,pythonPackages27.tensorflow,pythonPackages36.tensorflow.bazel-watcherrequires #51723.Things done
sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)nix path-info -Sbefore and after)