Skip to content

Commit

Permalink
bzlmod: compatibility_level support
Browse files Browse the repository at this point in the history
(#13316)

The compatibility_level is essentially the "major version" in semver, except that it's not embedded in the version string, but exists as a separate field. Modules with different compatibility levels participate in version resolution as if they're modules with different names, but the final dependency graph cannot contain multiple modules with the same name but different compatibility levels.

PiperOrigin-RevId: 384185854
  • Loading branch information
Wyverald authored and copybara-github committed Jul 12, 2021
1 parent 811eca3 commit f1f5b5d
Show file tree
Hide file tree
Showing 6 changed files with 292 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ public abstract class Module {
/** The version of the module. Must be empty iff the module has a {@link NonRegistryOverride}. */
public abstract String getVersion();

/**
* The compatibility level of the module, which essentially signifies the "major version" of the
* module in terms of SemVer.
*/
public abstract int getCompatibilityLevel();

/**
* The direct dependencies of this module. The key type is the repo name of the dep, and the value
* type is the ModuleKey (name+version) of the dep.
Expand All @@ -55,7 +61,7 @@ public abstract class Module {

/** Returns a new, empty {@link Builder}. */
public static Builder builder() {
return new AutoValue_Module.Builder();
return new AutoValue_Module.Builder().setCompatibilityLevel(0);
}

/**
Expand All @@ -77,6 +83,9 @@ public abstract static class Builder {

public abstract Builder setVersion(String value);

/** Optional; defaults to {@code 0}. */
public abstract Builder setCompatibilityLevel(int value);

public abstract Builder setDeps(ImmutableMap<String, ModuleKey> value);

public abstract Builder setRegistry(Registry value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,18 @@ public class ModuleFileGlobals implements ModuleFileGlobalsApi<ModuleFileFunctio
public ModuleFileGlobals() {}

@Override
public void module(String name, String version, String compatibilityLevel) throws EvalException {
public void module(String name, String version, StarlarkInt compatibilityLevel)
throws EvalException {
if (moduleCalled) {
throw Starlark.errorf("the module() directive can only be called once");
}
moduleCalled = true;
// TODO(wyv): add validation logic for name (alphanumerical) and version (use ParsedVersion) &
// others in the future
module.setName(name).setVersion(version);
// TODO(wyv): compatibility level
module
.setName(name)
.setVersion(version)
.setCompatibilityLevel(compatibilityLevel.toInt("compatibility_level"));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,16 @@

package com.google.devtools.build.lib.bazel.bzlmod;

import com.google.auto.value.AutoValue;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
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 javax.annotation.Nullable;

/**
* Runs module selection. This step of module resolution reads the output of {@link
Expand All @@ -30,6 +33,19 @@
*/
public class SelectionFunction implements SkyFunction {

/** During selection, a version is selected for each distinct "selection group". */
@AutoValue
abstract static class SelectionGroup {
static SelectionGroup of(Module module) {
return new AutoValue_SelectionFunction_SelectionGroup(
module.getName(), module.getCompatibilityLevel());
}

abstract String getModuleName();

abstract int getCompatibilityLevel();
}

@Override
public SkyValue compute(SkyKey skyKey, Environment env)
throws SkyFunctionException, InterruptedException {
Expand All @@ -38,27 +54,32 @@ public SkyValue compute(SkyKey skyKey, Environment env)
return null;
}

// TODO(wyv): compatibility_level, multiple_version_override
// TODO(wyv): multiple_version_override

// First figure out the version to select for every module.
// First figure out the version to select for every selection group.
ImmutableMap<ModuleKey, Module> depGraph = discovery.getDepGraph();
Map<String, ParsedVersion> selectedVersionForEachModule = new HashMap<>();
for (ModuleKey key : depGraph.keySet()) {
Map<SelectionGroup, ParsedVersion> selectedVersions = new HashMap<>();
for (Map.Entry<ModuleKey, Module> entry : depGraph.entrySet()) {
ModuleKey key = entry.getKey();
Module module = entry.getValue();

ParsedVersion parsedVersion;
try {
ParsedVersion parsedVersion = ParsedVersion.parse(key.getVersion());
selectedVersionForEachModule.merge(key.getName(), parsedVersion, ParsedVersion::max);
parsedVersion = ParsedVersion.parse(key.getVersion());
} catch (ParsedVersion.ParseException e) {
throw new SelectionFunctionException(e);
}
selectedVersions.merge(SelectionGroup.of(module), parsedVersion, ParsedVersion::max);
}

// Now build a new dep graph where deps with unselected versions are removed.
ImmutableMap.Builder<ModuleKey, Module> newDepGraphBuilder = new ImmutableMap.Builder<>();
for (Map.Entry<ModuleKey, Module> entry : depGraph.entrySet()) {
ModuleKey moduleKey = entry.getKey();
Module module = entry.getValue();

// Remove any dep whose version isn't selected.
String selectedVersion = selectedVersionForEachModule.get(moduleKey.getName()).getOriginal();
String selectedVersion = selectedVersions.get(SelectionGroup.of(module)).getOriginal();
if (!moduleKey.getVersion().equals(selectedVersion)) {
continue;
}
Expand All @@ -70,31 +91,94 @@ public SkyValue compute(SkyKey skyKey, Environment env)
depKey ->
ModuleKey.create(
depKey.getName(),
selectedVersionForEachModule.get(depKey.getName()).getOriginal())));
selectedVersions
.get(SelectionGroup.of(depGraph.get(depKey)))
.getOriginal())));
}
ImmutableMap<ModuleKey, Module> newDepGraph = newDepGraphBuilder.build();

// Further remove unreferenced modules from the graph. We can find out which modules are
// referenced by collecting deps transitively from the root.
HashMap<ModuleKey, Module> finalDepGraph = new HashMap<>();
collectDeps(ModuleKey.create(discovery.getRootModuleName(), ""), newDepGraph, finalDepGraph);
// 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);
try {
walker.walk(ModuleKey.create(discovery.getRootModuleName(), ""), null);
} catch (SelectionException e) {
throw new SelectionFunctionException(e);
}

return SelectionValue.create(
discovery.getRootModuleName(),
ImmutableMap.copyOf(finalDepGraph),
discovery.getOverrides());
discovery.getRootModuleName(), walker.getNewDepGraph(), discovery.getOverrides());
}

