From 5ac09b35a262e68fae86f748d81baf7c900f8224 Mon Sep 17 00:00:00 2001 From: Luca Di Grazia Date: Sun, 4 Sep 2022 15:53:58 +0200 Subject: [PATCH] bzlmod: Store canonical repo names in SelectionValue (https://github.com/bazelbuild/bazel/issues/13316) Up until now we assume the canonical repo name of a module is simply the module name. This doesn't work with multiple-version overrides. This CL addresses this issue; the canonical repo name will be ${moduleName}.${moduleVersion} for any module. This does mean that we need to put in some extra work to build reverse lookup maps, and especially add extra logic in RepositoryMappingFunction to make sure that WORKSPACE references to repos generated by modules can still work (i.e. if B is a module, something that points to "@B" gets rewritten to "@B.1.0", the canonical repo name) PiperOrigin-RevId: 385785118 --- .../bzlmod/BzlmodRepoRuleHelperImpl.java | 42 ++-- .../lib/bazel/bzlmod/DiscoveryFunction.java | 45 ++-- .../lib/bazel/bzlmod/ModuleFileFunction.java | 24 +- .../lib/bazel/bzlmod/ModuleFileValue.java | 42 +++- .../build/lib/bazel/bzlmod/ModuleKey.java | 5 + .../lib/bazel/bzlmod/SelectionFunction.java | 156 ++++++------- .../lib/bazel/bzlmod/SelectionValue.java | 2 +- .../skyframe/RepositoryMappingFunction.java | 109 ++++++++- .../bzlmod/BzlmodRepoRuleHelperTest.java | 27 +-- .../bazel/bzlmod/ModuleFileFunctionTest.java | 18 +- .../bazel/bzlmod/SelectionFunctionTest.java | 219 +----------------- .../repository/RepositoryDelegatorTest.java | 9 +- .../RepositoryMappingFunctionTest.java | 2 +- 13 files changed, 308 insertions(+), 392 deletions(-) diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleHelperImpl.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleHelperImpl.java index 9065bf15c4c..a0f9defe5dc 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleHelperImpl.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleHelperImpl.java @@ -18,6 +18,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.RootModuleFileValue; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.skyframe.SkyFunction.Environment; import java.io.IOException; @@ -30,14 +31,15 @@ public final class BzlmodRepoRuleHelperImpl implements BzlmodRepoRuleHelper { public Optional getRepoSpec(Environment env, String repositoryName) throws InterruptedException, IOException { - ModuleFileValue root = (ModuleFileValue) env.getValue(ModuleFileValue.keyForRootModule()); + RootModuleFileValue root = + (RootModuleFileValue) env.getValue(ModuleFileValue.keyForRootModule()); if (env.valuesMissing()) { return null; } ImmutableMap overrides = root.getOverrides(); // Step 1: Look for repositories defined by non-registry overrides. - Optional repoSpec = checkRepoFromNonRegistryOverrides(overrides, repositoryName); + Optional repoSpec = checkRepoFromNonRegistryOverrides(root, repositoryName); if (repoSpec.isPresent()) { return repoSpec; } @@ -61,36 +63,30 @@ public Optional getRepoSpec(Environment env, String repositoryName) } private static Optional checkRepoFromNonRegistryOverrides( - ImmutableMap overrides, String repositoryName) { - if (overrides.containsKey(repositoryName)) { - ModuleOverride override = overrides.get(repositoryName); - if (override instanceof NonRegistryOverride) { - return Optional.of(((NonRegistryOverride) override).getRepoSpec(repositoryName)); - } + RootModuleFileValue root, String repositoryName) { + String moduleName = root.getNonRegistryOverrideCanonicalRepoNameLookup().get(repositoryName); + if (moduleName == null) { + return Optional.empty(); } - return Optional.empty(); + NonRegistryOverride override = (NonRegistryOverride) root.getOverrides().get(moduleName); + return Optional.of(override.getRepoSpec(repositoryName)); } private static Optional checkRepoFromBazelModules( SelectionValue selectionValue, ImmutableMap overrides, - ExtendedEventHandler eventlistener, + ExtendedEventHandler eventListener, String repositoryName) throws InterruptedException, IOException { - for (ModuleKey moduleKey : selectionValue.getDepGraph().keySet()) { - // TODO(pcloudy): Support multiple version override. - // Currently we assume there is only one version for each module, therefore the module name is - // the repository name, but that's not the case if multiple version of the same module are - // allowed. - if (moduleKey.getName().equals(repositoryName)) { - Module module = selectionValue.getDepGraph().get(moduleKey); - Registry registry = checkNotNull(module.getRegistry()); - RepoSpec repoSpec = registry.getRepoSpec(moduleKey, repositoryName, eventlistener); - repoSpec = maybeAppendAdditionalPatches(repoSpec, overrides.get(moduleKey.getName())); - return Optional.of(repoSpec); - } + ModuleKey moduleKey = selectionValue.getCanonicalRepoNameLookup().get(repositoryName); + if (moduleKey == null) { + return Optional.empty(); } - return Optional.empty(); + Module module = selectionValue.getDepGraph().get(moduleKey); + Registry registry = checkNotNull(module.getRegistry()); + RepoSpec repoSpec = registry.getRepoSpec(moduleKey, repositoryName, eventListener); + repoSpec = maybeAppendAdditionalPatches(repoSpec, overrides.get(moduleKey.getName())); + return Optional.of(repoSpec); } private static RepoSpec maybeAppendAdditionalPatches(RepoSpec repoSpec, ModuleOverride override) { diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/DiscoveryFunction.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/DiscoveryFunction.java index e855d3b6dbf..6bb81bf2bce 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/DiscoveryFunction.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/DiscoveryFunction.java @@ -16,14 +16,17 @@ package com.google.devtools.build.lib.bazel.bzlmod; import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.RootModuleFileValue; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionException; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; import java.util.ArrayDeque; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; import java.util.Queue; +import java.util.Set; import javax.annotation.Nullable; /** @@ -37,33 +40,40 @@ public class DiscoveryFunction implements SkyFunction { @Override public SkyValue compute(SkyKey skyKey, Environment env) throws SkyFunctionException, InterruptedException { - ModuleFileValue root = (ModuleFileValue) env.getValue(ModuleFileValue.keyForRootModule()); + RootModuleFileValue root = + (RootModuleFileValue) env.getValue(ModuleFileValue.keyForRootModule()); if (root == null) { return null; } - ModuleKey rootModuleKey = ModuleKey.create(root.getModule().getName(), ""); + ModuleKey rootModuleKey = ModuleKey.create(root.getModule().getName(), Version.EMPTY); ImmutableMap overrides = root.getOverrides(); Map depGraph = new HashMap<>(); depGraph.put( rootModuleKey, rewriteDepKeys(root.getModule(), overrides, rootModuleKey.getName())); Queue unexpanded = new ArrayDeque<>(); unexpanded.add(rootModuleKey); - // TODO(wyv): currently we expand the "unexpanded" keys one by one. We should try to expand them - // all at once, using `env.getValues`. while (!unexpanded.isEmpty()) { - Module module = depGraph.get(unexpanded.remove()); - for (ModuleKey depKey : module.getDeps().values()) { - if (depGraph.containsKey(depKey)) { - continue; + Set unexpandedSkyKeys = new HashSet<>(); + while (!unexpanded.isEmpty()) { + Module module = depGraph.get(unexpanded.remove()); + for (ModuleKey depKey : module.getDeps().values()) { + if (depGraph.containsKey(depKey)) { + continue; + } + unexpandedSkyKeys.add(ModuleFileValue.key(depKey, overrides.get(depKey.getName()))); } - ModuleFileValue dep = - (ModuleFileValue) - env.getValue(ModuleFileValue.key(depKey, overrides.get(depKey.getName()))); - if (dep == null) { + } + Map result = env.getValues(unexpandedSkyKeys); + for (Map.Entry entry : result.entrySet()) { + ModuleKey depKey = ((ModuleFileValue.Key) entry.getKey()).getModuleKey(); + ModuleFileValue moduleFileValue = (ModuleFileValue) entry.getValue(); + if (moduleFileValue == null) { // Don't return yet. Try to expand any other unexpanded nodes before returning. depGraph.put(depKey, null); } else { - depGraph.put(depKey, rewriteDepKeys(dep.getModule(), overrides, rootModuleKey.getName())); + depGraph.put( + depKey, + rewriteDepKeys(moduleFileValue.getModule(), overrides, rootModuleKey.getName())); unexpanded.add(depKey); } } @@ -71,21 +81,20 @@ public SkyValue compute(SkyKey skyKey, Environment env) if (env.valuesMissing()) { return null; } - return DiscoveryValue.create( - root.getModule().getName(), ImmutableMap.copyOf(depGraph), overrides); + return DiscoveryValue.create(root.getModule().getName(), ImmutableMap.copyOf(depGraph)); } private static Module rewriteDepKeys( Module module, ImmutableMap overrides, String rootModuleName) { return module.withDepKeysTransformed( depKey -> { - String newVersion = depKey.getVersion(); + Version newVersion = depKey.getVersion(); @Nullable ModuleOverride override = overrides.get(depKey.getName()); if (override instanceof NonRegistryOverride || rootModuleName.equals(depKey.getName())) { - newVersion = ""; + newVersion = Version.EMPTY; } else if (override instanceof SingleVersionOverride) { - String overrideVersion = ((SingleVersionOverride) override).getVersion(); + Version overrideVersion = ((SingleVersionOverride) override).getVersion(); if (!overrideVersion.isEmpty()) { newVersion = overrideVersion; } diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java index 2ac635e4036..51689b5fa37 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java @@ -15,9 +15,14 @@ package com.google.devtools.build.lib.bazel.bzlmod; +import static com.google.common.collect.ImmutableMap.toImmutableMap; + import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Maps; import com.google.devtools.build.lib.actions.FileValue; +import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.NonRootModuleFileValue; +import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.RootModuleFileValue; import com.google.devtools.build.lib.cmdline.LabelConstants; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.events.Event; @@ -106,7 +111,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) throw errorf("The MODULE.bazel file of %s declares overrides", moduleKey); } - return ModuleFileValue.create(module, ImmutableMap.of()); + return NonRootModuleFileValue.create(module); } private SkyValue computeForRootModule(StarlarkSemantics starlarkSemantics, Environment env) @@ -128,7 +133,16 @@ private SkyValue computeForRootModule(StarlarkSemantics starlarkSemantics, Envir if (rootOverride != null) { throw errorf("invalid override for the root module found: %s", rootOverride); } - return ModuleFileValue.create(module, overrides); + ImmutableMap nonRegistryOverrideCanonicalRepoNameLookup = + Maps.filterValues(overrides, override -> override instanceof NonRegistryOverride) + .keySet() + .stream() + .collect( + toImmutableMap( + name -> ModuleKey.create(name, Version.EMPTY).getCanonicalRepoName(), + name -> name)); + return RootModuleFileValue.create( + module, overrides, nonRegistryOverrideCanonicalRepoNameLookup); } private ModuleFileGlobals execModuleFile( @@ -169,14 +183,12 @@ private GetModuleFileResult getModuleFile( // If there is a non-registry override for this module, we need to fetch the corresponding repo // first and read the module file from there. if (override instanceof NonRegistryOverride) { - // The canonical repo name of a module with a non-registry override is always the name of the - // module. - String repoName = key.getName(); + String canonicalRepoName = key.getCanonicalRepoName(); RepositoryDirectoryValue repoDir = (RepositoryDirectoryValue) env.getValue( RepositoryDirectoryValue.key( - RepositoryName.createFromValidStrippedName(repoName))); + RepositoryName.createFromValidStrippedName(canonicalRepoName))); if (repoDir == null) { return null; } diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileValue.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileValue.java index 37a80add679..293bb6b21ce 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileValue.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileValue.java @@ -28,10 +28,9 @@ import javax.annotation.Nullable; /** The result of {@link ModuleFileFunction}. */ -@AutoValue public abstract class ModuleFileValue implements SkyValue { - public static final ModuleKey ROOT_MODULE_KEY = ModuleKey.create("", ""); + public static final ModuleKey ROOT_MODULE_KEY = ModuleKey.create("", Version.EMPTY); /** * The module resulting from the module file evaluation. Note, in particular, that the version of @@ -40,15 +39,40 @@ public abstract class ModuleFileValue implements SkyValue { */ public abstract Module getModule(); + /** The {@link ModuleFileValue} for non-root modules. */ + @AutoValue + public abstract static class NonRootModuleFileValue extends ModuleFileValue { + + public static NonRootModuleFileValue create(Module module) { + return new AutoValue_ModuleFileValue_NonRootModuleFileValue(module); + } + } + /** - * The overrides specified by the evaluated module file. The key is the module name and the value - * is the override itself. + * The {@link ModuleFileValue} for the root module, containing additional information about + * overrides. */ - public abstract ImmutableMap getOverrides(); + @AutoValue + public abstract static class RootModuleFileValue extends ModuleFileValue { + /** + * The overrides specified by the evaluated module file. The key is the module name and the + * value is the override itself. + */ + public abstract ImmutableMap getOverrides(); - public static ModuleFileValue create( - Module module, ImmutableMap overrides) { - return new AutoValue_ModuleFileValue(module, overrides); + /** + * A mapping from a canonical repo name to the name of the module. Only works for modules with + * non-registry overrides. + */ + public abstract ImmutableMap getNonRegistryOverrideCanonicalRepoNameLookup(); + + public static RootModuleFileValue create( + Module module, + ImmutableMap overrides, + ImmutableMap nonRegistryOverrideCanonicalRepoNameLookup) { + return new AutoValue_ModuleFileValue_RootModuleFileValue( + module, overrides, nonRegistryOverrideCanonicalRepoNameLookup); + } } public static Key key(ModuleKey moduleKey, @Nullable ModuleOverride override) { @@ -65,7 +89,6 @@ public static Key keyForRootModule() { } /** {@link SkyKey} for {@link ModuleFileValue} computation. */ - @AutoCodec.VisibleForSerialization @AutoCodec @AutoValue abstract static class Key implements SkyKey { @@ -76,7 +99,6 @@ abstract static class Key implements SkyKey { @Nullable abstract ModuleOverride getOverride(); - @AutoCodec.VisibleForSerialization @AutoCodec.Instantiator static Key create(ModuleKey moduleKey, @Nullable ModuleOverride override) { return interner.intern(new AutoValue_ModuleFileValue_Key(moduleKey, override)); diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleKey.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleKey.java index 25c0b797a82..1973d95fb07 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleKey.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleKey.java @@ -37,4 +37,9 @@ public final String toString() { + "@" + (getVersion().isEmpty() ? "_" : getVersion().toString()); } + + /** Returns the canonical name of the repo backing this module. */ + public String getCanonicalRepoName() { + return getName() + "." + getVersion(); + } } diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SelectionFunction.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SelectionFunction.java index f0077a4392e..9ed0b1405eb 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SelectionFunction.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SelectionFunction.java @@ -26,17 +26,12 @@ import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Maps; import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.RootModuleFileValue; -import com.google.devtools.build.lib.server.FailureDetails.ExternalDeps.Code; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionException; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; -import java.util.ArrayDeque; import java.util.HashMap; -import java.util.HashSet; import java.util.Map; -import java.util.Queue; -import java.util.Set; import javax.annotation.Nullable; /** @@ -108,7 +103,7 @@ static ModuleNameAndCompatibilityLevel create(String moduleName, int compatibili private static ImmutableMap> computeAllowedVersionSets( ImmutableMap overrides, ImmutableMap depGraph) - throws ExternalDepsException { + throws SelectionException { Map> allowedVersionSets = new HashMap<>(); for (Map.Entry overrideEntry : overrides.entrySet()) { @@ -121,12 +116,11 @@ static ModuleNameAndCompatibilityLevel create(String moduleName, int compatibili for (Version allowedVersion : allowedVersions) { Module allowedVersionModule = depGraph.get(ModuleKey.create(moduleName, allowedVersion)); if (allowedVersionModule == null) { - throw ExternalDepsException.withMessage( - Code.VERSION_RESOLUTION_ERROR, - "multiple_version_override for module %s contains version %s, but it doesn't" - + " exist in the dependency graph", - moduleName, - allowedVersion); + throw new SelectionException( + String.format( + "multiple_version_override for module %s contains version %s, but it doesn't" + + " exist in the dependency graph", + moduleName, allowedVersion)); } ImmutableSortedSet.Builder allowedVersionSet = allowedVersionSets.computeIfAbsent( @@ -190,7 +184,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) ImmutableMap> allowedVersionSets; try { allowedVersionSets = computeAllowedVersionSets(overrides, depGraph); - } catch (ExternalDepsException e) { + } catch (SelectionException e) { throw new SelectionFunctionException(e); } @@ -238,23 +232,29 @@ public SkyValue compute(SkyKey skyKey, Environment env) // We can also take this opportunity to check that none of the remaining modules conflict with // each other (e.g. same module name but different compatibility levels, or not satisfying // multiple_version_override). - DepGraphWalker walker = - new DepGraphWalker(newDepGraph, discovery.getRootModuleName(), overrides, selectionGroups); + DepGraphWalker walker = new DepGraphWalker(newDepGraph, overrides, selectionGroups); try { - newDepGraph = walker.walk(); - } catch (ExternalDepsException e) { + walker.walk(ModuleKey.create(discovery.getRootModuleName(), Version.EMPTY), null); + } catch (SelectionException e) { throw new SelectionFunctionException(e); } + newDepGraph = walker.getNewDepGraph(); ImmutableMap canonicalRepoNameLookup = - newDepGraph.keySet().stream() + depGraph.keySet().stream() .collect(toImmutableMap(ModuleKey::getCanonicalRepoName, key -> key)); ImmutableMap moduleNameLookup = - newDepGraph.keySet().stream() - .filter(key -> !(overrides.get(key.getName()) instanceof MultipleVersionOverride)) + Maps.filterKeys( + newDepGraph, + key -> !(overrides.get(key.getName()) instanceof MultipleVersionOverride)) + .keySet() + .stream() .collect(toImmutableMap(ModuleKey::getName, key -> key)); return SelectionValue.create( - discovery.getRootModuleName(), newDepGraph, canonicalRepoNameLookup, moduleNameLookup); + discovery.getRootModuleName(), + walker.getNewDepGraph(), + canonicalRepoNameLookup, + moduleNameLookup); } /** @@ -264,51 +264,33 @@ public SkyValue compute(SkyKey skyKey, Environment env) static class DepGraphWalker { private static final Joiner JOINER = Joiner.on(", "); private final ImmutableMap oldDepGraph; - private final ModuleKey rootModuleKey; private final ImmutableMap overrides; private final ImmutableMap selectionGroups; + private final HashMap newDepGraph; private final HashMap moduleByName; DepGraphWalker( ImmutableMap oldDepGraph, - String rootModuleName, ImmutableMap overrides, ImmutableMap selectionGroups) { this.oldDepGraph = oldDepGraph; - this.rootModuleKey = ModuleKey.create(rootModuleName, Version.EMPTY); this.overrides = overrides; this.selectionGroups = selectionGroups; + this.newDepGraph = new HashMap<>(); this.moduleByName = new HashMap<>(); } - /** - * Walks the old dep graph and builds a new dep graph containing only deps reachable from the - * root module. The returned map has a guaranteed breadth-first iteration order. - */ - ImmutableMap walk() throws ExternalDepsException { - ImmutableMap.Builder newDepGraph = ImmutableMap.builder(); - Set known = new HashSet<>(); - Queue toVisit = new ArrayDeque<>(); - toVisit.add(ModuleKeyAndDependent.create(rootModuleKey, null)); - known.add(rootModuleKey); - while (!toVisit.isEmpty()) { - ModuleKeyAndDependent moduleKeyAndDependent = toVisit.remove(); - ModuleKey key = moduleKeyAndDependent.getModuleKey(); - Module module = oldDepGraph.get(key); - visit(key, module, moduleKeyAndDependent.getDependent()); - - for (ModuleKey depKey : module.getDeps().values()) { - if (known.add(depKey)) { - toVisit.add(ModuleKeyAndDependent.create(depKey, key)); - } - } - newDepGraph.put(key, module); - } - return newDepGraph.build(); + ImmutableMap getNewDepGraph() { + return ImmutableMap.copyOf(newDepGraph); } - void visit(ModuleKey key, Module module, @Nullable ModuleKey from) - throws ExternalDepsException { + void walk(ModuleKey key, @Nullable ModuleKey from) throws SelectionException { + if (newDepGraph.containsKey(key)) { + return; + } + Module module = oldDepGraph.get(key); + newDepGraph.put(key, module); + ModuleOverride override = overrides.get(key.getName()); if (override instanceof MultipleVersionOverride) { if (selectionGroups.get(key).getTargetAllowedVersion().isEmpty()) { @@ -316,14 +298,14 @@ void visit(ModuleKey key, Module module, @Nullable ModuleKey from) // higher than its version at the same compatibility level. Preconditions.checkState( from != null, "the root module cannot have a multiple version override"); - throw ExternalDepsException.withMessage( - Code.VERSION_RESOLUTION_ERROR, - "%s depends on %s which is not allowed by the multiple_version_override on %s," - + " which allows only [%s]", - from, - key, - key.getName(), - JOINER.join(((MultipleVersionOverride) override).getVersions())); + throw new SelectionException( + String.format( + "%s depends on %s which is not allowed by the multiple_version_override on %s," + + " which allows only [%s]", + from, + key, + key.getName(), + JOINER.join(((MultipleVersionOverride) override).getVersions()))); } } else { ExistingModule existingModuleWithSameName = @@ -335,16 +317,16 @@ void visit(ModuleKey key, Module module, @Nullable ModuleKey from) Preconditions.checkState( from != null && existingModuleWithSameName.getDependent() != null, "the root module cannot possibly exist more than once in the dep graph"); - throw ExternalDepsException.withMessage( - Code.VERSION_RESOLUTION_ERROR, - "%s depends on %s with compatibility level %d, but %s depends on %s with" - + " compatibility level %d which is different", - from, - key, - module.getCompatibilityLevel(), - existingModuleWithSameName.getDependent(), - existingModuleWithSameName.getModuleKey(), - existingModuleWithSameName.getCompatibilityLevel()); + throw new SelectionException( + String.format( + "%s depends on %s with compatibility level %d, but %s depends on %s with" + + " compatibility level %d which is different", + from, + key, + module.getCompatibilityLevel(), + existingModuleWithSameName.getDependent(), + existingModuleWithSameName.getModuleKey(), + existingModuleWithSameName.getCompatibilityLevel())); } } @@ -355,30 +337,18 @@ void visit(ModuleKey key, Module module, @Nullable ModuleKey from) ModuleKey depKey = depEntry.getValue(); String previousRepoName = depKeyToRepoName.put(depKey, repoName); if (previousRepoName != null) { - throw ExternalDepsException.withMessage( - Code.VERSION_RESOLUTION_ERROR, - "%s depends on %s at least twice (with repo names %s and %s). Consider adding a" - + " multiple_version_override if you want to depend on multiple versions of" - + " %s simultaneously", - key, - depKey, - repoName, - previousRepoName, - key.getName()); + throw new SelectionException( + String.format( + "%s depends on %s at least twice (with repo names %s and %s). Consider adding a" + + " multiple_version_override if you want to depend on multiple versions of" + + " %s simultaneously", + key, depKey, repoName, previousRepoName, key.getName())); } } - } - - @AutoValue - abstract static class ModuleKeyAndDependent { - abstract ModuleKey getModuleKey(); - - @Nullable - abstract ModuleKey getDependent(); - static ModuleKeyAndDependent create(ModuleKey moduleKey, @Nullable ModuleKey dependent) { - return new AutoValue_SelectionFunction_DepGraphWalker_ModuleKeyAndDependent( - moduleKey, dependent); + // Now visit our dependencies. + for (ModuleKey depKey : module.getDeps().values()) { + walk(depKey, key); } } @@ -409,4 +379,12 @@ static final class SelectionFunctionException extends SkyFunctionException { super(cause, Transience.PERSISTENT); } } + + // TODO(wyv): Replace this with a DetailedException (possibly named ModuleException or + // ExternalDepsException) and use it consistently across the space. + static final class SelectionException extends Exception { + SelectionException(String message) { + super(message); + } + } } diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SelectionValue.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SelectionValue.java index 91aafcbee49..2f9b6eb29c6 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SelectionValue.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SelectionValue.java @@ -39,7 +39,7 @@ public static SelectionValue create( public abstract String getRootModuleName(); - /** The post-selection dep graph. Must have BFS iteration order, starting from the root module. */ + /** The post-selection dep graph. */ public abstract ImmutableMap getDepGraph(); /** A mapping from a canonical repo name to the key of the module backing it. */ diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunction.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunction.java index 0dcf2351386..e7d91a917ae 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunction.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunction.java @@ -14,15 +14,25 @@ package com.google.devtools.build.lib.skyframe; +import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Maps; +import com.google.devtools.build.lib.bazel.bzlmod.Module; +import com.google.devtools.build.lib.bazel.bzlmod.ModuleKey; +import com.google.devtools.build.lib.bazel.bzlmod.SelectionValue; +import com.google.devtools.build.lib.bazel.bzlmod.Version; import com.google.devtools.build.lib.cmdline.LabelConstants; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException; import com.google.devtools.build.lib.packages.Package; +import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionException; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; import javax.annotation.Nullable; /** {@link SkyFunction} for {@link RepositoryMappingValue}s. */ @@ -32,19 +42,108 @@ public class RepositoryMappingFunction implements SkyFunction { @Override public SkyValue compute(SkyKey skyKey, Environment env) throws SkyFunctionException, InterruptedException { + RepositoryName repositoryName = (RepositoryName) skyKey.argument(); + + SelectionValue selectionValue = null; + if (Preconditions.checkNotNull(RepositoryDelegatorFunction.ENABLE_BZLMOD.get(env))) { + selectionValue = (SelectionValue) env.getValue(SelectionValue.KEY); + if (env.valuesMissing()) { + return null; + } + + Optional> mapping = + computeFromBzlmod(repositoryName, selectionValue); + if (mapping.isPresent()) { + return RepositoryMappingValue.withMapping(mapping.get()); + } + } + SkyKey externalPackageKey = PackageValue.key(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER); PackageValue externalPackageValue = (PackageValue) env.getValue(externalPackageKey); if (env.valuesMissing()) { return null; } + + return computeFromWorkspace(repositoryName, externalPackageValue, selectionValue); + } + + /** + * Calculate repo mappings for Bzlmod generated repository. + * + * @return the repo mappings for the repo if it's generated by Bzlmod, otherwise return + * Optional.empty(). + */ + private Optional> computeFromBzlmod( + RepositoryName repositoryName, SelectionValue selectionValue) { + ModuleKey moduleKey = + repositoryName.isMain() + ? ModuleKey.create(selectionValue.getRootModuleName(), Version.EMPTY) + : selectionValue.getCanonicalRepoNameLookup().get(repositoryName.strippedName()); + if (moduleKey == null) { + return Optional.empty(); + } + Module module = selectionValue.getDepGraph().get(moduleKey); + ImmutableMap.Builder repoMapping = ImmutableMap.builder(); + // In case of a repository refers to the main repository with @
, we should + // map it to the main repo identifier to avoid "fetching" the main repo. + repoMapping.put( + RepositoryName.createFromValidStrippedName(selectionValue.getRootModuleName()), + RepositoryName.MAIN); + // module.getDeps() contains a mapping of Bazel module dependencies from the required repo name + // to the module key. Go through them to construct the repo mappings. + for (Map.Entry dep : module.getDeps().entrySet()) { + String expectedRepoName = dep.getKey(); + String canonicalRepoName = dep.getValue().getCanonicalRepoName(); + if (expectedRepoName.equals(canonicalRepoName)) { + continue; + } + repoMapping.put( + RepositoryName.createFromValidStrippedName(expectedRepoName), + RepositoryName.createFromValidStrippedName(canonicalRepoName)); + } + return Optional.of(repoMapping.build()); + } + + private SkyValue computeFromWorkspace( + RepositoryName repositoryName, + PackageValue externalPackageValue, + @Nullable SelectionValue selectionValue) + throws RepositoryMappingFunctionException { Package externalPackage = externalPackageValue.getPackage(); if (externalPackage.containsErrors()) { throw new RepositoryMappingFunctionException(); } - - ImmutableMap mapping = - externalPackage.getRepositoryMapping((RepositoryName) skyKey.argument()); - return RepositoryMappingValue.withMapping(mapping); + if (selectionValue == null) { + return RepositoryMappingValue.withMapping( + externalPackage.getRepositoryMapping(repositoryName)); + } + // If bzlmod is in play, we need to transform mappings to "foo" into mappings for "foo.1.3" (if + // there is a module called "foo" in the dep graph and its version is 1.3, that is). + ImmutableMap moduleNameLookup = selectionValue.getModuleNameLookup(); + HashMap mapping = new HashMap<>(); + mapping.putAll( + Maps.transformValues( + externalPackage.getRepositoryMapping(repositoryName), + toRepo -> { + if (toRepo.isMain()) { + return toRepo; + } + ModuleKey moduleKey = moduleNameLookup.get(toRepo.strippedName()); + return moduleKey == null + ? toRepo + : RepositoryName.createFromValidStrippedName(moduleKey.getCanonicalRepoName()); + })); + // If there's no existing mapping to "foo", we should add a mapping from "foo" to "foo.1.3" + // anyways. + for (Map.Entry entry : moduleNameLookup.entrySet()) { + if (entry.getKey().equals(selectionValue.getRootModuleName())) { + continue; + } + mapping.putIfAbsent( + RepositoryName.createFromValidStrippedName(entry.getKey()), + RepositoryName.createFromValidStrippedName(entry.getValue().getCanonicalRepoName())); + } + return RepositoryMappingValue.withMapping(ImmutableMap.copyOf(mapping)); } @Nullable @@ -53,7 +152,7 @@ public String extractTag(SkyKey skyKey) { return null; } - private class RepositoryMappingFunctionException extends SkyFunctionException { + private static class RepositoryMappingFunctionException extends SkyFunctionException { RepositoryMappingFunctionException() { super( new BuildFileContainsErrorsException(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER), diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleHelperTest.java b/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleHelperTest.java index 5806546fe4b..09ec29ca06e 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleHelperTest.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleHelperTest.java @@ -134,7 +134,7 @@ public void getRepoSpec_bazelModule() throws Exception { "bazel_dep(name='B',version='1.0')"); FakeRegistry registry = registryFactory - .newFakeRegistry("/usr/local/modules") + .newFakeRegistry() .addModule( createModuleKey("B", "1.0"), "module(name='B', version='1.0');bazel_dep(name='C',version='2.0')") @@ -151,8 +151,8 @@ public void getRepoSpec_bazelModule() throws Exception { assertThat(repoSpec) .hasValue( RepoSpec.builder() - .setRuleClassName("local_repository") - .setAttributes(ImmutableMap.of("name", "C.2.0", "path", "/usr/local/modules/C.2.0")) + .setRuleClassName("fake_http_archive_rule") + .setAttributes(ImmutableMap.of("repo_name", "C.2.0")) .build()); } @@ -165,7 +165,7 @@ public void getRepoSpec_nonRegistryOverride() throws Exception { "local_path_override(module_name='C',path='/foo/bar/C')"); FakeRegistry registry = registryFactory - .newFakeRegistry("/usr/local/modules") + .newFakeRegistry() .addModule( createModuleKey("B", "1.0"), "module(name='B', version='1.0');bazel_dep(name='C',version='2.0')") @@ -200,7 +200,7 @@ public void getRepoSpec_singleVersionOverride() throws Exception { " module_name='C',version='3.0',patches=['//:foo.patch'],patch_strip=1)"); FakeRegistry registry = registryFactory - .newFakeRegistry("/usr/local/modules") + .newFakeRegistry() .addModule( createModuleKey("B", "1.0"), "module(name='B', version='1.0');bazel_dep(name='C',version='2.0')") @@ -218,16 +218,11 @@ public void getRepoSpec_singleVersionOverride() throws Exception { assertThat(repoSpec) .hasValue( RepoSpec.builder() - // This obviously wouldn't work in the real world since local_repository doesn't - // support patches -- but in the real world, registries also don't use - // local_repository. - .setRuleClassName("local_repository") + .setRuleClassName("fake_http_archive_rule") .setAttributes( ImmutableMap.of( - "name", + "repo_name", "C.3.0", - "path", - "/usr/local/modules/C.3.0", "patches", ImmutableList.of("//:foo.patch"), "patch_args", @@ -245,7 +240,7 @@ public void getRepoSpec_multipleVersionOverride() throws Exception { "multiple_version_override(module_name='D',versions=['1.0','2.0'])"); FakeRegistry registry = registryFactory - .newFakeRegistry("/usr/local/modules") + .newFakeRegistry() .addModule( createModuleKey("B", "1.0"), "module(name='B', version='1.0');bazel_dep(name='D',version='1.0')") @@ -266,8 +261,8 @@ public void getRepoSpec_multipleVersionOverride() throws Exception { assertThat(repoSpec) .hasValue( RepoSpec.builder() - .setRuleClassName("local_repository") - .setAttributes(ImmutableMap.of("name", "D.2.0", "path", "/usr/local/modules/D.2.0")) + .setRuleClassName("fake_http_archive_rule") + .setAttributes(ImmutableMap.of("repo_name", "D.2.0")) .build()); } @@ -279,7 +274,7 @@ public void getRepoSpec_notFound() throws Exception { "bazel_dep(name='B',version='1.0')"); FakeRegistry registry = registryFactory - .newFakeRegistry("/usr/local/modules") + .newFakeRegistry() .addModule(createModuleKey("B", "1.0"), "module(name='B', version='1.0')"); ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl())); diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java b/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java index 87d1eed8fbc..6d383a096f0 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java @@ -177,7 +177,7 @@ public void testRootModule() throws Exception { "local_path_override(module_name='E',path='somewhere/else')", "multiple_version_override(module_name='F',versions=['1.0','2.0'])", "archive_override(module_name='G',urls=['https://hello.com/world.zip'])"); - FakeRegistry registry = registryFactory.newFakeRegistry("/foo"); + FakeRegistry registry = registryFactory.newFakeRegistry(); ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl())); EvaluationResult result = @@ -218,7 +218,7 @@ public void testRootModule_noModuleFunctionIsOkay() throws Exception { scratch.file( rootDirectory.getRelative("MODULE.bazel").getPathString(), "bazel_dep(name='B',version='1.0')"); - FakeRegistry registry = registryFactory.newFakeRegistry("/foo"); + FakeRegistry registry = registryFactory.newFakeRegistry(); ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl())); EvaluationResult result = @@ -239,7 +239,7 @@ public void testRootModule_badSelfOverride() throws Exception { rootDirectory.getRelative("MODULE.bazel").getPathString(), "module(name='A')", "single_version_override(module_name='A',version='7')"); - FakeRegistry registry = registryFactory.newFakeRegistry("/foo"); + FakeRegistry registry = registryFactory.newFakeRegistry(); ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl())); EvaluationResult result = @@ -252,16 +252,16 @@ public void testRootModule_badSelfOverride() throws Exception { public void testRegistriesCascade() throws Exception { // Registry1 has no module B@1.0; registry2 and registry3 both have it. We should be using the // B@1.0 from registry2. - FakeRegistry registry1 = registryFactory.newFakeRegistry("/foo"); + FakeRegistry registry1 = registryFactory.newFakeRegistry(); FakeRegistry registry2 = registryFactory - .newFakeRegistry("/bar") + .newFakeRegistry() .addModule( createModuleKey("B", "1.0"), "module(name='B',version='1.0');bazel_dep(name='C',version='2.0')"); FakeRegistry registry3 = registryFactory - .newFakeRegistry("/baz") + .newFakeRegistry() .addModule( createModuleKey("B", "1.0"), "module(name='B',version='1.0');bazel_dep(name='D',version='3.0')"); @@ -300,7 +300,7 @@ public void testLocalPathOverride() throws Exception { scratch.file(rootDirectory.getRelative("code_for_b/WORKSPACE").getPathString()); FakeRegistry registry = registryFactory - .newFakeRegistry("/foo") + .newFakeRegistry() .addModule( createModuleKey("B", "1.0"), "module(name='B',version='1.0');bazel_dep(name='C',version='3.0')"); @@ -328,14 +328,14 @@ public void testLocalPathOverride() throws Exception { public void testRegistryOverride() throws Exception { FakeRegistry registry1 = registryFactory - .newFakeRegistry("/foo") + .newFakeRegistry() .addModule( createModuleKey("B", "1.0"), "module(name='B',version='1.0',compatibility_level=4)\n" + "bazel_dep(name='C',version='2.0')"); FakeRegistry registry2 = registryFactory - .newFakeRegistry("/foo") + .newFakeRegistry() .addModule( createModuleKey("B", "1.0"), "module(name='B',version='1.0',compatibility_level=6)\n" diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/SelectionFunctionTest.java b/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/SelectionFunctionTest.java index 72da54b526b..155f34eca7a 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/SelectionFunctionTest.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/SelectionFunctionTest.java @@ -182,28 +182,7 @@ public void testSimpleDiamond() throws Exception { .setName("D") .setVersion(Version.parse("2.0")) .setCompatibilityLevel(1) - .build()) - .inOrder(); - assertThat(selectionValue.getCanonicalRepoNameLookup()) - .containsExactly( - "A.", - createModuleKey("A", ""), - "B.1.0", - createModuleKey("B", "1.0"), - "C.2.0", - createModuleKey("C", "2.0"), - "D.2.0", - createModuleKey("D", "2.0")); - assertThat(selectionValue.getModuleNameLookup()) - .containsExactly( - "A", - createModuleKey("A", ""), - "B", - createModuleKey("B", "1.0"), - "C", - createModuleKey("C", "2.0"), - "D", - createModuleKey("D", "2.0")); + .build()); } @Test @@ -280,28 +259,7 @@ public void testDiamondWithFurtherRemoval() throws Exception { .addDep("D", createModuleKey("D", "2.0")) .build(), createModuleKey("D", "2.0"), - Module.builder().setName("D").setVersion(Version.parse("2.0")).build()) - .inOrder(); - assertThat(selectionValue.getCanonicalRepoNameLookup()) - .containsExactly( - "A.", - createModuleKey("A", ""), - "B.1.0", - createModuleKey("B", "1.0"), - "C.2.0", - createModuleKey("C", "2.0"), - "D.2.0", - createModuleKey("D", "2.0")); - assertThat(selectionValue.getModuleNameLookup()) - .containsExactly( - "A", - createModuleKey("A", ""), - "B", - createModuleKey("B", "1.0"), - "C", - createModuleKey("C", "2.0"), - "D", - createModuleKey("D", "2.0")); + Module.builder().setName("D").setVersion(Version.parse("2.0")).build()); } @Test @@ -369,25 +327,8 @@ public void testCircularDependencyDueToSelection() throws Exception { .setName("C") .setVersion(Version.parse("2.0")) .addDep("B", createModuleKey("B", "1.0")) - .build()) - .inOrder(); + .build()); // D is completely gone. - assertThat(selectionValue.getCanonicalRepoNameLookup()) - .containsExactly( - "A.", - createModuleKey("A", ""), - "B.1.0", - createModuleKey("B", "1.0"), - "C.2.0", - createModuleKey("C", "2.0")); - assertThat(selectionValue.getModuleNameLookup()) - .containsExactly( - "A", - createModuleKey("A", ""), - "B", - createModuleKey("B", "1.0"), - "C", - createModuleKey("C", "2.0")); } @Test @@ -552,32 +493,7 @@ public void differentCompatibilityLevelIsOkIfUnreferenced() throws Exception { .setName("E") .setVersion(Version.parse("1.0")) .addDep("C", createModuleKey("C", "1.1")) - .build()) - .inOrder(); - assertThat(selectionValue.getCanonicalRepoNameLookup()) - .containsExactly( - "A.", - createModuleKey("A", ""), - "B.1.1", - createModuleKey("B", "1.1"), - "C.1.1", - createModuleKey("C", "1.1"), - "D.1.0", - createModuleKey("D", "1.0"), - "E.1.0", - createModuleKey("E", "1.0")); - assertThat(selectionValue.getModuleNameLookup()) - .containsExactly( - "A", - createModuleKey("A", ""), - "B", - createModuleKey("B", "1.1"), - "C", - createModuleKey("C", "1.1"), - "D", - createModuleKey("D", "1.0"), - "E", - createModuleKey("E", "1.0")); + .build()); } @Test @@ -661,18 +577,7 @@ public void multipleVersionOverride_fork_goodCase() throws Exception { createModuleKey("B", "1.0"), Module.builder().setName("B").setVersion(Version.parse("1.0")).build(), createModuleKey("B", "2.0"), - Module.builder().setName("B").setVersion(Version.parse("2.0")).build()) - .inOrder(); - assertThat(selectionValue.getCanonicalRepoNameLookup()) - .containsExactly( - "A.", - createModuleKey("A", ""), - "B.1.0", - createModuleKey("B", "1.0"), - "B.2.0", - createModuleKey("B", "2.0")); - // No B in the module name lookup because there's a multiple-version override. - assertThat(selectionValue.getModuleNameLookup()).containsExactly("A", createModuleKey("A", "")); + Module.builder().setName("B").setVersion(Version.parse("2.0")).build()); } @Test @@ -799,28 +704,7 @@ public void multipleVersionOverride_diamond_differentCompatibilityLevels() throw .setName("D") .setVersion(Version.parse("2.0")) .setCompatibilityLevel(2) - .build()) - .inOrder(); - assertThat(selectionValue.getCanonicalRepoNameLookup()) - .containsExactly( - "A.", - createModuleKey("A", ""), - "B.1.0", - createModuleKey("B", "1.0"), - "C.2.0", - createModuleKey("C", "2.0"), - "D.1.0", - createModuleKey("D", "1.0"), - "D.2.0", - createModuleKey("D", "2.0")); - assertThat(selectionValue.getModuleNameLookup()) - .containsExactly( - "A", - createModuleKey("A", ""), - "B", - createModuleKey("B", "1.0"), - "C", - createModuleKey("C", "2.0")); + .build()); } @Test @@ -893,28 +777,7 @@ public void multipleVersionOverride_diamond_sameCompatibilityLevel() throws Exce createModuleKey("D", "1.0"), Module.builder().setName("D").setVersion(Version.parse("1.0")).build(), createModuleKey("D", "2.0"), - Module.builder().setName("D").setVersion(Version.parse("2.0")).build()) - .inOrder(); - assertThat(selectionValue.getCanonicalRepoNameLookup()) - .containsExactly( - "A.", - createModuleKey("A", ""), - "B.1.0", - createModuleKey("B", "1.0"), - "C.2.0", - createModuleKey("C", "2.0"), - "D.1.0", - createModuleKey("D", "1.0"), - "D.2.0", - createModuleKey("D", "2.0")); - assertThat(selectionValue.getModuleNameLookup()) - .containsExactly( - "A", - createModuleKey("A", ""), - "B", - createModuleKey("B", "1.0"), - "C", - createModuleKey("C", "2.0")); + Module.builder().setName("D").setVersion(Version.parse("2.0")).build()); } @Test @@ -1086,42 +949,7 @@ public void multipleVersionOverride_diamond_snappingToNextHighestVersion() throw .setName("C") .setVersion(Version.parse("2.0")) .setCompatibilityLevel(2) - .build()) - .inOrder(); - assertThat(selectionValue.getCanonicalRepoNameLookup()) - .containsExactly( - "A.", - createModuleKey("A", ""), - "B1.1.0", - createModuleKey("B1", "1.0"), - "B2.1.0", - createModuleKey("B2", "1.0"), - "B3.1.0", - createModuleKey("B3", "1.0"), - "B4.1.0", - createModuleKey("B4", "1.0"), - "B5.1.0", - createModuleKey("B5", "1.0"), - "C.1.3", - createModuleKey("C", "1.3"), - "C.1.7", - createModuleKey("C", "1.7"), - "C.2.0", - createModuleKey("C", "2.0")); - assertThat(selectionValue.getModuleNameLookup()) - .containsExactly( - "A", - createModuleKey("A", ""), - "B1", - createModuleKey("B1", "1.0"), - "B2", - createModuleKey("B2", "1.0"), - "B3", - createModuleKey("B3", "1.0"), - "B4", - createModuleKey("B4", "1.0"), - "B5", - createModuleKey("B5", "1.0")); + .build()); } @Test @@ -1419,35 +1247,6 @@ public void multipleVersionOverride_diamond_badVersionsAreOkayIfUnreferenced() t .setName("C") .setVersion(Version.parse("2.0")) .setCompatibilityLevel(2) - .build()) - .inOrder(); - assertThat(selectionValue.getCanonicalRepoNameLookup()) - .containsExactly( - "A.", - createModuleKey("A", ""), - "B1.1.0", - createModuleKey("B1", "1.0"), - "B2.1.1", - createModuleKey("B2", "1.1"), - "B3.1.0", - createModuleKey("B3", "1.0"), - "B4.1.1", - createModuleKey("B4", "1.1"), - "C.1.0", - createModuleKey("C", "1.0"), - "C.2.0", - createModuleKey("C", "2.0")); - assertThat(selectionValue.getModuleNameLookup()) - .containsExactly( - "A", - createModuleKey("A", ""), - "B1", - createModuleKey("B1", "1.0"), - "B2", - createModuleKey("B2", "1.1"), - "B3", - createModuleKey("B3", "1.0"), - "B4", - createModuleKey("B4", "1.1")); + .build()); } } diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java b/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java index eb05da19829..afbdad140a6 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java @@ -44,6 +44,7 @@ import com.google.devtools.build.lib.events.StoredEventHandler; import com.google.devtools.build.lib.packages.PackageFactory; import com.google.devtools.build.lib.packages.Rule; +import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException; import com.google.devtools.build.lib.packages.WorkspaceFileValue; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue.SuccessfulRepositoryDirectoryValue; @@ -447,7 +448,7 @@ public void loadRepositoryFromBzlmod() throws Exception { "bazel_dep(name='B',version='1.0')"); FakeRegistry registry = registryFactory - .newFakeRegistry(scratch.dir("modules").getPathString()) + .newFakeRegistry() .addModule(createModuleKey("B", "1.0"), "module(name='B', version='1.0');"); ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl())); scratch.file(rootPath.getRelative("BUILD").getPathString()); @@ -473,12 +474,12 @@ public void loadRepositoryFromBzlmod() throws Exception { EvaluationResult result = driver.evaluate(ImmutableList.of(key), evaluationContext); // B.1.0 should be fetched from MODULE.bazel file instead of WORKSPACE file. - // Because FakeRegistry will look for the contents of B.1.0 under $scratch/modules/B.1.0 which - // doesn't exist, the fetch should fail as expected. + // Because FakeRegistry returns a fake_http_archive_rule, the fetch should fail as expected. assertThat(result.hasError()).isTrue(); + assertThat(result.getError().getException()).isInstanceOf(InvalidRuleException.class); assertThat(result.getError().getException()) .hasMessageThat() - .contains("but it does not exist or is not a directory"); + .isEqualTo("Unrecognized native repository rule: fake_http_archive_rule"); // C should still be fetched from WORKSPACE successfully. loadRepo("C"); diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunctionTest.java b/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunctionTest.java index 9de81f69948..4645c06bab9 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunctionTest.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunctionTest.java @@ -64,7 +64,7 @@ protected boolean enableBzlmod() { @Before public void setUpForBzlmod() throws IOException, ParseException { scratch.file("MODULE.bazel", "module()"); - registry = FakeRegistry.DEFAULT_FACTORY.newFakeRegistry(scratch.dir("modules").getPathString()); + registry = FakeRegistry.DEFAULT_FACTORY.newFakeRegistry(); ModuleFileFunction.REGISTRIES.set( getSkyframeExecutor().getDifferencerForTesting(), ImmutableList.of(registry.getUrl())); }