Skip to content

Commit 4a0180e

Browse files
committed
Allow root module's override tags to take precedence over the overrides from the transitive deps.
1 parent 1202665 commit 4a0180e

File tree

6 files changed

+114
-4
lines changed

6 files changed

+114
-4
lines changed

MODULE.bazel

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -732,6 +732,25 @@ dev_maven.install(
732732
],
733733
)
734734

735+
736+
dev_maven.install(
737+
name = "root_module_can_override",
738+
artifacts = ["com.squareup:javapoet:1.11.1"],
739+
)
740+
741+
bazel_dep(name = "transitive_module_can_override", version = "0.0.0")
742+
local_path_override(
743+
module_name = "transitive_module_can_override",
744+
path = "tests/integration/override_targets/module",
745+
)
746+
747+
dev_maven.override(
748+
# This override demonstrates that this root module's override takes precedence over that transitive override definition.
749+
# Use something absurd for testing, like overriding okhttp3 to javapoet.
750+
coordinates = "com.squareup.okhttp3:okhttp",
751+
target = "@root_module_can_override//:com_squareup_javapoet",
752+
)
753+
735754
# Where there are file locks, the pinned and unpinned repos are listed
736755
# next to each other. Where compat repositories are created, they are
737756
# listed next to the repo that created them. The list is otherwise kept
@@ -815,6 +834,7 @@ use_repo(
815834
"starlark_aar_import_test",
816835
"starlark_aar_import_with_sources_test",
817836
"strict_visibility_testing",
837+
"root_module_can_override",
818838

819839
# Repo with compat repos
820840
"com_google_http_client_google_http_client_gson",

WORKSPACE

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -937,3 +937,18 @@ maven_install(
937937
"https://repo1.maven.org/maven2",
938938
],
939939
)
940+
941+
# This failure mode is bzlmod only. But the test still runs on Bazel 5/6, which
942+
# is WORKSPACE based, so we add a shim here to keep the test passing until
943+
# WORKSPACE support is no longer needed.
944+
maven_install(
945+
name = "root_module_can_override",
946+
artifacts = [
947+
"com.squareup:javapoet:1.11.1",
948+
"com.squareup.okhttp3:okhttp:4.12.0",
949+
],
950+
override_targets = {
951+
"com.squareup.okhttp3:okhttp": "@root_module_can_override//:com_squareup_javapoet",
952+
},
953+
repositories = ["https://repo1.maven.org/maven2"],
954+
)

private/extensions/maven.bzl

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -227,13 +227,22 @@ def maven_impl(mctx):
227227
# module attempts to update a maven repo (which is normally undesired behaviour)
228228
repo_name_2_module_name = {}
229229

230-
for mod in mctx.modules:
230+
# First compute the overrides. The order of the transitive overrides do not matter, but the root
231+
# overrides take precedence over all transitive ones.
232+
for idx, mod in enumerate(reversed(mctx.modules)):
233+
# Rotate the root module to the last to be visited.
234+
is_root_module = idx == (len(mctx.modules) - 1)
231235
for override in mod.tags.override:
232236
value = str(override.target)
233-
current = overrides.get(override.coordinates, None)
234-
to_use = _fail_if_different("Target of override for %s" % override.coordinates, current, value, [None])
235-
overrides.update({override.coordinates: to_use})
237+
if is_root_module:
238+
# Allow the root module's overrides to take precedence over any transitive overrides.
239+
to_use = value
240+
else:
241+
current = overrides.get(override.coordinates, None)
242+
to_use = _fail_if_different("Target of override for %s" % override.coordinates, current, value, [None])
243+
overrides.update({override.coordinates: value})
236244

245+
for mod in mctx.modules:
237246
for artifact in mod.tags.artifact:
238247
_check_repo_name(repo_name_2_module_name, artifact.name, mod.name)
239248

tests/integration/override_targets/BUILD

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,3 +69,20 @@ sh_test(
6969
"@bazel_tools//tools/bash/runfiles",
7070
],
7171
)
72+
73+
genquery(
74+
name = "root_module_can_override",
75+
expression = "deps(@root_module_can_override//:com_squareup_okhttp3_okhttp)",
76+
opts = [
77+
"--nohost_deps",
78+
"--noimplicit_deps",
79+
],
80+
scope = ["@root_module_can_override//:com_squareup_okhttp3_okhttp"],
81+
)
82+
83+
sh_test(
84+
name = "root_module_can_override_test",
85+
srcs = ["root_module_can_override_test.sh"],
86+
data = [":root_module_can_override"],
87+
deps = ["@bazel_tools//tools/bash/runfiles"],
88+
)
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
module(name = "transitive_module_can_override", version = "0.0.0")
2+
3+
bazel_dep(name = "rules_jvm_external", version = "0.0")
4+
local_path_override(
5+
module_name = "rules_jvm_external",
6+
path = "../../../..",
7+
)
8+
9+
maven = use_extension("@rules_jvm_external//:extensions.bzl", "maven")
10+
maven.install(
11+
name = "root_module_can_override",
12+
artifacts = ["com.squareup.okhttp3:okhttp:4.12.0"],
13+
)
14+
15+
maven.override(
16+
coordinates = "com.squareup.okhttp3:okhttp3",
17+
target = "//:poison_pill_non_existent_target",
18+
)
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
# --- begin runfiles.bash initialization v2 ---
2+
# Copy-pasted from the Bazel Bash runfiles library v2.
3+
set -uo pipefail; f=bazel_tools/tools/bash/runfiles/runfiles.bash
4+
source "${RUNFILES_DIR:-/dev/null}/$f" 2>/dev/null || \
5+
source "$(grep -sm1 "^$f " "${RUNFILES_MANIFEST_FILE:-/dev/null}" | cut -f2- -d' ')" 2>/dev/null || \
6+
source "$0.runfiles/$f" 2>/dev/null || \
7+
source "$(grep -sm1 "^$f " "$0.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \
8+
source "$(grep -sm1 "^$f " "$0.exe.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \
9+
{ echo>&2 "ERROR: cannot find $f"; exit 1; }; f=; set -e
10+
# --- end runfiles.bash initialization v2 ---
11+
12+
set -euox pipefail
13+
14+
deps_file=$(rlocation rules_jvm_external/tests/integration/override_targets/root_module_can_override)
15+
16+
function clean_up_workspace_names() {
17+
local file_name="$1"
18+
local target="$2"
19+
# The first `sed` command replaces `@@` with `@`. The second extracts the visible name
20+
# from the bzlmod mangled workspace name
21+
cat "$file_name" | sed -e 's|^@@|@|g; s|\r||g' | sed -e 's|^@[^/]*[+~]|@|g; s|\r||g' | grep "$target"
22+
cat "$file_name" | sed -e 's|^@@|@|g; s|\r||g' | sed -e 's|^@[^/]*[+~]|@|g; s|\r||g' | grep -q "$target"
23+
}
24+
25+
if ! clean_up_workspace_names "$deps_file" "@root_module_can_override//:com_squareup_okhttp3_okhttp"; then
26+
exit 1
27+
fi
28+
29+
if ! clean_up_workspace_names "$deps_file" "@root_module_can_override//:com_squareup_javapoet"; then
30+
exit 1
31+
fi

0 commit comments

Comments
 (0)