Document precedence of JAVABIN on java_binary rule

[samshadwell]

Ran into some confusion around this at work. We recently changed our Bazel repo so that everything was compiling/running with Java 17, but some folks would get issues when bazel runing java_binary targets. Error messages indicated they were still running on a Java 8 VM despite the --java_runtime_version flag.

Looking at where the stub template is filled in, I noticed that JAVABIN environment variable takes precedence over the java_executable: https://cs.opensource.google/bazel/bazel/+/master:src/main/starlark/builtins_bzl/bazel/java/bazel_java_binary.bzl;l=183?q=javabin&ss=bazel%2Fbazel&start=11

The folks running into issues set their JAVABIN in .zshrc files, so that was taking precedence over the Bazel flags and incorrectly running the Java 17 compiled code on a Java 8 VM. Seems reasonable that JAVABIN take precedence, but I figure this should probably be documented somewhere to save other folks the trouble of digging into the guts to figure out what was going on.

[fmeum]

Good catch. To me this looks like a bug as it is a very unexpected source of non-hermeticity. A wrapper script flag could serve as a less surprising knob for this.

[samshadwell]

Agreed, it was surprising @fmeum! Though I guess the hermiticity expectations of run are maybe a little lower than for build. It might be useful to disable via a flag, but I figured a doc-only change might be the least objectionable way to act on the learning

[fmeum]

Yes, thank you very much for fixing the docs!

Looking into https://cs.opensource.google/bazel/bazel/+/b3602eb14cf27494a0a754bc215ec2b94d13d89b:src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java;l=441, this does seem to be intentional.

[samshadwell]

@fmeum what's the process for getting this merged? This is my first PR to bazel 😅

[fmeum]

I am not a Bazel team member and thus can't review PRs. Your PR got assigned to the Java team for review. Just sit back and wait for someone from that team to review the PR, feel free to ping the PR if that doesn't happen within a reasonable amount of time (say two weeks).

[fmeum]

@cushon Could you take a look? This is a docs-only change.