Skip to content

Commit 216d333

Browse files
fmeumcopybara-github
authored andcommitted
Disallow output bases under GC-able directories
We have had user's report spurious build failures due to the disk cache GC collecting files under the output base when output base and disk cache were set up at the same path. Closes #26138. PiperOrigin-RevId: 785412502 Change-Id: I44865ce44782a21a4cc15b61f0e0c9d4f13ea194
1 parent b69620d commit 216d333

File tree

2 files changed

+78
-1
lines changed

2 files changed

+78
-1
lines changed

src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@
108108
import com.google.devtools.build.lib.vfs.PathFragment;
109109
import com.google.devtools.common.options.OptionsBase;
110110
import com.google.devtools.common.options.OptionsParsingResult;
111+
import java.io.FileNotFoundException;
111112
import java.io.IOException;
112113
import java.time.Instant;
113114
import java.util.LinkedHashMap;
@@ -324,6 +325,50 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
324325
repositoryCache.getRepoContentsCache().setPath(toPath(repoOptions.repoContentsCache, env));
325326
}
326327
Path repoContentsCachePath = repositoryCache.getRepoContentsCache().getPath();
328+
if (repoContentsCachePath != null) {
329+
// Check that the repo contents cache directory, which is managed by a garbage collecting
330+
// idle task, does not contain the output base. Since the specified output base path may be
331+
// a symlink, we resolve it fully. Intermediate symlinks do not have to be checked as the
332+
// garbage collector ignores symlinks. We also resolve the repo contents cache directory,
333+
// where intermediate symlinks also don't matter since deletion only occurs under the fully
334+
// resolved path.
335+
Path resolvedOutputBase = env.getOutputBase();
336+
try {
337+
resolvedOutputBase = resolvedOutputBase.resolveSymbolicLinks();
338+
} catch (FileNotFoundException ignored) {
339+
// Will be created later.
340+
} catch (IOException e) {
341+
throw new AbruptExitException(
342+
detailedExitCode(
343+
"could not resolve output base: %s".formatted(e.getMessage()),
344+
Code.BAD_REPO_CONTENTS_CACHE),
345+
e);
346+
}
347+
Path resolvedRepoContentsCache = repoContentsCachePath;
348+
try {
349+
resolvedRepoContentsCache = resolvedRepoContentsCache.resolveSymbolicLinks();
350+
} catch (FileNotFoundException ignored) {
351+
// Will be created later.
352+
} catch (IOException e) {
353+
throw new AbruptExitException(
354+
detailedExitCode(
355+
"could not resolve repo contents cache path: %s".formatted(e.getMessage()),
356+
Code.BAD_REPO_CONTENTS_CACHE),
357+
e);
358+
}
359+
if (resolvedOutputBase.startsWith(resolvedRepoContentsCache)) {
360+
// This is dangerous as the repo contents cache GC may delete files in the output base.
361+
throw new AbruptExitException(
362+
detailedExitCode(
363+
"""
364+
The output base [%s] is inside the repo contents cache [%s]. This can cause \
365+
spurious failures. Disable the repo contents cache with `--repo_contents_cache=`, \
366+
or specify `--repo_contents_cache=<path that doesn't contain the output base>`.
367+
"""
368+
.formatted(resolvedOutputBase, resolvedRepoContentsCache),
369+
Code.BAD_REPO_CONTENTS_CACHE));
370+
}
371+
}
327372
if (repoContentsCachePath != null
328373
&& env.getWorkspace() != null
329374
&& repoContentsCachePath.startsWith(env.getWorkspace())) {

src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@
112112
import io.netty.handler.codec.DecoderException;
113113
import io.netty.handler.codec.http.HttpResponseStatus;
114114
import io.reactivex.rxjava3.plugins.RxJavaPlugins;
115+
import java.io.FileNotFoundException;
115116
import java.io.IOException;
116117
import java.net.URI;
117118
import java.net.URISyntaxException;
@@ -348,6 +349,38 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
348349
boolean enableRemoteDownloader = shouldEnableRemoteDownloader(remoteOptions);
349350

350351
if (enableDiskCache) {
352+
// Check that the disk cache directory, which is managed by a garbage collecting idle task,
353+
// does not contain the output base. Since the specified output base path may be a symlink,
354+
// we resolve it fully. Intermediate symlinks do not have to be checked as the garbage
355+
// collector ignores symlinks. We also resolve the disk cache directory, where intermediate
356+
// symlinks also don't matter since deletion only occurs under the fully resolved path.
357+
Path resolvedOutputBase = env.getOutputBase();
358+
try {
359+
resolvedOutputBase = resolvedOutputBase.resolveSymbolicLinks();
360+
} catch (FileNotFoundException ignored) {
361+
// Will be created later.
362+
} catch (IOException e) {
363+
throw createOptionsExitException(
364+
"Failed to resolve output base: %s".formatted(e.getMessage()),
365+
FailureDetails.RemoteOptions.Code.EXECUTION_WITH_INVALID_CACHE);
366+
}
367+
Path resolvedDiskCache = env.getWorkingDirectory().getRelative(remoteOptions.diskCache);
368+
try {
369+
resolvedDiskCache = resolvedDiskCache.resolveSymbolicLinks();
370+
} catch (FileNotFoundException ignored) {
371+
// Will be created later.
372+
} catch (IOException e) {
373+
throw createOptionsExitException(
374+
"Failed to resolve disk cache directory: %s".formatted(e.getMessage()),
375+
FailureDetails.RemoteOptions.Code.EXECUTION_WITH_INVALID_CACHE);
376+
}
377+
if (resolvedOutputBase.startsWith(resolvedDiskCache)) {
378+
// This is dangerous as the disk cache GC may delete files in the output base.
379+
throw createOptionsExitException(
380+
"The output base [%s] cannot be a subdirectory of the --disk_cache directory [%s]"
381+
.formatted(resolvedOutputBase, resolvedDiskCache),
382+
FailureDetails.RemoteOptions.Code.EXECUTION_WITH_INVALID_CACHE);
383+
}
351384
var gcIdleTask =
352385
DiskCacheGarbageCollectorIdleTask.create(remoteOptions, env.getWorkingDirectory());
353386
if (gcIdleTask != null) {
@@ -1229,5 +1262,4 @@ static Credentials createCredentials(
12291262
Downloader getRemoteDownloader() {
12301263
return remoteDownloader;
12311264
}
1232-
12331265
}

0 commit comments

Comments
 (0)