Skip to content
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

Fix collection of Maven runtime deps #1308

Merged
merged 2 commits into from
Jan 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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",
)