From c22475165f67390c31393fe62a98556f5c9deda5 Mon Sep 17 00:00:00 2001 From: Peter Lobsinger Date: Sun, 15 Dec 2024 17:08:23 -0500 Subject: [PATCH 1/4] Plumb targets_compatible_with setting from source to use-site The user sets it in `MODULE.bazl`/`WORKSPACE` to affect `java_import` targets in the `maven.install` repo. --- private/dependency_tree_parser.bzl | 33 +++++++++++++++++++++++++++--- private/extensions/maven.bzl | 15 ++++++++++++++ private/rules/coursier.bzl | 8 ++++++++ private/rules/maven_install.bzl | 7 ++++++- 4 files changed, 59 insertions(+), 4 deletions(-) diff --git a/private/dependency_tree_parser.bzl b/private/dependency_tree_parser.bzl index d778bab43..8f50d7f47 100644 --- a/private/dependency_tree_parser.bzl +++ b/private/dependency_tree_parser.bzl @@ -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", @@ -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", @@ -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( diff --git a/private/extensions/maven.bzl b/private/extensions/maven.bzl index 929b6e89d..550f9db7d 100644 --- a/private/extensions/maven.bzl +++ b/private/extensions/maven.bzl @@ -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."), @@ -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 @@ -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(): @@ -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 "@" @@ -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"): diff --git a/private/rules/coursier.bzl b/private/rules/coursier.bzl index 69b4f4fb6..cab6419c1 100644 --- a/private/rules/coursier.bzl +++ b/private/rules/coursier.bzl @@ -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, ) @@ -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", diff --git a/private/rules/maven_install.bzl b/private/rules/maven_install.bzl index 3378e0c8d..8da25cb84 100644 --- a/private/rules/maven_install.bzl +++ b/private/rules/maven_install.bzl @@ -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 @@ -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.") @@ -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: @@ -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, ) From 2e614542fb50b25298afac6a69f9714e87525868 Mon Sep 17 00:00:00 2001 From: Peter Lobsinger Date: Sun, 15 Dec 2024 23:35:23 -0500 Subject: [PATCH 2/4] Document targets_compatible_with feature --- README.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index eb0b3bc4b..ae11e0051 100644 --- a/README.md +++ b/README.md @@ -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` @@ -548,6 +550,7 @@ maven_install( repositories = [ "https://repo1.maven.org/maven2", ], + targets_compatible_with = ["@platforms//os:linux"], ) maven_install( @@ -558,6 +561,7 @@ maven_install( repositories = [ "https://repo1.maven.org/maven2", ], + targets_compatible_with = ["@platforms//os:android"], ) ``` From 34a02fd6787edb435e5394dd1c447ff0b6956bfb Mon Sep 17 00:00:00 2001 From: Peter Lobsinger Date: Mon, 16 Dec 2024 14:03:02 -0500 Subject: [PATCH 3/4] Provide a unit test for targets_compatible_with feature --- MODULE.bazel | 3 ++ WORKSPACE | 3 ++ tests/unit/jvm_import/BUILD | 38 +++++++++++++++++++ tests/unit/jvm_import/jvm_import_test.bzl | 14 +++++++ .../jvm_import/platform_transition_jar.bzl | 28 ++++++++++++++ 5 files changed, 86 insertions(+) create mode 100644 tests/unit/jvm_import/platform_transition_jar.bzl diff --git a/MODULE.bazel b/MODULE.bazel index 682c38aaf..e0f24f2d2 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -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", diff --git a/WORKSPACE b/WORKSPACE index 543b0e6dd..73a2e9fd6 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -526,6 +526,9 @@ maven_install( repositories = [ "https://repo1.maven.org/maven2", ], + targets_compatible_with = [ + "@platforms//os:android", + ], ) maven_install( diff --git a/tests/unit/jvm_import/BUILD b/tests/unit/jvm_import/BUILD index 6b0abc47a..5bafb3d6d 100644 --- a/tests/unit/jvm_import/BUILD +++ b/tests/unit/jvm_import/BUILD @@ -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", @@ -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") diff --git a/tests/unit/jvm_import/jvm_import_test.bzl b/tests/unit/jvm_import/jvm_import_test.bzl index ccb7c14df..b6755d90f 100644 --- a/tests/unit/jvm_import/jvm_import_test.bzl +++ b/tests/unit/jvm_import/jvm_import_test.bzl @@ -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) @@ -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( diff --git a/tests/unit/jvm_import/platform_transition_jar.bzl b/tests/unit/jvm_import/platform_transition_jar.bzl new file mode 100644 index 000000000..400ecf6d9 --- /dev/null +++ b/tests/unit/jvm_import/platform_transition_jar.bzl @@ -0,0 +1,28 @@ +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(), + }, + 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. + """, +) From b1fbc25a55a3097f420a44d9d8e18451f41f3eb9 Mon Sep 17 00:00:00 2001 From: Peter Lobsinger Date: Mon, 16 Dec 2024 14:24:30 -0500 Subject: [PATCH 4/4] Opt-in to transitions Necessary to use outbound transitions on Bazel versions prior to v7.1. --- tests/unit/jvm_import/platform_transition_jar.bzl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/unit/jvm_import/platform_transition_jar.bzl b/tests/unit/jvm_import/platform_transition_jar.bzl index 400ecf6d9..442d36f0d 100644 --- a/tests/unit/jvm_import/platform_transition_jar.bzl +++ b/tests/unit/jvm_import/platform_transition_jar.bzl @@ -19,6 +19,9 @@ platform_transition_jar = rule( 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.