diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java index c9c38d62f0e10a..2c19fce133cf75 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java @@ -186,8 +186,10 @@ public SkyValue compute(SkyKey skyKey, Environment env) // Dev dependencies should always be ignored if the current module isn't the root module /* ignoreDevDeps= */ true, builtinModules, - // We don't want non-root modules to print anything. - /* printIsNoop= */ true, + // Disable printing for modules from registries. We don't want them to be able to spam + // the console during resolution, but module files potentially edited by the user as + // part of a non-registry override should permit printing to aid debugging. + /* printIsNoop= */ getModuleFileResult.registry != null, starlarkSemantics, env.getListener(), SymbolGenerator.create(skyKey)); diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunctionTest.java index 7fd91f6fd46e1d..218a36cb303bb8 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunctionTest.java @@ -24,15 +24,24 @@ import com.google.devtools.build.lib.actions.FileValue; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.analysis.BlazeVersionInfo; +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.RepositoryOptions.BazelCompatibilityMode; import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.CheckDirectDepsMode; import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode; +import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager; +import com.google.devtools.build.lib.bazel.repository.starlark.StarlarkRepositoryFunction; +import com.google.devtools.build.lib.bazel.repository.starlark.StarlarkRepositoryModule; import com.google.devtools.build.lib.clock.BlazeClock; import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; 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.ClientEnvironmentFunction; import com.google.devtools.build.lib.skyframe.ExternalFilesHelper; import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction; @@ -41,6 +50,7 @@ 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; @@ -57,12 +67,15 @@ import com.google.devtools.build.skyframe.SkyFunctionName; import java.io.IOException; import java.util.Map; +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; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +import org.mockito.Mockito; /** Tests for {@link BazelModuleResolutionFunction}. */ @RunWith(JUnit4.class) @@ -97,6 +110,19 @@ 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 + .clearWorkspaceFilePrefixForTesting() + .clearWorkspaceFileSuffixForTesting() + .addStarlarkBootstrap(new RepositoryBootstrap(new StarlarkRepositoryModule())); + + ConfiguredRuleClassProvider ruleClassProvider = builder.build(); + ImmutableMap repositoryHandlers = + ImmutableMap.of(LocalRepositoryRule.NAME, new LocalRepositoryFunction()); + DownloadManager downloadManager = Mockito.mock(DownloadManager.class); + StarlarkRepositoryFunction starlarkRepositoryFunction = + new StarlarkRepositoryFunction(downloadManager); evaluator = new InMemoryMemoizingEvaluator( @@ -128,6 +154,18 @@ public void setup() throws Exception { .put( SkyFunctions.CLIENT_ENVIRONMENT_VARIABLE, new ClientEnvironmentFunction(new AtomicReference<>(ImmutableMap.of()))) + .put( + SkyFunctions.REPOSITORY_DIRECTORY, + new RepositoryDelegatorFunction( + repositoryHandlers, + starlarkRepositoryFunction, + new AtomicBoolean(true), + ImmutableMap::of, + directories, + BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER)) + .put( + BzlmodRepoRuleValue.BZLMOD_REPO_RULE, + new BzlmodRepoRuleFunction(ruleClassProvider, directories)) .buildOrThrow(), differencer); @@ -142,6 +180,10 @@ public void setup() throws Exception { differencer, BazelCompatibilityMode.ERROR); BazelLockFileFunction.LOCKFILE_MODE.set(differencer, LockfileMode.UPDATE); YankedVersionsUtil.ALLOWED_YANKED_VERSIONS.set(differencer, ImmutableList.of()); + RepositoryDelegatorFunction.FORCE_FETCH.set( + differencer, RepositoryDelegatorFunction.FORCE_FETCH_DISABLED); + RepositoryDelegatorFunction.REPOSITORY_OVERRIDES.set(differencer, ImmutableMap.of()); + RepositoryDelegatorFunction.VENDOR_DIRECTORY.set(differencer, Optional.empty()); } @Test @@ -369,54 +411,13 @@ private void setupModulesForYankedVersion() throws Exception { } @Test - public void testYankedVersionSideEffects_equalCompatibilityLevel() throws Exception { - scratch.overwriteFile( - rootDirectory.getRelative("MODULE.bazel").getPathString(), - "module(name='mod', version='1.0')", - "bazel_dep(name = 'a', version = '1.0')", - "bazel_dep(name = 'b', version = '1.1')"); - - FakeRegistry registry = - registryFactory - .newFakeRegistry("/bar") - .addModule( - createModuleKey("a", "1.0"), - "module(name='a', version='1.0')", - "bazel_dep(name='b', version='1.0')") - .addModule(createModuleKey("c", "1.0"), "module(name='c', version='1.0')") - .addModule(createModuleKey("c", "1.1"), "module(name='c', version='1.1')") - .addModule( - createModuleKey("b", "1.0"), - "module(name='b', version='1.0', compatibility_level = 2)", - "bazel_dep(name='c', version='1.1')", - "print('hello from yanked version')") - .addModule( - createModuleKey("b", "1.1"), - "module(name='b', version='1.1', compatibility_level = 2)", - "bazel_dep(name='c', version='1.0')") - .addYankedVersion("b", ImmutableMap.of(Version.parse("1.0"), "1.0 is a bad version!")); - - ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl())); - EvaluationResult result = - evaluator.evaluate(ImmutableList.of(BazelModuleResolutionValue.KEY), evaluationContext); - - assertThat(result.hasError()).isFalse(); - assertThat(result.get(BazelModuleResolutionValue.KEY).getResolvedDepGraph().keySet()) - .containsExactly( - ModuleKey.ROOT, - createModuleKey("a", "1.0"), - createModuleKey("b", "1.1"), - createModuleKey("c", "1.0")); - assertDoesNotContainEvent("hello from yanked version"); - } - - @Test - public void testYankedVersionSideEffects_differentCompatibilityLevel() throws Exception { + public void overrideOnNonexistentModule() throws Exception { scratch.overwriteFile( rootDirectory.getRelative("MODULE.bazel").getPathString(), "module(name='mod', version='1.0')", "bazel_dep(name = 'a', version = '1.0')", - "bazel_dep(name = 'b', version = '1.1')"); + "bazel_dep(name = 'b', version = '1.1')", + "local_path_override(module_name='d', path='whatevs')"); FakeRegistry registry = registryFactory @@ -429,14 +430,12 @@ public void testYankedVersionSideEffects_differentCompatibilityLevel() throws Ex .addModule(createModuleKey("c", "1.1"), "module(name='c', version='1.1')") .addModule( createModuleKey("b", "1.0"), - "module(name='b', version='1.0', compatibility_level = 2)", - "bazel_dep(name='c', version='1.1')", - "print('hello from yanked version')") + "module(name='b', version='1.0')", + "bazel_dep(name='c', version='1.1')") .addModule( createModuleKey("b", "1.1"), - "module(name='b', version='1.1', compatibility_level = 3)", - "bazel_dep(name='c', version='1.0')") - .addYankedVersion("b", ImmutableMap.of(Version.parse("1.0"), "1.0 is a bad version!")); + "module(name='b', version='1.1')", + "bazel_dep(name='c', version='1.0')"); ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl())); EvaluationResult result = @@ -444,20 +443,24 @@ public void testYankedVersionSideEffects_differentCompatibilityLevel() throws Ex assertThat(result.hasError()).isTrue(); assertThat(result.getError().toString()) - .contains( - "a@1.0 depends on b@1.0 with compatibility level 2, but depends on b@1.1 with" - + " compatibility level 3 which is different"); - assertDoesNotContainEvent("hello from yanked version"); + .contains("the root module specifies overrides on nonexistent module(s): d"); } @Test - public void overrideOnNonexistentModule() throws Exception { + public void testPrintBehavior() throws Exception { scratch.overwriteFile( rootDirectory.getRelative("MODULE.bazel").getPathString(), "module(name='mod', version='1.0')", + "print('hello from root module')", "bazel_dep(name = 'a', version = '1.0')", "bazel_dep(name = 'b', version = '1.1')", - "local_path_override(module_name='d', path='whatevs')"); + "single_version_override(module_name = 'b', version = '1.1')", + "local_path_override(module_name='a', path='a')"); + scratch.file( + "a/MODULE.bazel", + "module(name='a', version='1.0')", + "print('hello from overridden a')", + "bazel_dep(name='b', version='1.0')"); FakeRegistry registry = registryFactory @@ -465,24 +468,29 @@ public void overrideOnNonexistentModule() throws Exception { .addModule( createModuleKey("a", "1.0"), "module(name='a', version='1.0')", + "print('hello from a@1.0')", "bazel_dep(name='b', version='1.0')") .addModule(createModuleKey("c", "1.0"), "module(name='c', version='1.0')") .addModule(createModuleKey("c", "1.1"), "module(name='c', version='1.1')") .addModule( createModuleKey("b", "1.0"), - "module(name='b', version='1.0')", - "bazel_dep(name='c', version='1.1')") + "module(name='b', version='1.0', compatibility_level = 2)", + "bazel_dep(name='c', version='1.1')", + "print('hello from b@1.0')") .addModule( createModuleKey("b", "1.1"), - "module(name='b', version='1.1')", - "bazel_dep(name='c', version='1.0')"); + "module(name='b', version='1.1', compatibility_level = 3)", + "bazel_dep(name='c', version='1.0')", + "print('hello from b@1.1')"); ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl())); EvaluationResult result = evaluator.evaluate(ImmutableList.of(BazelModuleResolutionValue.KEY), evaluationContext); - assertThat(result.hasError()).isTrue(); - assertThat(result.getError().toString()) - .contains("the root module specifies overrides on nonexistent module(s): d"); + assertContainsEvent("hello from root module"); + assertContainsEvent("hello from overridden a"); + assertDoesNotContainEvent("hello from a@1.0"); + assertDoesNotContainEvent("hello from b@1.0"); + assertDoesNotContainEvent("hello from b@1.1"); } }