Allow passing in additional javac_opts to java_grpc_library

[honnix]

Is your feature request related to a problem?

java_grpc_library passes the resolved java toolchain to java_common.compile to compile generated code. However, there are cases that users might need to further customize javac_opts, where the customization is not suitable to be done directly on the toolchain itself.

Describe the solution you'd like

_java_grpc_library and _java_lite_grpc_library (used by java_grpc_library) expose an attribute to receive customized javac_opts and pass it down to java_common.compile.

Describe alternatives you've considered

I was not able to find a reasonable alternative but I could have missed something.

Additional context

[honnix]

Alternatively, this can be done similarly as https://github.com/bazelbuild/bazel/blob/5d5da86dd7a6ad94e755489e743fdd58c480a116/src/main/starlark/builtins_bzl/common/java/proto/java_proto_library.bzl#L121

[temawi]

@ejona86 Any thoughts on plumbing through additional javac options in this manner?

[honnix]

FWIW, we applied the following patch and it worked as expected:

diff --git a/java_grpc_library.bzl b/java_grpc_library.bzl
index ed9900917..ec11f2453 100644
--- a/java_grpc_library.bzl
+++ b/java_grpc_library.bzl
@@ -104,13 +104,15 @@ def _java_rpc_library_impl(ctx):
     )
 
     deps_java_info = java_common.merge([dep[JavaInfo] for dep in ctx.attr.deps])
+    java_toolchain = toolchain.java_toolchain[java_common.JavaToolchainInfo]
 
     java_info = java_common.compile(
         ctx,
-        java_toolchain = toolchain.java_toolchain[java_common.JavaToolchainInfo],
+        java_toolchain = java_toolchain,
         source_jars = [srcjar],
         output = ctx.outputs.jar,
         output_source_jar = ctx.outputs.srcjar,
+        javac_opts = java_toolchain._compatible_javacopts.get("grpc", depset()),
         plugins = [plugin[JavaPluginInfo] for plugin in toolchain.java_plugins],
         deps = [
             java_common.make_non_strict(deps_java_info),

And this is how we configure toolchain:

default_java_toolchain(
    ...
    compatible_javacopts = {
        "grpc": [...],
    },
    ...
)

[ejona86]

I'm really not up-to-date on how toolchains are configured. It seems this sort of option could maybe be in java_rpc_toolchain, although right now grpc_java_library uses a hard-coded toolchain.

I'm generally very fine with copying proto_java_library, but _compatible_javacopts is private API.

What type of javac options are you wanting to set? Linter, warnings, or something else?

[honnix]

@ejona86 It could be the case having a toolchain configured globally to target one Java version, while for proto/grpc to target another version, for compatibility reasons. It could also be disabling or enabling for example -Werror.

but _compatible_javacopts is private API

Yeah that is very valid concern.

[ejona86]

Looks like compatible_javacopts in java_toolchain is also private. "Internal API, do not use!":
https://bazel.build/reference/be/java#java_toolchain.compatible_javacopts

having a toolchain configured globally to target one Java version, while for proto/grpc to target another version, for compatibility reasons

Although in that case "the Bazel way" would have a full new toolchain, not just javacopts. Right?

It could also be disabling or enabling for example -Werror.

If our generated code is producing a warning, I'd be interested in fixing that. And there's not much point in enabling -Werror only for the generated code.

Are either of these issues a problem you are actually having?

[honnix]

Although in that case "the Bazel way" would have a full new toolchain, not just javacopts. Right?

Yeah that could be an alternative if java_grpc_library could accept a Java toolchain. I think right now it uses whatever resolved by Bazel via @bazel_tools//tools/jdk:current_java_toolchain if I'm not mistaken. Or we could perhaps create a full java_grpc_library_toolchain and pass it in here

grpc-java/java_grpc_library.bzl

Line 134 in 34e241a

"_toolchain": attr.label(

.

If our generated code is producing a warning, I'd be interested in fixing that. And there's not much point in enabling -Werror only for the generated code.

It could be a temporary deprecation warning.

Are either of these issues a problem you are actually having?

We sort of have both, but primarily the first case where we need to compile proto+grpc targeting a lower version of Java.