Skip to content

Adopt proto_lang_toolchain for java_grpc_library() - #12994

Open
AgraVator wants to merge 6 commits into
grpc:masterfrom
AgraVator:adopt-proto-lang-toolchain
Open

Adopt proto_lang_toolchain for java_grpc_library()#12994
AgraVator wants to merge 6 commits into
grpc:masterfrom
AgraVator:adopt-proto-lang-toolchain

Conversation

@AgraVator

Copy link
Copy Markdown
Contributor

No description provided.

@AgraVator
AgraVator requested a review from ejona86 August 18, 2026 17:09
@AgraVator
AgraVator marked this pull request as ready for review August 18, 2026 17:09
Comment thread compiler/BUILD.bazel Outdated
Comment thread java_grpc_library.bzl
Comment thread java_grpc_library.bzl Outdated
Comment thread java_grpc_library.bzl Outdated
@AgraVator
AgraVator requested a review from ejona86 August 20, 2026 07:04
Comment thread java_grpc_library.bzl
Comment thread compiler/BUILD.bazel Outdated
Comment thread java_grpc_library.bzl Outdated
java_info = java_common.compile(
ctx,
java_toolchain = toolchain.java_toolchain[java_common.JavaToolchainInfo],
java_toolchain = ctx.toolchains["@bazel_tools//tools/jdk:toolchain_type"].java,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised to see a literal being passed here. Is that not defined somewhere?

@AgraVator AgraVator Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bazel does not expose an importable constant for this toolchain type, so using the literal string as was being done at https://github.com/protocolbuffers/protobuf/blob/main/bazel/private/java_proto_support.bzl#L43

Comment thread java_grpc_library.bzl
source_jars = [srcjar],
output = ctx.outputs.jar,
output_source_jar = ctx.outputs.srcjar,
plugins = [plugin[JavaPluginInfo] for plugin in toolchain.java_plugins],

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like we need to still support this, as there are usages internally. I don't know if that means we need a second toolchain, or what.

@AgraVator
AgraVator requested a review from ejona86 September 2, 2026 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants