Skip to content

Allow root module's override tags to take precedence over the overridees from transitive deps. #1381

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

Merged
merged 9 commits into from
Jun 24, 2025
Merged
22 changes: 22 additions & 0 deletions MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -828,6 +828,27 @@ local_path_override(
path = "tests/integration/root_wins_layer",
)


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", dev_dependency = True)
local_path_override(
module_name = "transitive_module_can_override",
path = "tests/integration/override_targets/module",
)

dev_maven.override(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the comment please mention the test this is used in?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

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",
)

# 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
Expand Down Expand Up @@ -914,6 +935,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",
Expand Down
15 changes: 15 additions & 0 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -1006,3 +1006,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"],
)
14 changes: 11 additions & 3 deletions private/extensions/maven.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -373,13 +373,21 @@ def maven_impl(mctx):
repo_name_2_module_name = {}

# Process overrides first (they don't need deduplication)
for mod in mctx.modules:
# 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])
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: to_use})

# First pass: process the module tags, but keep root and non-root modules separately
Expand Down
28 changes: 28 additions & 0 deletions tests/integration/override_targets/BUILD
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
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")
load("//tests/integration:is_bzlmod_enabled.bzl", "is_bzlmod_enabled")

build_test(
name = "override_targets",
Expand Down Expand Up @@ -69,3 +71,29 @@ 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"],
)

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",
file1 = ":root_module_can_override.golden",
file2 = ":root_module_can_override_sorted.txt",
# This test only makes sense if we're running with `bzlmod` enabled
tags = [] if is_bzlmod_enabled() else ["manual"],
)
18 changes: 18 additions & 0 deletions tests/integration/override_targets/module/MODULE.bazel
Original file line number Diff line number Diff line change
@@ -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",
)
Original file line number Diff line number Diff line change
@@ -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