Repository: bazelbuild/bazel
Issue: Unexpected interaction between toolchain resolution and bzlmod
Labels: type: bug
P2
team-ExternalDeps
area-Bzlmod

### Description of the bug:


When bzlmod is enabled (as it is the case in Bazel 7.0.0), toolchain registered by default can unexpectedly shadow toolchains declared in the `WORKSPACE` file. This makes the oucome of toolchain resolution dependent on whether bzlmod is enabled, even if the `MODULE.bazel` file of the main repository is empty (as it is by default)

The root cause seems to be different from #17289 : there, AFAIU that one is caused by a subtle interaction between label mapping and toolchain resolution, here it's much more straightforward.

Some debugging indicates that what happens is that `RegisteredToolchainsFunction` creates the list of registered toolchains by concatenating two lists: those from bzlmod, then those from the WORKSPACE file.  `rules_python` is by default and immutably (except if bzlmod is disabled) a bzlmod module and it  registers `@bazel_tools//tools/python:_autodetecting_py_runtime_pair` as a Python toolchain. Since that toolchain doesn't have any exec or target constraints, it seems to win toolchain selection by default, regardless of what other toolchains come after.

Therefore, no matter what is written in the `WORKSPACE` file, one gets the autodetecting Python toolchain.

I'm not sure if this is WAI; it could be construed as such, but it sure is surprising and a hindrance to migration to bzlmod.

### Which category does this issue belong to?


_No response_

### What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.


```
cat > WORKSPACE <<'EOF'
register_toolchains("//tc:py_toolchain")
EOF

cat > BUILD <<'EOF'
py_binary(
    name = "py",
    srcs = ["py.py"],
)
EOF

cat > py.py <<'EOF'
print("Hello, Python!")
EOF

mkdir tc

cat > tc/BUILD <<'EOF'
load("@bazel_tools//tools/python:toolchain.bzl", "py_runtime_pair")

py_runtime(
    name = "python2",
    files = ["ok.py2"],
    interpreter = "ok.py2",
    python_version = "PY2",
)

py_runtime(
    name = "python3",
    files = ["ok.py3"],
    interpreter = "ok.py3",
    python_version = "PY3",
)

py_runtime_pair(
    name = "pair",
    py2_runtime = ":python2",
    py3_runtime = ":python3",
)

toolchain(
    name = "py_toolchain",
    toolchain = ":pair",
    toolchain_type = "@bazel_tools//tools/python:toolchain_type",
)
EOF

bazel run --enable_bzlmod :py  # BUG: Prints "Hello, Python!", ignoring the toolchain
bazel run --noenable_bzlmod :py  # WORKS: Fails to find the dummy interpreter defined above
```

### Which operating system are you running Bazel on?


Linux

### What is the output of `bazel info release`?


development version

### If `bazel info release` returns `development version` or `(@non-git)`, tell us how you built Bazel.


From sources

### What's the output of `git remote get-url origin; git rev-parse master; git rev-parse HEAD` ?


```text
4a29f0851d1cde0240793cdc7a2e2cab926d31b7
```


### Is this a regression? If yes, please try to identify the Bazel commit where the bug was introduced.


Pretty obviously when `--enable_bzlmod` was flipped to true.

### Have you found anything relevant by searching the web?

There are a number of pertinent bugs:
* https://github.com/bazelbuild/bazel/issues/17289 (but that has a different root cause, if I understand correctly)
* https://github.com/bazelbuild/rules_python/issues/1313 (seems to be very similar, but I haven't seen evidence that the bzlmod folks looked at this and has less information as to why this happens)

### Any other information, logs, or outputs that you want to share?

_No response_

Issue comments:
cc @meteorcloudy 
One possible solution I see is to change the order of registration to the following in order of decreasing precedence:
1. Toolchains registered in the root module's MODULE.bazel file
2. Toolchains registered in the WORKSPACE file
3. Toolchains registered in dependencies' MODULE.bazel files in BFS order

Independent of that particular issue, the auto-detecting Python toolchain should have constraints to match on the auto-detected host platform, otherwise it probably breaks remote execution of Python tools.
> RegisteredToolchainsFunction creates the list of registered toolchains by concatenating two lists: those from bzlmod, then those from the WORKSPACE file.

Yes, this is WAI. But we probably should document the priorities of toolchain registration more clearly.

> Toolchains registered in the root module's MODULE.bazel file
> Toolchains registered in the WORKSPACE file
> Toolchains registered in dependencies' MODULE.bazel files in BFS order

This does look like a better order since it gives more control to users. But still could be surprising and should be documented.

> Independent of that particular issue, the auto-detecting Python toolchain should have constraints to match on the auto-detected host platform, otherwise it probably breaks remote execution of Python tools.

/cc @rickeylev 
Yep, I find the order proposed by @fmeum more intuitive than what's at HEAD.
There is a potential problem with this new order though: We can't distinguish top-level toolchain registrations from those coming from WORKSPACE macros or even the suffix. This could result in unexpected results of a different kind.
Ideally toolchains registered in the workplace suffix would have the lowest priority (that's what effectively happened before bzlmod was a thing).

We've discussed (and I can't currently find the issue) adding explicit priority levels to `register_toolchains`: that could be used with some defaults to set the order of toolchains as described, but I'd want to take some time and flesh out the problem.

