Remove deploy jar Java runtime dependency for non-executable targets

[fmeum]

This allows deploy jars for java_binary targets with create_executable = False to compile for target platforms for which no standalone Java runtime is available (e.g. Android).

Along the way, this removes a redundant check (runtime.hermetic_files is never None) and ensures that files coming from the hermetic runtime are only added if hermetic is set to True.

Work towards #17085

[fmeum]

@hvadehra In case it's public, could you perhaps provide a bit more context on what is part of the hermetic runtime feature? I find it tricky to propose changes to the Java rules without (roughly) knowing how this is supposed to work.

For example, depending on how this feature works, it could make more sense to guard the runtime dependency behind the value of create_executable.

[hvadehra]

In case it's public, could you perhaps provide a bit more context on what is part of the hermetic runtime feature?

cc @cushon @jianglizhou

[fmeum]

I just checked that with this and bazelbuild/rules_java#114 and bazelbuild/rules_kotlin#1000, I can now build android_binary targets involving Java, Kotlin and deploy JARs with --java_runtime_version=remotejdk_17 (instead of local_jdk).

[cushon]

@hvadehra In case it's public, could you perhaps provide a bit more context on what is part of the hermetic runtime feature? I find it tricky to propose changes to the Java rules without (roughly) knowing how this is supposed to work.

For example, depending on how this feature works, it could make more sense to guard the runtime dependency behind the value of create_executable.

There's some background on the hermetic runtime feature here: https://mail.openjdk.org/pipermail/leyden-dev/2023-February/000107.html

For the Java rules support, it's basically bundling a JDK into the _deploy.jar when an executable is created

[fmeum]

@cushon Thanks for the pointer.

What I find surprising is that the logic at https://github.com/bazelbuild/bazel/pull/19140/files#diff-ed65b74742411654fece564fdcfdb107625e42d1383d58b73ba32d0334896dcbL212 seems to run regardless of whether the java_binary has create_executable = True. If I understood it correctly, your last statement would indicate that that's not true.

[cushon]

It should be fine to disable that logic if create_executable = False.

The internal java_binary wrapper guarantees that hermetic is only ever enabled if create_executable is also enabled:

    if not ctx.attr.create_executable and ctx.attr.hermetic:
        fail("hermetic specified but create_executable is false")

[fmeum]

@cushon Thanks for the confirmation.

I am slightly worried about this line as it isn't conditional on hemertic: https://cs.opensource.google/bazel/bazel/+/master:src/main/starlark/builtins_bzl/common/java/java_binary_deploy_jar.bzl;l=212

I will push a new version that removes the runtime dependency if create_executable = False.

[cushon]

@cushon Thanks for the confirmation.

I am slightly worried about this line as it isn't conditional on hemertic: https://cs.opensource.google/bazel/bazel/+/master:src/main/starlark/builtins_bzl/common/java/java_binary_deploy_jar.bzl;l=212

Good catch, I am preparing a fix for that

I will push a new version that removes the runtime dependency if create_executable = False.

👍

[fmeum]

I pushed the new version.

[fmeum]

@cushon I rebased onto your commit and simplified the implementation by using that hermetic implies create_executable.