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

feat: allow attaching platform constraints to dependency trees imported through maven.install #1298

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
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: 3 additions & 0 deletions MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,9 @@ dev_maven.install(
"https://repo1.maven.org/maven2",
"https://maven.google.com",
],
targets_compatible_with = [
"@platforms//os:android",
],
)
dev_maven.install(
name = "m2local_testing",
Expand Down
6 changes: 5 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,9 @@ java_library(

If your WORKSPACE contains several projects that use different versions of the
same artifact, you can specify multiple `maven_install` declarations in the
WORKSPACE, with a unique repository name for each of them.
WORKSPACE, with a unique repository name for each of them. You can ensure that
builds do not cross this division using [platform
constraints](https://bazel.build/extending/platforms).

For example, if you want to use the JRE version of Guava for a server app, and
the Android version for an Android app, you can specify two `maven_install`
Expand All @@ -548,6 +550,7 @@ maven_install(
repositories = [
"https://repo1.maven.org/maven2",
],
targets_compatible_with = ["@platforms//os:linux"],
)

maven_install(
Expand All @@ -558,6 +561,7 @@ maven_install(
repositories = [
"https://repo1.maven.org/maven2",
],
targets_compatible_with = ["@platforms//os:android"],
)
```

Expand Down
3 changes: 3 additions & 0 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,9 @@ maven_install(
repositories = [
"https://repo1.maven.org/maven2",
],
targets_compatible_with = [
"@platforms//os:android",
],
)

maven_install(
Expand Down
33 changes: 30 additions & 3 deletions private/dependency_tree_parser.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,34 @@ package_info(
target_import_string.append("\tvisibility = [%s]," % (",".join(["\"%s\"" % t for t in target_visibilities])))
alias_visibility = "\tvisibility = [%s],\n" % (",".join(["\"%s\"" % t for t in target_visibilities]))

# 9. Finish the java_import rule.
# 9. If `targets_compatible_with` is configured, set it on non-transitive dependencies; the effect applies transitively already.
# visibility only for non-transitive dependencies.
#
# java_import(
# name = "org_hamcrest_hamcrest_library",
# jars = ["https/repo1.maven.org/maven2/org/hamcrest/hamcrest-library/1.3/hamcrest-library-1.3.jar"],
# srcjar = "https/repo1.maven.org/maven2/org/hamcrest/hamcrest-library/1.3/hamcrest-library-1.3-sources.jar",
# deps = [
# ":org_hamcrest_hamcrest_core",
# ],
# tags = [
# "maven_coordinates=org.hamcrest:hamcrest.library:1.3"],
# "maven_url=https://repo1.maven.org/maven/org/hamcrest/hamcrest-core/1.3/hamcrest-core-1.3.jar",
# "maven:compile-only",
# ],
# neverlink = True,
# testonly = True,
# visibility = ["//visibility:public"],
# targets_compatible_with = [
# "@platforms//os:linux",
# ],
if repository_ctx.attr.targets_compatible_with:
target_import_string.append("\ttarget_compatible_with = [")
for t in repository_ctx.attr.targets_compatible_with:
target_import_string.append("\t\t\"%s\"," % t)
target_import_string.append("\t],")

# 10. Finish the java_import rule.
#
# java_import(
# name = "org_hamcrest_hamcrest_library",
Expand All @@ -334,7 +361,7 @@ package_info(

to_return.append("\n".join(target_import_string))

# 10. Create a versionless alias target
# 11. Create a versionless alias target
#
# alias(
# name = "org_hamcrest_hamcrest_library_1_3",
Expand All @@ -360,7 +387,7 @@ processor_class = "{processor_class}",
),
)

# 11. If using maven_install.json, use a genrule to copy the file from the http_file
# 12. If using maven_install.json, use a genrule to copy the file from the http_file
# repository into this repository.
#
# genrule(
Expand Down
15 changes: 15 additions & 0 deletions private/extensions/maven.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ install = tag_class(
),
"strict_visibility_value": attr.label_list(default = ["//visibility:private"]),

"targets_compatible_with": attr.label_list(
doc = "Platform constraints to add to the repository's targets.",
default = [],
),

# Android support
"aar_import_bzl_label": attr.string(default = DEFAULT_AAR_IMPORT_LABEL, doc = "The label (as a string) to use to import aar_import from"),
"use_starlark_android_rules": attr.bool(default = False, doc = "Whether to use the native or Starlark version of the Android rules."),
Expand Down Expand Up @@ -222,6 +227,7 @@ def maven_impl(mctx):
# - version_conflict_policy: string. Fails build if different and not a default.
# - ignore_empty_files: Treat jars that are empty as if they were not found.
# - additional_coursier_options: Additional options that will be passed to coursier.
# - targets_compatible_with: a string list. Build will fail is duplicated and different.

# Mapping of `name`s to a list of `bazel_module.name`. This will allow us to
# warn users when more than one module attempts to update a maven repo
Expand Down Expand Up @@ -344,6 +350,13 @@ def maven_impl(mctx):

repo["additional_coursier_options"] = repo.get("additional_coursier_options", []) + getattr(install, "additional_coursier_options", [])

repo["targets_compatible_with"] = _fail_if_different(
"targets_compatible_with",
repo.get("targets_compatible_with"),
install.targets_compatible_with,
[None, []],
)

repos[install.name] = repo

for (repo_name, known_names) in repo_name_2_module_name.items():
Expand Down Expand Up @@ -441,6 +454,7 @@ def maven_impl(mctx):
duplicate_version_warning = repo.get("duplicate_version_warning"),
ignore_empty_files = repo.get("ignore_empty_files"),
additional_coursier_options = repo.get("additional_coursier_options"),
targets_compatible_with = repo.get("targets_compatible_with"),
)
else:
workspace_prefix = "@@" if bazel_features.external_deps.is_bzlmod_enabled else "@"
Expand Down Expand Up @@ -502,6 +516,7 @@ def maven_impl(mctx):
duplicate_version_warning = repo.get("duplicate_version_warning"),
excluded_artifacts = excluded_artifacts_json,
repin_instructions = repo.get("repin_instructions"),
targets_compatible_with = repo.get("targets_compatible_with"),
)

if repo.get("generate_compat_repositories"):
Expand Down
8 changes: 8 additions & 0 deletions private/rules/coursier.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -1427,6 +1427,10 @@ pinned_coursier_fetch = repository_rule(
"excluded_artifacts": attr.string_list(default = []), # only used for hash generation
# Use @@// to refer to the main repo with Bzlmod.
"_workspace_label": attr.label(default = ("@@" if str(Label("//:invalid")).startswith("@@") else "@") + "//does/not:exist"),
"targets_compatible_with": attr.label_list(
doc = "Platform constraints to add to the repository's targets.",
default = [],
),
},
implementation = _pinned_coursier_fetch_impl,
)
Expand Down Expand Up @@ -1494,6 +1498,10 @@ coursier_fetch = repository_rule(
doc = "Name of the corresponding pinned repo for this repo. Presence implies that this is an unpinned repo.",
mandatory = False,
),
"targets_compatible_with": attr.label_list(
doc = "Platform constraints to add to the repository's targets.",
default = [],
),
},
environ = [
"JAVA_HOME",
Expand Down
7 changes: 6 additions & 1 deletion private/rules/maven_install.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ def maven_install(
duplicate_version_warning = "warn",
repin_instructions = None,
ignore_empty_files = False,
additional_coursier_options = []):
additional_coursier_options = [],
targets_compatible_with = [],
):
"""Resolves and fetches artifacts transitively from Maven repositories.

This macro runs a repository rule that invokes the Coursier CLI to resolve
Expand Down Expand Up @@ -80,6 +82,7 @@ def maven_install(
repin_instructions: Instructions to re-pin dependencies in your repository. Will be shown when re-pinning is required.
ignore_empty_files: Treat jars that are empty as if they were not found.
additional_coursier_options: Additional options that will be passed to coursier.
targets_compatible_with: Platform constraints to add to the repository's targets.
"""
if boms and resolver == "coursier":
fail("The coursier resolver does not support resolving Maven BOMs. Please use another resolver.")
Expand Down Expand Up @@ -143,6 +146,7 @@ def maven_install(
duplicate_version_warning = duplicate_version_warning,
ignore_empty_files = ignore_empty_files,
additional_coursier_options = additional_coursier_options,
targets_compatible_with = targets_compatible_with,
)

else:
Expand Down Expand Up @@ -173,6 +177,7 @@ def maven_install(
use_starlark_android_rules = use_starlark_android_rules,
aar_import_bzl_label = aar_import_bzl_label,
repin_instructions = repin_instructions,
targets_compatible_with = targets_compatible_with,
# Extra arguments only used for hash generation
excluded_artifacts = excluded_artifacts_json_strings,
)
38 changes: 38 additions & 0 deletions tests/unit/jvm_import/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
load("@bazel_skylib//rules:build_test.bzl", "build_test")
load("@rules_java//java:defs.bzl", "java_import")
load(":jvm_import_test.bzl", "jvm_import_test_suite")
load(":platform_transition_jar.bzl", "platform_transition_jar")

java_import(
name = "java_import_that_consumes_the_downloaded_file_directly",
Expand All @@ -29,4 +30,41 @@ build_test(
],
)

platform(
name = "android",
constraint_values = ["@platforms//os:android"],
)

platform(
name = "linux",
constraint_values = ["@platforms//os:linux"],
)

# Platform constraint is valid on Android but invalid on Linux:
#
# maven_install(
# name = "jvm_import_test",
# # ...
# targets_compatible_with = ["@platforms//os:android"],
# # ...
# )
platform_transition_jar(
name = "findbugs_for_android",
src = "@jvm_import_test//:com_google_code_findbugs_jsr305_3_0_2",
platform = ":android",
)

platform_transition_jar(
name = "findbugs_for_linux",
src = "@jvm_import_test//:com_google_code_findbugs_jsr305_3_0_2",
platform = ":linux",
)

build_test(
name = "test_does_jar_artifact_work_with_matching_platform_constraint",
targets = [
":findbugs_for_android",
],
)

jvm_import_test_suite(name = "jvm_import_tests")
14 changes: 14 additions & 0 deletions tests/unit/jvm_import/jvm_import_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,16 @@ does_jvm_import_export_a_package_provider_test = analysistest.make(
},
)

def _does_jvm_import_enforce_platform_constraints_impl(ctx):
env = analysistest.begin(ctx)
asserts.expect_failure(env, expected_failure_msg = "didn't satisfy constraint")
return analysistest.end(env)

does_jvm_import_enforce_platform_constraints_test = analysistest.make(
_does_jvm_import_enforce_platform_constraints_impl,
expect_failure = True,
)

def _does_non_jvm_import_target_carry_metadata(ctx):
env = analysistest.begin(ctx)

Expand Down Expand Up @@ -121,6 +131,10 @@ def jvm_import_test_suite(name):
target_under_test = "@jvm_import_test//:com_google_code_findbugs_jsr305",
src = "@jvm_import_test//:com_google_code_findbugs_jsr305",
)
does_jvm_import_enforce_platform_constraints_test(
name = "does_jvm_import_enforce_platform_constraints_test",
target_under_test = ":findbugs_for_linux",
)

# TODO: restore once https://github.com/bazelbuild/rules_license/issues/154 is resolved
# does_non_jvm_import_target_carry_metadata_test(
Expand Down
31 changes: 31 additions & 0 deletions tests/unit/jvm_import/platform_transition_jar.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
def _xsitn_impl(settings, attr):
return {"//command_line_option:platforms": str(attr.platform)}

_transition = transition(
implementation = _xsitn_impl,
inputs = [],
outputs = ["//command_line_option:platforms"],
)

def _rule_impl(ctx):
return [
DefaultInfo(files = depset(transitive = [ctx.attr.src[0][DefaultInfo].files])),
]

platform_transition_jar = rule(
implementation = _rule_impl,
attrs = {
"src": attr.label(
cfg = _transition,
),
"platform": attr.label(),
"_allowlist_function_transition": attr.label(
default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
),
},
doc = """
Depend on a JAR for a specified platform.
This isn't typical usage - for the most part, Java builds should use a consistent config.
Using a transition enables modelling multi-platform builds within the build system for the purposes of testing.
""",
)