Skip to content

Commit

Permalink
Ensure runfiles output manifest is gone before making it a link.
Browse files Browse the repository at this point in the history
After 0763dd0, the test included in this CL fails like this:
```
** test_switch_runfiles_from_enabled_to_disabled *******************************
Computing main repo mapping:
Loading:
Loading: 0 packages loaded
Analyzing: target //:out (1 packages loaded, 0 targets configured)
INFO: Analyzed target //:out (42 packages loaded, 172 targets configured).
INFO: Found 1 target...
[0 / 2] [Prepa] BazelWorkspaceStatusAction stable-status.txt
Target //:out up-to-date:
  bazel-bin/out
INFO: Elapsed time: 0.391s, Critical Path: 0.02s
INFO: 5 processes: 4 internal, 1 local.
INFO: Build completed successfully, 5 total actions
Computing main repo mapping:
Loading:
Loading: 0 packages loaded
WARNING: Build option --enable_runfiles has changed, discarding analysis cache (this can be expensive, see https://bazel.build/advanced/performance/iteration-speed).
Analyzing: target //:out (1 packages loaded, 0 targets configured)
INFO: Analyzed target //:out (1 packages loaded, 173 targets configured).
INFO: Found 1 target...
[0 / 3] [Prepa] BazelWorkspaceStatusAction stable-status.txt
ERROR: workspace/BUILD:6:8: Executing genrule //:g failed: java.io.IOException: execroot/main/bazel-out/k8-opt-exec-ST-e846b08c7501/bin/cmd.runfiles/MANIFEST (File exists)
Target //:out failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 0.177s, Critical Path: 0.01s
INFO: 4 processes: 4 internal.
ERROR: Build did NOT complete successfully
```
This is because `FileSystemUtils.copyFile` ensured that the target was removed before writing to the target while `Path.createSymbolicLink` does not. Insert a `delete()` call to fix the problem.

Also, rename `copyManifest` to `linkManifest` for accuracy.

Closes #19382.

PiperOrigin-RevId: 562519152
Change-Id: I8683f84daaa6f827a3022fead50687e4d2eb34d9
  • Loading branch information
benjaminp authored and copybara-github committed Sep 4, 2023
1 parent 93b679f commit 695d747
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ private void updateRunfilesTree(

switch (runfileSymlinksMode) {
case SKIP:
helper.copyManifest();
helper.linkManifest();
break;
case EXTERNAL:
helper.createSymlinksUsingCommand(execRoot, binTools, env, outErr);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,13 @@ public void clearRunfilesDirectory() throws ExecException {
}
}

/** Copies the input manifest to the output manifest. */
public void copyManifest() throws ExecException {
/** Links the output manifest to the input manifest. */
public void linkManifest() throws ExecException {
// Pretend we created the runfiles tree by symlinking the output manifest to the input manifest.
Path outputManifest = getOutputManifest();
try {
symlinkTreeRoot.createDirectoryAndParents();
outputManifest.delete();
outputManifest.createSymbolicLink(inputManifest);
} catch (IOException e) {
throw new EnvironmentalExecException(e, Code.SYMLINK_TREE_MANIFEST_LINK_IO_EXCEPTION);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public void createSymlinks(
// Skyframe does not clear the directory for us.
var helper = createSymlinkTreeHelper(action, actionExecutionContext);
helper.clearRunfilesDirectory();
helper.copyManifest();
helper.linkManifest();
} else if (action.getRunfileSymlinksMode() == RunfileSymlinksMode.INTERNAL
&& !action.isFilesetTree()) {
try {
Expand Down
22 changes: 22 additions & 0 deletions src/test/shell/bazel/runfiles_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -239,4 +239,26 @@ EOF
[[ -f bazel-bin/world.runfiles/MANIFEST ]] || fail "expected output manifest world to exist"
}

function test_switch_runfiles_from_enabled_to_disabled {
echo '#!/bin/bash' > cmd.sh
chmod 755 cmd.sh
cat > BUILD <<'EOF'
sh_binary(
name = "cmd",
srcs = ["cmd.sh"],
data = glob(["data-*"]),
)
genrule(
name = "g",
cmd = "$(location :cmd) > $@",
outs = ["out"],
tools = [":cmd"],
)
EOF

bazel build --spawn_strategy=local --nobuild_runfile_links //:out
touch data-1
bazel build --spawn_strategy=local --nobuild_runfile_links --enable_runfiles=false //:out
}

run_suite "runfiles tests"

0 comments on commit 695d747

Please sign in to comment.