improve diagnostics for incorrect java_runtime configuration

[cushon]

cc @lberki

Creating a java_runtime.java_home that isn't actually a JDK should result in a better error than it does currently:

bazel run --javabase @bazel_tools//tools/jdk:absolute_javabase --define=ABSOLUTE_JAVABASE=/not/actually/a/jdk <some java binary>
...
/usr/local/google/home/cushon/.cache/bazel/_bazel_cushon/8c38d57ae237ae214c53581f830f9e07/execroot/__main__/bazel-out/k8-fastbuild/bin/java/j: line 355: /not/actually/a/jdk/bin/java: No such file or directory

[iirina]

@lberki can you also assign a priority?

[cushon]

We should probably fix this before flipping the default on #6105, but it's not currently on fire.

[cushon]

The reason I didn't just do this as part of #6105 is that it wasn't obvious to me how to actually report a better error. For java_runtime rules using a local JDK we don't know what the contents are at analysis time, so we can't detect that it's empty. I guess we could create an action associated with the java_runtime that validates some well-known files are present or else fails with a better error.

Does that sounds reasonable, or is there a nicer way to do this?

[lberki]

I think that sounds reasonable, but I don't see an obvious way of emitting that error message so we'll probably have to compromise and I'd rather have #6105 fixed now than be delayed for the fix of this issue, especially with the imminent cut of 0.18.

I can't think of any way of doing this other than an action that checks were java_runtime.java_home points to and is somehow a dependency of all java_binary rules. Or else we can do it in the Java launcher, but that would mean doing it over and over on every invocation of a java_binary.

[cushon]

an action that checks where java_runtime.java_home points to and is somehow a dependency of all java_binary rules

Could the action be created by java_runtime and produce an empty file that we add to the rule's files-to-build? That's the 'validation action' hack we uses for stuff like one version enforcement.

(There's a related feature request somewhere for first-class 'validation action' support, so we could wire actions like that into the graph without having to produce empty files.)

[lberki]

Yep, it rhymes pretty well with the concept of "validation actions".

[mattgodbolt]

I just hit this following the build instructions on https://github.com/bazelbuild/intellij#building-the-plugin on a fresh new computer, and I'm not even sure how to fix this. Presumably I need to install a system-wide JDK? [edit: after installing a system JDK I was good]

[ulfjack]

I'm seeing the issue described in #6475 on Windows, even with a JDK installed, unless I also set JAVA_HOME. It would be very good if the error message could be improved because this just ate a couple of hours of eng time.

To make it more interesting, some command-line variants of setting JAVA_HOME result in a warning that JAVA_HOME does not point to a JDK. For example, the standard terminal auto-completes like so: set JAVA_HOME="C:\Program Files\Java\", but then the resulting environment variable contains double quote characters (").

[comius]

I propose we remove --define=ABSOLUTE_JAVABASE mechanism. "Define" flags or any other flags are impossible to process in repository rules, which can check the installation. Macros and rules cannot check the installation until the build phase. So to provide a better error message we'd need to have a second java_runtime implementation just to emit an error message.

Alternative is to use local_java_repostiory in the WORKSPACE file. Or we could even extend it to use an environment variable ABSOLUTE_JAVABASE which is available to repository rules.

[lberki]

Environment variables work pretty well for Bazel (not for Blaze, but the solutions in the two places don't need to be the same)