Change --extra_toolchains precedence to last-wins, instead of first…

…-wins.

This only changes the precedence within all uses of `--extra_toolchains`, it does not change the precedence for `register_toolchains` calls.

After this, the priority order for toolchains is:
1. Consider toolchains registered via `--extra_toolchains`
  1. Within this set, the last mentioned toolchain has highest priority
2. Consider toolchains registered via `register_toolchains`
  1. Within this set, the first mentioned toolchain has highest priority

In all cases, pseudo-targets like `:all` and `/...` are ordered by Bazel's package loading mechanism, which is currently using a lexicographic ordering.

Fixes #19945.

Closes #19942. Closes #20031.

PiperOrigin-RevId: 579845262
Change-Id: Ibd9c32e5479a6a27717734852b875f9b3f31b510

site/en/extending/toolchains.md

@@ -508,6 +508,18 @@ The host platform is automatically included as an available execution platform.
 Available platforms and toolchains are tracked as ordered lists for determinism,
 with preference given to earlier items in the list.
 
+The set of available toolchains, in priority order, is created from
+`--extra_toolchains` and `register_toolchains`:
+
+1. Toolchains registered using `--extra_toolchains` are added first.
+  1. Within these, the **last** toolchain has highest priority.
+2. Toolchains registered using `register_toolchains`
+  1. Within these, the **first** mentioned toolchain has highest priority.
+
+**NOTE:** [Pseudo-targets like `:all`, `:*`, and
+`/...`](/run/build#specifying-build-targets) are ordered by Bazel's package
+loading mechanism, which uses a lexicographic ordering.
+
 The resolution steps are as follows.
 
 1. A `target_compatible_with` or `exec_compatible_with` clause *matches* a

src/main/java/com/google/devtools/build/lib/skyframe/toolchains/RegisteredToolchainsFunction.java

@@ -83,12 +83,13 @@ public SkyValue compute(SkyKey skyKey, Environment env)
     ImmutableList.Builder<SignedTargetPattern> targetPatternBuilder = new ImmutableList.Builder<>();
 
     // Get the toolchains from the configuration.
+    // Reverse the list so the last one defined takes precedences.
     PlatformConfiguration platformConfiguration =
         configuration.getFragment(PlatformConfiguration.class);
     try {
       targetPatternBuilder.addAll(
           TargetPatternUtil.parseAllSigned(
-              platformConfiguration.getExtraToolchains(), mainRepoParser));
+              platformConfiguration.getExtraToolchains().reverse(), mainRepoParser));
     } catch (InvalidTargetPatternException e) {
       throw new RegisteredToolchainsFunctionException(
           new InvalidToolchainLabelException(e), Transience.PERSISTENT);

src/test/java/com/google/devtools/build/lib/skyframe/toolchains/RegisteredToolchainsFunctionTest.java

@@ -153,8 +153,8 @@ public void testRegisteredToolchains_flagOverride_multiple() throws Exception {
     // Verify that the target registered with the extra_toolchains flag is first in the list.
     assertToolchainLabels(result.get(toolchainsKey))
         .containsAtLeast(
-            Label.parseCanonicalUnchecked("//extra:extra_toolchain_impl_1"),
             Label.parseCanonicalUnchecked("//extra:extra_toolchain_impl_2"),
+            Label.parseCanonicalUnchecked("//extra:extra_toolchain_impl_1"),
             Label.parseCanonicalUnchecked("//toolchain:toolchain_1_impl"))
         .inOrder();
   }

src/test/shell/integration/java_integration_test.sh

@@ -267,7 +267,8 @@ EOF
   # Set javabase to an absolute path.
   bazel build //$pkg/java/hello:hello //$pkg/java/hello:hello_deploy.jar \
       "$stamp_arg" \
-      --extra_toolchains="//$pkg/jvm:all,//tools/jdk:all" \
+      --extra_toolchains="//tools/jdk:all" \
+      --extra_toolchains="//$pkg/jvm:all" \
       --platforms="//$pkg/jvm:platform" \
       "$embed_label" >&"$TEST_log" \
       || fail "Build failed"

src/test/shell/integration/toolchain_test.sh

@@ -2781,6 +2781,87 @@ EOF
   expect_log "Selected execution platform //${pkg}/platforms:platform2"
 }
 
+# Regression test for https://github.com/bazelbuild/bazel/issues/19945.
+function test_extra_toolchain_precedence {
+  local -r pkg="${FUNCNAME[0]}"
+
+  write_test_toolchain "${pkg}"
+  write_test_rule "${pkg}"
+
+  cat > WORKSPACE <<EOF
+register_toolchains('//${pkg}:toolchain_1')
+EOF
+
+  cat > "${pkg}/BUILD" <<EOF
+load('//${pkg}/toolchain:toolchain_test_toolchain.bzl', 'test_toolchain')
+
+package(default_visibility = ["//visibility:public"])
+
+# Define and declare four identical toolchains.
+[
+  [
+    test_toolchain(
+      name = 'toolchain_impl_' + str(i),
+      extra_str = 'foo from toolchain_' + str(i),
+    ),
+    toolchain(
+      name = 'toolchain_' + str(i),
+      toolchain_type = '//${pkg}/toolchain:test_toolchain',
+      toolchain = ':toolchain_impl_' + str(i)
+    ),
+  ]
+  for i in range(1, 5)
+]
+EOF
+
+  mkdir -p "${pkg}/demo"
+  cat > "${pkg}/demo/BUILD" <<EOF
+load('//${pkg}/toolchain:rule_use_toolchain.bzl', 'use_toolchain')
+package(default_visibility = ["//visibility:public"])
+
+# Use the toolchain.
+use_toolchain(
+    name = 'use',
+    message = 'this is the rule')
+EOF
+
+  bazel query "//${pkg}:*"
+
+  bazel \
+    build \
+    "//${pkg}/demo:use" &> $TEST_log || fail "Build failed"
+  expect_log 'Using toolchain: rule message: "this is the rule", toolchain extra_str: "foo from toolchain_1"'
+
+  # Test that bazelrc options take precedence over registered toolchains
+  cat > "${pkg}/toolchain_rc" <<EOF
+import ${bazelrc}
+build --extra_toolchains=//${pkg}:toolchain_2
+EOF
+
+  bazel \
+    --${PRODUCT_NAME}rc="${pkg}/toolchain_rc" \
+    build \
+    "//${pkg}/demo:use" &> $TEST_log || fail "Build failed"
+  expect_log 'Using toolchain: rule message: "this is the rule", toolchain extra_str: "foo from toolchain_2"'
+
+  # Test that command-line options take precedence over other toolchains
+  bazel \
+    --${PRODUCT_NAME}rc="${pkg}/toolchain_rc" \
+    build \
+    --extra_toolchains=//${pkg}:toolchain_3 \
+    "//${pkg}/demo:use" &> $TEST_log || fail "Build failed"
+  expect_log 'Using toolchain: rule message: "this is the rule", toolchain extra_str: "foo from toolchain_3"'
+
+  # Test that the last --extra_toolchains takes precedence
+  bazel \
+    --${PRODUCT_NAME}rc="${pkg}/toolchain_rc" \
+    build \
+    --extra_toolchains=//${pkg}:toolchain_3 \
+    --extra_toolchains=//${pkg}:toolchain_4 \
+    "//${pkg}/demo:use" &> $TEST_log || fail "Build failed"
+  expect_log 'Using toolchain: rule message: "this is the rule", toolchain extra_str: "foo from toolchain_4"'
+}
+
 # TODO(katre): Test using toolchain-provided make variables from a genrule.
 
 run_suite "toolchain tests"