diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java index 5dbd6702309363..f60177c23913da 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java @@ -208,10 +208,8 @@ public void serverInit(OptionsParsingResult startupOptions, ServerBuilder builde @Override public void workspaceInit( BlazeRuntime runtime, BlazeDirectories directories, WorkspaceBuilder builder) { - // TODO(b/27143724): Remove this guard when Google-internal flavor no longer uses repositories. - if ("bazel".equals(runtime.getProductName())) { - builder.allowExternalRepositories(true); - } + builder.allowExternalRepositories(true); + builder.setRepoContentsCachePathSupplier(repositoryCache.getRepoContentsCache()::getPath); repositoryFetchFunction = new RepositoryFetchFunction( diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryFetchFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryFetchFunction.java index 7e49e2b505cbe4..bfbab75b949330 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryFetchFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryFetchFunction.java @@ -21,6 +21,7 @@ import com.google.common.base.Throwables; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +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.bazel.bzlmod.NonRegistryOverride; @@ -297,16 +298,40 @@ public SkyValue compute(SkyKey skyKey, Environment env) e), Transience.TRANSIENT); } - // Don't forget to register a FileValue on the cache repo dir, so that we know to - // refetch - // if the cache entry gets GC'd from under us. + // Don't forget to register a FileStateValue on the cache repo dir, so that we know to + // refetch if the cache entry gets GC'd from under us or the entire cache is deleted. + // + // Note that registering a FileValue dependency instead would lead to subtly incorrect + // behavior when the repo contents cache directory is deleted between builds: + // 1. We register a FileValue dependency on the cache entry. + // 2. Before the next build, the repo contents cache directory is deleted. + // 3. On the next build, FileSystemValueChecker invalidates the underlying + // FileStateValue, which in turn results in the FileValue and the current + // RepositoryDirectoryValue being marked as dirty. + // 4. Skyframe visits the dirty nodes bottom up to check for actual changes. In + // particular, it reevaluates FileFunction before RepositoryFetchFunction and thus + // the FileValue of the repo contents cache directory is locked in as non-existent + // before RepositoryFetchFunction can recreate it. + // 5. Any other SkyFunction that depends on the FileValue of a file in the repo (e.g. + // PackageFunction) will report that file as missing since the resolved path has a + // parent that is non-existent. + // By using FileStateValue directly, which benefits from special logic built into + // DirtinessCheckerUtils that recognizes the repo contents cache directories with + // non-UUID names and prevents locking in their value during dirtiness checking, we + // avoid 4. and thus the incorrect missing file errors in 5. if (env.getValue( - FileValue.key( + FileStateValue.key( RootedPath.toRootedPath( Root.absoluteRoot(cachedRepoDir.getFileSystem()), cachedRepoDir))) == null) { return null; } + // This is never reached: the repo dir in the repo contents cache is created under a new + // UUID-named directory and thus the FileStateValue above will always be missing from + // Skyframe. After the restart, the repo will either encounter the just created cache + // entry as a candidate or will create a new one if it got GC'd in the meantime. + throw new IllegalStateException( + "FileStateValue unexpectedly present for " + cachedRepoDir); } if (remoteRepoContentsCache != null) { remoteRepoContentsCache.addToCache( diff --git a/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java b/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java index 297ba824b147d6..a5dab67d48dd03 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java @@ -31,6 +31,7 @@ import com.google.devtools.build.lib.skyframe.serialization.ObjectCodecRegistry; import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCachingServicesSupplier; import com.google.devtools.build.lib.util.AbruptExitException; +import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.SingleFileSystemSyscallCache; import com.google.devtools.build.lib.vfs.SyscallCache; import com.google.devtools.build.skyframe.SkyFunction; @@ -62,6 +63,7 @@ public final class WorkspaceBuilder { private SyscallCache syscallCache; private boolean allowExternalRepositories = true; + private Supplier repoContentsCachePathSupplier = () -> null; @Nullable private Supplier analysisCodecRegistrySupplier = null; @Nullable @@ -123,6 +125,7 @@ BlazeWorkspace build( skyFunctions.buildOrThrow(), singleFsSyscallCache, allowExternalRepositories, + repoContentsCachePathSupplier, skyKeyStateReceiver == null ? SkyframeExecutor.SkyKeyStateReceiver.NULL_INSTANCE : skyKeyStateReceiver, @@ -223,6 +226,13 @@ public WorkspaceBuilder allowExternalRepositories(boolean allowExternalRepositor return this; } + @CanIgnoreReturnValue + public WorkspaceBuilder setRepoContentsCachePathSupplier( + Supplier repoContentsCachePathSupplier) { + this.repoContentsCachePathSupplier = repoContentsCachePathSupplier; + return this; + } + @CanIgnoreReturnValue public WorkspaceBuilder setSkyKeyStateReceiver( SkyframeExecutor.SkyKeyStateReceiver skyKeyStateReceiver) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/DirtinessCheckerUtils.java b/src/main/java/com/google/devtools/build/lib/skyframe/DirtinessCheckerUtils.java index 129cdbe7550bb5..e4c2f995bed91e 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/DirtinessCheckerUtils.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/DirtinessCheckerUtils.java @@ -167,7 +167,7 @@ Map getDirtyExternalRepos() { private static boolean isCacheableType(FileType fileType) { return switch (fileType) { case INTERNAL, EXTERNAL_OTHER, BUNDLED -> true; - case EXTERNAL_REPO, OUTPUT -> false; + case EXTERNAL_REPO, OUTPUT, REPO_CONTENTS_CACHE_DIRS -> false; }; } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java b/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java index f238e1fb86b7b6..79e115b1a9267d 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java @@ -40,6 +40,7 @@ import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Supplier; import javax.annotation.Nullable; /** Common utilities for dealing with paths outside the package roots. */ @@ -50,6 +51,7 @@ public class ExternalFilesHelper { private final ExternalFileAction externalFileAction; private final BlazeDirectories directories; private final int maxNumExternalFilesToLog; + private final Supplier repoContentsCachePathSupplier; private final AtomicInteger numExternalFilesLogged = new AtomicInteger(0); private static final int MAX_EXTERNAL_FILES_TO_TRACK = 2500; @@ -66,31 +68,52 @@ private ExternalFilesHelper( AtomicReference pkgLocator, ExternalFileAction externalFileAction, BlazeDirectories directories, + Supplier repoContentsCachePathSupplier, int maxNumExternalFilesToLog) { this.pkgLocator = pkgLocator; this.externalFileAction = externalFileAction; this.directories = directories; + this.repoContentsCachePathSupplier = repoContentsCachePathSupplier; this.maxNumExternalFilesToLog = maxNumExternalFilesToLog; } public static ExternalFilesHelper create( AtomicReference pkgLocator, ExternalFileAction externalFileAction, - BlazeDirectories directories) { + BlazeDirectories directories, + Supplier repoContentsCachePathSupplier) { return TestType.isInTest() - ? createForTesting(pkgLocator, externalFileAction, directories) + ? createForTesting( + pkgLocator, externalFileAction, directories, repoContentsCachePathSupplier) : new ExternalFilesHelper( - pkgLocator, externalFileAction, directories, /* maxNumExternalFilesToLog= */ 100); + pkgLocator, + externalFileAction, + directories, + repoContentsCachePathSupplier, + /* maxNumExternalFilesToLog= */ 100); } public static ExternalFilesHelper createForTesting( AtomicReference pkgLocator, ExternalFileAction externalFileAction, BlazeDirectories directories) { + return createForTesting( + pkgLocator, + externalFileAction, + directories, + /* repoContentsCachePathSupplier= */ () -> null); + } + + public static ExternalFilesHelper createForTesting( + AtomicReference pkgLocator, + ExternalFileAction externalFileAction, + BlazeDirectories directories, + Supplier repoContentsCachePathSupplier) { return new ExternalFilesHelper( pkgLocator, externalFileAction, directories, + repoContentsCachePathSupplier, // These log lines are mostly spam during unit and integration tests. /* maxNumExternalFilesToLog= */ 0); } @@ -155,11 +178,26 @@ public enum FileType { */ EXTERNAL_REPO, + /** + * The directory containing the repo contents cache entries as well as direct children + * corresponding to individual predeclared input hashes. These directories are created by Bazel + * but may be deleted when users delete the entire repo contents cache. + * + *

These files' handling differs from EXTERNAL_REPO as they are never modified after they are + * created and don't live under the external directory, as well as from EXTERNAL_OTHER as they + * can be recreated by Bazel after diff detection. + * + *

The contents of these directories are considered EXTERNAL_OTHER as they carry UUID names + * and are thus never reused. + */ + REPO_CONTENTS_CACHE_DIRS, + /** * None of the above. We encounter these paths when outputs, source files or external repos * symlink to files outside aforementioned Bazel-managed directories. For example, C compilation * by the host compiler may depend on /usr/bin/gcc. Bazel makes a best-effort attempt to detect - * changes in such files. + * changes in such files. This type also includes files in repo contents cache entries, which + * are created by Bazel but never modified after creation. */ EXTERNAL_OTHER, } @@ -207,7 +245,11 @@ void setExternalFilesKnowledge(ExternalFilesKnowledge externalFilesKnowledge) { ExternalFilesHelper cloneWithFreshExternalFilesKnowledge() { return new ExternalFilesHelper( - pkgLocator, externalFileAction, directories, maxNumExternalFilesToLog); + pkgLocator, + externalFileAction, + directories, + repoContentsCachePathSupplier, + maxNumExternalFilesToLog); } public FileType getAndNoteFileType(RootedPath rootedPath) { @@ -240,6 +282,12 @@ private FileType detectFileType(RootedPath rootedPath) { if (packageLocator.getPathEntries().contains(rootedPath.getRoot())) { return FileType.INTERNAL; } + var repoContentsCachePath = repoContentsCachePathSupplier.get(); + if (repoContentsCachePath != null + && rootedPath.asPath().startsWith(repoContentsCachePath) + && !rootedPath.asPath().relativeTo(repoContentsCachePath).isMultiSegment()) { + return FileType.REPO_CONTENTS_CACHE_DIRS; + } // The outputBase may be null if we're not actually running a build. Path outputBase = packageLocator.getOutputBase(); if (outputBase == null) { @@ -270,6 +318,7 @@ FileType maybeHandleExternalFile(RootedPath rootedPath, SkyFunction.Environment switch (fileType) { case BUNDLED: case INTERNAL: + case REPO_CONTENTS_CACHE_DIRS: break; case EXTERNAL_OTHER: if (numExternalFilesLogged.incrementAndGet() < maxNumExternalFilesToLog) { @@ -294,14 +343,12 @@ FileType maybeHandleExternalFile(RootedPath rootedPath, SkyFunction.Environment /** * For files that are under $OUTPUT_BASE/external, add a dependency on the corresponding repo so - * that if the repo definition changes, the File/DirectoryStateValue will be re-evaluated. + * that if the repo is refetched, the {File,DirectoryListing}StateValue's of files underneath will + * be re-evaluated. * - *

Note that: - We don't add a dependency on the parent directory at the package root boundary, - * so the only transitive dependencies from files inside the package roots to external files are - * through symlinks. So the upwards transitive closure of external files is small. - The only way - * other than external repositories for external source files to get into the skyframe graph in - * the first place is through symlinks outside the package roots, which we neither want to - * encourage nor optimize for since it is not common. So the set of external files is small. + *

Note that we don't add a dependency on the parent directory at the package root boundary, so + * the only transitive dependencies from files inside the package roots to external files are + * through symlinks. So the upwards transitive closure of external files is small. */ private void addExternalFilesDependencies(RootedPath rootedPath, Environment env) throws InterruptedException { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java index 236c857be13e4c..80e0a5c21a2270 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java @@ -79,6 +79,7 @@ import com.google.devtools.build.lib.vfs.FileStateKey; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.ModifiedFileSet; +import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.SyscallCache; import com.google.devtools.build.skyframe.DelegatingGraphInconsistencyReceiver; import com.google.devtools.build.skyframe.EmittedEventState; @@ -115,6 +116,7 @@ import java.util.concurrent.Future; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; +import java.util.function.Supplier; import javax.annotation.Nullable; /** @@ -168,6 +170,7 @@ protected SequencedSkyframeExecutor( CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy, ImmutableList buildFilesByPriority, boolean allowExternalRepositories, + Supplier repoContentsCachePathSupplier, ActionOnIOExceptionReadingBuildFile actionOnIOExceptionReadingBuildFile, ActionOnFilesystemErrorCodeLoadingBzlFile actionOnFilesystemErrorCodeLoadingBzlFile, boolean shouldUseRepoDotBazel, @@ -200,6 +203,7 @@ protected SequencedSkyframeExecutor( workspaceInfoFromDiffReceiver, new SequencedRecordingDifferencer(), allowExternalRepositories, + repoContentsCachePathSupplier, globUnderSingleDep, diffCheckNotificationOptions); } @@ -828,6 +832,7 @@ public static final class Builder { private WorkspaceInfoFromDiffReceiver workspaceInfoFromDiffReceiver = (ignored1, ignored2) -> {}; private boolean allowExternalRepositories = false; + private Supplier repoContentsCachePathSupplier = () -> null; private Consumer skyframeExecutorConsumerOnInit = skyframeExecutor -> {}; private SkyFunction ignoredSubdirectoriesFunction; private BugReporter bugReporter = BugReporter.defaultInstance(); @@ -866,6 +871,7 @@ public SequencedSkyframeExecutor build() { crossRepositoryLabelViolationStrategy, buildFilesByPriority, allowExternalRepositories, + repoContentsCachePathSupplier, actionOnIOExceptionReadingBuildFile, actionOnFilesystemErrorCodeLoadingBzlFile, shouldUseRepoDotBazel, @@ -946,6 +952,12 @@ public Builder allowExternalRepositories(boolean allowExternalRepositories) { return this; } + @CanIgnoreReturnValue + public Builder setRepoContentsCachePathSupplier(Supplier repoContentsCachePathSupplier) { + this.repoContentsCachePathSupplier = repoContentsCachePathSupplier; + return this; + } + @CanIgnoreReturnValue public Builder setCrossRepositoryLabelViolationStrategy( CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java index 958776668ffabe..a9bbebf1616c4c 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java @@ -20,9 +20,11 @@ import com.google.devtools.build.lib.bugreport.BugReporter; import com.google.devtools.build.lib.packages.PackageFactory; import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.SyscallCache; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionName; +import java.util.function.Supplier; /** A factory of SkyframeExecutors that returns SequencedSkyframeExecutor. */ public final class SequencedSkyframeExecutorFactory implements SkyframeExecutorFactory { @@ -38,6 +40,7 @@ public SkyframeExecutor create( ImmutableMap extraSkyFunctions, SyscallCache syscallCache, boolean allowExternalRepositories, + Supplier repoContentsCachePathSupplier, SkyframeExecutor.SkyKeyStateReceiver skyKeyStateReceiver, BugReporter bugReporter) { return BazelSkyframeExecutorConstants.newBazelSkyframeExecutorBuilder() @@ -50,6 +53,7 @@ public SkyframeExecutor create( .setExtraSkyFunctions(extraSkyFunctions) .setSyscallCache(syscallCache) .allowExternalRepositories(allowExternalRepositories) + .setRepoContentsCachePathSupplier(repoContentsCachePathSupplier) .setSkyKeyStateReceiver(skyKeyStateReceiver) .setBugReporter(bugReporter) .build(); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index ae3c1a239e1a6d..eed51785ec7235 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -688,6 +688,7 @@ protected SkyframeExecutor( @Nullable WorkspaceInfoFromDiffReceiver workspaceInfoFromDiffReceiver, @Nullable RecordingDifferencer recordingDiffer, boolean allowExternalRepositories, + Supplier repoContentsCachePathSupplier, boolean globUnderSingleDep, Optional diffCheckNotificationOptions) { // Strictly speaking, these arguments are not required for initialization, but all current @@ -733,7 +734,8 @@ protected SkyframeExecutor( this.skyframeBuildView = new SkyframeBuildView(artifactFactory, this, ruleClassProvider, actionKeyContext); this.externalFilesHelper = - ExternalFilesHelper.create(pkgLocator, externalFileAction, directories); + ExternalFilesHelper.create( + pkgLocator, externalFileAction, directories, repoContentsCachePathSupplier); this.crossRepositoryLabelViolationStrategy = crossRepositoryLabelViolationStrategy; this.buildFilesByPriority = buildFilesByPriority; this.actionOnIOExceptionReadingBuildFile = actionOnIOExceptionReadingBuildFile; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutorFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutorFactory.java index a2756443f8a25f..7ec2b33ab9299d 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutorFactory.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutorFactory.java @@ -21,9 +21,11 @@ import com.google.devtools.build.lib.packages.PackageFactory; import com.google.devtools.build.lib.util.AbruptExitException; import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.SyscallCache; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionName; +import java.util.function.Supplier; /** A factory that creates instances of SkyframeExecutor. */ public interface SkyframeExecutorFactory { @@ -50,6 +52,7 @@ SkyframeExecutor create( ImmutableMap extraSkyFunctions, SyscallCache syscallCache, boolean allowExternalRepositories, + Supplier repoContentsCachePathSupplier, SkyframeExecutor.SkyKeyStateReceiver skyKeyStateReceiver, BugReporter bugReporter) throws AbruptExitException; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java index 7aeb7afa9411b9..746b0ee18f3d9e 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java @@ -288,7 +288,11 @@ protected void validate() { public final PackageLoader build() { validate(); externalFilesHelper = - ExternalFilesHelper.create(pkgLocatorRef, externalFileAction, directories); + ExternalFilesHelper.create( + pkgLocatorRef, + externalFileAction, + directories, + /* repoContentsCachePathSupplier= */ () -> null); return buildImpl(); } diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/AbstractCollectPackagesUnderDirectoryTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/AbstractCollectPackagesUnderDirectoryTest.java index 2d7b0dc690e088..262dba90c75b97 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/AbstractCollectPackagesUnderDirectoryTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/AbstractCollectPackagesUnderDirectoryTest.java @@ -318,6 +318,7 @@ private void initEvaluator() getExtraSkyFunctions(), SyscallCache.NO_CACHE, /* allowExternalRepositories= */ false, + /* repoContentsCachePathSupplier= */ () -> null, SkyframeExecutor.SkyKeyStateReceiver.NULL_INSTANCE, BugReporter.defaultInstance()); skyframeExecutor.injectExtraPrecomputedValues( diff --git a/src/test/py/bazel/BUILD b/src/test/py/bazel/BUILD index 4dbd5ecd53c9dc..25a178d87dd733 100644 --- a/src/test/py/bazel/BUILD +++ b/src/test/py/bazel/BUILD @@ -392,6 +392,7 @@ py_test( name = "repo_contents_cache_test", size = "large", srcs = ["bzlmod/repo_contents_cache_test.py"], + shard_count = 4, tags = ["requires-network"], deps = [ ":bzlmod_test_utils", diff --git a/src/test/py/bazel/bzlmod/repo_contents_cache_test.py b/src/test/py/bazel/bzlmod/repo_contents_cache_test.py index 3ff7ee84a4541e..efaa4e182e6be7 100644 --- a/src/test/py/bazel/bzlmod/repo_contents_cache_test.py +++ b/src/test/py/bazel/bzlmod/repo_contents_cache_test.py @@ -14,7 +14,9 @@ # limitations under the License. # pylint: disable=g-long-ternary +import json import os +import pathlib import tempfile import time @@ -54,6 +56,18 @@ def sleepUntilCacheEmpty(self): time.sleep(0.5) self.fail('repo contents cache still not empty after 5 seconds') + def repoDir(self, repo_name, cwd=None): + _, stdout, _ = self.RunBazel(['info', 'output_base'], cwd=cwd) + self.assertLen(stdout, 1) + output_base = stdout[0].strip() + + _, stdout, _ = self.RunBazel(['mod', 'dump_repo_mapping', ''], cwd=cwd) + self.assertLen(stdout, 1) + mapping = json.loads(stdout[0]) + canonical_repo_name = mapping[repo_name] + + return output_base + '/external/' + canonical_repo_name + def testCachedAfterCleanExpunge(self): self.ScratchFile( 'MODULE.bazel', @@ -76,6 +90,21 @@ def testCachedAfterCleanExpunge(self): # First fetch: not cached _, _, stderr = self.RunBazel(['build', '@my_repo//:haha']) self.assertIn('JUST FETCHED', '\n'.join(stderr)) + # Verify that the repo directory under the output base is a symlink or + # junction into the repo contents cache. + repo_dir = self.repoDir('my_repo') + self.assertTrue(os.path.islink(repo_dir) or os.path.isjunction(repo_dir)) + target_path = os.readlink(repo_dir) + real_target_path = os.path.realpath(target_path) + real_repo_contents_cache = os.path.realpath(self.repo_contents_cache) + for parent in pathlib.Path(real_target_path).parents: + if parent.samefile(real_repo_contents_cache): + break + else: + self.fail( + 'repo target dir %s is not in the repo contents cache %s' + % (real_target_path, real_repo_contents_cache) + ) # After expunging: cached self.RunBazel(['clean', '--expunge']) @@ -295,7 +324,9 @@ def testGc_singleServer_gcAfterCacheHit(self): # GC'd while server is alive: not cached, but also no crash self.sleepUntilCacheEmpty() _, _, stderr = self.RunBazel(['build', '@my_repo//:haha']) - self.assertIn('JUST FETCHED', '\n'.join(stderr)) + stderr = '\n'.join(stderr) + self.assertIn('JUST FETCHED', stderr) + self.assertNotIn('WARNING', stderr) def testGc_singleServer_gcAfterCacheMiss(self): self.ScratchFile( @@ -328,7 +359,9 @@ def testGc_singleServer_gcAfterCacheMiss(self): # GC'd while server is alive: not cached, but also no crash self.sleepUntilCacheEmpty() _, _, stderr = self.RunBazel(['build', '@my_repo//:haha']) - self.assertIn('JUST FETCHED', '\n'.join(stderr)) + stderr = '\n'.join(stderr) + self.assertIn('JUST FETCHED', stderr) + self.assertNotIn('WARNING', stderr) def testGc_multipleServers(self): module_bazel_lines = [ @@ -383,13 +416,17 @@ def testGc_multipleServers(self): ], cwd=dir_a, ) - self.assertIn('JUST FETCHED', '\n'.join(stderr)) + stderr = '\n'.join(stderr) + self.assertIn('JUST FETCHED', stderr) + self.assertNotIn('WARNING', stderr) # GC'd while B's server is alive (after B's earlier cache hit): # not cached, but also no crash self.sleepUntilCacheEmpty() _, _, stderr = self.RunBazel(['build', '@my_repo//:haha'], cwd=dir_b) - self.assertIn('JUST FETCHED', '\n'.join(stderr)) + stderr = '\n'.join(stderr) + self.assertIn('JUST FETCHED', stderr) + self.assertNotIn('WARNING', stderr) def testReverseDependencyDirection(self): # Set up two repos that retain their predeclared input hashes across two @@ -452,6 +489,109 @@ def testReverseDependencyDirection(self): self.assertIn('| @@+repo+bar', stderr) self.assertIn('`-- @@+repo+foo', stderr) + def doTestRepoContentsCacheDeleted(self, check_external_repository_files): + repo_contents_cache = self.ScratchDir('repo_contents_cache') + workspace = self.ScratchDir('workspace') + extra_args = [ + '--experimental_check_external_repository_files=%s' + % str(check_external_repository_files).lower(), + '--repo_contents_cache=%s' % repo_contents_cache, + ] + + self.ScratchFile( + 'workspace/MODULE.bazel', + [ + 'repo = use_repo_rule("//:repo.bzl", "repo")', + 'repo(name = "my_repo")', + ], + ) + self.ScratchFile('workspace/BUILD.bazel') + self.ScratchFile( + 'workspace/repo.bzl', + [ + 'def _repo_impl(rctx):', + ' rctx.file("BUILD", "filegroup(name=\'haha\')")', + ' print("JUST FETCHED")', + ' return rctx.repo_metadata(reproducible=True)', + 'repo = repository_rule(_repo_impl)', + ], + ) + # First fetch: not cached + _, _, stderr = self.RunBazel( + [ + 'build', + '@my_repo//:haha', + ] + + extra_args, + cwd=workspace, + ) + self.assertIn('JUST FETCHED', '\n'.join(stderr)) + + # Second fetch: cached + _, _, stderr = self.RunBazel( + [ + 'build', + '@my_repo//:haha', + ] + + extra_args, + cwd=workspace, + ) + self.assertNotIn('JUST FETCHED', '\n'.join(stderr)) + + # Delete the entire repo contents cache and fetch again: not cached + # Avoid access denied on Windows due to files being read-only by moving to + # a different location instead. + os.rename(repo_contents_cache, repo_contents_cache + '_deleted') + _, _, stderr = self.RunBazel( + [ + 'build', + '@my_repo//:haha', + ] + + extra_args, + cwd=workspace, + ) + stderr = '\n'.join(stderr) + self.assertIn('JUST FETCHED', stderr) + self.assertNotIn('WARNING', stderr) + + # Second fetch after deletion: cached + _, _, stderr = self.RunBazel( + [ + 'build', + '@my_repo//:haha', + ] + + extra_args, + cwd=workspace, + ) + self.assertNotIn('JUST FETCHED', '\n'.join(stderr)) + self.assertNotIn('WARNING', '\n'.join(stderr)) + + # Delete the entire repo contents cache and fetch again with a different + # path: not cached + # Avoid access denied on Windows due to files being read-only by moving to + # a different location instead. + os.rename(repo_contents_cache, repo_contents_cache + '_deleted_again') + _, _, stderr = self.RunBazel( + [ + 'build', + '@my_repo//:haha', + ] + + extra_args + + [ + '--repo_contents_cache=%s' % repo_contents_cache + '2', + ], + cwd=workspace, + ) + stderr = '\n'.join(stderr) + self.assertIn('JUST FETCHED', stderr) + self.assertNotIn('WARNING', stderr) + + def testRepoContentsCacheDeleted_withCheckExternalRepositoryFiles(self): + self.doTestRepoContentsCacheDeleted(check_external_repository_files=True) + + def testRepoContentsCacheDeleted_withoutCheckExternalRepositoryFiles(self): + self.doTestRepoContentsCacheDeleted(check_external_repository_files=False) + if __name__ == '__main__': absltest.main()