Skip to content

Propagate .jdeps for java_import #363

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.

Sign up for GitHub

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

Open
fmeum wants to merge 2 commits into
base: master
Choose a base branch
from
+96 −11
Open

Propagate .jdeps for java_import #363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
08211fd
Make `java_import` participate in Java classpath reduction.
fmeum Jun 30, 2026
463ff11
Address review comments
fmeum Jul 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 15 additions & 8 deletions java/common/rules/impl/bazel_java_import_impl.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,16 +115,22 @@ def bazel_java_import_rule(
collected_jars = _collect_jars(ctx, jars)
all_deps = _filter_provider(JavaInfo, deps, exports)

jdeps_artifact = None
merged_java_info = java_common.merge(all_deps)

# import_deps_check is always run to generate the jdeps proto for downstream compile actions,
# but may be silenced.
if not skip_incomplete_deps_check and "incomplete-deps" not in ctx.attr.tags:
jdeps_artifact = import_deps_check(
ctx,
collected_jars,
merged_java_info.compile_jars,
merged_java_info.transitive_compile_time_jars,
"java_import",
)
checking_mode = "error"
else:
checking_mode = "silence"
jdeps_artifact = import_deps_check(
ctx,
collected_jars,
merged_java_info.compile_jars,
merged_java_info.transitive_compile_time_jars,
"java_import",
checking_mode = checking_mode,
)

compilation_to_runtime_jar_map = _process_with_ijars_if_needed(collected_jars, ctx)
runtime_deps_list = [runtime_dep[JavaInfo] for runtime_dep in runtime_deps if JavaInfo in runtime_dep]
Expand All @@ -134,6 +140,7 @@ def bazel_java_import_rule(
java_infos.append(JavaInfo(
output_jar = jar,
compile_jar = compilation_to_runtime_jar_map[jar],
compile_jdeps = jdeps_artifact,
deps = all_deps,
runtime_deps = runtime_deps_list,
neverlink = neverlink,
Expand Down
10 changes: 7 additions & 3 deletions java/common/rules/impl/import_deps_check.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ def import_deps_check(
jars_to_check,
declared_deps,
transitive_deps,
rule_class):
rule_class,
checking_mode = "error"):
"""
Creates actions that checks import deps for java rules.

Expand All @@ -33,6 +34,7 @@ def import_deps_check(
declared_deps: (list[File]) A list of direct dependencies.
transitive_deps: (list[File]) A list of transitive dependencies.
rule_class: (String) Rule class.
checking_mode: (String) One of "error", "warning" or "silence".

Returns:
(File) Output file of the created action.
Expand All @@ -53,9 +55,11 @@ def import_deps_check(
before_each = "--classpath_entry",
)
args.add_all(java_toolchain.bootclasspath, before_each = "--bootclasspath_entry")
args.add("--checking_mode=error")
args.add(checking_mode, format = "--checking_mode=%s")
args.add("--jdeps_output", jdeps_output)
args.add("--rule_label", ctx.label)

# ctx.label can start with an @ and must not be understood as a flagfile.
args.add(ctx.label, format = "--rule_label=%s")

semantics.update_args_for_import_deps(ctx, args)
inputs = depset(
Expand Down
73 changes: 73 additions & 0 deletions test/java/common/rules/java_import_tests.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,77 @@ def _test_deps_impl(env, targets):
"{package}/depjar.jar",
])

# Regression test for https://github.com/bazelbuild/rules_java/issues/362: every java_import
# records a jdeps proto as its compile_jdeps so that reduced classpaths of downstream compilations
# are not missing its transitive dependencies.
def _test_compile_jdeps_propagated_for_deps(name):
util.helper_target(
java_import,
name = name + "/import-jar",
jars = ["import.jar"],
deps = [name + "/depjar"],
)
util.helper_target(
java_import,
name = name + "/incomplete-jar",
jars = ["incomplete.jar"],
deps = [name + "/depjar"],
tags = ["incomplete-deps"],
)
Comment thread
fmeum marked this conversation as resolved.
util.helper_target(
java_import,
name = name + "/depjar",
jars = ["depjar.jar"],
)

analysis_test(
name = name,
impl = _test_compile_jdeps_propagated_for_deps_impl,
targets = {
"incomplete_deps": name + "/incomplete-jar",
"with_deps": name + "/import-jar",
"without_deps": name + "/depjar",
},
# The rules_java Starlark implementation is used from Bazel 8 on.
attr_values = {"tags": ["min_bazel_8"]},
)

def _test_compile_jdeps_propagated_for_deps_impl(env, targets):
for target in [targets.with_deps, targets.incomplete_deps, targets.without_deps]:
assert_compilation_args = java_info_subject.from_target(env, target).compilation_args()
assert_compilation_args.compile_time_java_dependencies().contains_exactly([
"{package}/_java_import/{name}/jdeps.proto",
])

def _test_import_deps_checker_checking_mode(name):
util.helper_target(
java_import,
name = name + "/import-jar",
jars = ["import.jar"],
deps = [name + "/depjar"],
)
util.helper_target(
java_import,
name = name + "/depjar",
jars = ["depjar.jar"],
)

analysis_test(
name = name,
impl = _test_import_deps_checker_checking_mode_impl,
target = name + "/import-jar",
# The rules_java Starlark implementation is used from Bazel 8 on.
attr_values = {"tags": ["min_bazel_8"]},
)

def _test_import_deps_checker_checking_mode_impl(env, target):
# java_import only generates the jdeps proto, it never fails the build on incomplete deps.
assert_action = env.expect.that_target(target).action_named("ImportDepsChecker")
assert_action.contains_flag_values([
("--checking_mode", "silence"),
("--rule_label", "//{package}:{name}"),
])

# Regression test for b/262751943.
def _test_commandline_contains_target_label(name):
util.helper_target(
Expand Down Expand Up @@ -959,6 +1030,8 @@ def java_import_tests(name):
_test_simple,
_test_with_java_library,
_test_deps,
_test_compile_jdeps_propagated_for_deps,

@hvadehra hvadehra Jul 3, 2026

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 think tests for the ImportDepsChecker action would be useful as well - particularly around the value of --checking_mode

@fmeum fmeum Jul 7, 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.

I added a test and realized that Bazel always forces the silence mode. Which is good for backwards compatibility, I guess, but probably worth making configurable at some point.

_test_import_deps_checker_checking_mode,
_test_commandline_contains_target_label,
_test_java_library_allows_import_in_deps,
_test_module_flags,
Expand Down
1 change: 1 addition & 0 deletions toolchains/default_java_toolchain.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ _DEFAULT_JAVA_LANGUAGE_VERSION = "11"

# Default java_toolchain parameters
_BASE_TOOLCHAIN_CONFIGURATION = dict(
deps_checker = Label("@remote_java_tools//:ImportDepsChecker"),

@fmeum fmeum Jun 30, 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.

This is a breaking change for Bazel as the hardcoded mode is error. Can I add a new attribute that makes it configurable on the toolchain level? I can set the default to silence to match the current behavior or to error as a breaking change with a simple migration.

Are you precompiling the tool to a native image at Google?

@cushon cushon Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Are you precompiling the tool to a native image at Google?

It's a prebuilt jar that runs on the JVM, but it's also a validation action that is not on the critical path (#362 (comment)).

Using a native image seems like a good idea if it's going to run as a non-validation action so the .jdeps can be used for reduced classpaths.

@cushon cushon Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can I add a new attribute that makes it configurable on the toolchain level?

Bazel supports tags = ["incomplete-deps"] on java_import. Having a real attribute (on the toolchain and/or individual java_imports) is probably better than leaning on tags for this.

cc @hvadehra

@fmeum fmeum Jun 30, 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.

My main concern is that external repos using java_import would be very difficult to opt out unless there is a global (toolchain) switch.

@hvadehra hvadehra Jul 3, 2026

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.

This should only be a breaking change for Bazel 7, which is okay I guess. I also don't see why we need a separate switch - one can just unset deps_checker on the toolchain.

forcibly_disable_header_compilation = False,
genclass = Label("@remote_java_tools//:GenClass"),
header_compiler = Label("@remote_java_tools//:TurbineDirect"),
Expand Down