-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow passing in additional javac_opts to java_grpc_library #11097
Comments
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 |
@ejona86 Any thoughts on plumbing through additional javac options in this manner? |
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": [...],
},
...
) |
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? |
@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
Yeah that is very valid concern. |
Looks like
Although in that case "the Bazel way" would have a full new toolchain, not just javacopts. Right?
If our generated code is producing a warning, I'd be interested in fixing that. And there's not much point in enabling Are either of these issues a problem you are actually having? |
Yeah that could be an alternative if grpc-java/java_grpc_library.bzl Line 134 in 34e241a
It could be a temporary deprecation warning.
We sort of have both, but primarily the first case where we need to compile proto+grpc targeting a lower version of Java. |
There doesn't seem to be a way to support this as described, so I'm going to close this. Things have advanced in Bazel that the way forward for this is probably to let users define their own grpc-java toolchain and allow them to specify the java toolchain to use within it. Some of that is being discussed in #11764. I'm going to close this, in favor of #11764 and any future discussion of customizable toolchains. |
Is your feature request related to a problem?
java_grpc_library
passes the resolved java toolchain tojava_common.compile
to compile generated code. However, there are cases that users might need to further customizejavac_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 byjava_grpc_library
) expose an attribute to receive customizedjavac_opts
and pass it down tojava_common.compile
.Describe alternatives you've considered
I was not able to find a reasonable alternative but I could have missed something.
Additional context
The text was updated successfully, but these errors were encountered: