From db7453a53cc25ce6e67a8dfe05872965a8407504 Mon Sep 17 00:00:00 2001 From: Luca Di Grazia Date: Sun, 4 Sep 2022 15:57:01 +0200 Subject: [PATCH] bzlmod: compatibility_level support (https://github.com/bazelbuild/bazel/issues/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 --- .../build/lib/bazel/bzlmod/Module.java | 11 +- .../lib/bazel/bzlmod/ModuleFileGlobals.java | 9 +- .../lib/bazel/bzlmod/SelectionFunction.java | 3 +- .../repository/ModuleFileGlobalsApi.java | 15 +- .../bazel/bzlmod/ModuleFileFunctionTest.java | 152 +++--------------- .../bazel/bzlmod/SelectionFunctionTest.java | 2 +- 6 files changed, 42 insertions(+), 150 deletions(-) diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Module.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Module.java index e5a7f744214..eb1f0d3019c 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Module.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Module.java @@ -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 @@ -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); } /** @@ -82,11 +79,9 @@ public Module withDepKeysTransformed(UnaryOperator 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); diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileGlobals.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileGlobals.java index 1bf9128af9c..336292049c8 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileGlobals.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileGlobals.java @@ -37,15 +37,18 @@ public class ModuleFileGlobals implements ModuleFileGlobalsApi repositoryHandlers = - ImmutableMap.of(LocalRepositoryRule.NAME, new LocalRepositoryFunction()); MemoizingEvaluator evaluator = new InMemoryMemoizingEvaluator( ImmutableMap.builder() @@ -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 @@ -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())); @@ -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 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 @@ -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 result = driver.evaluate(ImmutableList.of(skyKey), evaluationContext); if (result.hasError()) { @@ -267,50 +188,13 @@ 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 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 { @@ -318,14 +202,14 @@ public void testRegistryOverride() throws Exception { 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())); @@ -333,8 +217,8 @@ public void testRegistryOverride() throws Exception { // 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 result = driver.evaluate(ImmutableList.of(skyKey), evaluationContext); if (result.hasError()) { @@ -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()); } 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 cceab189146..510de3ae3d7 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 @@ -63,7 +63,7 @@ private void setUpDiscoveryResult(String rootModuleName, ImmutableMap