Skip to content

Commit

Permalink
Fix collection of Maven runtime deps (#1308)
Browse files Browse the repository at this point in the history
A transitive dependency of a `maven_project_jar` is a runtime dependency if and only if each path to it from the root goes through at least one `runtime_deps` edge. Before this commit, every transitive dependency that was reachable from the root via at least one path with a `runtime_deps` edge was considered to be a runtime dependency, even if other paths require it as a compile dependency.

Also show a helpful error when users attempt to move a dep from `runtime_deps` to `deps` on a `java_export` target with no `srcs`.
  • Loading branch information
fmeum authored Jan 17, 2025
1 parent c1ca608 commit a1d4e4f
Show file tree
Hide file tree
Showing 10 changed files with 201 additions and 27 deletions.
3 changes: 2 additions & 1 deletion .bazelci/presubmit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ tasks:
- "//..."
ubuntu2204_latest:
platform: ubuntu2204
bazel: latest
# TODO: Revert to latest after fixing incompatibilities with Bazel 8.0.0
bazel: 7.x
environment:
# This tests custom cache locations.
# https://github.com/bazelbuild/rules_jvm_external/pull/316
Expand Down
2 changes: 2 additions & 0 deletions private/lib/coordinates.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ def unpack_coordinates(coords):
return None

pieces = coords.split(":")
if len(pieces) < 2:
fail("Could not parse maven coordinate: %s" % coords)
group = pieces[0]
artifact = pieces[1]

Expand Down
38 changes: 22 additions & 16 deletions private/rules/has_maven_deps.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@ MavenInfo = provider(
fields = {
# Fields to do with maven coordinates
"coordinates": "Maven coordinates for the project, which may be None",
"maven_deps": "Depset of first-order maven dependencies",
"maven_runtime_deps": "Depset of first-order maven runtime dependencies",
"as_maven_dep": "Depset of this project if used as a maven dependency",
"maven_deps": "Depset of coordinates of all transitive maven dependencies",
"maven_compile_deps": "Depset of coordinates of all transitive maven dependencies with compile scope",

# Fields used for generating artifacts
"artifact_infos": "Depset of JavaInfo instances of targets to include in the maven artifact",
Expand All @@ -29,7 +28,7 @@ to offer `tags` to be used to infer maven information.
_EMPTY_INFO = MavenInfo(
coordinates = None,
maven_deps = depset(),
as_maven_dep = depset(),
maven_compile_deps = depset(),
artifact_infos = depset(),
dep_infos = depset(),
label_to_javainfo = {},
Expand All @@ -39,7 +38,7 @@ _EMPTY_INFO = MavenInfo(
_STOPPED_INFO = MavenInfo(
coordinates = "STOPPED",
maven_deps = depset(),
as_maven_dep = depset(),
maven_compile_deps = depset(),
artifact_infos = depset(),
dep_infos = depset(),
label_to_javainfo = {},
Expand Down Expand Up @@ -109,7 +108,8 @@ def calculate_artifact_source_jars(maven_info):
_gathered = provider(
fields = [
"all_infos",
"runtime_infos",
"all_deps",
"compile_deps",
"label_to_javainfo",
"artifact_infos",
"transitive_exports",
Expand All @@ -122,20 +122,26 @@ def _extract_from(gathered, maven_info, dep, include_transitive_exports, is_runt

gathered.all_infos.append(maven_info)

if is_runtime_dep:
gathered.runtime_infos.append(maven_info)

gathered.label_to_javainfo.update(maven_info.label_to_javainfo)
if java_info:
if maven_info.coordinates == "STOPPED":
if maven_info.coordinates == _STOPPED_INFO.coordinates:
pass
if maven_info.coordinates:
elif maven_info.coordinates:
gathered.dep_infos.append(dep[JavaInfo])

own_coordinates = depset([maven_info.coordinates])
gathered.all_deps.append(own_coordinates)
if not is_runtime_dep:
gathered.compile_deps.append(own_coordinates)
else:
gathered.artifact_infos.append(dep[JavaInfo])
if include_transitive_exports:
gathered.transitive_exports.append(maven_info.transitive_exports)

gathered.all_deps.append(maven_info.maven_deps)
if not is_runtime_dep:
gathered.compile_deps.append(maven_info.maven_compile_deps)

def _has_maven_deps_impl(target, ctx):
if not JavaInfo in target:
return [_EMPTY_INFO]
Expand All @@ -157,7 +163,8 @@ def _has_maven_deps_impl(target, ctx):

gathered = _gathered(
all_infos = [],
runtime_infos = [],
all_deps = [],
compile_deps = [],
artifact_infos = [target[JavaInfo]],
transitive_exports = [],
dep_infos = [],
Expand All @@ -181,8 +188,8 @@ def _has_maven_deps_impl(target, ctx):
transitive_exports_from_deps = gathered.transitive_exports
dep_infos = gathered.dep_infos
label_to_javainfo = gathered.label_to_javainfo
maven_deps = depset(transitive = [i.as_maven_dep for i in all_infos])
maven_runtime_deps = depset(transitive = [i.as_maven_dep for i in gathered.runtime_infos])
maven_deps = depset(transitive = gathered.all_deps)
maven_compile_deps = depset(transitive = gathered.compile_deps)

transitive_exports_from_exports = depset()
if hasattr(ctx.rule.attr, "exports"):
Expand All @@ -195,8 +202,7 @@ def _has_maven_deps_impl(target, ctx):
info = MavenInfo(
coordinates = coordinates,
maven_deps = maven_deps,
maven_runtime_deps = maven_runtime_deps,
as_maven_dep = depset([coordinates]) if coordinates else maven_deps,
maven_compile_deps = maven_compile_deps,
artifact_infos = depset(direct = artifact_infos),
dep_infos = depset(direct = dep_infos, transitive = [i.dep_infos for i in all_infos]),
label_to_javainfo = label_to_javainfo,
Expand Down
6 changes: 6 additions & 0 deletions private/rules/java_export.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,12 @@ def java_export(
doc_resources = kwargs.pop("doc_resources", [])
toolchains = kwargs.pop("toolchains", [])

# java_library doesn't allow srcs without deps, but users may try to specify deps rather than
# runtime_deps on java_export to indicate that the generated POM should list the deps as compile
# deps.
if kwargs.get("deps") and not kwargs.get("srcs"):
fail("deps not allowed without srcs; move to runtime_deps (for 'runtime' scope in the generated POM) or exports (for 'compile' scope)")

# Construct the java_library we'll export from here.
java_library(
name = lib_name,
Expand Down
6 changes: 5 additions & 1 deletion private/rules/maven_bom.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,16 @@ def _label(label_or_string):

def _maven_bom_impl(ctx):
fragments = [f[MavenBomFragmentInfo] for f in ctx.attr.fragments]
dep_coordinates = [f.coordinates for f in fragments]

# Expand maven coordinates for any variables to be replaced.
coordinates = ctx.expand_make_variables("coordinates", ctx.attr.maven_coordinates, {})

bom = generate_pom(
ctx,
coordinates = coordinates,
versioned_dep_coordinates = [f[MavenBomFragmentInfo].coordinates for f in ctx.attr.fragments],
versioned_dep_coordinates = dep_coordinates,
versioned_compile_dep_coordinates = dep_coordinates,
pom_template = ctx.file.pom_template,
out_name = "%s.xml" % ctx.label.name,
)
Expand Down Expand Up @@ -70,12 +72,14 @@ def _maven_dependencies_bom_impl(ctx):
first_order_deps = [f[MavenBomFragmentInfo].coordinates for f in ctx.attr.fragments]
all_deps = depset(transitive = [f.maven_info.maven_deps for f in fragments]).to_list()
combined_deps = [a for a in all_deps if a not in first_order_deps]
compile_deps = depset(transitive = [f.maven_info.maven_compile_deps for f in fragments]).to_list()

unpacked = unpack_coordinates(ctx.attr.bom_coordinates)
dependencies_bom = generate_pom(
ctx,
coordinates = ctx.attr.maven_coordinates,
versioned_dep_coordinates = combined_deps + ["%s:%s:pom:%s" % (unpacked.group, unpacked.artifact, unpacked.version)],
versioned_compile_dep_coordinates = compile_deps,
pom_template = ctx.file.pom_template,
out_name = "%s.xml" % ctx.label.name,
indent = 12,
Expand Down
8 changes: 6 additions & 2 deletions private/rules/maven_utils.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,12 @@ def generate_pom(
parent = None,
versioned_dep_coordinates = [],
unversioned_dep_coordinates = [],
runtime_deps = [],
versioned_compile_dep_coordinates = [],
indent = 8):
versioned_compile_dep_coordinates_set = {
k: None
for k in versioned_compile_dep_coordinates
}
unpacked_coordinates = _unpack_coordinates(coordinates)
substitutions = {
"{groupId}": unpacked_coordinates.group,
Expand Down Expand Up @@ -93,7 +97,7 @@ def generate_pom(
include_version = dep in versioned_dep_coordinates
unpacked = _unpack_coordinates(dep)

new_scope = "runtime" if dep in runtime_deps else "compile"
new_scope = "compile" if dep in versioned_compile_dep_coordinates_set else "runtime"
if unpacked.packaging == "pom":
new_scope = "import"

Expand Down
16 changes: 9 additions & 7 deletions private/rules/pom_file.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,21 @@ def _pom_file_impl(ctx):
additional_deps = determine_additional_dependencies(artifact_jars, ctx.attr.additional_dependencies)

all_maven_deps = info.maven_deps.to_list()
runtime_maven_deps = info.maven_runtime_deps.to_list()
compile_maven_deps = info.maven_compile_deps.to_list()

for dep in additional_deps:
for coords in dep[MavenInfo].as_maven_dep.to_list():
all_maven_deps.append(coords)
dep_info = dep[MavenInfo]
dep_coordinates = [dep_info.coordinates] if dep_info.coordinates else dep_info.as_maven_dep.to_list()
all_maven_deps.extend(dep_coordinates)
compile_maven_deps.extend(dep_coordinates)

expanded_maven_deps = [
ctx.expand_make_variables("additional_deps", coords, ctx.var)
for coords in all_maven_deps
]
expanded_runtime_deps = [
ctx.expand_make_variables("maven_runtime_deps", coords, ctx.var)
for coords in runtime_maven_deps
expanded_compile_deps = [
ctx.expand_make_variables("maven_compile_deps", coords, ctx.var)
for coords in compile_maven_deps
]

# Expand maven coordinates for any variables to be replaced.
Expand All @@ -35,7 +37,7 @@ def _pom_file_impl(ctx):
ctx,
coordinates = coordinates,
versioned_dep_coordinates = sorted(expanded_maven_deps),
runtime_deps = expanded_runtime_deps,
versioned_compile_dep_coordinates = expanded_compile_deps,
pom_template = ctx.file.pom_template,
out_name = "%s.xml" % ctx.label.name,
)
Expand Down
3 changes: 3 additions & 0 deletions tests/unit/runtime_scope/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
load("//tests/unit/runtime_scope:runtime_scope_test.bzl", "runtime_scope_tests")

runtime_scope_tests(name = "runtime_scope_tests")
Empty file.
146 changes: 146 additions & 0 deletions tests/unit/runtime_scope/runtime_scope_test.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
"""Unit tests for java_export runtime_deps behavior."""

load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts")
load("@rules_java//java:defs.bzl", "JavaInfo", "java_library")
load("//private/rules:has_maven_deps.bzl", "MavenInfo", "has_maven_deps")
load("//private/rules:maven_project_jar.bzl", "maven_project_jar")

def _runtime_scope_test_impl(ctx):
env = analysistest.begin(ctx)
tut = analysistest.target_under_test(env)

asserts.equals(env, [
# keep sorted
"example:dep_leaf:1.0.0",
"example:exported_leaf:1.0.0",
"example:leaf:1.0.0",
"example:runtime_dep_and_dep_leaf:1.0.0",
"example:runtime_dep_leaf:1.0.0",
], sorted(tut[MavenInfo].maven_deps.to_list()))
asserts.equals(env, [
# keep sorted
"example:exported_leaf:1.0.0",
"example:leaf:1.0.0",
"example:runtime_dep_and_dep_leaf:1.0.0",
], sorted(tut[MavenInfo].maven_compile_deps.to_list()))

return analysistest.end(env)

_runtime_scope_test = analysistest.make(
_runtime_scope_test_impl,
extra_target_under_test_aspects = [has_maven_deps],
)

def runtime_scope_tests(name):
java_library(
name = "library_to_test",
deps = [
":is_dep_has_exports",
":is_dep_has_runtime_deps",
],
exports = [":is_export_has_deps"],
runtime_deps = [":is_runtime_dep_has_deps"],
srcs = ["Foo.java"],
)

java_library(
name = "is_dep_has_exports",
exports = [
":leaf",
":runtime_dep_and_dep_leaf",
":stop_propagation",
],
srcs = ["Foo.java"],
)

java_library(
name = "is_dep_has_runtime_deps",
runtime_deps = [
":runtime_dep_leaf",
":runtime_dep_and_dep_leaf",
],
srcs = ["Foo.java"],
)

java_library(
name = "is_export_has_deps",
deps = [":exported_leaf"],
srcs = ["Foo.java"],
)

java_library(
name = "is_runtime_dep_has_deps",
deps = [":dep_leaf"],
srcs = ["Foo.java"],
)

java_library(
name = "leaf",
srcs = ["Foo.java"],
tags = [
"maven_coordinates=example:leaf:1.0.0",
],
)

java_library(
name = "exported_leaf",
srcs = ["Foo.java"],
tags = [
"maven_coordinates=example:exported_leaf:1.0.0",
],
)

java_library(
name = "dep_leaf",
srcs = ["Foo.java"],
tags = [
"maven_coordinates=example:dep_leaf:1.0.0",
],
)

java_library(
name = "runtime_dep_leaf",
srcs = ["Foo.java"],
tags = [
"maven_coordinates=example:runtime_dep_leaf:1.0.0",
],
)

java_library(
name = "runtime_dep_and_dep_leaf",
srcs = ["Foo.java"],
tags = [
"maven_coordinates=example:runtime_dep_and_dep_leaf:1.0.0",
],
deps = [":transitive_maven_dep"],
)

java_library(
name = "stop_propagation",
srcs = ["Foo.java"],
tags = [
"no-maven",
],
deps = [":not_included_leaf"],
)

java_library(
name = "not_included_leaf",
srcs = ["Foo.java"],
tags = [
"maven_coordinates=example:not_included_leaf:1.0.0",
],
)

java_library(
name = "transitive_maven_dep",
srcs = ["Foo.java"],
tags = [
"maven_coordinates=example:transitive_maven_dep:1.0.0",
],
)

_runtime_scope_test(
name = name,
target_under_test = ":library_to_test",
)

0 comments on commit a1d4e4f

Please sign in to comment.