Skip to content

Commit 457fc9b

Browse files
bazel-ioWyverald
andauthored
[7.0.2] Disregard WORKSPACE while verifying lockfile repo mapping entries in extension eval (#21003)
See code comment and linked issue for more information. Fixes #20942. Closes #20982. Commit 21508b1 PiperOrigin-RevId: 600856392 Change-Id: I5b8a0ed3a38e37ab51ffb49b19a59f2e161b9a33 --------- Co-authored-by: Xdng Yng <[email protected]>
1 parent 2634a6e commit 457fc9b

File tree

2 files changed

+59
-2
lines changed

2 files changed

+59
-2
lines changed

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java

+15-2
Original file line numberDiff line numberDiff line change
@@ -342,10 +342,19 @@ public String getRecordedDiffMessages() {
342342
private static boolean didRepoMappingsChange(
343343
Environment env, ImmutableTable<RepositoryName, String, RepositoryName> recordedRepoMappings)
344344
throws InterruptedException, NeedsSkyframeRestartException {
345+
// Request repo mappings for any 'source repos' in the recorded mapping entries.
346+
// Note specially that the main repo needs to be treated differently: if any .bzl file from the
347+
// main repo was used for module extension eval, it _has_ to be before WORKSPACE is evaluated
348+
// (see relevant code in BzlLoadFunction#getRepositoryMapping), so we only request the main repo
349+
// mapping _without_ WORKSPACE repos. See #20942 for more information.
345350
SkyframeLookupResult result =
346351
env.getValuesAndExceptions(
347352
recordedRepoMappings.rowKeySet().stream()
348-
.map(RepositoryMappingValue::key)
353+
.map(
354+
repoName ->
355+
repoName.isMain()
356+
? RepositoryMappingValue.KEY_FOR_ROOT_MODULE_WITHOUT_WORKSPACE_REPOS
357+
: RepositoryMappingValue.key(repoName))
349358
.collect(toImmutableSet()));
350359
if (env.valuesMissing()) {
351360
// This likely means that one of the 'source repos' in the recorded mapping entries is no
@@ -354,7 +363,11 @@ private static boolean didRepoMappingsChange(
354363
}
355364
for (Table.Cell<RepositoryName, String, RepositoryName> cell : recordedRepoMappings.cellSet()) {
356365
RepositoryMappingValue repoMappingValue =
357-
(RepositoryMappingValue) result.get(RepositoryMappingValue.key(cell.getRowKey()));
366+
(RepositoryMappingValue)
367+
result.get(
368+
cell.getRowKey().isMain()
369+
? RepositoryMappingValue.KEY_FOR_ROOT_MODULE_WITHOUT_WORKSPACE_REPOS
370+
: RepositoryMappingValue.key(cell.getRowKey()));
358371
if (repoMappingValue == null) {
359372
throw new NeedsSkyframeRestartException();
360373
}

src/test/py/bazel/bzlmod/bazel_lockfile_test.py

+44
Original file line numberDiff line numberDiff line change
@@ -1863,6 +1863,50 @@ def testExtensionRepoMappingChange_sourceRepoNoLongerExistent(self):
18631863
self.assertIn('ran the extension!', '\n'.join(stderr))
18641864
self.assertIn('STR=@@quux~1.0//:quux.h', '\n'.join(stderr))
18651865

1866+
def testExtensionRepoMappingChange_mainRepoEvalCycleWithWorkspace(self):
1867+
# Regression test for #20942
1868+
self.main_registry.createCcModule('foo', '1.0')
1869+
self.ScratchFile(
1870+
'MODULE.bazel',
1871+
[
1872+
'bazel_dep(name="foo",version="1.0")',
1873+
'ext = use_extension(":ext.bzl", "ext")',
1874+
'use_repo(ext, "repo")',
1875+
],
1876+
)
1877+
self.ScratchFile(
1878+
'BUILD.bazel',
1879+
[
1880+
'load("@repo//:defs.bzl", "STR")',
1881+
'print("STR="+STR)',
1882+
'filegroup(name="lol")',
1883+
],
1884+
)
1885+
self.ScratchFile(
1886+
'ext.bzl',
1887+
[
1888+
'def _repo_impl(rctx):',
1889+
' rctx.file("BUILD")',
1890+
' rctx.file("defs.bzl", "STR = " + repr(str(rctx.attr.value)))',
1891+
'repo = repository_rule(_repo_impl,attrs={"value":attr.label()})',
1892+
'def _ext_impl(mctx):',
1893+
' print("ran the extension!")',
1894+
' repo(name = "repo", value = Label("@foo//:lib_foo"))',
1895+
'ext = module_extension(_ext_impl)',
1896+
],
1897+
)
1898+
# any `load` in WORKSPACE should trigger the bug
1899+
self.ScratchFile('WORKSPACE.bzlmod', ['load("@repo//:defs.bzl","STR")'])
1900+
1901+
_, _, stderr = self.RunBazel(['build', ':lol'])
1902+
self.assertIn('STR=@@foo~1.0//:lib_foo', '\n'.join(stderr))
1903+
1904+
# Shutdown bazel to make sure we rely on the lockfile and not skyframe
1905+
self.RunBazel(['shutdown'])
1906+
# Build again. This should _NOT_ trigger a failure!
1907+
_, _, stderr = self.RunBazel(['build', ':lol'])
1908+
self.assertNotIn('ran the extension!', '\n'.join(stderr))
1909+
18661910

18671911
if __name__ == '__main__':
18681912
absltest.main()

0 commit comments

Comments
 (0)