Fixing the python toolchains should be very easy, if @rickeylev wants help I can provide pointers.

What's the priority on fixing bzlmod/workspace toolchain ordering?
If you told me the behavior at HEAD is by design, I'd grudgingly accept that and ask how we could help people like the Kleaf folks with their migration. They do have a point that this (and other behavior similarly surprising, if it exists) doesn't make migration to bzlmod easy. I have a number of ideas which could make this more intuitive, but given that I you and @Wyverald follow toolchains and bzlmod much closer than I do, I'd rather not voice them before being asked.

Practically speaking, I don't think there is a rush, at least from this project, given that they also need bzlmod vendoring to be able to turn bzlmod on. But let's not fall into the trap where both issues get deferred because they expect the other one to progress slowly :)

Went ahead and wrote up #20356 for the python part: @lberki, while tests are running, can you see if this addresses the original issue?
It doesn't fix the repro in the description of this bug, so I guess it does not?
That's unfortunate. Can you share the repro with me?
It's in the description of https://github.com/bazelbuild/bazel/issues/20354 , complete with a shell snippet to produce a repository from scratch and example Bazel invocations.
Whoops, sorry
Is it feasible for Kleaf to move/copy their toolchain registrations from WORKSPACE to MODULE.bazel? That would be the immediate fix.
Using `--extra_toolchains` in a bazelrc would also be a workaround.
>  rules_python is by default and immutably (except if bzlmod is disabled) a bzlmod module and it registers @bazel_tools//tools/python:_autodetecting_py_runtime_pair as a Python toolchain.

Well, yes and no. There's three different mechanisms that can cause it to get registered when you don't expect it:

1. As part of Bazel itself. I don't know where this puts it in the toolchain priorities, but it happens very early, when Bazel is initializing the rule types it knows about. See https://github.com/bazelbuild/bazel/blob/38e8da1b571de21e51d92cce750875f0a9f280ca/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java#L479 and https://github.com/bazelbuild/bazel/blob/38e8da1b571de21e51d92cce750875f0a9f280ca/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python.WORKSPACE#L4
2. As part of older rules_python "releases" (0.4 to 0.12). I put releases in quotes because, from what I can tell, those releases are of rules_python versions that didn't actually have bzlmod support. I'm guessing they were manually created to help bootstrap things. After that, the autodetecting toolchain is no longer registered. Starting in 0.23.0, a hermetic toolchain is automatically registered. Also note that Bazel itself depends on rules_python 0.4.0 right now (I have a pending change to upgrade that to 0.22.0, which doesn't register the autodetecting toolchain). See https://github.com/search?q=repo%3Abazelbuild%2Fbazel-central-registry+path%3Amodules%2Frules_python+autodetecting_toolchain&type=code
4. The rules_bazel_integration_test module registers the autodetecting toolchain. So if that's in the module graph, that'll happen. See https://github.com/search?q=repo%3Abazelbuild%2Fbazel-central-registry+-path%3Amodules%2Frules_python+autodetecting_toolchain&type=code

> https://github.com/bazelbuild/rules_python/issues/1313 

That actually sounds exactly like this issue: the user has an empty module, a workspace registering a toolchain, but then when they enable bzlmod, their toolchain is ignored.

As an aside, don't use the stuff in `@bazel_tools//tools/python`. Use `@rules_python` instead.
> There is a potential problem with this new order though: We can't distinguish top-level toolchain registrations from those coming from WORKSPACE macros or even the suffix. This could result in unexpected results of a different kind.

This is a fixable problem, actually. We could annotate toolchains registered in WORKSPACE suffixes with a "low priority" bit (messy, but technically possible), then order the toolchains per 1) root MODULE.bazel 2) WORKSPACE sans suffix 3) non-root Bzlmod stuff 4) WORKSPACE suffix. This would address the Kleaf use case.
I believe that this is catching more users by surprise: https://bazelbuild.slack.com/archives/CA306CEV6/p1701394561232659

