Skip to content

Commit c1fea13

Browse files
Wyveraldbrentleyjones
Wyverald
and
brentleyjones
authored
Introduce max_compatibility_level for bazel_dep (#18178)
* Add InterimModule to represent a Module before resolution finishes The class Module currently holds data that's no longer needed after resolution finishes (such as the compatibility level, bazel compatibility, the registry where it comes from, etc). Conversely, the repo spec field is not computed until the end of resolution. To remove runtime checks and reduce cognitive overhead, this CL splits the Module class into two; one only used after resolution finishes (Module), and one only used before (InterimModule). This allows us to introduce max_compatibility_level in a follow CL. Work towards #17378 Co-authored-by: Brentley Jones <github@brentleyjones.com> PiperOrigin-RevId: 525780111 Change-Id: I2df8d78d324b3c8744ba0a4eda405d162e9bbb8c * Selection with max_compatibility_level See code comments for more details. tl;dr: we use the new `bazel_dep(max_compatibility_level=)` attribute to influence version selection. Fixes #17378 RELNOTES: Added a new `max_compatibility_level` attribute to the `bazel_dep` directive, which allows version selection to upgrade a dependency up to the specified compatibility level. Co-authored-by: Brentley Jones <github@brentleyjones.com> PiperOrigin-RevId: 526118928 Change-Id: I332eb3761e0dee0cb7f318cb5d8d1780fca91be8 --------- Co-authored-by: Brentley Jones <github@brentleyjones.com>
1 parent 6c61110 commit c1fea13

23 files changed

Lines changed: 1505 additions & 744 deletions

‎src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,10 @@ java_library(
9494
"BazelModuleResolutionValue.java",
9595
"BzlmodFlagsAndEnvVars.java",
9696
"GitOverride.java",
97+
"InterimModule.java",
9798
"LocalPathOverride.java",
9899
"Module.java",
100+
"ModuleBase.java",
99101
"ModuleExtensionEvalStarlarkThreadContext.java",
100102
"ModuleFileValue.java",
101103
"ModuleOverride.java",

‎src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleInspectorFunction.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
5555
return null;
5656
}
5757
ImmutableMap<String, ModuleOverride> overrides = root.getOverrides();
58-
ImmutableMap<ModuleKey, Module> unprunedDepGraph = resolutionValue.getUnprunedDepGraph();
58+
ImmutableMap<ModuleKey, InterimModule> unprunedDepGraph = resolutionValue.getUnprunedDepGraph();
5959
ImmutableMap<ModuleKey, Module> resolvedDepGraph = resolutionValue.getResolvedDepGraph();
6060

6161
ImmutableMap<ModuleKey, AugmentedModule> depGraph =
@@ -74,7 +74,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
7474
}
7575

7676
public static ImmutableMap<ModuleKey, AugmentedModule> computeAugmentedGraph(
77-
ImmutableMap<ModuleKey, Module> unprunedDepGraph,
77+
ImmutableMap<ModuleKey, InterimModule> unprunedDepGraph,
7878
ImmutableSet<ModuleKey> usedModules,
7979
ImmutableMap<String, ModuleOverride> overrides) {
8080
Map<ModuleKey, AugmentedModule.Builder> depGraphAugmentBuilder = new HashMap<>();
@@ -83,9 +83,9 @@ public static ImmutableMap<ModuleKey, AugmentedModule> computeAugmentedGraph(
8383
// to their children AugmentedModule as dependant. Also fill in their own AugmentedModule
8484
// with a map from their dependencies to the resolution reason that was applied to each.
8585
// The newly created graph will also contain ModuleAugments for non-loaded modules.
86-
for (Entry<ModuleKey, Module> e : unprunedDepGraph.entrySet()) {
86+
for (Entry<ModuleKey, InterimModule> e : unprunedDepGraph.entrySet()) {
8787
ModuleKey parentKey = e.getKey();
88-
Module parentModule = e.getValue();
88+
InterimModule parentModule = e.getValue();
8989

9090
AugmentedModule.Builder parentBuilder =
9191
depGraphAugmentBuilder
@@ -95,10 +95,10 @@ public static ImmutableMap<ModuleKey, AugmentedModule> computeAugmentedGraph(
9595
.setLoaded(true);
9696

9797
for (String childDep : parentModule.getDeps().keySet()) {
98-
ModuleKey originalKey = parentModule.getOriginalDeps().get(childDep);
99-
Module originalModule = unprunedDepGraph.get(originalKey);
100-
ModuleKey key = parentModule.getDeps().get(childDep);
101-
Module module = unprunedDepGraph.get(key);
98+
ModuleKey originalKey = parentModule.getOriginalDeps().get(childDep).toModuleKey();
99+
InterimModule originalModule = unprunedDepGraph.get(originalKey);
100+
ModuleKey key = parentModule.getDeps().get(childDep).toModuleKey();
101+
InterimModule module = unprunedDepGraph.get(key);
102102

103103
AugmentedModule.Builder originalChildBuilder =
104104
depGraphAugmentBuilder.computeIfAbsent(originalKey, AugmentedModule::builder);

‎src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java‎

Lines changed: 83 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,10 @@
2121
import com.google.common.collect.ImmutableList;
2222
import com.google.common.collect.ImmutableMap;
2323
import com.google.common.collect.ImmutableSet;
24+
import com.google.common.collect.Maps;
2425
import com.google.devtools.build.lib.analysis.BlazeVersionInfo;
2526
import com.google.devtools.build.lib.bazel.BazelVersion;
27+
import com.google.devtools.build.lib.bazel.bzlmod.InterimModule.DepSpec;
2628
import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.RootModuleFileValue;
2729
import com.google.devtools.build.lib.bazel.bzlmod.Version.ParseException;
2830
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.BazelCompatibilityMode;
@@ -78,18 +80,18 @@ public SkyValue compute(SkyKey skyKey, Environment env)
7880
if (root == null) {
7981
return null;
8082
}
81-
ImmutableMap<ModuleKey, Module> initialDepGraph = Discovery.run(env, root);
83+
ImmutableMap<ModuleKey, InterimModule> initialDepGraph = Discovery.run(env, root);
8284
if (initialDepGraph == null) {
8385
return null;
8486
}
8587

86-
BazelModuleResolutionValue selectionResultValue;
88+
Selection.Result selectionResult;
8789
try {
88-
selectionResultValue = Selection.run(initialDepGraph, root.getOverrides());
90+
selectionResult = Selection.run(initialDepGraph, root.getOverrides());
8991
} catch (ExternalDepsException e) {
9092
throw new BazelModuleResolutionFunctionException(e, Transience.PERSISTENT);
9193
}
92-
ImmutableMap<ModuleKey, Module> resolvedDepGraph = selectionResultValue.getResolvedDepGraph();
94+
ImmutableMap<ModuleKey, InterimModule> resolvedDepGraph = selectionResult.getResolvedDepGraph();
9395

9496
verifyRootModuleDirectDepsAreAccurate(
9597
initialDepGraph.get(ModuleKey.ROOT),
@@ -109,40 +111,15 @@ public SkyValue compute(SkyKey skyKey, Environment env)
109111
Objects.requireNonNull(ALLOWED_YANKED_VERSIONS.get(env))),
110112
env.getListener());
111113

112-
// Add repo spec to each module and remove registry
113-
try {
114-
ImmutableMap.Builder<ModuleKey, Module> mapBuilder = ImmutableMap.builder();
115-
for (Map.Entry<ModuleKey, Module> entry : resolvedDepGraph.entrySet()) {
116-
Module module = entry.getValue();
117-
// Only change modules with registry (not overridden)
118-
if (module.getRegistry() != null) {
119-
RepoSpec moduleRepoSpec =
120-
module
121-
.getRegistry()
122-
.getRepoSpec(module.getKey(), module.getCanonicalRepoName(), env.getListener());
123-
ModuleOverride override = root.getOverrides().get(entry.getKey().getName());
124-
moduleRepoSpec = maybeAppendAdditionalPatches(moduleRepoSpec, override);
125-
module = module.toBuilder().setRepoSpec(moduleRepoSpec).setRegistry(null).build();
126-
}
127-
mapBuilder.put(entry.getKey(), module);
128-
}
129-
resolvedDepGraph = mapBuilder.buildOrThrow();
130-
} catch (IOException e) {
131-
throw new BazelModuleResolutionFunctionException(
132-
ExternalDepsException.withMessage(
133-
Code.ERROR_ACCESSING_REGISTRY,
134-
"Unable to get module repo spec from registry: %s",
135-
e.getMessage()),
136-
Transience.PERSISTENT);
137-
}
114+
ImmutableMap<ModuleKey, Module> finalDepGraph =
115+
computeFinalDepGraph(resolvedDepGraph, root.getOverrides(), env.getListener());
138116

139-
return BazelModuleResolutionValue.create(
140-
resolvedDepGraph, selectionResultValue.getUnprunedDepGraph());
117+
return BazelModuleResolutionValue.create(finalDepGraph, selectionResult.getUnprunedDepGraph());
141118
}
142119

143120
private static void verifyRootModuleDirectDepsAreAccurate(
144-
Module discoveredRootModule,
145-
Module resolvedRootModule,
121+
InterimModule discoveredRootModule,
122+
InterimModule resolvedRootModule,
146123
CheckDirectDepsMode mode,
147124
EventHandler eventHandler)
148125
throws BazelModuleResolutionFunctionException {
@@ -151,14 +128,14 @@ private static void verifyRootModuleDirectDepsAreAccurate(
151128
}
152129

153130
boolean failure = false;
154-
for (Map.Entry<String, ModuleKey> dep : discoveredRootModule.getDeps().entrySet()) {
155-
ModuleKey resolved = resolvedRootModule.getDeps().get(dep.getKey());
156-
if (!dep.getValue().equals(resolved)) {
131+
for (Map.Entry<String, DepSpec> dep : discoveredRootModule.getDeps().entrySet()) {
132+
ModuleKey resolved = resolvedRootModule.getDeps().get(dep.getKey()).toModuleKey();
133+
if (!dep.getValue().toModuleKey().equals(resolved)) {
157134
String message =
158135
String.format(
159136
"For repository '%s', the root module requires module version %s, but got %s in the"
160137
+ " resolved dependency graph.",
161-
dep.getKey(), dep.getValue(), resolved);
138+
dep.getKey(), dep.getValue().toModuleKey(), resolved);
162139
if (mode == CheckDirectDepsMode.WARNING) {
163140
eventHandler.handle(Event.warn(message));
164141
} else {
@@ -177,7 +154,9 @@ private static void verifyRootModuleDirectDepsAreAccurate(
177154
}
178155

179156
public static void checkBazelCompatibility(
180-
ImmutableCollection<Module> modules, BazelCompatibilityMode mode, EventHandler eventHandler)
157+
ImmutableCollection<InterimModule> modules,
158+
BazelCompatibilityMode mode,
159+
EventHandler eventHandler)
181160
throws BazelModuleResolutionFunctionException {
182161
if (mode == BazelCompatibilityMode.OFF) {
183162
return;
@@ -189,7 +168,7 @@ public static void checkBazelCompatibility(
189168
}
190169

191170
BazelVersion curVersion = BazelVersion.parse(currentBazelVersion);
192-
for (Module module : modules) {
171+
for (InterimModule module : modules) {
193172
for (String compatVersion : module.getBazelCompatibility()) {
194173
if (!curVersion.satisfiesCompatibility(compatVersion)) {
195174
String message =
@@ -306,14 +285,14 @@ private boolean parseModuleKeysFromString(
306285
return false;
307286
}
308287

309-
private void verifyYankedVersions(
310-
ImmutableMap<ModuleKey, Module> depGraph,
288+
private static void verifyYankedVersions(
289+
ImmutableMap<ModuleKey, InterimModule> depGraph,
311290
Optional<ImmutableSet<ModuleKey>> allowedYankedVersions,
312291
ExtendedEventHandler eventHandler)
313292
throws BazelModuleResolutionFunctionException, InterruptedException {
314293
// Check whether all resolved modules are either not yanked or allowed. Modules with a
315294
// NonRegistryOverride are ignored as their metadata is not available whatsoever.
316-
for (Module m : depGraph.values()) {
295+
for (InterimModule m : depGraph.values()) {
317296
if (m.getKey().equals(ModuleKey.ROOT) || m.getRegistry() == null) {
318297
continue;
319298
}
@@ -349,7 +328,7 @@ private void verifyYankedVersions(
349328
}
350329
}
351330

352-
private RepoSpec maybeAppendAdditionalPatches(RepoSpec repoSpec, ModuleOverride override) {
331+
private static RepoSpec maybeAppendAdditionalPatches(RepoSpec repoSpec, ModuleOverride override) {
353332
if (!(override instanceof SingleVersionOverride)) {
354333
return repoSpec;
355334
}
@@ -369,6 +348,66 @@ private RepoSpec maybeAppendAdditionalPatches(RepoSpec repoSpec, ModuleOverride
369348
.build();
370349
}
371350

351+
@Nullable
352+
private static RepoSpec computeRepoSpec(
353+
InterimModule interimModule, ModuleOverride override, ExtendedEventHandler eventHandler)
354+
throws BazelModuleResolutionFunctionException, InterruptedException {
355+
if (interimModule.getRegistry() == null) {
356+
// This module has a non-registry override. We don't need to store the repo spec in this case.
357+
return null;
358+
}
359+
try {
360+
RepoSpec moduleRepoSpec =
361+
interimModule
362+
.getRegistry()
363+
.getRepoSpec(
364+
interimModule.getKey(), interimModule.getCanonicalRepoName(), eventHandler);
365+
return maybeAppendAdditionalPatches(moduleRepoSpec, override);
366+
} catch (IOException e) {
367+
throw new BazelModuleResolutionFunctionException(
368+
ExternalDepsException.withMessage(
369+
Code.ERROR_ACCESSING_REGISTRY,
370+
"Unable to get module repo spec from registry: %s",
371+
e.getMessage()),
372+
Transience.PERSISTENT);
373+
}
374+
}
375+
376+
/**
377+
* Builds a {@link Module} from an {@link InterimModule}, discarding unnecessary fields and adding
378+
* extra necessary ones (such as the repo spec).
379+
*/
380+
static Module moduleFromInterimModule(
381+
InterimModule interim, ModuleOverride override, ExtendedEventHandler eventHandler)
382+
throws BazelModuleResolutionFunctionException, InterruptedException {
383+
return Module.builder()
384+
.setName(interim.getName())
385+
.setVersion(interim.getVersion())
386+
.setKey(interim.getKey())
387+
.setRepoName(interim.getRepoName())
388+
.setExecutionPlatformsToRegister(interim.getExecutionPlatformsToRegister())
389+
.setToolchainsToRegister(interim.getToolchainsToRegister())
390+
.setDeps(ImmutableMap.copyOf(Maps.transformValues(interim.getDeps(), DepSpec::toModuleKey)))
391+
.setRepoSpec(computeRepoSpec(interim, override, eventHandler))
392+
.setExtensionUsages(interim.getExtensionUsages())
393+
.build();
394+
}
395+
396+
private static ImmutableMap<ModuleKey, Module> computeFinalDepGraph(
397+
ImmutableMap<ModuleKey, InterimModule> resolvedDepGraph,
398+
ImmutableMap<String, ModuleOverride> overrides,
399+
ExtendedEventHandler eventHandler)
400+
throws BazelModuleResolutionFunctionException, InterruptedException {
401+
ImmutableMap.Builder<ModuleKey, Module> finalDepGraph = ImmutableMap.builder();
402+
for (Map.Entry<ModuleKey, InterimModule> entry : resolvedDepGraph.entrySet()) {
403+
finalDepGraph.put(
404+
entry.getKey(),
405+
moduleFromInterimModule(
406+
entry.getValue(), overrides.get(entry.getKey().getName()), eventHandler));
407+
}
408+
return finalDepGraph.buildOrThrow();
409+
}
410+
372411
static class BazelModuleResolutionFunctionException extends SkyFunctionException {
373412
BazelModuleResolutionFunctionException(ExternalDepsException e, Transience transience) {
374413
super(e, transience);

‎src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionValue.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,11 @@ abstract class BazelModuleResolutionValue implements SkyValue {
4444
* overridden by {@code single_version_override} or {@link NonRegistryOverride}, only by {@code
4545
* multiple_version_override}.
4646
*/
47-
abstract ImmutableMap<ModuleKey, Module> getUnprunedDepGraph();
47+
abstract ImmutableMap<ModuleKey, InterimModule> getUnprunedDepGraph();
4848

4949
static BazelModuleResolutionValue create(
5050
ImmutableMap<ModuleKey, Module> resolvedDepGraph,
51-
ImmutableMap<ModuleKey, Module> unprunedDepGraph) {
51+
ImmutableMap<ModuleKey, InterimModule> unprunedDepGraph) {
5252
return new AutoValue_BazelModuleResolutionValue(resolvedDepGraph, unprunedDepGraph);
5353
}
5454
}

‎src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Discovery.java‎

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@
1616
package com.google.devtools.build.lib.bazel.bzlmod;
1717

1818
import com.google.common.collect.ImmutableMap;
19+
import com.google.devtools.build.lib.bazel.bzlmod.InterimModule.DepSpec;
1920
import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.RootModuleFileValue;
2021
import com.google.devtools.build.skyframe.SkyFunction.Environment;
21-
import com.google.devtools.build.skyframe.SkyFunctionException;
2222
import com.google.devtools.build.skyframe.SkyKey;
2323
import com.google.devtools.build.skyframe.SkyframeLookupResult;
2424
import java.util.ArrayDeque;
@@ -42,23 +42,24 @@ private Discovery() {}
4242
* dependency is missing and this function needs a restart).
4343
*/
4444
@Nullable
45-
public static ImmutableMap<ModuleKey, Module> run(Environment env, RootModuleFileValue root)
46-
throws SkyFunctionException, InterruptedException {
45+
public static ImmutableMap<ModuleKey, InterimModule> run(
46+
Environment env, RootModuleFileValue root) throws InterruptedException {
4747
String rootModuleName = root.getModule().getName();
4848
ImmutableMap<String, ModuleOverride> overrides = root.getOverrides();
49-
Map<ModuleKey, Module> depGraph = new HashMap<>();
50-
depGraph.put(ModuleKey.ROOT, rewriteDepKeys(root.getModule(), overrides, rootModuleName));
49+
Map<ModuleKey, InterimModule> depGraph = new HashMap<>();
50+
depGraph.put(ModuleKey.ROOT, rewriteDepSpecs(root.getModule(), overrides, rootModuleName));
5151
Queue<ModuleKey> unexpanded = new ArrayDeque<>();
5252
unexpanded.add(ModuleKey.ROOT);
5353
while (!unexpanded.isEmpty()) {
5454
Set<SkyKey> unexpandedSkyKeys = new HashSet<>();
5555
while (!unexpanded.isEmpty()) {
56-
Module module = depGraph.get(unexpanded.remove());
57-
for (ModuleKey depKey : module.getDeps().values()) {
58-
if (depGraph.containsKey(depKey)) {
56+
InterimModule module = depGraph.get(unexpanded.remove());
57+
for (DepSpec depSpec : module.getDeps().values()) {
58+
if (depGraph.containsKey(depSpec.toModuleKey())) {
5959
continue;
6060
}
61-
unexpandedSkyKeys.add(ModuleFileValue.key(depKey, overrides.get(depKey.getName())));
61+
unexpandedSkyKeys.add(
62+
ModuleFileValue.key(depSpec.toModuleKey(), overrides.get(depSpec.getName())));
6263
}
6364
}
6465
SkyframeLookupResult result = env.getValuesAndExceptions(unexpandedSkyKeys);
@@ -70,7 +71,7 @@ public static ImmutableMap<ModuleKey, Module> run(Environment env, RootModuleFil
7071
depGraph.put(depKey, null);
7172
} else {
7273
depGraph.put(
73-
depKey, rewriteDepKeys(moduleFileValue.getModule(), overrides, rootModuleName));
74+
depKey, rewriteDepSpecs(moduleFileValue.getModule(), overrides, rootModuleName));
7475
unexpanded.add(depKey);
7576
}
7677
}
@@ -81,16 +82,16 @@ public static ImmutableMap<ModuleKey, Module> run(Environment env, RootModuleFil
8182
return ImmutableMap.copyOf(depGraph);
8283
}
8384

84-
private static Module rewriteDepKeys(
85-
Module module, ImmutableMap<String, ModuleOverride> overrides, String rootModuleName) {
86-
return module.withDepKeysTransformed(
87-
depKey -> {
88-
if (rootModuleName.equals(depKey.getName())) {
89-
return ModuleKey.ROOT;
85+
private static InterimModule rewriteDepSpecs(
86+
InterimModule module, ImmutableMap<String, ModuleOverride> overrides, String rootModuleName) {
87+
return module.withDepSpecsTransformed(
88+
depSpec -> {
89+
if (rootModuleName.equals(depSpec.getName())) {
90+
return DepSpec.fromModuleKey(ModuleKey.ROOT);
9091
}
9192

92-
Version newVersion = depKey.getVersion();
93-
@Nullable ModuleOverride override = overrides.get(depKey.getName());
93+
Version newVersion = depSpec.getVersion();
94+
@Nullable ModuleOverride override = overrides.get(depSpec.getName());
9495
if (override instanceof NonRegistryOverride) {
9596
newVersion = Version.EMPTY;
9697
} else if (override instanceof SingleVersionOverride) {
@@ -100,7 +101,7 @@ private static Module rewriteDepKeys(
100101
}
101102
}
102103

103-
return ModuleKey.create(depKey.getName(), newVersion);
104+
return DepSpec.create(depSpec.getName(), newVersion, depSpec.getMaxCompatibilityLevel());
104105
});
105106
}
106107
}

0 commit comments

Comments
 (0)