private void collectDeps(
ModuleKey key,
ImmutableMap<ModuleKey, Module> oldDepGraph,
HashMap<ModuleKey, Module> newDepGraph) {
if (newDepGraph.containsKey(key)) {
return;
/**
* Walks the dependency graph from the root node, collecting any reachable nodes through deps into
* a new dep graph and checking that nothing conflicts.
*/
static class DepGraphWalker {
private final ImmutableMap<ModuleKey, Module> oldDepGraph;
private final HashMap<ModuleKey, Module> newDepGraph;
private final HashMap<String, ExistingModule> moduleByName;

DepGraphWalker(ImmutableMap<ModuleKey, Module> oldDepGraph) {
this.oldDepGraph = oldDepGraph;
this.newDepGraph = new HashMap<>();
this.moduleByName = new HashMap<>();
}
Module module = oldDepGraph.get(key);
newDepGraph.put(key, module);
for (ModuleKey depKey : module.getDeps().values()) {
collectDeps(depKey, oldDepGraph, newDepGraph);

ImmutableMap<ModuleKey, Module> getNewDepGraph() {
return ImmutableMap.copyOf(newDepGraph);
}

void walk(ModuleKey key, @Nullable ModuleKey from) throws SelectionException {
if (newDepGraph.containsKey(key)) {
return;
}
Module module = oldDepGraph.get(key);
newDepGraph.put(key, module);

ExistingModule existingModuleWithSameName =
moduleByName.put(
module.getName(), ExistingModule.create(key, module.getCompatibilityLevel(), from));
if (existingModuleWithSameName != null) {
// This has to mean that a module with the same name but a different compatibility level was
// also selected.
Preconditions.checkState(
from != null && existingModuleWithSameName.getDependent() != null,
"the root module cannot possibly exist more than once in the dep graph");
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()));
}

for (ModuleKey depKey : module.getDeps().values()) {
walk(depKey, key);
}
}

@AutoValue
abstract static class ExistingModule {
abstract ModuleKey getModuleKey();

abstract int getCompatibilityLevel();

@Nullable
abstract ModuleKey getDependent();

static ExistingModule create(
ModuleKey moduleKey, int compatibilityLevel, ModuleKey dependent) {
return new AutoValue_SelectionFunction_DepGraphWalker_ExistingModule(
moduleKey, compatibilityLevel, dependent);
}
}
}

Expand All @@ -108,4 +192,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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ public interface ModuleFileGlobalsApi<ModuleFileFunctionExceptionT extends Excep
parameters = {
@Param(
name = "name",
// TODO(wyv): explain module name format
doc =
"The name of the module. Can be omitted only if this module is the root module (as"
+ " in, if it's not going to be depended on by another module).",
Expand All @@ -45,6 +46,7 @@ public interface ModuleFileGlobalsApi<ModuleFileFunctionExceptionT extends Excep
defaultValue = "''"),
@Param(
name = "version",
// TODO(wyv): explain version format
doc =
"The version of the module. Can be omitted only if this module is the root module"
+ " (as in, if it's not going to be depended on by another module).",
Expand All @@ -53,15 +55,22 @@ public interface ModuleFileGlobalsApi<ModuleFileFunctionExceptionT extends Excep
defaultValue = "''"),
@Param(
name = "compatibility_level",
// TODO(wyv): See X for more details; also mention multiple_version_override
doc =
"The compatibility level of the module; this should be changed every time a major"
+ " incompatible change is introduced.", // TODO(wyv): See X for more details.
+ " incompatible change is introduced. This is essentially the \"major"
+ " version\" of the module in terms of SemVer, except that it's not embedded"
+ " in the version string itself, but exists as a separate field. Modules with"
+ " different compatibility levels participate in version resolution as if"
+ " they're modules with different names, but the final dependency graph cannot"
+ " contain multiple modules with the same name but different compatibility"
+ " levels.",
named = true,
positional = false,
defaultValue = "''"),
defaultValue = "0"),
// TODO(wyv): bazel_compatibility, module_rule_exports, toolchains & platforms
})
void module(String name, String version, String compatibilityLevel)
void module(String name, String version, StarlarkInt compatibilityLevel)
throws EvalException, InterruptedException;

