Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Introduce max_compatibility_level for bazel_dep #18178

Merged
merged 2 commits into from
Apr 21, 2023
Merged

Introduce max_compatibility_level for bazel_dep #18178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
74d7243
Add InterimModule to represent a Module before resolution finishes
Wyverald Apr 20, 2023
4c083ea
Selection with max_compatibility_level
Wyverald Apr 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,10 @@ java_library(
"BazelModuleResolutionValue.java",
"BzlmodFlagsAndEnvVars.java",
"GitOverride.java",
"InterimModule.java",
"LocalPathOverride.java",
"Module.java",
"ModuleBase.java",
"ModuleExtensionEvalStarlarkThreadContext.java",
"ModuleFileValue.java",
"ModuleOverride.java",
Expand Down
16 changes: 8 additions & 8 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleInspectorFunction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
return null;
}
ImmutableMap<String, ModuleOverride> overrides = root.getOverrides();
ImmutableMap<ModuleKey, Module> unprunedDepGraph = resolutionValue.getUnprunedDepGraph();
ImmutableMap<ModuleKey, InterimModule> unprunedDepGraph = resolutionValue.getUnprunedDepGraph();
ImmutableMap<ModuleKey, Module> resolvedDepGraph = resolutionValue.getResolvedDepGraph();

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

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

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

for (String childDep : parentModule.getDeps().keySet()) {
ModuleKey originalKey = parentModule.getOriginalDeps().get(childDep);
Module originalModule = unprunedDepGraph.get(originalKey);
ModuleKey key = parentModule.getDeps().get(childDep);
Module module = unprunedDepGraph.get(key);
ModuleKey originalKey = parentModule.getOriginalDeps().get(childDep).toModuleKey();
InterimModule originalModule = unprunedDepGraph.get(originalKey);
ModuleKey key = parentModule.getDeps().get(childDep).toModuleKey();
InterimModule module = unprunedDepGraph.get(key);

AugmentedModule.Builder originalChildBuilder =
depGraphAugmentBuilder.computeIfAbsent(originalKey, AugmentedModule::builder);
Expand Down
127 changes: 83 additions & 44 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
import com.google.devtools.build.lib.analysis.BlazeVersionInfo;
import com.google.devtools.build.lib.bazel.BazelVersion;
import com.google.devtools.build.lib.bazel.bzlmod.InterimModule.DepSpec;
import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.RootModuleFileValue;
import com.google.devtools.build.lib.bazel.bzlmod.Version.ParseException;
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.BazelCompatibilityMode;
Expand Down Expand Up @@ -78,18 +80,18 @@ public SkyValue compute(SkyKey skyKey, Environment env)
if (root == null) {
return null;
}
ImmutableMap<ModuleKey, Module> initialDepGraph = Discovery.run(env, root);
ImmutableMap<ModuleKey, InterimModule> initialDepGraph = Discovery.run(env, root);
if (initialDepGraph == null) {
return null;
}

BazelModuleResolutionValue selectionResultValue;
Selection.Result selectionResult;
try {
selectionResultValue = Selection.run(initialDepGraph, root.getOverrides());
selectionResult = Selection.run(initialDepGraph, root.getOverrides());
} catch (ExternalDepsException e) {
throw new BazelModuleResolutionFunctionException(e, Transience.PERSISTENT);
}
ImmutableMap<ModuleKey, Module> resolvedDepGraph = selectionResultValue.getResolvedDepGraph();
ImmutableMap<ModuleKey, InterimModule> resolvedDepGraph = selectionResult.getResolvedDepGraph();

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

// Add repo spec to each module and remove registry
try {
ImmutableMap.Builder<ModuleKey, Module> mapBuilder = ImmutableMap.builder();
for (Map.Entry<ModuleKey, Module> entry : resolvedDepGraph.entrySet()) {
Module module = entry.getValue();
// Only change modules with registry (not overridden)
if (module.getRegistry() != null) {
RepoSpec moduleRepoSpec =
module
.getRegistry()
.getRepoSpec(module.getKey(), module.getCanonicalRepoName(), env.getListener());
ModuleOverride override = root.getOverrides().get(entry.getKey().getName());
moduleRepoSpec = maybeAppendAdditionalPatches(moduleRepoSpec, override);
module = module.toBuilder().setRepoSpec(moduleRepoSpec).setRegistry(null).build();
}
mapBuilder.put(entry.getKey(), module);
}
resolvedDepGraph = mapBuilder.buildOrThrow();
} catch (IOException e) {
throw new BazelModuleResolutionFunctionException(
ExternalDepsException.withMessage(
Code.ERROR_ACCESSING_REGISTRY,
"Unable to get module repo spec from registry: %s",
e.getMessage()),
Transience.PERSISTENT);
}
ImmutableMap<ModuleKey, Module> finalDepGraph =
computeFinalDepGraph(resolvedDepGraph, root.getOverrides(), env.getListener());

return BazelModuleResolutionValue.create(
resolvedDepGraph, selectionResultValue.getUnprunedDepGraph());
return BazelModuleResolutionValue.create(finalDepGraph, selectionResult.getUnprunedDepGraph());
}

