Skip to content

Commit

Permalink
bzlmod: compatibility_level support
Browse files Browse the repository at this point in the history
    (bazelbuild/bazel#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
Luca Di Grazia committed Sep 4, 2022
1 parent 82d59bd commit db7453a
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 150 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public abstract class Module {
public abstract String getName();

/** The version of the module. Must be empty iff the module has a {@link NonRegistryOverride}. */
public abstract Version getVersion();
public abstract String getVersion();

/**
* The compatibility level of the module, which essentially signifies the "major version" of the
Expand All @@ -61,10 +61,7 @@ public abstract class Module {

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

/**
Expand All @@ -82,11 +79,9 @@ public Module withDepKeysTransformed(UnaryOperator<ModuleKey> transform) {
/** Builder type for {@link Module}. */
@AutoValue.Builder
public abstract static class Builder {
/** Optional; defaults to the empty string. */
public abstract Builder setName(String value);

/** Optional; defaults to {@link Version#EMPTY}. */
public abstract Builder setVersion(Version value);
public abstract Builder setVersion(String value);

/** Optional; defaults to {@code 0}. */
public abstract Builder setCompatibilityLevel(int 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 @@ -109,7 +109,8 @@ public SkyValue compute(SkyKey skyKey, Environment env)
throw new SelectionFunctionException(e);
}

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

/**
Expand Down
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 @@ -16,38 +16,25 @@
package com.google.devtools.build.lib.bazel.bzlmod;

import static com.google.common.truth.Truth.assertThat;
import static com.google.devtools.build.lib.bazel.bzlmod.BzlmodTestUtil.createModuleKey;
import static org.junit.Assert.fail;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.FileStateValue;
import com.google.devtools.build.lib.actions.FileValue;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.analysis.ServerDirectories;
import com.google.devtools.build.lib.analysis.util.AnalysisMock;
import com.google.devtools.build.lib.bazel.repository.starlark.StarlarkRepositoryModule;
import com.google.devtools.build.lib.packages.PackageFactory;
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
import com.google.devtools.build.lib.rules.repository.LocalRepositoryFunction;
import com.google.devtools.build.lib.rules.repository.LocalRepositoryRule;
import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction;
import com.google.devtools.build.lib.rules.repository.RepositoryFunction;
import com.google.devtools.build.lib.skyframe.BazelSkyframeExecutorConstants;
import com.google.devtools.build.lib.skyframe.BzlmodRepoRuleFunction;
import com.google.devtools.build.lib.skyframe.ExternalFilesHelper;
import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction;
import com.google.devtools.build.lib.skyframe.FileFunction;
import com.google.devtools.build.lib.skyframe.FileStateFunction;
import com.google.devtools.build.lib.skyframe.ManagedDirectoriesKnowledge;
import com.google.devtools.build.lib.skyframe.PrecomputedFunction;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.skyframe.SkyFunctions;
import com.google.devtools.build.lib.starlarkbuildapi.repository.RepositoryBootstrap;
import com.google.devtools.build.lib.testutil.FoundationTestCase;
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.lib.vfs.UnixGlob;
Expand All @@ -61,8 +48,6 @@
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionName;
import com.google.devtools.build.skyframe.SkyKey;
import java.util.Optional;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
import net.starlark.java.eval.StarlarkSemantics;
import org.junit.Before;
Expand Down Expand Up @@ -102,20 +87,7 @@ public void setup() throws Exception {
packageLocator,
ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_PATHS,
directories);
ConfiguredRuleClassProvider.Builder builder = new ConfiguredRuleClassProvider.Builder();
TestRuleClassProvider.addStandardRules(builder);
builder
.clearWorkspaceFileSuffixForTesting()
.addStarlarkBootstrap(new RepositoryBootstrap(new StarlarkRepositoryModule()));
ConfiguredRuleClassProvider ruleClassProvider = builder.build();

PackageFactory packageFactory =
AnalysisMock.get()
.getPackageFactoryBuilderForTesting(directories)
.build(ruleClassProvider, fileSystem);

ImmutableMap<String, RepositoryFunction> repositoryHandlers =
ImmutableMap.of(LocalRepositoryRule.NAME, new LocalRepositoryFunction());
MemoizingEvaluator evaluator =
new InMemoryMemoizingEvaluator(
ImmutableMap.<SkyFunctionName, SkyFunction>builder()
Expand All @@ -130,39 +102,11 @@ public void setup() throws Exception {
SkyFunctions.MODULE_FILE,
new ModuleFileFunction(registryFactory, rootDirectory))
.put(SkyFunctions.PRECOMPUTED, new PrecomputedFunction())
.put(
SkyFunctions.REPOSITORY_DIRECTORY,
new RepositoryDelegatorFunction(
repositoryHandlers,
null,
new AtomicBoolean(true),
ImmutableMap::of,
directories,
ManagedDirectoriesKnowledge.NO_MANAGED_DIRECTORIES,
BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER))
.put(
BzlmodRepoRuleValue.BZLMOD_REPO_RULE,
new BzlmodRepoRuleFunction(
packageFactory,
ruleClassProvider,
directories,
new BzlmodRepoRuleHelperImpl()))
.build(),
differencer);
driver = new SequentialBuildDriver(evaluator);

PrecomputedValue.STARLARK_SEMANTICS.set(differencer, StarlarkSemantics.DEFAULT);
RepositoryDelegatorFunction.REPOSITORY_OVERRIDES.set(differencer, ImmutableMap.of());
RepositoryDelegatorFunction.DEPENDENCY_FOR_UNCONDITIONAL_FETCHING.set(
differencer, RepositoryDelegatorFunction.DONT_FETCH_UNCONDITIONALLY);
PrecomputedValue.PATH_PACKAGE_LOCATOR.set(differencer, packageLocator.get());
RepositoryDelegatorFunction.RESOLVED_FILE_INSTEAD_OF_WORKSPACE.set(
differencer, Optional.empty());
PrecomputedValue.REPO_ENV.set(differencer, ImmutableMap.of());
RepositoryDelegatorFunction.OUTPUT_VERIFICATION_REPOSITORY_RULES.set(
differencer, ImmutableSet.of());
RepositoryDelegatorFunction.RESOLVED_FILE_FOR_VERIFICATION.set(differencer, Optional.empty());
RepositoryDelegatorFunction.ENABLE_BZLMOD.set(differencer, true);
}

@Test
Expand All @@ -173,8 +117,7 @@ public void testRootModule() throws Exception {
"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')",
"local_path_override(module_name='E',path='somewhere/else')",
"multiple_version_override(module_name='F',versions=['1.0','2.0'])");
"local_path_override(module_name='E',path='somewhere/else')");
FakeRegistry registry = registryFactory.newFakeRegistry();
ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl()));

Expand All @@ -188,37 +131,15 @@ public void testRootModule() throws Exception {
.isEqualTo(
Module.builder()
.setName("A")
.setVersion(Version.parse("0.1"))
.setVersion("0.1")
.setCompatibilityLevel(4)
.addDep("B", createModuleKey("B", "1.0"))
.addDep("see", createModuleKey("C", "2.0"))
.addDep("B", ModuleKey.create("B", "1.0"))
.addDep("see", ModuleKey.create("C", "2.0"))
.build());
assertThat(moduleFileValue.getOverrides())
.containsExactly(
"D", SingleVersionOverride.create(Version.parse("18"), "", ImmutableList.of(), 0),
"E", LocalPathOverride.create("somewhere/else"),
"F",
MultipleVersionOverride.create(
ImmutableList.of(Version.parse("1.0"), Version.parse("2.0")), ""));
}

@Test
public void testRootModule_noModuleFunctionIsOkay() throws Exception {
scratch.file(
rootDirectory.getRelative("MODULE.bazel").getPathString(),
"bazel_dep(name='B',version='1.0')");
FakeRegistry registry = registryFactory.newFakeRegistry();
ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl()));

EvaluationResult<ModuleFileValue> result =
driver.evaluate(ImmutableList.of(ModuleFileValue.keyForRootModule()), evaluationContext);
if (result.hasError()) {
fail(result.getError().toString());
}
ModuleFileValue moduleFileValue = result.get(ModuleFileValue.keyForRootModule());
assertThat(moduleFileValue.getModule())
.isEqualTo(Module.builder().addDep("B", createModuleKey("B", "1.0")).build());
assertThat(moduleFileValue.getOverrides()).isEmpty();
"D", SingleVersionOverride.create("18", "", ImmutableList.of(), 0),
"E", LocalPathOverride.create("somewhere/else"));
}

@Test
Expand All @@ -245,18 +166,18 @@ public void testRegistriesCascade() throws Exception {
registryFactory
.newFakeRegistry()
.addModule(
createModuleKey("B", "1.0"),
ModuleKey.create("B", "1.0"),
"module(name='B',version='1.0');bazel_dep(name='C',version='2.0')");
FakeRegistry registry3 =
registryFactory
.newFakeRegistry()
.addModule(
createModuleKey("B", "1.0"),
ModuleKey.create("B", "1.0"),
"module(name='B',version='1.0');bazel_dep(name='D',version='3.0')");
ModuleFileFunction.REGISTRIES.set(
differencer, ImmutableList.of(registry1.getUrl(), registry2.getUrl(), registry3.getUrl()));

SkyKey skyKey = ModuleFileValue.key(createModuleKey("B", "1.0"), null);
SkyKey skyKey = ModuleFileValue.key(ModuleKey.create("B", "1.0"), null);
EvaluationResult<ModuleFileValue> result =
driver.evaluate(ImmutableList.of(skyKey), evaluationContext);
if (result.hasError()) {
Expand All @@ -267,74 +188,37 @@ public void testRegistriesCascade() throws Exception {
.isEqualTo(
Module.builder()
.setName("B")
.setVersion(Version.parse("1.0"))
.addDep("C", createModuleKey("C", "2.0"))
.setVersion("1.0")
.addDep("C", ModuleKey.create("C", "2.0"))
.setRegistry(registry2)
.build());
}

@Test
public void testLocalPathOverride() throws Exception {
// There is an override for B to use the local path "code_for_b", so we shouldn't even be
// looking at the registry.
scratch.file(
rootDirectory.getRelative("MODULE.bazel").getPathString(),
"module(name='A',version='0.1')",
"local_path_override(module_name='B',path='code_for_b')");
scratch.file(
rootDirectory.getRelative("code_for_b/MODULE.bazel").getPathString(),
"module(name='B',version='1.0')",
"bazel_dep(name='C',version='2.0')");
scratch.file(rootDirectory.getRelative("code_for_b/WORKSPACE").getPathString());
FakeRegistry registry =
registryFactory
.newFakeRegistry()
.addModule(
createModuleKey("B", "1.0"),
"module(name='B',version='1.0');bazel_dep(name='C',version='3.0')");
ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl()));

// The version is empty here due to the override.
SkyKey skyKey =
ModuleFileValue.key(createModuleKey("B", ""), LocalPathOverride.create("code_for_b"));
EvaluationResult<ModuleFileValue> result =
driver.evaluate(ImmutableList.of(skyKey), evaluationContext);
if (result.hasError()) {
fail(result.getError().toString());
}
ModuleFileValue moduleFileValue = result.get(skyKey);
assertThat(moduleFileValue.getModule())
.isEqualTo(
Module.builder()
.setName("B")
.setVersion(Version.parse("1.0"))
.addDep("C", createModuleKey("C", "2.0"))
.build());
}
// TODO: test local path override

@Test
public void testRegistryOverride() throws Exception {
FakeRegistry registry1 =
registryFactory
.newFakeRegistry()
.addModule(
createModuleKey("B", "1.0"),
ModuleKey.create("B", "1.0"),
"module(name='B',version='1.0',compatibility_level=4)\n"
+ "bazel_dep(name='C',version='2.0')");
FakeRegistry registry2 =
registryFactory
.newFakeRegistry()
.addModule(
createModuleKey("B", "1.0"),
ModuleKey.create("B", "1.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).
SkyKey skyKey =
ModuleFileValue.key(
createModuleKey("B", "1.0"),
SingleVersionOverride.create(Version.EMPTY, registry2.getUrl(), ImmutableList.of(), 0));
ModuleKey.create("B", "1.0"),
SingleVersionOverride.create("", registry2.getUrl(), ImmutableList.of(), 0));
EvaluationResult<ModuleFileValue> result =
driver.evaluate(ImmutableList.of(skyKey), evaluationContext);
if (result.hasError()) {
Expand All @@ -345,9 +229,9 @@ public void testRegistryOverride() throws Exception {
.isEqualTo(
Module.builder()
.setName("B")
.setVersion(Version.parse("1.0"))
.setVersion("1.0")
.setCompatibilityLevel(6)
.addDep("C", createModuleKey("C", "3.0"))
.addDep("C", ModuleKey.create("C", "3.0"))
.setRegistry(registry2)
.build());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ private void setUpDiscoveryResult(String rootModuleName, ImmutableMap<ModuleKey,
new SkyFunction() {
@Override
public SkyValue compute(SkyKey skyKey, Environment env) {
return DiscoveryValue.create(rootModuleName, depGraph);
return DiscoveryValue.create(rootModuleName, depGraph, ImmutableMap.of());
}

@Override
Expand Down

0 comments on commit db7453a

Please sign in to comment.