It's pretty hard to come up with why this is failing if users are not aware of `MODULE.bazel` at all, as in this case.
@bazel-io fork 7.0.0
Since Bzlmod is enabled by default in Bazel 7 and that could trigger this issue for many users, we should probably address this in 7.0.0.
A fix for this issue has been included in [Bazel 7.0.0 RC6](https://releases.bazel.build/7.0.0/rc6/index.html). Please test out the release candidate and report any issues as soon as possible. Thanks!

Pull request review comments:

Patch that resolved the issue:
diff --git a/site/en/external/migration.md b/site/en/external/migration.md
index efd1cd1bc51bd9..c4eb0c4050dbab 100644
--- a/site/en/external/migration.md
+++ b/site/en/external/migration.md
@@ -453,6 +453,19 @@ toolchain.
     register_toolchains("@local_config_sh//:local_sh_toolchain")
     ```
 
+The toolchains and execution platforms registered in `WORKSPACE`,
+`WORKSPACE.bzlmod` and each Bazel module's `MODULE.bazel` file follow this
+order of precedence during toolchain selection (from highest to lowest):
+
+1. toolchains and execution platforms registered in the root module's
+   `MODULE.bazel` file.
+2. toolchains and execution platforms registered in the `WORKSPACE` or
+   `WORKSPACE.bzlmod` file.
+3. toolchains and execution platforms registered by modules that are
+   (transitive) dependencies of the root module.
+4. when not using `WORKSPACE.bzlmod`: toolchains registered in the `WORKSPACE`
+   [suffix](/external/migration#builtin-default-deps).
+
 [register_execution_platforms]: /rules/lib/globals/module#register_execution_platforms
 
 ### Introduce local repositories {:#introduce-local-deps}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java
index 742c237f096ecc..09dfa240a3a0db 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Package.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -72,6 +72,7 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.OptionalInt;
 import java.util.Set;
 import java.util.TreeMap;
 import java.util.concurrent.Semaphore;
@@ -239,7 +240,7 @@ public enum ConfigSettingVisibilityPolicy {
 
   private ImmutableList<TargetPattern> registeredExecutionPlatforms;
   private ImmutableList<TargetPattern> registeredToolchains;
-
+  private OptionalInt firstWorkspaceSuffixRegisteredToolchain;
   private long computationSteps;
 
   // These two fields are mutually exclusive. Which one is set depends on
@@ -402,6 +403,7 @@ private void finishInit(Builder builder) {
     this.failureDetail = builder.getFailureDetail();
     this.registeredExecutionPlatforms = ImmutableList.copyOf(builder.registeredExecutionPlatforms);
     this.registeredToolchains = ImmutableList.copyOf(builder.registeredToolchains);
+    this.firstWorkspaceSuffixRegisteredToolchain = builder.firstWorkspaceSuffixRegisteredToolchain;
     this.repositoryMapping = Preconditions.checkNotNull(builder.repositoryMapping);
     this.mainRepositoryMapping = Preconditions.checkNotNull(builder.mainRepositoryMapping);
     ImmutableMap.Builder<RepositoryName, ImmutableMap<String, RepositoryName>>
@@ -722,6 +724,23 @@ public ImmutableList<TargetPattern> getRegisteredToolchains() {
     return registeredToolchains;
   }
 
+  public ImmutableList<TargetPattern> getUserRegisteredToolchains() {
+    return getRegisteredToolchains()
+        .subList(
+            0, firstWorkspaceSuffixRegisteredToolchain.orElse(getRegisteredToolchains().size()));
+  }
+
+  public ImmutableList<TargetPattern> getWorkspaceSuffixRegisteredToolchains() {
+    return getRegisteredToolchains()
+        .subList(
+            firstWorkspaceSuffixRegisteredToolchain.orElse(getRegisteredToolchains().size()),
+            getRegisteredToolchains().size());
+  }
+
+  OptionalInt getFirstWorkspaceSuffixRegisteredToolchain() {
+    return firstWorkspaceSuffixRegisteredToolchain;
+  }
+
   @Override
   public String toString() {
     return "Package("
@@ -936,8 +955,16 @@ default boolean precomputeTransitiveLoads() {
     private final List<TargetPattern> registeredToolchains = new ArrayList<>();
 
     /**
-     * True iff the "package" function has already been called in this package.
+     * Tracks the index within {@link #registeredToolchains} of the first toolchain registered from
+     * the WORKSPACE suffixes rather than the WORKSPACE file (if any).
+     *
+     * <p>This is needed to distinguish between these toolchains during resolution: toolchains
+     * registered in WORKSPACE have precedence over those defined in non-root Bazel modules, which
+     * in turn have precedence over those from the WORKSPACE suffixes.
      */
+    private OptionalInt firstWorkspaceSuffixRegisteredToolchain = OptionalInt.empty();
+
+    /** True iff the "package" function has already been called in this package. */
     private boolean packageFunctionUsed;
 
     /**
@@ -1620,10 +1647,18 @@ void addRegisteredExecutionPlatforms(List<TargetPattern> platforms) {
       this.registeredExecutionPlatforms.addAll(platforms);
     }
 
-    void addRegisteredToolchains(List<TargetPattern> toolchains) {
+    void addRegisteredToolchains(List<TargetPattern> toolchains, boolean forWorkspaceSuffix) {
+      if (forWorkspaceSuffix && firstWorkspaceSuffixRegisteredToolchain.isEmpty()) {
+        firstWorkspaceSuffixRegisteredToolchain = OptionalInt.of(registeredToolchains.size());
+      }
       this.registeredToolchains.addAll(toolchains);
     }
 
+    void setFirstWorkspaceSuffixRegisteredToolchain(
+        OptionalInt firstWorkspaceSuffixRegisteredToolchain) {
+      this.firstWorkspaceSuffixRegisteredToolchain = firstWorkspaceSuffixRegisteredToolchain;
+    }
+
     @CanIgnoreReturnValue
     private Builder beforeBuild(boolean discoverAssumedInputFiles) throws NoSuchPackageException {
       Preconditions.checkNotNull(pkg);
diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
index d5901190b10058..095fd305412c78 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
@@ -185,7 +185,10 @@ public void setParent(
       builder.setFailureDetailOverride(aPackage.getFailureDetail());
     }
     builder.addRegisteredExecutionPlatforms(aPackage.getRegisteredExecutionPlatforms());
-    builder.addRegisteredToolchains(aPackage.getRegisteredToolchains());
+    builder.addRegisteredToolchains(
+        aPackage.getRegisteredToolchains(), /* forWorkspaceSuffix= */ false);
+    builder.setFirstWorkspaceSuffixRegisteredToolchain(
+        aPackage.getFirstWorkspaceSuffixRegisteredToolchain());
     builder.addRepositoryMappings(aPackage);
     for (Rule rule : aPackage.getTargets(Rule.class)) {
       try {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java
index 1f514a4f43cbcc..0f2847470d452d 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java
@@ -36,6 +36,13 @@
 /** A helper for the {@link WorkspaceFactory} to create repository rules */
 public final class WorkspaceFactoryHelper {
 
+  public static final String DEFAULT_WORKSPACE_SUFFIX_FILE = "/DEFAULT.WORKSPACE.SUFFIX";
+
+  public static boolean originatesInWorkspaceSuffix(
+      ImmutableList<StarlarkThread.CallStackEntry> callstack) {
+    return callstack.get(0).location.file().equals(DEFAULT_WORKSPACE_SUFFIX_FILE);
+  }
+
   @CanIgnoreReturnValue
   public static Rule createAndAddRepositoryRule(
       Package.Builder pkg,
@@ -70,7 +77,7 @@ public static Rule createAndAddRepositoryRule(
         throw new LabelSyntaxException(e.getMessage());
       }
     }
-    pkg.addRegisteredToolchains(toolchains.build());
+    pkg.addRegisteredToolchains(toolchains.build(), originatesInWorkspaceSuffix(callstack));
     return rule;
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java
index 61255637295fc9..1ef44d40f422d2 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java
@@ -14,6 +14,7 @@
 
 package com.google.devtools.build.lib.packages;
 
+import static com.google.devtools.build.lib.packages.WorkspaceFactoryHelper.originatesInWorkspaceSuffix;
 import static net.starlark.java.eval.Starlark.NONE;
 
 import com.google.common.collect.ImmutableList;
@@ -114,7 +115,9 @@ public void registerToolchains(Sequence<?> toolchainLabels, StarlarkThread threa
     // Add to the package definition for later.
     Package.Builder builder = PackageFactory.getContext(thread).pkgBuilder;
     List<String> patterns = Sequence.cast(toolchainLabels, String.class, "toolchain_labels");
-    builder.addRegisteredToolchains(parsePatterns(patterns, builder, thread));
+    builder.addRegisteredToolchains(
+        parsePatterns(patterns, builder, thread),
+        originatesInWorkspaceSuffix(thread.getCallStack()));
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java
index 8ae2ad808423fc..6d6186548494a0 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java
@@ -14,6 +14,7 @@
 
 package com.google.devtools.build.lib.skyframe;
 
+import static com.google.devtools.build.lib.packages.WorkspaceFactoryHelper.DEFAULT_WORKSPACE_SUFFIX_FILE;
 import static com.google.devtools.build.lib.rules.repository.ResolvedFileValue.ATTRIBUTES;
 import static com.google.devtools.build.lib.rules.repository.ResolvedFileValue.NATIVE;
 import static com.google.devtools.build.lib.rules.repository.ResolvedFileValue.REPOSITORIES;
@@ -220,7 +221,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
       StarlarkFile file =
           StarlarkFile.parse(
               ParserInput.fromString(
-                  ruleClassProvider.getDefaultWorkspaceSuffix(), "/DEFAULT.WORKSPACE.SUFFIX"),
+                  ruleClassProvider.getDefaultWorkspaceSuffix(), DEFAULT_WORKSPACE_SUFFIX_FILE),
               // The DEFAULT.WORKSPACE.SUFFIX file breaks through the usual privacy mechanism.
               options.toBuilder().allowLoadPrivateSymbols(true).build());
       if (!file.ok()) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/toolchains/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/toolchains/BUILD
index 15c3c703e1d64f..e08f783554c4ab 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/toolchains/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/toolchains/BUILD
@@ -90,6 +90,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/analysis:platform_configuration",
         "//src/main/java/com/google/devtools/build/lib/analysis/platform",
         "//src/main/java/com/google/devtools/build/lib/analysis/platform:utils",
+        "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:common",
         "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:resolution",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/packages",
@@ -160,6 +161,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/analysis:platform_configuration",
         "//src/main/java/com/google/devtools/build/lib/analysis/platform",
         "//src/main/java/com/google/devtools/build/lib/analysis/platform:utils",
+        "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:common",
         "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:exception",
         "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:resolution",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/toolchains/RegisteredExecutionPlatformsFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/toolchains/RegisteredExecutionPlatformsFunction.java
index 4fb87c27f2b02e..671f9f906d676e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/toolchains/RegisteredExecutionPlatformsFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/toolchains/RegisteredExecutionPlatformsFunction.java
@@ -26,6 +26,7 @@
 import com.google.devtools.build.lib.analysis.platform.PlatformProviderUtils;
 import com.google.devtools.build.lib.bazel.bzlmod.BazelDepGraphValue;
 import com.google.devtools.build.lib.bazel.bzlmod.Module;
+import com.google.devtools.build.lib.bazel.bzlmod.ModuleKey;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelConstants;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
@@ -104,21 +105,31 @@ public SkyValue compute(SkyKey skyKey, Environment env)
       }
     }
 
-    // Get registered execution platforms from bzlmod.
-    ImmutableList<TargetPattern> bzlmodExecutionPlatforms =
-        getBzlmodExecutionPlatforms(starlarkSemantics, env);
-    if (bzlmodExecutionPlatforms == null) {
+    // Get registered execution platforms from the root Bazel module.
+    ImmutableList<TargetPattern> bzlmodRootModuleExecutionPlatforms =
+        getBzlmodExecutionPlatforms(starlarkSemantics, env, /* forRootModule= */ true);
+    if (bzlmodRootModuleExecutionPlatforms == null) {
       return null;
     }
-    targetPatternBuilder.addAll(TargetPatternUtil.toSigned(bzlmodExecutionPlatforms));
+    targetPatternBuilder.addAll(TargetPatternUtil.toSigned(bzlmodRootModuleExecutionPlatforms));
 
     // Get the registered execution platforms from the WORKSPACE.
+    // The WORKSPACE suffixes don't register any execution platforms, so we can register all
+    // platforms in WORKSPACE before those in non-root Bazel modules.
     ImmutableList<TargetPattern> workspaceExecutionPlatforms = getWorkspaceExecutionPlatforms(env);
     if (workspaceExecutionPlatforms == null) {
       return null;
     }
     targetPatternBuilder.addAll(TargetPatternUtil.toSigned(workspaceExecutionPlatforms));
 
+    // Get registered execution platforms from the non-root Bazel modules.
+    ImmutableList<TargetPattern> bzlmodNonRootModuleExecutionPlatforms =
+        getBzlmodExecutionPlatforms(starlarkSemantics, env, /* forRootModule= */ false);
+    if (bzlmodNonRootModuleExecutionPlatforms == null) {
+      return null;
+    }
+    targetPatternBuilder.addAll(TargetPatternUtil.toSigned(bzlmodNonRootModuleExecutionPlatforms));
+
     // Expand target patterns.
     ImmutableList<Label> platformLabels;
     try {
@@ -164,7 +175,7 @@ public static ImmutableList<TargetPattern> getWorkspaceExecutionPlatforms(Enviro
 
   @Nullable
   private static ImmutableList<TargetPattern> getBzlmodExecutionPlatforms(
-      StarlarkSemantics semantics, Environment env)
+      StarlarkSemantics semantics, Environment env, boolean forRootModule)
       throws InterruptedException, RegisteredExecutionPlatformsFunctionException {
     if (!semantics.getBool(BuildLanguageOptions.ENABLE_BZLMOD)) {
       return ImmutableList.of();
@@ -176,6 +187,9 @@ private static ImmutableList<TargetPattern> getBzlmodExecutionPlatforms(
     }
     ImmutableList.Builder<TargetPattern> executionPlatforms = ImmutableList.builder();
     for (Module module : bazelDepGraphValue.getDepGraph().values()) {
+      if (forRootModule != module.getKey().equals(ModuleKey.ROOT)) {
+        continue;
+      }
       TargetPattern.Parser parser =
           new TargetPattern.Parser(
               PathFragment.EMPTY_FRAGMENT,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/toolchains/RegisteredToolchainsFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/toolchains/RegisteredToolchainsFunction.java
index cec918ff94e919..c01b3e6ee712f9 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/toolchains/RegisteredToolchainsFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/toolchains/RegisteredToolchainsFunction.java
@@ -28,6 +28,7 @@
 import com.google.devtools.build.lib.bazel.bzlmod.BazelDepGraphValue;
 import com.google.devtools.build.lib.bazel.bzlmod.ExternalDepsException;
 import com.google.devtools.build.lib.bazel.bzlmod.Module;
+import com.google.devtools.build.lib.bazel.bzlmod.ModuleKey;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelConstants;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
@@ -95,19 +96,37 @@ public SkyValue compute(SkyKey skyKey, Environment env)
           new InvalidToolchainLabelException(e), Transience.PERSISTENT);
     }
 
-    // Get registered toolchains from bzlmod.
-    ImmutableList<TargetPattern> bzlmodToolchains = getBzlmodToolchains(starlarkSemantics, env);
-    if (bzlmodToolchains == null) {
+    // Get registered toolchains from the root Bazel module.
+    ImmutableList<TargetPattern> bzlmodRootModuleToolchains =
+        getBzlmodToolchains(starlarkSemantics, env, /* forRootModule= */ true);
+    if (bzlmodRootModuleToolchains == null) {
       return null;
     }
-    targetPatternBuilder.addAll(TargetPatternUtil.toSigned(bzlmodToolchains));
+    targetPatternBuilder.addAll(TargetPatternUtil.toSigned(bzlmodRootModuleToolchains));
 
-    // Get the registered toolchains from the WORKSPACE.
-    ImmutableList<TargetPattern> workspaceToolchains = getWorkspaceToolchains(env);
-    if (workspaceToolchains == null) {
+    // Get the toolchains from the user-supplied WORKSPACE file.
+    ImmutableList<TargetPattern> userRegisteredWorkspaceToolchains =
+        getWorkspaceToolchains(env, /* userRegistered= */ true);
+    if (userRegisteredWorkspaceToolchains == null) {
       return null;
     }
-    targetPatternBuilder.addAll(TargetPatternUtil.toSigned(workspaceToolchains));
+    targetPatternBuilder.addAll(TargetPatternUtil.toSigned(userRegisteredWorkspaceToolchains));
+
+    // Get registered toolchains from non-root Bazel modules.
+    ImmutableList<TargetPattern> bzlmodNonRootModuleToolchains =
+        getBzlmodToolchains(starlarkSemantics, env, /* forRootModule= */ false);
+    if (bzlmodNonRootModuleToolchains == null) {
+      return null;
+    }
+    targetPatternBuilder.addAll(TargetPatternUtil.toSigned(bzlmodNonRootModuleToolchains));
+
+    // Get the toolchains from the Bazel-supplied WORKSPACE suffix.
+    ImmutableList<TargetPattern> workspaceSuffixToolchains =
+        getWorkspaceToolchains(env, /* userRegistered= */ false);
+    if (workspaceSuffixToolchains == null) {
+      return null;
+    }
+    targetPatternBuilder.addAll(TargetPatternUtil.toSigned(workspaceSuffixToolchains));
 
     // Expand target patterns.
     ImmutableList<Label> toolchainLabels;
@@ -140,8 +159,8 @@ public SkyValue compute(SkyKey skyKey, Environment env)
    */
   @Nullable
   @VisibleForTesting
-  public static ImmutableList<TargetPattern> getWorkspaceToolchains(Environment env)
-      throws InterruptedException {
+  public static ImmutableList<TargetPattern> getWorkspaceToolchains(
+      Environment env, boolean userRegistered) throws InterruptedException {
     PackageValue externalPackageValue =
         (PackageValue) env.getValue(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER);
     if (externalPackageValue == null) {
@@ -149,12 +168,16 @@ public static ImmutableList<TargetPattern> getWorkspaceToolchains(Environment en
     }
 
     Package externalPackage = externalPackageValue.getPackage();
-    return externalPackage.getRegisteredToolchains();
+    if (userRegistered) {
+      return externalPackage.getUserRegisteredToolchains();
+    } else {
+      return externalPackage.getWorkspaceSuffixRegisteredToolchains();
+    }
   }
 
   @Nullable
   private static ImmutableList<TargetPattern> getBzlmodToolchains(
-      StarlarkSemantics semantics, Environment env)
+      StarlarkSemantics semantics, Environment env, boolean forRootModule)
       throws InterruptedException, RegisteredToolchainsFunctionException {
     if (!semantics.getBool(BuildLanguageOptions.ENABLE_BZLMOD)) {
       return ImmutableList.of();
@@ -166,6 +189,9 @@ private static ImmutableList<TargetPattern> getBzlmodToolchains(
     }
     ImmutableList.Builder<TargetPattern> toolchains = ImmutableList.builder();
     for (Module module : bazelDepGraphValue.getDepGraph().values()) {
+      if (forRootModule != module.getKey().equals(ModuleKey.ROOT)) {
+        continue;
+      }
       TargetPattern.Parser parser =
           new TargetPattern.Parser(
               PathFragment.EMPTY_FRAGMENT,


Test patch:
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java
index bdcf861e3571d3..ba0fbd957955c4 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java
@@ -501,7 +501,8 @@ public void setupMockClient(MockToolsConfig config, List<String> workspaceConten
         "def rules_java_dependencies():",
         "    pass",
         "def rules_java_toolchains():",
-        "    pass");
+        "    native.register_toolchains('//java/toolchains/runtime:all')",
+        "    native.register_toolchains('//java/toolchains/javac:all')");
 
     config.create(
         "rules_java_workspace/java/toolchains/runtime/BUILD",
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/LoadingMock.java b/src/test/java/com/google/devtools/build/lib/packages/util/LoadingMock.java
index 9cc9715233fce6..bc49f4c2fd0129 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/LoadingMock.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/LoadingMock.java
@@ -39,4 +39,8 @@ public PackageFactoryBuilderWithSkyframeForTesting getPackageFactoryBuilderForTe
   public ConfiguredRuleClassProvider createRuleClassProvider() {
     return TestRuleClassProvider.getRuleClassProviderWithClearedSuffix();
   }
+
+  public ConfiguredRuleClassProvider createRuleClassProviderWithSuffix() {
+    return TestRuleClassProvider.getRuleClassProvider();
+  }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/repository/ExternalPackageHelperTest.java b/src/test/java/com/google/devtools/build/lib/repository/ExternalPackageHelperTest.java
index 3e6388aa44f923..289d61b157cff0 100644
--- a/src/test/java/com/google/devtools/build/lib/repository/ExternalPackageHelperTest.java
+++ b/src/test/java/com/google/devtools/build/lib/repository/ExternalPackageHelperTest.java
@@ -82,6 +82,7 @@
 import java.util.Optional;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.Stream;
 import javax.annotation.Nullable;
 import org.junit.Before;
 import org.junit.Test;
@@ -411,13 +412,19 @@ private static final class GetRegisteredToolchainsFunction implements SkyFunctio
     @Override
     public SkyValue compute(SkyKey skyKey, Environment env)
         throws SkyFunctionException, InterruptedException {
-      List<TargetPattern> registeredToolchains =
-          RegisteredToolchainsFunction.getWorkspaceToolchains(env);
-      if (registeredToolchains == null) {
+      ImmutableList<TargetPattern> userRegisteredToolchains =
+          RegisteredToolchainsFunction.getWorkspaceToolchains(env, /* userRegistered= */ true);
+      if (userRegisteredToolchains == null) {
+        return null;
+      }
+      ImmutableList<TargetPattern> workspaceSuffixRegisteredToolchains =
+          RegisteredToolchainsFunction.getWorkspaceToolchains(env, /* userRegistered= */ false);
+      if (workspaceSuffixRegisteredToolchains == null) {
         return null;
       }
       return GetRegisteredToolchainsValue.create(
-          registeredToolchains.stream()
+          Stream.concat(
+                  userRegisteredToolchains.stream(), workspaceSuffixRegisteredToolchains.stream())
               .map(TargetPattern::getOriginalPattern)
               .collect(toImmutableList()));
     }
diff --git a/src/test/java/com/google/devtools/build/lib/rules/platform/ToolchainTestCase.java b/src/test/java/com/google/devtools/build/lib/rules/platform/ToolchainTestCase.java
index 78e06b21e93940..3e21c1e58c6555 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/platform/ToolchainTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/platform/ToolchainTestCase.java
@@ -224,7 +224,8 @@ public void createToolchains() throws Exception {
     scratch.file(
         "toolchain/BUILD",
         "toolchain_type(name = 'test_toolchain')",
-        "toolchain_type(name = 'optional_toolchain')");
+        "toolchain_type(name = 'optional_toolchain')",
+        "toolchain_type(name = 'workspace_suffix_toolchain')");
 
     testToolchainTypeLabel = Label.parseCanonicalUnchecked("//toolchain:test_toolchain");
     testToolchainType = ToolchainTypeRequirement.create(testToolchainTypeLabel);
@@ -247,6 +248,22 @@ public void createToolchains() throws Exception {
         ImmutableList.of("//constraints:mac"),
         ImmutableList.of("//constraints:linux"),
         "bar");
+    Label suffixToolchainTypeLabel =
+        Label.parseCanonicalUnchecked("//toolchain:workspace_suffix_toolchain");
+    addToolchain(
+        "toolchain",
+        "suffix_toolchain_1",
+        suffixToolchainTypeLabel,
+        ImmutableList.of(),
+        ImmutableList.of(),
+        "suffix1");
+    addToolchain(
+        "toolchain",
+        "suffix_toolchain_2",
+        suffixToolchainTypeLabel,
+        ImmutableList.of(),
+        ImmutableList.of(),
+        "suffix2");
   }
 
   protected EvaluationResult<RegisteredToolchainsValue> requestToolchainsFromSkyframe(
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/toolchains/BUILD b/src/test/java/com/google/devtools/build/lib/skyframe/toolchains/BUILD
index 943c9ddea6a78c..08252c34a218c8 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/toolchains/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/toolchains/BUILD
@@ -66,14 +66,10 @@ java_test(
     deps = [
         "//src/main/java/com/google/devtools/build/lib/analysis:view_creation_failed_exception",
         "//src/main/java/com/google/devtools/build/lib/analysis/platform",
-        "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:resolution_impl",
-        "//src/main/java/com/google/devtools/build/lib/bazel/repository:repository_options",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_key",
-        "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:platform_lookup_util",
         "//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:registered_execution_platforms_value",
-        "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/skyframe",
         "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
         "//src/test/java/com/google/devtools/build/lib/bazel/bzlmod:util",
@@ -92,17 +88,16 @@ java_test(
     name = "RegisteredToolchainsFunctionTest",
     srcs = ["RegisteredToolchainsFunctionTest.java"],
     deps = [
+        "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
         "//src/main/java/com/google/devtools/build/lib/analysis/platform",
-        "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:resolution_impl",
-        "//src/main/java/com/google/devtools/build/lib/bazel/repository:repository_options",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
-        "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:registered_toolchains_value",
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/skyframe",
         "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
         "//src/test/java/com/google/devtools/build/lib/bazel/bzlmod:util",
         "//src/test/java/com/google/devtools/build/lib/rules/platform:testutil",
+        "//src/test/java/com/google/devtools/build/lib/testutil",
         "//src/test/java/com/google/devtools/build/skyframe:testutil",
         "//third_party:guava",
         "//third_party:guava-testlib",
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/toolchains/RegisteredExecutionPlatformsFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/toolchains/RegisteredExecutionPlatformsFunctionTest.java
index 5d9688faa85b33..91b3981b8f518a 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/toolchains/RegisteredExecutionPlatformsFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/toolchains/RegisteredExecutionPlatformsFunctionTest.java
@@ -339,8 +339,13 @@ public void testRegisteredExecutionPlatforms_bzlmod() throws Exception {
           "platform(name='plat')");
     }
     scratch.overwriteFile(
-        "BUILD", "platform(name='plat')", "platform(name='dev_plat')", "platform(name='wsplat')");
-    rewriteWorkspace("register_execution_platforms('//:wsplat')");
+        "BUILD",
+        "platform(name='plat')",
+        "platform(name='dev_plat')",
+        "platform(name='wsplat')",
+        "platform(name='wsplat2')");
+    rewriteWorkspace(
+        "register_execution_platforms('//:wsplat')", "register_execution_platforms('//:wsplat2')");
 
     SkyKey executionPlatformsKey = RegisteredExecutionPlatformsValue.key(targetConfigKey);
     EvaluationResult<RegisteredExecutionPlatformsValue> result =
@@ -354,13 +359,17 @@ public void testRegisteredExecutionPlatforms_bzlmod() throws Exception {
     // WORKSPACE registrations.
     assertExecutionPlatformLabels(result.get(executionPlatformsKey))
         .containsExactly(
+            // Root module platforms
             Label.parseCanonical("//:plat"),
             Label.parseCanonical("//:dev_plat"),
+            // WORKSPACE platforms
+            Label.parseCanonical("//:wsplat"),
+            Label.parseCanonical("//:wsplat2"),
+            // Other modules' toolchains
             Label.parseCanonical("@@bbb~1.0//:plat"),
             Label.parseCanonical("@@ccc~1.1//:plat"),
             Label.parseCanonical("@@eee~1.0//:plat"),
-            Label.parseCanonical("@@ddd~1.1//:plat"),
-            Label.parseCanonical("//:wsplat"))
+            Label.parseCanonical("@@ddd~1.1//:plat"))
         .inOrder();
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/toolchains/RegisteredToolchainsFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/toolchains/RegisteredToolchainsFunctionTest.java
index 0892a303c2a771..15b934e3befd61 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/toolchains/RegisteredToolchainsFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/toolchains/RegisteredToolchainsFunctionTest.java
@@ -20,15 +20,18 @@
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.testing.EqualsTester;
+import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
 import com.google.devtools.build.lib.analysis.platform.DeclaredToolchainInfo;
 import com.google.devtools.build.lib.analysis.platform.ToolchainTypeInfo;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.rules.platform.ToolchainTestCase;
+import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.skyframe.EvaluationResult;
 import com.google.devtools.build.skyframe.SkyKey;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -37,6 +40,17 @@
 @RunWith(JUnit4.class)
 public class RegisteredToolchainsFunctionTest extends ToolchainTestCase {
 
+  @Override
+  protected ConfiguredRuleClassProvider createRuleClassProvider() {
+    // testRegisteredToolchains_bzlmod uses the WORKSPACE suffixes.
+    ConfiguredRuleClassProvider.Builder builder = new ConfiguredRuleClassProvider.Builder();
+    TestRuleClassProvider.addStandardRules(builder);
+    builder.clearWorkspaceFileSuffixForTesting();
+    builder.addWorkspaceFileSuffix(
+        "register_toolchains('//toolchain:suffix_toolchain_1', '//toolchain:suffix_toolchain_2')");
+    return builder.build();
+  }
+
   @Test
   public void testRegisteredToolchains() throws Exception {
     // Request the toolchains.
@@ -423,8 +437,24 @@ public void testRegisteredToolchains_bzlmod() throws Exception {
         "load('@toolchain_def//:toolchain_def.bzl', 'declare_toolchain')",
         "declare_toolchain(name='dev_tool')",
         "declare_toolchain(name='tool')",
-        "declare_toolchain(name='wstool')");
-    rewriteWorkspace("register_toolchains('//:wstool')");
+        "declare_toolchain(name='wstool')",
+        "declare_toolchain(name='wstool2')");
+    scratch.overwriteFile(
+        "WORKSPACE",
+        Stream.concat(
+                analysisMock.getWorkspaceContents(mockToolsConfig).stream()
+                    // The register_toolchains calls usually live in the WORKSPACE suffixes.
+                    // BazelAnalysisMock moves the mock registrations to the actual WORKSPACE file
+                    // as most Java tests don't run with the suffixes. This test class does, so we
+                    // skip over the "unnatural" registrations.
+                    .filter(line -> !line.startsWith("register_toolchains(")),
+                // Register a toolchain explicitly that is also registered in the WORKSPACE suffix.
+                Stream.of(
+                    "register_toolchains('//:wstool')",
+                    "register_toolchains('//toolchain:suffix_toolchain_2')",
+                    "register_toolchains('//:wstool2')"))
+            .toArray(String[]::new));
+    invalidatePackages();
 
     SkyKey toolchainsKey = RegisteredToolchainsValue.key(targetConfigKey);
     EvaluationResult<RegisteredToolchainsValue> result =
@@ -438,13 +468,20 @@ public void testRegisteredToolchains_bzlmod() throws Exception {
     // registrations.
     assertToolchainLabels(result.get(toolchainsKey))
         .containsAtLeast(
+            // Root module toolchains
             Label.parseCanonical("//:tool_impl"),
             Label.parseCanonical("//:dev_tool_impl"),
+            // WORKSPACE toolchains
+            Label.parseCanonical("//:wstool_impl"),
+            Label.parseCanonical("//toolchain:suffix_toolchain_2_impl"),
+            Label.parseCanonical("//:wstool2_impl"),
+            // Other modules' toolchains
             Label.parseCanonical("@@bbb~1.0//:tool_impl"),
             Label.parseCanonical("@@ccc~1.1//:tool_impl"),
             Label.parseCanonical("@@eee~1.0//:tool_impl"),
             Label.parseCanonical("@@ddd~1.1//:tool_impl"),
-            Label.parseCanonical("//:wstool_impl"))
+            // WORKSPACE suffix toolchains
+            Label.parseCanonical("//toolchain:suffix_toolchain_1_impl"))
         .inOrder();
   }