private static void verifyRootModuleDirectDepsAreAccurate(
Module discoveredRootModule,
Module resolvedRootModule,
InterimModule discoveredRootModule,
InterimModule resolvedRootModule,
CheckDirectDepsMode mode,
EventHandler eventHandler)
throws BazelModuleResolutionFunctionException {
Expand All @@ -151,14 +128,14 @@ private static void verifyRootModuleDirectDepsAreAccurate(
}

boolean failure = false;
for (Map.Entry<String, ModuleKey> dep : discoveredRootModule.getDeps().entrySet()) {
ModuleKey resolved = resolvedRootModule.getDeps().get(dep.getKey());
if (!dep.getValue().equals(resolved)) {
for (Map.Entry<String, DepSpec> dep : discoveredRootModule.getDeps().entrySet()) {
ModuleKey resolved = resolvedRootModule.getDeps().get(dep.getKey()).toModuleKey();
if (!dep.getValue().toModuleKey().equals(resolved)) {
String message =
String.format(
"For repository '%s', the root module requires module version %s, but got %s in the"
+ " resolved dependency graph.",
dep.getKey(), dep.getValue(), resolved);
dep.getKey(), dep.getValue().toModuleKey(), resolved);
if (mode == CheckDirectDepsMode.WARNING) {
eventHandler.handle(Event.warn(message));
} else {
Expand All @@ -177,7 +154,9 @@ private static void verifyRootModuleDirectDepsAreAccurate(
}

public static void checkBazelCompatibility(
ImmutableCollection<Module> modules, BazelCompatibilityMode mode, EventHandler eventHandler)
ImmutableCollection<InterimModule> modules,
BazelCompatibilityMode mode,
EventHandler eventHandler)
throws BazelModuleResolutionFunctionException {
if (mode == BazelCompatibilityMode.OFF) {
return;
Expand All @@ -189,7 +168,7 @@ public static void checkBazelCompatibility(
}

BazelVersion curVersion = BazelVersion.parse(currentBazelVersion);
for (Module module : modules) {
for (InterimModule module : modules) {
for (String compatVersion : module.getBazelCompatibility()) {
if (!curVersion.satisfiesCompatibility(compatVersion)) {
String message =
Expand Down Expand Up @@ -306,14 +285,14 @@ private boolean parseModuleKeysFromString(
return false;
}

private void verifyYankedVersions(
ImmutableMap<ModuleKey, Module> depGraph,
private static void verifyYankedVersions(
ImmutableMap<ModuleKey, InterimModule> depGraph,
Optional<ImmutableSet<ModuleKey>> allowedYankedVersions,
ExtendedEventHandler eventHandler)
throws BazelModuleResolutionFunctionException, InterruptedException {
// Check whether all resolved modules are either not yanked or allowed. Modules with a
// NonRegistryOverride are ignored as their metadata is not available whatsoever.
for (Module m : depGraph.values()) {
for (InterimModule m : depGraph.values()) {
if (m.getKey().equals(ModuleKey.ROOT) || m.getRegistry() == null) {
continue;
}
Expand Down Expand Up @@ -349,7 +328,7 @@ private void verifyYankedVersions(
}
}

private RepoSpec maybeAppendAdditionalPatches(RepoSpec repoSpec, ModuleOverride override) {
private static RepoSpec maybeAppendAdditionalPatches(RepoSpec repoSpec, ModuleOverride override) {
if (!(override instanceof SingleVersionOverride)) {
return repoSpec;
}
Expand All @@ -369,6 +348,66 @@ private RepoSpec maybeAppendAdditionalPatches(RepoSpec repoSpec, ModuleOverride
.build();
}

@Nullable
private static RepoSpec computeRepoSpec(
InterimModule interimModule, ModuleOverride override, ExtendedEventHandler eventHandler)
throws BazelModuleResolutionFunctionException, InterruptedException {
if (interimModule.getRegistry() == null) {
// This module has a non-registry override. We don't need to store the repo spec in this case.
return null;
}
try {
RepoSpec moduleRepoSpec =
interimModule
.getRegistry()
.getRepoSpec(
interimModule.getKey(), interimModule.getCanonicalRepoName(), eventHandler);
return maybeAppendAdditionalPatches(moduleRepoSpec, override);
} catch (IOException e) {
throw new BazelModuleResolutionFunctionException(
ExternalDepsException.withMessage(
Code.ERROR_ACCESSING_REGISTRY,
"Unable to get module repo spec from registry: %s",
e.getMessage()),
Transience.PERSISTENT);
}
}

/**
* Builds a {@link Module} from an {@link InterimModule}, discarding unnecessary fields and adding
* extra necessary ones (such as the repo spec).
*/
static Module moduleFromInterimModule(
InterimModule interim, ModuleOverride override, ExtendedEventHandler eventHandler)
throws BazelModuleResolutionFunctionException, InterruptedException {
return Module.builder()
.setName(interim.getName())
.setVersion(interim.getVersion())
.setKey(interim.getKey())
.setRepoName(interim.getRepoName())
.setExecutionPlatformsToRegister(interim.getExecutionPlatformsToRegister())
.setToolchainsToRegister(interim.getToolchainsToRegister())
.setDeps(ImmutableMap.copyOf(Maps.transformValues(interim.getDeps(), DepSpec::toModuleKey)))
.setRepoSpec(computeRepoSpec(interim, override, eventHandler))
.setExtensionUsages(interim.getExtensionUsages())
.build();
}

private static ImmutableMap<ModuleKey, Module> computeFinalDepGraph(
ImmutableMap<ModuleKey, InterimModule> resolvedDepGraph,
ImmutableMap<String, ModuleOverride> overrides,
ExtendedEventHandler eventHandler)
throws BazelModuleResolutionFunctionException, InterruptedException {
ImmutableMap.Builder<ModuleKey, Module> finalDepGraph = ImmutableMap.builder();
for (Map.Entry<ModuleKey, InterimModule> entry : resolvedDepGraph.entrySet()) {
finalDepGraph.put(
entry.getKey(),
moduleFromInterimModule(
entry.getValue(), overrides.get(entry.getKey().getName()), eventHandler));
}
return finalDepGraph.buildOrThrow();
}

static class BazelModuleResolutionFunctionException extends SkyFunctionException {
BazelModuleResolutionFunctionException(ExternalDepsException e, Transience transience) {
super(e, transience);
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionValue.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,11 @@ abstract class BazelModuleResolutionValue implements SkyValue {
* overridden by {@code single_version_override} or {@link NonRegistryOverride}, only by {@code
* multiple_version_override}.
*/
abstract ImmutableMap<ModuleKey, Module> getUnprunedDepGraph();
abstract ImmutableMap<ModuleKey, InterimModule> getUnprunedDepGraph();

static BazelModuleResolutionValue create(
ImmutableMap<ModuleKey, Module> resolvedDepGraph,
ImmutableMap<ModuleKey, Module> unprunedDepGraph) {
ImmutableMap<ModuleKey, InterimModule> unprunedDepGraph) {
return new AutoValue_BazelModuleResolutionValue(resolvedDepGraph, unprunedDepGraph);
}
}
39 changes: 20 additions & 19 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Discovery.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
package com.google.devtools.build.lib.bazel.bzlmod;

import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.bazel.bzlmod.InterimModule.DepSpec;
import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.RootModuleFileValue;
import com.google.devtools.build.skyframe.SkyFunction.Environment;
import com.google.devtools.build.skyframe.SkyFunctionException;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import java.util.ArrayDeque;
Expand All @@ -42,23 +42,24 @@ private Discovery() {}
* dependency is missing and this function needs a restart).
*/
@Nullable
public static ImmutableMap<ModuleKey, Module> run(Environment env, RootModuleFileValue root)
throws SkyFunctionException, InterruptedException {
public static ImmutableMap<ModuleKey, InterimModule> run(
Environment env, RootModuleFileValue root) throws InterruptedException {
String rootModuleName = root.getModule().getName();
ImmutableMap<String, ModuleOverride> overrides = root.getOverrides();
Map<ModuleKey, Module> depGraph = new HashMap<>();
depGraph.put(ModuleKey.ROOT, rewriteDepKeys(root.getModule(), overrides, rootModuleName));
Map<ModuleKey, InterimModule> depGraph = new HashMap<>();
depGraph.put(ModuleKey.ROOT, rewriteDepSpecs(root.getModule(), overrides, rootModuleName));
Queue<ModuleKey> unexpanded = new ArrayDeque<>();
unexpanded.add(ModuleKey.ROOT);
while (!unexpanded.isEmpty()) {
Set<SkyKey> unexpandedSkyKeys = new HashSet<>();
while (!unexpanded.isEmpty()) {
Module module = depGraph.get(unexpanded.remove());
for (ModuleKey depKey : module.getDeps().values()) {
if (depGraph.containsKey(depKey)) {
InterimModule module = depGraph.get(unexpanded.remove());
for (DepSpec depSpec : module.getDeps().values()) {
if (depGraph.containsKey(depSpec.toModuleKey())) {
continue;
}
unexpandedSkyKeys.add(ModuleFileValue.key(depKey, overrides.get(depKey.getName())));
unexpandedSkyKeys.add(
ModuleFileValue.key(depSpec.toModuleKey(), overrides.get(depSpec.getName())));
}
}
SkyframeLookupResult result = env.getValuesAndExceptions(unexpandedSkyKeys);
Expand All @@ -70,7 +71,7 @@ public static ImmutableMap<ModuleKey, Module> run(Environment env, RootModuleFil
depGraph.put(depKey, null);
} else {
depGraph.put(
depKey, rewriteDepKeys(moduleFileValue.getModule(), overrides, rootModuleName));
depKey, rewriteDepSpecs(moduleFileValue.getModule(), overrides, rootModuleName));
unexpanded.add(depKey);
}
}
Expand All @@ -81,16 +82,16 @@ public static ImmutableMap<ModuleKey, Module> run(Environment env, RootModuleFil
return ImmutableMap.copyOf(depGraph);
}

private static Module rewriteDepKeys(
Module module, ImmutableMap<String, ModuleOverride> overrides, String rootModuleName) {
return module.withDepKeysTransformed(
depKey -> {
if (rootModuleName.equals(depKey.getName())) {
return ModuleKey.ROOT;
private static InterimModule rewriteDepSpecs(
InterimModule module, ImmutableMap<String, ModuleOverride> overrides, String rootModuleName) {
return module.withDepSpecsTransformed(
depSpec -> {
if (rootModuleName.equals(depSpec.getName())) {
return DepSpec.fromModuleKey(ModuleKey.ROOT);
}

Version newVersion = depKey.getVersion();
@Nullable ModuleOverride override = overrides.get(depKey.getName());
Version newVersion = depSpec.getVersion();
@Nullable ModuleOverride override = overrides.get(depSpec.getName());
if (override instanceof NonRegistryOverride) {
newVersion = Version.EMPTY;
} else if (override instanceof SingleVersionOverride) {
Expand All @@ -100,7 +101,7 @@ private static Module rewriteDepKeys(
}
}

return ModuleKey.create(depKey.getName(), newVersion);
return DepSpec.create(depSpec.getName(), newVersion, depSpec.getMaxCompatibilityLevel());
});
}
}
Loading