Skip to content

Commit 96b3612

Browse files
fmeumWyverald
authored and
copybara-github
committed
Give WORKSPACE toolchains and platforms precedence over non-root modules
RELNOTES[INC]: Toolchains and execution platforms are now registered in the following order with `--enable_bzlmod`: 1. root module's module file 2. `WORKSPACE` or `WORKSPACE.bzlmod` 3. non-root modules' module files 4. default toolchains registered by Bazel (does not apply with `WORKSPACE.bzlmod` or execution platforms) Fixes #20354 Closes #20407. Co-authored-by: Xudong Yang <wyv@google.com> PiperOrigin-RevId: 587826082 Change-Id: Ia98da6ef07b2fbf589ef369d986af2323af6f72a
1 parent ec4da1b commit 96b3612

16 files changed

Lines changed: 219 additions & 45 deletions

File tree

‎site/en/external/migration.md‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,19 @@ toolchain.
453453
register_toolchains("@local_config_sh//:local_sh_toolchain")
454454
```
455455
456+
The toolchains and execution platforms registered in `WORKSPACE`,
457+
`WORKSPACE.bzlmod` and each Bazel module's `MODULE.bazel` file follow this
458+
order of precedence during toolchain selection (from highest to lowest):
459+
460+
1. toolchains and execution platforms registered in the root module's
461+
`MODULE.bazel` file.
462+
2. toolchains and execution platforms registered in the `WORKSPACE` or
463+
`WORKSPACE.bzlmod` file.
464+
3. toolchains and execution platforms registered by modules that are
465+
(transitive) dependencies of the root module.
466+
4. when not using `WORKSPACE.bzlmod`: toolchains registered in the `WORKSPACE`
467+
[suffix](/external/migration#builtin-default-deps).
468+
456469
[register_execution_platforms]: /rules/lib/globals/module#register_execution_platforms
457470
458471
### Introduce local repositories {:#introduce-local-deps}

‎src/main/java/com/google/devtools/build/lib/packages/Package.java‎

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272
import java.util.Map;
7373
import java.util.Objects;
7474
import java.util.Optional;
75+
import java.util.OptionalInt;
7576
import java.util.OptionalLong;
7677
import java.util.Set;
7778
import java.util.TreeMap;
@@ -262,7 +263,7 @@ void setPackageOverhead(OptionalLong packageOverhead) {
262263

263264
private ImmutableList<TargetPattern> registeredExecutionPlatforms;
264265
private ImmutableList<TargetPattern> registeredToolchains;
265-
266+
private OptionalInt firstWorkspaceSuffixRegisteredToolchain;
266267
private long computationSteps;
267268

268269
// These two fields are mutually exclusive. Which one is set depends on
@@ -425,6 +426,7 @@ private void finishInit(Builder builder) {
425426
this.failureDetail = builder.getFailureDetail();
426427
this.registeredExecutionPlatforms = ImmutableList.copyOf(builder.registeredExecutionPlatforms);
427428
this.registeredToolchains = ImmutableList.copyOf(builder.registeredToolchains);
429+
this.firstWorkspaceSuffixRegisteredToolchain = builder.firstWorkspaceSuffixRegisteredToolchain;
428430
this.repositoryMapping = Preconditions.checkNotNull(builder.repositoryMapping);
429431
this.mainRepositoryMapping = Preconditions.checkNotNull(builder.mainRepositoryMapping);
430432
ImmutableMap.Builder<RepositoryName, ImmutableMap<String, RepositoryName>>
@@ -764,6 +766,23 @@ public ImmutableList<TargetPattern> getRegisteredToolchains() {
764766
return registeredToolchains;
765767
}
766768

769+
public ImmutableList<TargetPattern> getUserRegisteredToolchains() {
770+
return getRegisteredToolchains()
771+
.subList(
772+
0, firstWorkspaceSuffixRegisteredToolchain.orElse(getRegisteredToolchains().size()));
773+
}
774+
775+
public ImmutableList<TargetPattern> getWorkspaceSuffixRegisteredToolchains() {
776+
return getRegisteredToolchains()
777+
.subList(
778+
firstWorkspaceSuffixRegisteredToolchain.orElse(getRegisteredToolchains().size()),
779+
getRegisteredToolchains().size());
780+
}
781+
782+
OptionalInt getFirstWorkspaceSuffixRegisteredToolchain() {
783+
return firstWorkspaceSuffixRegisteredToolchain;
784+
}
785+
767786
@Override
768787
public String toString() {
769788
return "Package("
@@ -984,8 +1003,16 @@ default boolean precomputeTransitiveLoads() {
9841003
private final List<TargetPattern> registeredToolchains = new ArrayList<>();
9851004

9861005
/**
987-
* True iff the "package" function has already been called in this package.
1006+
* Tracks the index within {@link #registeredToolchains} of the first toolchain registered from
1007+
* the WORKSPACE suffixes rather than the WORKSPACE file (if any).
1008+
*
1009+
* <p>This is needed to distinguish between these toolchains during resolution: toolchains
1010+
* registered in WORKSPACE have precedence over those defined in non-root Bazel modules, which
1011+
* in turn have precedence over those from the WORKSPACE suffixes.
9881012
*/
1013+
private OptionalInt firstWorkspaceSuffixRegisteredToolchain = OptionalInt.empty();
1014+
1015+
/** True iff the "package" function has already been called in this package. */
9891016
private boolean packageFunctionUsed;
9901017

9911018
/**
@@ -1670,10 +1697,18 @@ void addRegisteredExecutionPlatforms(List<TargetPattern> platforms) {
16701697
this.registeredExecutionPlatforms.addAll(platforms);
16711698
}
16721699

1673-
void addRegisteredToolchains(List<TargetPattern> toolchains) {
1700+
void addRegisteredToolchains(List<TargetPattern> toolchains, boolean forWorkspaceSuffix) {
1701+
if (forWorkspaceSuffix && firstWorkspaceSuffixRegisteredToolchain.isEmpty()) {
1702+
firstWorkspaceSuffixRegisteredToolchain = OptionalInt.of(registeredToolchains.size());
1703+
}
16741704
this.registeredToolchains.addAll(toolchains);
16751705
}
16761706

1707+
void setFirstWorkspaceSuffixRegisteredToolchain(
1708+
OptionalInt firstWorkspaceSuffixRegisteredToolchain) {
1709+
this.firstWorkspaceSuffixRegisteredToolchain = firstWorkspaceSuffixRegisteredToolchain;
1710+
}
1711+
16771712
@CanIgnoreReturnValue
16781713
private Builder beforeBuild(boolean discoverAssumedInputFiles) throws NoSuchPackageException {
16791714
Preconditions.checkNotNull(pkg);

‎src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,10 @@ public void setParent(
185185
builder.setFailureDetailOverride(aPackage.getFailureDetail());
186186
}
187187
builder.addRegisteredExecutionPlatforms(aPackage.getRegisteredExecutionPlatforms());
188-
builder.addRegisteredToolchains(aPackage.getRegisteredToolchains());
188+
builder.addRegisteredToolchains(
189+
aPackage.getRegisteredToolchains(), /* forWorkspaceSuffix= */ false);
190+
builder.setFirstWorkspaceSuffixRegisteredToolchain(
191+
aPackage.getFirstWorkspaceSuffixRegisteredToolchain());
189192
builder.addRepositoryMappings(aPackage);
190193
for (Rule rule : aPackage.getTargets(Rule.class)) {
191194
try {

‎src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,13 @@
3636
/** A helper for the {@link WorkspaceFactory} to create repository rules */
3737
public final class WorkspaceFactoryHelper {
3838

39+
public static final String DEFAULT_WORKSPACE_SUFFIX_FILE = "/DEFAULT.WORKSPACE.SUFFIX";
40+
41+
public static boolean originatesInWorkspaceSuffix(
42+
ImmutableList<StarlarkThread.CallStackEntry> callstack) {
43+
return callstack.get(0).location.file().equals(DEFAULT_WORKSPACE_SUFFIX_FILE);
44+
}
45+
3946
@CanIgnoreReturnValue
4047
public static Rule createAndAddRepositoryRule(
4148
Package.Builder pkg,
@@ -70,7 +77,7 @@ public static Rule createAndAddRepositoryRule(
7077
throw new LabelSyntaxException(e.getMessage());
7178
}
7279
}
73-
pkg.addRegisteredToolchains(toolchains.build());
80+
pkg.addRegisteredToolchains(toolchains.build(), originatesInWorkspaceSuffix(callstack));
7481
return rule;
7582
}
7683

‎src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
package com.google.devtools.build.lib.packages;
1616

17+
import static com.google.devtools.build.lib.packages.WorkspaceFactoryHelper.originatesInWorkspaceSuffix;
1718
import static net.starlark.java.eval.Starlark.NONE;
1819

1920
import com.google.common.collect.ImmutableList;
@@ -114,7 +115,9 @@ public void registerToolchains(Sequence<?> toolchainLabels, StarlarkThread threa
114115
// Add to the package definition for later.
115116
Package.Builder builder = PackageFactory.getContext(thread).pkgBuilder;
116117
List<String> patterns = Sequence.cast(toolchainLabels, String.class, "toolchain_labels");
117-
builder.addRegisteredToolchains(parsePatterns(patterns, builder, thread));
118+
builder.addRegisteredToolchains(
119+
parsePatterns(patterns, builder, thread),
120+
originatesInWorkspaceSuffix(thread.getCallStack()));
118121
}
119122

120123
@Override

‎src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
package com.google.devtools.build.lib.skyframe;
1616

17+
import static com.google.devtools.build.lib.packages.WorkspaceFactoryHelper.DEFAULT_WORKSPACE_SUFFIX_FILE;
1718
import static com.google.devtools.build.lib.rules.repository.ResolvedFileValue.ATTRIBUTES;
1819
import static com.google.devtools.build.lib.rules.repository.ResolvedFileValue.NATIVE;
1920
import static com.google.devtools.build.lib.rules.repository.ResolvedFileValue.REPOSITORIES;
@@ -220,7 +221,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
220221
StarlarkFile file =
221222
StarlarkFile.parse(
222223
ParserInput.fromString(
223-
ruleClassProvider.getDefaultWorkspaceSuffix(), "/DEFAULT.WORKSPACE.SUFFIX"),
224+
ruleClassProvider.getDefaultWorkspaceSuffix(), DEFAULT_WORKSPACE_SUFFIX_FILE),
224225
// The DEFAULT.WORKSPACE.SUFFIX file breaks through the usual privacy mechanism.
225226
options.toBuilder().allowLoadPrivateSymbols(true).build());
226227
if (!file.ok()) {

‎src/main/java/com/google/devtools/build/lib/skyframe/toolchains/BUILD‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ java_library(
9090
"//src/main/java/com/google/devtools/build/lib/analysis:platform_configuration",
9191
"//src/main/java/com/google/devtools/build/lib/analysis/platform",
9292
"//src/main/java/com/google/devtools/build/lib/analysis/platform:utils",
93+
"//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:common",
9394
"//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:resolution",
9495
"//src/main/java/com/google/devtools/build/lib/cmdline",
9596
"//src/main/java/com/google/devtools/build/lib/packages",
@@ -161,6 +162,7 @@ java_library(
161162
"//src/main/java/com/google/devtools/build/lib/analysis:platform_configuration",
162163
"//src/main/java/com/google/devtools/build/lib/analysis/platform",
163164
"//src/main/java/com/google/devtools/build/lib/analysis/platform:utils",
165+
"//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:common",
164166
"//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:exception",
165167
"//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:resolution",
166168
"//src/main/java/com/google/devtools/build/lib/cmdline",

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

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import com.google.devtools.build.lib.analysis.platform.PlatformProviderUtils;
2727
import com.google.devtools.build.lib.bazel.bzlmod.BazelDepGraphValue;
2828
import com.google.devtools.build.lib.bazel.bzlmod.Module;
29+
import com.google.devtools.build.lib.bazel.bzlmod.ModuleKey;
2930
import com.google.devtools.build.lib.cmdline.Label;
3031
import com.google.devtools.build.lib.cmdline.LabelConstants;
3132
import com.google.devtools.build.lib.cmdline.RepositoryName;
@@ -104,21 +105,31 @@ public SkyValue compute(SkyKey skyKey, Environment env)
104105
}
105106
}
106107

107-
// Get registered execution platforms from bzlmod.
108-
ImmutableList<TargetPattern> bzlmodExecutionPlatforms =
109-
getBzlmodExecutionPlatforms(starlarkSemantics, env);
110-
if (bzlmodExecutionPlatforms == null) {
108+
// Get registered execution platforms from the root Bazel module.
109+
ImmutableList<TargetPattern> bzlmodRootModuleExecutionPlatforms =
110+
getBzlmodExecutionPlatforms(starlarkSemantics, env, /* forRootModule= */ true);
111+
if (bzlmodRootModuleExecutionPlatforms == null) {
111112
return null;
112113
}
113-
targetPatternBuilder.addAll(TargetPatternUtil.toSigned(bzlmodExecutionPlatforms));
114+
targetPatternBuilder.addAll(TargetPatternUtil.toSigned(bzlmodRootModuleExecutionPlatforms));
114115

115116
// Get the registered execution platforms from the WORKSPACE.
117+
// The WORKSPACE suffixes don't register any execution platforms, so we can register all
118+
// platforms in WORKSPACE before those in non-root Bazel modules.
116119
ImmutableList<TargetPattern> workspaceExecutionPlatforms = getWorkspaceExecutionPlatforms(env);
117120
if (workspaceExecutionPlatforms == null) {
118121
return null;
119122
}
120123
targetPatternBuilder.addAll(TargetPatternUtil.toSigned(workspaceExecutionPlatforms));
121124

125+
// Get registered execution platforms from the non-root Bazel modules.
126+
ImmutableList<TargetPattern> bzlmodNonRootModuleExecutionPlatforms =
127+
getBzlmodExecutionPlatforms(starlarkSemantics, env, /* forRootModule= */ false);
128+
if (bzlmodNonRootModuleExecutionPlatforms == null) {
129+
return null;
130+
}
131+
targetPatternBuilder.addAll(TargetPatternUtil.toSigned(bzlmodNonRootModuleExecutionPlatforms));
132+
122133
// Expand target patterns.
123134
ImmutableList<Label> platformLabels;
124135
try {
@@ -164,7 +175,7 @@ public static ImmutableList<TargetPattern> getWorkspaceExecutionPlatforms(Enviro
164175

165176
@Nullable
166177
private static ImmutableList<TargetPattern> getBzlmodExecutionPlatforms(
167-
StarlarkSemantics semantics, Environment env)
178+
StarlarkSemantics semantics, Environment env, boolean forRootModule)
168179
throws InterruptedException, RegisteredExecutionPlatformsFunctionException {
169180
if (!semantics.getBool(BuildLanguageOptions.ENABLE_BZLMOD)) {
170181
return ImmutableList.of();
@@ -176,6 +187,9 @@ private static ImmutableList<TargetPattern> getBzlmodExecutionPlatforms(
176187
}
177188
ImmutableList.Builder<TargetPattern> executionPlatforms = ImmutableList.builder();
178189
for (Module module : bazelDepGraphValue.getDepGraph().values()) {
190+
if (forRootModule != module.getKey().equals(ModuleKey.ROOT)) {
191+
continue;
192+
}
179193
TargetPattern.Parser parser =
180194
new TargetPattern.Parser(
181195
PathFragment.EMPTY_FRAGMENT,

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

Lines changed: 38 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import com.google.devtools.build.lib.bazel.bzlmod.BazelDepGraphValue;
2929
import com.google.devtools.build.lib.bazel.bzlmod.ExternalDepsException;
3030
import com.google.devtools.build.lib.bazel.bzlmod.Module;
31+
import com.google.devtools.build.lib.bazel.bzlmod.ModuleKey;
3132
import com.google.devtools.build.lib.cmdline.Label;
3233
import com.google.devtools.build.lib.cmdline.LabelConstants;
3334
import com.google.devtools.build.lib.cmdline.RepositoryName;
@@ -95,19 +96,37 @@ public SkyValue compute(SkyKey skyKey, Environment env)
9596
new InvalidToolchainLabelException(e), Transience.PERSISTENT);
9697
}
9798

98-
// Get registered toolchains from bzlmod.
99-
ImmutableList<TargetPattern> bzlmodToolchains = getBzlmodToolchains(starlarkSemantics, env);
100-
if (bzlmodToolchains == null) {
99+
// Get registered toolchains from the root Bazel module.
100+
ImmutableList<TargetPattern> bzlmodRootModuleToolchains =
101+
getBzlmodToolchains(starlarkSemantics, env, /* forRootModule= */ true);
102+
if (bzlmodRootModuleToolchains == null) {
101103
return null;
102104
}
103-
targetPatternBuilder.addAll(TargetPatternUtil.toSigned(bzlmodToolchains));
105+
targetPatternBuilder.addAll(TargetPatternUtil.toSigned(bzlmodRootModuleToolchains));
104106

105-
// Get the registered toolchains from the WORKSPACE.
106-
ImmutableList<TargetPattern> workspaceToolchains = getWorkspaceToolchains(env);
107-
if (workspaceToolchains == null) {
107+
// Get the toolchains from the user-supplied WORKSPACE file.
108+
ImmutableList<TargetPattern> userRegisteredWorkspaceToolchains =
109+
getWorkspaceToolchains(env, /* userRegistered= */ true);
110+
if (userRegisteredWorkspaceToolchains == null) {
108111
return null;
109112
}
110-
targetPatternBuilder.addAll(TargetPatternUtil.toSigned(workspaceToolchains));
113+
targetPatternBuilder.addAll(TargetPatternUtil.toSigned(userRegisteredWorkspaceToolchains));
114+
115+
// Get registered toolchains from non-root Bazel modules.
116+
ImmutableList<TargetPattern> bzlmodNonRootModuleToolchains =
117+
getBzlmodToolchains(starlarkSemantics, env, /* forRootModule= */ false);
118+
if (bzlmodNonRootModuleToolchains == null) {
119+
return null;
120+
}
121+
targetPatternBuilder.addAll(TargetPatternUtil.toSigned(bzlmodNonRootModuleToolchains));
122+
123+
// Get the toolchains from the Bazel-supplied WORKSPACE suffix.
124+
ImmutableList<TargetPattern> workspaceSuffixToolchains =
125+
getWorkspaceToolchains(env, /* userRegistered= */ false);
126+
if (workspaceSuffixToolchains == null) {
127+
return null;
128+
}
129+
targetPatternBuilder.addAll(TargetPatternUtil.toSigned(workspaceSuffixToolchains));
111130

112131
// Expand target patterns.
113132
ImmutableList<Label> toolchainLabels;
@@ -140,21 +159,25 @@ public SkyValue compute(SkyKey skyKey, Environment env)
140159
*/
141160
@Nullable
142161
@VisibleForTesting
143-
public static ImmutableList<TargetPattern> getWorkspaceToolchains(Environment env)
144-
throws InterruptedException {
162+
public static ImmutableList<TargetPattern> getWorkspaceToolchains(
163+
Environment env, boolean userRegistered) throws InterruptedException {
145164
PackageValue externalPackageValue =
146165
(PackageValue) env.getValue(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER);
147166
if (externalPackageValue == null) {
148167
return null;
149168
}
150169

151170
Package externalPackage = externalPackageValue.getPackage();
152-
return externalPackage.getRegisteredToolchains();
171+
if (userRegistered) {
172+
return externalPackage.getUserRegisteredToolchains();
173+
} else {
174+
return externalPackage.getWorkspaceSuffixRegisteredToolchains();
175+
}
153176
}
154177

155178
@Nullable
156179
private static ImmutableList<TargetPattern> getBzlmodToolchains(
157-
StarlarkSemantics semantics, Environment env)
180+
StarlarkSemantics semantics, Environment env, boolean forRootModule)
158181
throws InterruptedException, RegisteredToolchainsFunctionException {
159182
if (!semantics.getBool(BuildLanguageOptions.ENABLE_BZLMOD)) {
160183
return ImmutableList.of();
@@ -166,6 +189,9 @@ private static ImmutableList<TargetPattern> getBzlmodToolchains(
166189
}
167190
ImmutableList.Builder<TargetPattern> toolchains = ImmutableList.builder();
168191
for (Module module : bazelDepGraphValue.getDepGraph().values()) {
192+
if (forRootModule != module.getKey().equals(ModuleKey.ROOT)) {
193+
continue;
194+
}
169195
TargetPattern.Parser parser =
170196
new TargetPattern.Parser(
171197
PathFragment.EMPTY_FRAGMENT,

‎src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,8 @@ public void setupMockClient(MockToolsConfig config, List<String> workspaceConten
462462
"def rules_java_dependencies():",
463463
" pass",
464464
"def rules_java_toolchains():",
465-
" pass");
465+
" native.register_toolchains('//java/toolchains/runtime:all')",
466+
" native.register_toolchains('//java/toolchains/javac:all')");
466467

467468
config.create(
468469
"rules_java_workspace/java/toolchains/runtime/BUILD",

0 commit comments

Comments
 (0)