@StarlarkMethod(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public void setup() throws Exception {
public void testRootModule() throws Exception {
scratch.file(
rootDirectory.getRelative("MODULE.bazel").getPathString(),
"module(name='A',version='0.1')",
"module(name='A',version='0.1',compatibility_level=4)",
"bazel_dep(name='B',version='1.0')",
"bazel_dep(name='C',version='2.0',repo_name='see')",
"single_version_override(module_name='D',version='18')",
Expand All @@ -132,6 +132,7 @@ public void testRootModule() throws Exception {
Module.builder()
.setName("A")
.setVersion("0.1")
.setCompatibilityLevel(4)
.addDep("B", ModuleKey.create("B", "1.0"))
.addDep("see", ModuleKey.create("C", "2.0"))
.build());
Expand Down Expand Up @@ -202,13 +203,15 @@ public void testRegistryOverride() throws Exception {
.newFakeRegistry()
.addModule(
ModuleKey.create("B", "1.0"),
"module(name='B',version='1.0');bazel_dep(name='C',version='2.0')");
"module(name='B',version='1.0',compatibility_level=4)\n"
+ "bazel_dep(name='C',version='2.0')");
FakeRegistry registry2 =
registryFactory
.newFakeRegistry()
.addModule(
ModuleKey.create("B", "1.0"),
"module(name='B',version='1.0');bazel_dep(name='C',version='3.0')");
"module(name='B',version='1.0',compatibility_level=6)\n"
+ "bazel_dep(name='C',version='3.0')");
ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry1.getUrl()));

// Override the registry for B to be registry2 (instead of the default registry1).
Expand All @@ -227,6 +230,7 @@ public void testRegistryOverride() throws Exception {
Module.builder()
.setName("B")
.setVersion("1.0")
.setCompatibilityLevel(6)
.addDep("C", ModuleKey.create("C", "3.0"))
.setRegistry(registry2)
.build());
Expand Down
Loading

0 comments on commit f1f5b5d

Please sign in to comment.