Skip to content

Commit

Permalink
Fix bug in reuse_sandbox_directories
Browse files Browse the repository at this point in the history
#19935 gives a clear repro. The problem appears when a stashed sandbox
contains a directory whose same path is a regular file in a later
execution. If we try to reuse the stashed sandbox we trigger an error if
the old directory contained any files.

This was due to simply calling delete() without checking first if it
was a directory. The fix is to call deleteTree() instead in those cases.
directory.

Fixes #19935

RELNOTES:none
PiperOrigin-RevId: 576889004
Change-Id: I73b145cd574b83c473ffaccd90b675eb5f086c0e
  • Loading branch information
oquenchil authored and copybara-github committed Oct 26, 2023
1 parent 0a9c9a7 commit 7d87996
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,8 @@ private static void cleanRecursively(
if (SYMLINK.equals(dirent.getType())
&& absPath.readSymbolicLink().equals(destination.get())) {
inputsToCreate.remove(pathRelativeToWorkDir);
} else if (absPath.isDirectory()) {
absPath.deleteTree();
} else {
absPath.delete();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,19 @@ public void cleanExisting_updatesDirs() throws IOException, InterruptedException
// outputDir only exists partially
execRootPath.getRelative(outputDir).getParentDirectory().createDirectoryAndParents();
execRootPath.getRelative("justSomeDir/thatIsDoomed").createDirectoryAndParents();
SandboxHelpers.cleanExisting(rootDir, inputs, inputsToCreate, dirsToCreate, execRootPath);
// `thiswillbeafile/output` simulates a directory that was in the stashed dir but whose same
// path is used later for a regular file.
scratch.dir("/execRoot/thiswillbeafile/output");
scratch.file("/execRoot/thiswillbeafile/output/file1");
dirsToCreate.add(PathFragment.create("thiswillbeafile"));
PathFragment input4 = PathFragment.create("thiswillbeafile/output");
SandboxInputs inputs2 =
new SandboxInputs(
ImmutableMap.of(input1, inputTxt, input2, inputTxt, input3, inputTxt, input4, inputTxt),
ImmutableMap.of(),
ImmutableMap.of(),
ImmutableMap.of());
SandboxHelpers.cleanExisting(rootDir, inputs2, inputsToCreate, dirsToCreate, execRootPath);
assertThat(dirsToCreate).containsExactly(inputDir2, inputDir3, outputDir);
assertThat(execRootPath.getRelative("existing/directory/with").exists()).isTrue();
assertThat(execRootPath.getRelative("partial").exists()).isTrue();
Expand Down

0 comments on commit 7d87996

Please sign in to comment.