From fcaff31c48a4ffa702e8ec32be9f7fd114c09551 Mon Sep 17 00:00:00 2001 From: Jingwen Chen Date: Tue, 5 Nov 2024 13:08:52 +0000 Subject: [PATCH 1/8] Allow root module's override tags to take precedence over the overrides from the transitive deps. --- MODULE.bazel | 20 ++++++++++++ WORKSPACE | 15 +++++++++ private/extensions/maven.bzl | 17 +++++++--- tests/integration/override_targets/BUILD | 17 ++++++++++ .../override_targets/module/MODULE.bazel | 18 +++++++++++ .../root_module_can_override_test.sh | 31 +++++++++++++++++++ 6 files changed, 114 insertions(+), 4 deletions(-) create mode 100644 tests/integration/override_targets/module/MODULE.bazel create mode 100755 tests/integration/override_targets/root_module_can_override_test.sh diff --git a/MODULE.bazel b/MODULE.bazel index 29863ee9f..a46b3d33e 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -775,6 +775,25 @@ dev_maven.install( ], ) + +dev_maven.install( + name = "root_module_can_override", + artifacts = ["com.squareup:javapoet:1.11.1"], +) + +bazel_dep(name = "transitive_module_can_override", version = "0.0.0") +local_path_override( + module_name = "transitive_module_can_override", + path = "tests/integration/override_targets/module", +) + +dev_maven.override( + # This override demonstrates that this root module's override takes precedence over that transitive override definition. + # Use something absurd for testing, like overriding okhttp3 to javapoet. + coordinates = "com.squareup.okhttp3:okhttp", + target = "@root_module_can_override//:com_squareup_javapoet", +) + # Where there are file locks, the pinned and unpinned repos are listed # next to each other. Where compat repositories are created, they are # listed next to the repo that created them. The list is otherwise kept @@ -860,6 +879,7 @@ use_repo( "starlark_aar_import_test", "starlark_aar_import_with_sources_test", "strict_visibility_testing", + "root_module_can_override", # Repo with compat repos "com_google_http_client_google_http_client_gson", diff --git a/WORKSPACE b/WORKSPACE index aad863e1c..e7c04e0e7 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -967,3 +967,18 @@ maven_install( "https://repo1.maven.org/maven2", ], ) + +# This failure mode is bzlmod only. But the test still runs on Bazel 5/6, which +# is WORKSPACE based, so we add a shim here to keep the test passing until +# WORKSPACE support is no longer needed. +maven_install( + name = "root_module_can_override", + artifacts = [ + "com.squareup:javapoet:1.11.1", + "com.squareup.okhttp3:okhttp:4.12.0", + ], + override_targets = { + "com.squareup.okhttp3:okhttp": "@root_module_can_override//:com_squareup_javapoet", + }, + repositories = ["https://repo1.maven.org/maven2"], +) diff --git a/private/extensions/maven.bzl b/private/extensions/maven.bzl index 745d0d9bb..da32a4363 100644 --- a/private/extensions/maven.bzl +++ b/private/extensions/maven.bzl @@ -228,15 +228,24 @@ def maven_impl(mctx): # can intentionally contribute to the default `maven` repo namespace.) repo_name_2_module_name = {} - for mod in mctx.modules: + # First compute the overrides. The order of the transitive overrides do not matter, but the root + # overrides take precedence over all transitive ones. + for idx, mod in enumerate(reversed(mctx.modules)): + # Rotate the root module to the last to be visited. + is_root_module = idx == (len(mctx.modules) - 1) for override in mod.tags.override: if not override.name in overrides: overrides[override.name] = {} value = str(override.target) - current = overrides[override.name].get(override.coordinates) - to_use = _fail_if_different("Target of override for %s" % override.coordinates, current, value, [None]) - overrides[override.name].update({override.coordinates: to_use}) + if is_root_module: + # Allow the root module's overrides to take precedence over any transitive overrides. + to_use = value + else: + current = overrides[override.name].get(override.coordinates) + to_use = _fail_if_different("Target of override for %s" % override.coordinates, current, value, [None]) + overrides[override.name].update({override.coordinates: value}) + for mod in mctx.modules: for artifact in mod.tags.artifact: _check_repo_name(repo_name_2_module_name, artifact.name, mod.name) diff --git a/tests/integration/override_targets/BUILD b/tests/integration/override_targets/BUILD index 64096f5c8..3b7efdc52 100644 --- a/tests/integration/override_targets/BUILD +++ b/tests/integration/override_targets/BUILD @@ -69,3 +69,20 @@ sh_test( "@bazel_tools//tools/bash/runfiles", ], ) + +genquery( + name = "root_module_can_override", + expression = "deps(@root_module_can_override//:com_squareup_okhttp3_okhttp)", + opts = [ + "--nohost_deps", + "--noimplicit_deps", + ], + scope = ["@root_module_can_override//:com_squareup_okhttp3_okhttp"], +) + +sh_test( + name = "root_module_can_override_test", + srcs = ["root_module_can_override_test.sh"], + data = [":root_module_can_override"], + deps = ["@bazel_tools//tools/bash/runfiles"], +) diff --git a/tests/integration/override_targets/module/MODULE.bazel b/tests/integration/override_targets/module/MODULE.bazel new file mode 100644 index 000000000..50210f61a --- /dev/null +++ b/tests/integration/override_targets/module/MODULE.bazel @@ -0,0 +1,18 @@ +module(name = "transitive_module_can_override", version = "0.0.0") + +bazel_dep(name = "rules_jvm_external", version = "0.0") +local_path_override( + module_name = "rules_jvm_external", + path = "../../../..", +) + +maven = use_extension("@rules_jvm_external//:extensions.bzl", "maven") +maven.install( + name = "root_module_can_override", + artifacts = ["com.squareup.okhttp3:okhttp:4.12.0"], +) + +maven.override( + coordinates = "com.squareup.okhttp3:okhttp3", + target = "//:poison_pill_non_existent_target", +) diff --git a/tests/integration/override_targets/root_module_can_override_test.sh b/tests/integration/override_targets/root_module_can_override_test.sh new file mode 100755 index 000000000..d1582a054 --- /dev/null +++ b/tests/integration/override_targets/root_module_can_override_test.sh @@ -0,0 +1,31 @@ +# --- begin runfiles.bash initialization v2 --- +# Copy-pasted from the Bazel Bash runfiles library v2. +set -uo pipefail; f=bazel_tools/tools/bash/runfiles/runfiles.bash +source "${RUNFILES_DIR:-/dev/null}/$f" 2>/dev/null || \ + source "$(grep -sm1 "^$f " "${RUNFILES_MANIFEST_FILE:-/dev/null}" | cut -f2- -d' ')" 2>/dev/null || \ + source "$0.runfiles/$f" 2>/dev/null || \ + source "$(grep -sm1 "^$f " "$0.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \ + source "$(grep -sm1 "^$f " "$0.exe.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \ + { echo>&2 "ERROR: cannot find $f"; exit 1; }; f=; set -e +# --- end runfiles.bash initialization v2 --- + +set -euox pipefail + +deps_file=$(rlocation rules_jvm_external/tests/integration/override_targets/root_module_can_override) + +function clean_up_workspace_names() { + local file_name="$1" + local target="$2" + # The first `sed` command replaces `@@` with `@`. The second extracts the visible name + # from the bzlmod mangled workspace name + cat "$file_name" | sed -e 's|^@@|@|g; s|\r||g' | sed -e 's|^@[^/]*[+~]|@|g; s|\r||g' | grep "$target" + cat "$file_name" | sed -e 's|^@@|@|g; s|\r||g' | sed -e 's|^@[^/]*[+~]|@|g; s|\r||g' | grep -q "$target" +} + +if ! clean_up_workspace_names "$deps_file" "@root_module_can_override//:com_squareup_okhttp3_okhttp"; then + exit 1 +fi + +if ! clean_up_workspace_names "$deps_file" "@root_module_can_override//:com_squareup_javapoet"; then + exit 1 +fi From ae6b331aac5c4c527d6e851600a20c4ae860cdc5 Mon Sep 17 00:00:00 2001 From: Sumeet Gajjar Date: Wed, 18 Jun 2025 17:16:16 -0700 Subject: [PATCH 2/8] Add name to okhttp3 root module override --- MODULE.bazel | 1 + 1 file changed, 1 insertion(+) diff --git a/MODULE.bazel b/MODULE.bazel index a46b3d33e..e2c63b2c9 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -788,6 +788,7 @@ local_path_override( ) dev_maven.override( + name = "root_module_can_override", # This override demonstrates that this root module's override takes precedence over that transitive override definition. # Use something absurd for testing, like overriding okhttp3 to javapoet. coordinates = "com.squareup.okhttp3:okhttp", From f6d88812721e5a0e8236694b235efbc6d480d96c Mon Sep 17 00:00:00 2001 From: Sumeet Gajjar Date: Wed, 18 Jun 2025 17:18:31 -0700 Subject: [PATCH 3/8] Fix non-root overrides update logic to use updated 'to_use' cooridnates instead of original value --- private/extensions/maven.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/private/extensions/maven.bzl b/private/extensions/maven.bzl index da32a4363..e88001541 100644 --- a/private/extensions/maven.bzl +++ b/private/extensions/maven.bzl @@ -243,7 +243,7 @@ def maven_impl(mctx): else: current = overrides[override.name].get(override.coordinates) to_use = _fail_if_different("Target of override for %s" % override.coordinates, current, value, [None]) - overrides[override.name].update({override.coordinates: value}) + overrides[override.name].update({override.coordinates: to_use}) for mod in mctx.modules: for artifact in mod.tags.artifact: From 6fbc63c6907356804967265ced94e400946df803 Mon Sep 17 00:00:00 2001 From: Sumeet Gajjar Date: Wed, 18 Jun 2025 17:28:14 -0700 Subject: [PATCH 4/8] Address review comments from OG PR --- MODULE.bazel | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MODULE.bazel b/MODULE.bazel index e2c63b2c9..a21e479a3 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -781,7 +781,7 @@ dev_maven.install( artifacts = ["com.squareup:javapoet:1.11.1"], ) -bazel_dep(name = "transitive_module_can_override", version = "0.0.0") +bazel_dep(name = "transitive_module_can_override", version = "0.0.0", dev_dependency = True) local_path_override( module_name = "transitive_module_can_override", path = "tests/integration/override_targets/module", From 183f0f5e5e1950d45b8b0c5c9c8d29f47ab8d9df Mon Sep 17 00:00:00 2001 From: Sumeet Gajjar Date: Mon, 23 Jun 2025 17:23:08 -0700 Subject: [PATCH 5/8] Address review comments --- MODULE.bazel | 1 + 1 file changed, 1 insertion(+) diff --git a/MODULE.bazel b/MODULE.bazel index a21e479a3..a4f8dfb6f 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -791,6 +791,7 @@ dev_maven.override( name = "root_module_can_override", # This override demonstrates that this root module's override takes precedence over that transitive override definition. # Use something absurd for testing, like overriding okhttp3 to javapoet. + # The //tests/integration/override_targets:root_module_can_override_test validates the root override take precedence over transitive ones. coordinates = "com.squareup.okhttp3:okhttp", target = "@root_module_can_override//:com_squareup_javapoet", ) From 5f5014b2167ce380d8e2aecbed241b8d733140df Mon Sep 17 00:00:00 2001 From: Sumeet Gajjar Date: Mon, 23 Jun 2025 17:57:19 -0700 Subject: [PATCH 6/8] Replace sh_test with diff_test --- tests/integration/override_targets/BUILD | 37 ++++++--- .../override_contains_additional_deps.golden | 79 +++++++++++++++++++ .../override_contains_additional_deps.sh | 49 ------------ .../root_module_can_override.golden | 3 + .../root_module_can_override_test.sh | 31 -------- 5 files changed, 106 insertions(+), 93 deletions(-) create mode 100755 tests/integration/override_targets/override_contains_additional_deps.golden delete mode 100755 tests/integration/override_targets/override_contains_additional_deps.sh create mode 100755 tests/integration/override_targets/root_module_can_override.golden delete mode 100755 tests/integration/override_targets/root_module_can_override_test.sh diff --git a/tests/integration/override_targets/BUILD b/tests/integration/override_targets/BUILD index 3b7efdc52..28ae1bb05 100644 --- a/tests/integration/override_targets/BUILD +++ b/tests/integration/override_targets/BUILD @@ -1,4 +1,5 @@ load("@bazel_skylib//rules:build_test.bzl", "build_test") +load("@bazel_skylib//rules:diff_test.bzl", "diff_test") load("@rules_android//android:rules.bzl", "aar_import", "android_binary") load("@rules_java//java:defs.bzl", "java_library") @@ -59,15 +60,18 @@ genquery( scope = ["@override_target_in_deps//:io_opentelemetry_opentelemetry_sdk"], ) -sh_test( - name = "override_contains_additional_deps", - srcs = ["override_contains_additional_deps.sh"], - data = [ - ":trace_otel_deps", - ], - deps = [ - "@bazel_tools//tools/bash/runfiles", - ], +genrule( + name = "override_contains_additional_deps_sorted", + testonly = 1, + srcs = [":trace_otel_deps"], + outs = ["override_contains_additional_deps_sorted.txt"], + cmd = "cat $< | sed -e 's|^@@|@|g; s|\r||g' | sed -e 's|^@[^/]*[+~]|@|g; s|\r||g' | sort > $@", +) + +diff_test( + name = "override_contains_additional_deps_test", + file1 = ":override_contains_additional_deps.golden", + file2 = ":override_contains_additional_deps_sorted.txt", ) genquery( @@ -80,9 +84,16 @@ genquery( scope = ["@root_module_can_override//:com_squareup_okhttp3_okhttp"], ) -sh_test( +genrule( + name = "root_module_can_override_sorted", + testonly = 1, + srcs = [":root_module_can_override"], + outs = ["root_module_can_override_sorted.txt"], + cmd = "cat $< | sed -e 's|^@@|@|g; s|\r||g' | sed -e 's|^@[^/]*[+~]|@|g; s|\r||g' | sort > $@", +) + +diff_test( name = "root_module_can_override_test", - srcs = ["root_module_can_override_test.sh"], - data = [":root_module_can_override"], - deps = ["@bazel_tools//tools/bash/runfiles"], + file1 = ":root_module_can_override.golden", + file2 = ":root_module_can_override_sorted.txt", ) diff --git a/tests/integration/override_targets/override_contains_additional_deps.golden b/tests/integration/override_targets/override_contains_additional_deps.golden new file mode 100755 index 000000000..a180e7b8a --- /dev/null +++ b/tests/integration/override_targets/override_contains_additional_deps.golden @@ -0,0 +1,79 @@ +//tests/integration/override_targets:additional_deps +@bazel_tools//src/conditions:host_windows +@bazel_tools//src/conditions:host_windows_arm64_constraint +@bazel_tools//src/conditions:host_windows_x64_constraint +@com_google_code_gson_gson_2_10_1//file:file +@com_google_code_gson_gson_2_10_1//file:v1/com/google/code/gson/gson/2.10.1/gson-2.10.1.jar +@io_opentelemetry_opentelemetry_api_1_28_0//file:file +@io_opentelemetry_opentelemetry_api_1_28_0//file:v1/io/opentelemetry/opentelemetry-api/1.28.0/opentelemetry-api-1.28.0.jar +@io_opentelemetry_opentelemetry_api_events_1_28_0_alpha//file:file +@io_opentelemetry_opentelemetry_api_events_1_28_0_alpha//file:v1/io/opentelemetry/opentelemetry-api-events/1.28.0-alpha/opentelemetry-api-events-1.28.0-alpha.jar +@io_opentelemetry_opentelemetry_context_1_28_0//file:file +@io_opentelemetry_opentelemetry_context_1_28_0//file:v1/io/opentelemetry/opentelemetry-context/1.28.0/opentelemetry-context-1.28.0.jar +@io_opentelemetry_opentelemetry_extension_incubator_1_28_0_alpha//file:file +@io_opentelemetry_opentelemetry_extension_incubator_1_28_0_alpha//file:v1/io/opentelemetry/opentelemetry-extension-incubator/1.28.0-alpha/opentelemetry-extension-incubator-1.28.0-alpha.jar +@io_opentelemetry_opentelemetry_sdk_1_28_0//file:file +@io_opentelemetry_opentelemetry_sdk_1_28_0//file:v1/io/opentelemetry/opentelemetry-sdk/1.28.0/opentelemetry-sdk-1.28.0.jar +@io_opentelemetry_opentelemetry_sdk_common_1_28_0//file:file +@io_opentelemetry_opentelemetry_sdk_common_1_28_0//file:v1/io/opentelemetry/opentelemetry-sdk-common/1.28.0/opentelemetry-sdk-common-1.28.0.jar +@io_opentelemetry_opentelemetry_sdk_logs_1_28_0//file:file +@io_opentelemetry_opentelemetry_sdk_logs_1_28_0//file:v1/io/opentelemetry/opentelemetry-sdk-logs/1.28.0/opentelemetry-sdk-logs-1.28.0.jar +@io_opentelemetry_opentelemetry_sdk_metrics_1_28_0//file:file +@io_opentelemetry_opentelemetry_sdk_metrics_1_28_0//file:v1/io/opentelemetry/opentelemetry-sdk-metrics/1.28.0/opentelemetry-sdk-metrics-1.28.0.jar +@io_opentelemetry_opentelemetry_sdk_trace_1_28_0//file:file +@io_opentelemetry_opentelemetry_sdk_trace_1_28_0//file:v1/io/opentelemetry/opentelemetry-sdk-trace/1.28.0/opentelemetry-sdk-trace-1.28.0.jar +@io_opentelemetry_opentelemetry_semconv_1_28_0_alpha//file:file +@io_opentelemetry_opentelemetry_semconv_1_28_0_alpha//file:v1/io/opentelemetry/opentelemetry-semconv/1.28.0-alpha/opentelemetry-semconv-1.28.0-alpha.jar +@org_apache_commons_commons_pool2_2_11_1//file:file +@org_apache_commons_commons_pool2_2_11_1//file:v1/org/apache/commons/commons-pool2/2.11.1/commons-pool2-2.11.1.jar +@org_json_json_20231013//file:file +@org_json_json_20231013//file:v1/org/json/json/20231013/json-20231013.jar +@org_slf4j_slf4j_api_1_7_36//file:file +@org_slf4j_slf4j_api_1_7_36//file:v1/org/slf4j/slf4j-api/1.7.36/slf4j-api-1.7.36.jar +@override_target_in_deps//:com/google/code/gson/gson/2.10.1/gson-2.10.1.jar +@override_target_in_deps//:com_google_code_gson_gson +@override_target_in_deps//:com_google_code_gson_gson_2_10_1_extension +@override_target_in_deps//:io/opentelemetry/opentelemetry-api-events/1.28.0-alpha/opentelemetry-api-events-1.28.0-alpha.jar +@override_target_in_deps//:io/opentelemetry/opentelemetry-api/1.28.0/opentelemetry-api-1.28.0.jar +@override_target_in_deps//:io/opentelemetry/opentelemetry-context/1.28.0/opentelemetry-context-1.28.0.jar +@override_target_in_deps//:io/opentelemetry/opentelemetry-extension-incubator/1.28.0-alpha/opentelemetry-extension-incubator-1.28.0-alpha.jar +@override_target_in_deps//:io/opentelemetry/opentelemetry-sdk-common/1.28.0/opentelemetry-sdk-common-1.28.0.jar +@override_target_in_deps//:io/opentelemetry/opentelemetry-sdk-logs/1.28.0/opentelemetry-sdk-logs-1.28.0.jar +@override_target_in_deps//:io/opentelemetry/opentelemetry-sdk-metrics/1.28.0/opentelemetry-sdk-metrics-1.28.0.jar +@override_target_in_deps//:io/opentelemetry/opentelemetry-sdk-trace/1.28.0/opentelemetry-sdk-trace-1.28.0.jar +@override_target_in_deps//:io/opentelemetry/opentelemetry-sdk/1.28.0/opentelemetry-sdk-1.28.0.jar +@override_target_in_deps//:io/opentelemetry/opentelemetry-semconv/1.28.0-alpha/opentelemetry-semconv-1.28.0-alpha.jar +@override_target_in_deps//:io_opentelemetry_opentelemetry_api_1_28_0_extension +@override_target_in_deps//:io_opentelemetry_opentelemetry_api_events +@override_target_in_deps//:io_opentelemetry_opentelemetry_api_events_1_28_0_alpha_extension +@override_target_in_deps//:io_opentelemetry_opentelemetry_context +@override_target_in_deps//:io_opentelemetry_opentelemetry_context_1_28_0_extension +@override_target_in_deps//:io_opentelemetry_opentelemetry_extension_incubator +@override_target_in_deps//:io_opentelemetry_opentelemetry_extension_incubator_1_28_0_alpha_extension +@override_target_in_deps//:io_opentelemetry_opentelemetry_sdk +@override_target_in_deps//:io_opentelemetry_opentelemetry_sdk_1_28_0_extension +@override_target_in_deps//:io_opentelemetry_opentelemetry_sdk_common +@override_target_in_deps//:io_opentelemetry_opentelemetry_sdk_common_1_28_0_extension +@override_target_in_deps//:io_opentelemetry_opentelemetry_sdk_logs +@override_target_in_deps//:io_opentelemetry_opentelemetry_sdk_logs_1_28_0_extension +@override_target_in_deps//:io_opentelemetry_opentelemetry_sdk_metrics +@override_target_in_deps//:io_opentelemetry_opentelemetry_sdk_metrics_1_28_0_extension +@override_target_in_deps//:io_opentelemetry_opentelemetry_sdk_trace +@override_target_in_deps//:io_opentelemetry_opentelemetry_sdk_trace_1_28_0_extension +@override_target_in_deps//:io_opentelemetry_opentelemetry_semconv +@override_target_in_deps//:io_opentelemetry_opentelemetry_semconv_1_28_0_alpha_extension +@override_target_in_deps//:org/apache/commons/commons-pool2/2.11.1/commons-pool2-2.11.1.jar +@override_target_in_deps//:org/json/json/20231013/json-20231013.jar +@override_target_in_deps//:org/slf4j/slf4j-api/1.7.36/slf4j-api-1.7.36.jar +@override_target_in_deps//:org_apache_commons_commons_pool2 +@override_target_in_deps//:org_apache_commons_commons_pool2_2_11_1_extension +@override_target_in_deps//:org_json_json +@override_target_in_deps//:org_json_json_20231013_extension +@override_target_in_deps//:org_slf4j_slf4j_api +@override_target_in_deps//:org_slf4j_slf4j_api_1_7_36_extension +@override_target_in_deps//:original_io_opentelemetry_opentelemetry_api +@override_target_in_deps//:redis/clients/jedis/5.0.2/jedis-5.0.2.jar +@override_target_in_deps//:redis_clients_jedis +@override_target_in_deps//:redis_clients_jedis_5_0_2_extension +@redis_clients_jedis_5_0_2//file:file +@redis_clients_jedis_5_0_2//file:v1/redis/clients/jedis/5.0.2/jedis-5.0.2.jar diff --git a/tests/integration/override_targets/override_contains_additional_deps.sh b/tests/integration/override_targets/override_contains_additional_deps.sh deleted file mode 100755 index 16bfdd774..000000000 --- a/tests/integration/override_targets/override_contains_additional_deps.sh +++ /dev/null @@ -1,49 +0,0 @@ -# --- begin runfiles.bash initialization v2 --- -# Copy-pasted from the Bazel Bash runfiles library v2. -set -uo pipefail; f=bazel_tools/tools/bash/runfiles/runfiles.bash -source "${RUNFILES_DIR:-/dev/null}/$f" 2>/dev/null || \ - source "$(grep -sm1 "^$f " "${RUNFILES_MANIFEST_FILE:-/dev/null}" | cut -f2- -d' ')" 2>/dev/null || \ - source "$0.runfiles/$f" 2>/dev/null || \ - source "$(grep -sm1 "^$f " "$0.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \ - source "$(grep -sm1 "^$f " "$0.exe.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \ - { echo>&2 "ERROR: cannot find $f"; exit 1; }; f=; set -e -# --- end runfiles.bash initialization v2 --- - -set -euox pipefail - -deps_file=$(rlocation rules_jvm_external/tests/integration/override_targets/trace_otel_deps) - -function clean_up_workspace_names() { - local file_name="$1" - local target="$2" - # The first `sed` command replaces `@@` with `@`. The second extracts the visible name - # from the bzlmod mangled workspace name - cat "$file_name" | sed -e 's|^@@|@|g; s|\r||g' | sed -e 's|^@[^/]*[+~]|@|g; s|\r||g' | grep "$target" - cat "$file_name" | sed -e 's|^@@|@|g; s|\r||g' | sed -e 's|^@[^/]*[+~]|@|g; s|\r||g' | grep -q "$target" -} - -# we should contain the original target -if ! clean_up_workspace_names "$deps_file" "@override_target_in_deps//:io_opentelemetry_opentelemetry_sdk"; then - echo "Unable to find SDK target" - exit 1 -fi - -# should contain the "raw" dep -if ! clean_up_workspace_names "$deps_file" "@override_target_in_deps//:original_io_opentelemetry_opentelemetry_api"; then - echo "Unable to find raw API target" - exit 1 -fi - -# the "context" dependency is depended upon by `io.opentelemetry:opentelemetry-api` and -# nothing else in the SDK. If we have built the raw dependency properly, this should -# also be present in the dependencies -if ! clean_up_workspace_names "$deps_file" "@override_target_in_deps//:io_opentelemetry_opentelemetry_context"; then - echo "Unable to find transitive dep of raw target" - exit 1 -fi - -# Finally, we expect jedis (which is not an OTel dep) to have also been added -if ! clean_up_workspace_names "$deps_file" "@override_target_in_deps//:redis_clients_jedis"; then - echo "Unable to find additional target added to a transitive dep of the SDK" - exit 1 -fi diff --git a/tests/integration/override_targets/root_module_can_override.golden b/tests/integration/override_targets/root_module_can_override.golden new file mode 100755 index 000000000..48c9a6011 --- /dev/null +++ b/tests/integration/override_targets/root_module_can_override.golden @@ -0,0 +1,3 @@ +@root_module_can_override//:com_squareup_javapoet +@root_module_can_override//:com_squareup_okhttp3_okhttp +@root_module_can_override//:v1/https/repo1.maven.org/maven2/com/squareup/javapoet/1.11.1/javapoet-1.11.1.jar diff --git a/tests/integration/override_targets/root_module_can_override_test.sh b/tests/integration/override_targets/root_module_can_override_test.sh deleted file mode 100755 index d1582a054..000000000 --- a/tests/integration/override_targets/root_module_can_override_test.sh +++ /dev/null @@ -1,31 +0,0 @@ -# --- begin runfiles.bash initialization v2 --- -# Copy-pasted from the Bazel Bash runfiles library v2. -set -uo pipefail; f=bazel_tools/tools/bash/runfiles/runfiles.bash -source "${RUNFILES_DIR:-/dev/null}/$f" 2>/dev/null || \ - source "$(grep -sm1 "^$f " "${RUNFILES_MANIFEST_FILE:-/dev/null}" | cut -f2- -d' ')" 2>/dev/null || \ - source "$0.runfiles/$f" 2>/dev/null || \ - source "$(grep -sm1 "^$f " "$0.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \ - source "$(grep -sm1 "^$f " "$0.exe.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \ - { echo>&2 "ERROR: cannot find $f"; exit 1; }; f=; set -e -# --- end runfiles.bash initialization v2 --- - -set -euox pipefail - -deps_file=$(rlocation rules_jvm_external/tests/integration/override_targets/root_module_can_override) - -function clean_up_workspace_names() { - local file_name="$1" - local target="$2" - # The first `sed` command replaces `@@` with `@`. The second extracts the visible name - # from the bzlmod mangled workspace name - cat "$file_name" | sed -e 's|^@@|@|g; s|\r||g' | sed -e 's|^@[^/]*[+~]|@|g; s|\r||g' | grep "$target" - cat "$file_name" | sed -e 's|^@@|@|g; s|\r||g' | sed -e 's|^@[^/]*[+~]|@|g; s|\r||g' | grep -q "$target" -} - -if ! clean_up_workspace_names "$deps_file" "@root_module_can_override//:com_squareup_okhttp3_okhttp"; then - exit 1 -fi - -if ! clean_up_workspace_names "$deps_file" "@root_module_can_override//:com_squareup_javapoet"; then - exit 1 -fi From cb199e51f8f17333e0c9b9f4bf11160649116cea Mon Sep 17 00:00:00 2001 From: Sumeet Gajjar Date: Mon, 23 Jun 2025 18:15:54 -0700 Subject: [PATCH 7/8] Remove unwanted changes --- tests/integration/override_targets/BUILD | 15 ++-- .../override_contains_additional_deps.golden | 79 ------------------- .../override_contains_additional_deps.sh | 49 ++++++++++++ 3 files changed, 58 insertions(+), 85 deletions(-) delete mode 100755 tests/integration/override_targets/override_contains_additional_deps.golden create mode 100755 tests/integration/override_targets/override_contains_additional_deps.sh diff --git a/tests/integration/override_targets/BUILD b/tests/integration/override_targets/BUILD index 93f71d52a..8a5ad24ad 100644 --- a/tests/integration/override_targets/BUILD +++ b/tests/integration/override_targets/BUILD @@ -69,12 +69,15 @@ genrule( cmd = "cat $< | sed -e 's|^@@|@|g; s|\r||g' | sed -e 's|^@[^/]*[+~]|@|g; s|\r||g' | sort > $@", ) -diff_test( - name = "override_contains_additional_deps_test", - file1 = ":override_contains_additional_deps.golden", - file2 = ":override_contains_additional_deps_sorted.txt", - # This test only makes sense if we're running with `bzlmod` enabled - tags = [] if is_bzlmod_enabled() else ["manual"], +sh_test( + name = "override_contains_additional_deps", + srcs = ["override_contains_additional_deps.sh"], + data = [ + ":trace_otel_deps", + ], + deps = [ + "@bazel_tools//tools/bash/runfiles", + ], ) genquery( diff --git a/tests/integration/override_targets/override_contains_additional_deps.golden b/tests/integration/override_targets/override_contains_additional_deps.golden deleted file mode 100755 index a180e7b8a..000000000 --- a/tests/integration/override_targets/override_contains_additional_deps.golden +++ /dev/null @@ -1,79 +0,0 @@ -//tests/integration/override_targets:additional_deps -@bazel_tools//src/conditions:host_windows -@bazel_tools//src/conditions:host_windows_arm64_constraint -@bazel_tools//src/conditions:host_windows_x64_constraint -@com_google_code_gson_gson_2_10_1//file:file -@com_google_code_gson_gson_2_10_1//file:v1/com/google/code/gson/gson/2.10.1/gson-2.10.1.jar -@io_opentelemetry_opentelemetry_api_1_28_0//file:file -@io_opentelemetry_opentelemetry_api_1_28_0//file:v1/io/opentelemetry/opentelemetry-api/1.28.0/opentelemetry-api-1.28.0.jar -@io_opentelemetry_opentelemetry_api_events_1_28_0_alpha//file:file -@io_opentelemetry_opentelemetry_api_events_1_28_0_alpha//file:v1/io/opentelemetry/opentelemetry-api-events/1.28.0-alpha/opentelemetry-api-events-1.28.0-alpha.jar -@io_opentelemetry_opentelemetry_context_1_28_0//file:file -@io_opentelemetry_opentelemetry_context_1_28_0//file:v1/io/opentelemetry/opentelemetry-context/1.28.0/opentelemetry-context-1.28.0.jar -@io_opentelemetry_opentelemetry_extension_incubator_1_28_0_alpha//file:file -@io_opentelemetry_opentelemetry_extension_incubator_1_28_0_alpha//file:v1/io/opentelemetry/opentelemetry-extension-incubator/1.28.0-alpha/opentelemetry-extension-incubator-1.28.0-alpha.jar -@io_opentelemetry_opentelemetry_sdk_1_28_0//file:file -@io_opentelemetry_opentelemetry_sdk_1_28_0//file:v1/io/opentelemetry/opentelemetry-sdk/1.28.0/opentelemetry-sdk-1.28.0.jar -@io_opentelemetry_opentelemetry_sdk_common_1_28_0//file:file -@io_opentelemetry_opentelemetry_sdk_common_1_28_0//file:v1/io/opentelemetry/opentelemetry-sdk-common/1.28.0/opentelemetry-sdk-common-1.28.0.jar -@io_opentelemetry_opentelemetry_sdk_logs_1_28_0//file:file -@io_opentelemetry_opentelemetry_sdk_logs_1_28_0//file:v1/io/opentelemetry/opentelemetry-sdk-logs/1.28.0/opentelemetry-sdk-logs-1.28.0.jar -@io_opentelemetry_opentelemetry_sdk_metrics_1_28_0//file:file -@io_opentelemetry_opentelemetry_sdk_metrics_1_28_0//file:v1/io/opentelemetry/opentelemetry-sdk-metrics/1.28.0/opentelemetry-sdk-metrics-1.28.0.jar -@io_opentelemetry_opentelemetry_sdk_trace_1_28_0//file:file -@io_opentelemetry_opentelemetry_sdk_trace_1_28_0//file:v1/io/opentelemetry/opentelemetry-sdk-trace/1.28.0/opentelemetry-sdk-trace-1.28.0.jar -@io_opentelemetry_opentelemetry_semconv_1_28_0_alpha//file:file -@io_opentelemetry_opentelemetry_semconv_1_28_0_alpha//file:v1/io/opentelemetry/opentelemetry-semconv/1.28.0-alpha/opentelemetry-semconv-1.28.0-alpha.jar -@org_apache_commons_commons_pool2_2_11_1//file:file -@org_apache_commons_commons_pool2_2_11_1//file:v1/org/apache/commons/commons-pool2/2.11.1/commons-pool2-2.11.1.jar -@org_json_json_20231013//file:file -@org_json_json_20231013//file:v1/org/json/json/20231013/json-20231013.jar -@org_slf4j_slf4j_api_1_7_36//file:file -@org_slf4j_slf4j_api_1_7_36//file:v1/org/slf4j/slf4j-api/1.7.36/slf4j-api-1.7.36.jar -@override_target_in_deps//:com/google/code/gson/gson/2.10.1/gson-2.10.1.jar -@override_target_in_deps//:com_google_code_gson_gson -@override_target_in_deps//:com_google_code_gson_gson_2_10_1_extension -@override_target_in_deps//:io/opentelemetry/opentelemetry-api-events/1.28.0-alpha/opentelemetry-api-events-1.28.0-alpha.jar -@override_target_in_deps//:io/opentelemetry/opentelemetry-api/1.28.0/opentelemetry-api-1.28.0.jar -@override_target_in_deps//:io/opentelemetry/opentelemetry-context/1.28.0/opentelemetry-context-1.28.0.jar -@override_target_in_deps//:io/opentelemetry/opentelemetry-extension-incubator/1.28.0-alpha/opentelemetry-extension-incubator-1.28.0-alpha.jar -@override_target_in_deps//:io/opentelemetry/opentelemetry-sdk-common/1.28.0/opentelemetry-sdk-common-1.28.0.jar -@override_target_in_deps//:io/opentelemetry/opentelemetry-sdk-logs/1.28.0/opentelemetry-sdk-logs-1.28.0.jar -@override_target_in_deps//:io/opentelemetry/opentelemetry-sdk-metrics/1.28.0/opentelemetry-sdk-metrics-1.28.0.jar -@override_target_in_deps//:io/opentelemetry/opentelemetry-sdk-trace/1.28.0/opentelemetry-sdk-trace-1.28.0.jar -@override_target_in_deps//:io/opentelemetry/opentelemetry-sdk/1.28.0/opentelemetry-sdk-1.28.0.jar -@override_target_in_deps//:io/opentelemetry/opentelemetry-semconv/1.28.0-alpha/opentelemetry-semconv-1.28.0-alpha.jar -@override_target_in_deps//:io_opentelemetry_opentelemetry_api_1_28_0_extension -@override_target_in_deps//:io_opentelemetry_opentelemetry_api_events -@override_target_in_deps//:io_opentelemetry_opentelemetry_api_events_1_28_0_alpha_extension -@override_target_in_deps//:io_opentelemetry_opentelemetry_context -@override_target_in_deps//:io_opentelemetry_opentelemetry_context_1_28_0_extension -@override_target_in_deps//:io_opentelemetry_opentelemetry_extension_incubator -@override_target_in_deps//:io_opentelemetry_opentelemetry_extension_incubator_1_28_0_alpha_extension -@override_target_in_deps//:io_opentelemetry_opentelemetry_sdk -@override_target_in_deps//:io_opentelemetry_opentelemetry_sdk_1_28_0_extension -@override_target_in_deps//:io_opentelemetry_opentelemetry_sdk_common -@override_target_in_deps//:io_opentelemetry_opentelemetry_sdk_common_1_28_0_extension -@override_target_in_deps//:io_opentelemetry_opentelemetry_sdk_logs -@override_target_in_deps//:io_opentelemetry_opentelemetry_sdk_logs_1_28_0_extension -@override_target_in_deps//:io_opentelemetry_opentelemetry_sdk_metrics -@override_target_in_deps//:io_opentelemetry_opentelemetry_sdk_metrics_1_28_0_extension -@override_target_in_deps//:io_opentelemetry_opentelemetry_sdk_trace -@override_target_in_deps//:io_opentelemetry_opentelemetry_sdk_trace_1_28_0_extension -@override_target_in_deps//:io_opentelemetry_opentelemetry_semconv -@override_target_in_deps//:io_opentelemetry_opentelemetry_semconv_1_28_0_alpha_extension -@override_target_in_deps//:org/apache/commons/commons-pool2/2.11.1/commons-pool2-2.11.1.jar -@override_target_in_deps//:org/json/json/20231013/json-20231013.jar -@override_target_in_deps//:org/slf4j/slf4j-api/1.7.36/slf4j-api-1.7.36.jar -@override_target_in_deps//:org_apache_commons_commons_pool2 -@override_target_in_deps//:org_apache_commons_commons_pool2_2_11_1_extension -@override_target_in_deps//:org_json_json -@override_target_in_deps//:org_json_json_20231013_extension -@override_target_in_deps//:org_slf4j_slf4j_api -@override_target_in_deps//:org_slf4j_slf4j_api_1_7_36_extension -@override_target_in_deps//:original_io_opentelemetry_opentelemetry_api -@override_target_in_deps//:redis/clients/jedis/5.0.2/jedis-5.0.2.jar -@override_target_in_deps//:redis_clients_jedis -@override_target_in_deps//:redis_clients_jedis_5_0_2_extension -@redis_clients_jedis_5_0_2//file:file -@redis_clients_jedis_5_0_2//file:v1/redis/clients/jedis/5.0.2/jedis-5.0.2.jar diff --git a/tests/integration/override_targets/override_contains_additional_deps.sh b/tests/integration/override_targets/override_contains_additional_deps.sh new file mode 100755 index 000000000..16bfdd774 --- /dev/null +++ b/tests/integration/override_targets/override_contains_additional_deps.sh @@ -0,0 +1,49 @@ +# --- begin runfiles.bash initialization v2 --- +# Copy-pasted from the Bazel Bash runfiles library v2. +set -uo pipefail; f=bazel_tools/tools/bash/runfiles/runfiles.bash +source "${RUNFILES_DIR:-/dev/null}/$f" 2>/dev/null || \ + source "$(grep -sm1 "^$f " "${RUNFILES_MANIFEST_FILE:-/dev/null}" | cut -f2- -d' ')" 2>/dev/null || \ + source "$0.runfiles/$f" 2>/dev/null || \ + source "$(grep -sm1 "^$f " "$0.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \ + source "$(grep -sm1 "^$f " "$0.exe.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \ + { echo>&2 "ERROR: cannot find $f"; exit 1; }; f=; set -e +# --- end runfiles.bash initialization v2 --- + +set -euox pipefail + +deps_file=$(rlocation rules_jvm_external/tests/integration/override_targets/trace_otel_deps) + +function clean_up_workspace_names() { + local file_name="$1" + local target="$2" + # The first `sed` command replaces `@@` with `@`. The second extracts the visible name + # from the bzlmod mangled workspace name + cat "$file_name" | sed -e 's|^@@|@|g; s|\r||g' | sed -e 's|^@[^/]*[+~]|@|g; s|\r||g' | grep "$target" + cat "$file_name" | sed -e 's|^@@|@|g; s|\r||g' | sed -e 's|^@[^/]*[+~]|@|g; s|\r||g' | grep -q "$target" +} + +# we should contain the original target +if ! clean_up_workspace_names "$deps_file" "@override_target_in_deps//:io_opentelemetry_opentelemetry_sdk"; then + echo "Unable to find SDK target" + exit 1 +fi + +# should contain the "raw" dep +if ! clean_up_workspace_names "$deps_file" "@override_target_in_deps//:original_io_opentelemetry_opentelemetry_api"; then + echo "Unable to find raw API target" + exit 1 +fi + +# the "context" dependency is depended upon by `io.opentelemetry:opentelemetry-api` and +# nothing else in the SDK. If we have built the raw dependency properly, this should +# also be present in the dependencies +if ! clean_up_workspace_names "$deps_file" "@override_target_in_deps//:io_opentelemetry_opentelemetry_context"; then + echo "Unable to find transitive dep of raw target" + exit 1 +fi + +# Finally, we expect jedis (which is not an OTel dep) to have also been added +if ! clean_up_workspace_names "$deps_file" "@override_target_in_deps//:redis_clients_jedis"; then + echo "Unable to find additional target added to a transitive dep of the SDK" + exit 1 +fi From f5d1a50216cdfd98ef3e4d08945e6398c1e76bfe Mon Sep 17 00:00:00 2001 From: Sumeet Gajjar Date: Mon, 23 Jun 2025 18:16:33 -0700 Subject: [PATCH 8/8] Remove unwanted changes --- tests/integration/override_targets/BUILD | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tests/integration/override_targets/BUILD b/tests/integration/override_targets/BUILD index 8a5ad24ad..438b4f27d 100644 --- a/tests/integration/override_targets/BUILD +++ b/tests/integration/override_targets/BUILD @@ -61,14 +61,6 @@ genquery( scope = ["@override_target_in_deps//:io_opentelemetry_opentelemetry_sdk"], ) -genrule( - name = "override_contains_additional_deps_sorted", - testonly = 1, - srcs = [":trace_otel_deps"], - outs = ["override_contains_additional_deps_sorted.txt"], - cmd = "cat $< | sed -e 's|^@@|@|g; s|\r||g' | sed -e 's|^@[^/]*[+~]|@|g; s|\r||g' | sort > $@", -) - sh_test( name = "override_contains_additional_deps", srcs = ["override_contains_additional_deps.sh"],