diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java index d6fde03c52cd22..78ae340e0a14d2 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java @@ -342,10 +342,19 @@ public String getRecordedDiffMessages() { private static boolean didRepoMappingsChange( Environment env, ImmutableTable recordedRepoMappings) throws InterruptedException, NeedsSkyframeRestartException { + // Request repo mappings for any 'source repos' in the recorded mapping entries. + // Note specially that the main repo needs to be treated differently: if any .bzl file from the + // main repo was used for module extension eval, it _has_ to be before WORKSPACE is evaluated + // (see relevant code in BzlLoadFunction#getRepositoryMapping), so we only request the main repo + // mapping _without_ WORKSPACE repos. See #20942 for more information. SkyframeLookupResult result = env.getValuesAndExceptions( recordedRepoMappings.rowKeySet().stream() - .map(RepositoryMappingValue::key) + .map( + repoName -> + repoName.isMain() + ? RepositoryMappingValue.KEY_FOR_ROOT_MODULE_WITHOUT_WORKSPACE_REPOS + : RepositoryMappingValue.key(repoName)) .collect(toImmutableSet())); if (env.valuesMissing()) { // This likely means that one of the 'source repos' in the recorded mapping entries is no @@ -354,7 +363,11 @@ private static boolean didRepoMappingsChange( } for (Table.Cell cell : recordedRepoMappings.cellSet()) { RepositoryMappingValue repoMappingValue = - (RepositoryMappingValue) result.get(RepositoryMappingValue.key(cell.getRowKey())); + (RepositoryMappingValue) + result.get( + cell.getRowKey().isMain() + ? RepositoryMappingValue.KEY_FOR_ROOT_MODULE_WITHOUT_WORKSPACE_REPOS + : RepositoryMappingValue.key(cell.getRowKey())); if (repoMappingValue == null) { throw new NeedsSkyframeRestartException(); } diff --git a/src/test/py/bazel/bzlmod/bazel_lockfile_test.py b/src/test/py/bazel/bzlmod/bazel_lockfile_test.py index 396d6b79c6f6ea..c7df0c9f97f038 100644 --- a/src/test/py/bazel/bzlmod/bazel_lockfile_test.py +++ b/src/test/py/bazel/bzlmod/bazel_lockfile_test.py @@ -1856,6 +1856,50 @@ def testExtensionRepoMappingChange_sourceRepoNoLongerExistent(self): self.assertIn('ran the extension!', '\n'.join(stderr)) self.assertIn('STR=@@quux~1.0//:quux.h', '\n'.join(stderr)) + def testExtensionRepoMappingChange_mainRepoEvalCycleWithWorkspace(self): + # Regression test for #20942 + self.main_registry.createCcModule('foo', '1.0') + self.ScratchFile( + 'MODULE.bazel', + [ + 'bazel_dep(name="foo",version="1.0")', + 'ext = use_extension(":ext.bzl", "ext")', + 'use_repo(ext, "repo")', + ], + ) + self.ScratchFile( + 'BUILD.bazel', + [ + 'load("@repo//:defs.bzl", "STR")', + 'print("STR="+STR)', + 'filegroup(name="lol")', + ], + ) + self.ScratchFile( + 'ext.bzl', + [ + 'def _repo_impl(rctx):', + ' rctx.file("BUILD")', + ' rctx.file("defs.bzl", "STR = " + repr(str(rctx.attr.value)))', + 'repo = repository_rule(_repo_impl,attrs={"value":attr.label()})', + 'def _ext_impl(mctx):', + ' print("ran the extension!")', + ' repo(name = "repo", value = Label("@foo//:lib_foo"))', + 'ext = module_extension(_ext_impl)', + ], + ) + # any `load` in WORKSPACE should trigger the bug + self.ScratchFile('WORKSPACE.bzlmod', ['load("@repo//:defs.bzl","STR")']) + + _, _, stderr = self.RunBazel(['build', '--enable_workspace', ':lol']) + self.assertIn('STR=@@foo~1.0//:lib_foo', '\n'.join(stderr)) + + # Shutdown bazel to make sure we rely on the lockfile and not skyframe + self.RunBazel(['shutdown']) + # Build again. This should _NOT_ trigger a failure! + _, _, stderr = self.RunBazel(['build', '--enable_workspace', ':lol']) + self.assertNotIn('ran the extension!', '\n'.join(stderr)) + if __name__ == '__main__': absltest.main()