Skip to content

Commit

Permalink
Remote: Don't check declared outputs for failed action (bazelbuild#15181
Browse files Browse the repository at this point in the history
)

Fix a bug that Bazel print message

```
Invalid action cache entry ...: expected output ... does not exist.
```

instead of the underlying error message when remote action failed.

Closes bazelbuild#15151.

PiperOrigin-RevId: 438815494

Co-authored-by: Chi Wang <chiwang@google.com>
  • Loading branch information
Wyverald and coeuvre authored Apr 5, 2022
1 parent df153df commit d418245
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1010,26 +1010,28 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re
metadata = parseActionResultMetadata(action, result);
}

// Check that all mandatory outputs are created.
for (ActionInput output : action.spawn.getOutputFiles()) {
if (action.spawn.isMandatoryOutput(output)) {
// Don't check output that is tree artifact since spawn could generate nothing under that
// directory. Remote server typically doesn't create directory ahead of time resulting in
// empty tree artifact missing from action cache entry.
if (output instanceof Artifact && ((Artifact) output).isTreeArtifact()) {
continue;
}
if (result.success()) {
// Check that all mandatory outputs are created.
for (ActionInput output : action.spawn.getOutputFiles()) {
if (action.spawn.isMandatoryOutput(output)) {
// Don't check output that is tree artifact since spawn could generate nothing under that
// directory. Remote server typically doesn't create directory ahead of time resulting in
// empty tree artifact missing from action cache entry.
if (output instanceof Artifact && ((Artifact) output).isTreeArtifact()) {
continue;
}

Path localPath = execRoot.getRelative(output.getExecPath());
if (!metadata.files.containsKey(localPath)
&& !metadata.directories.containsKey(localPath)
&& !metadata.symlinks.containsKey(localPath)) {
throw new IOException(
"Invalid action cache entry "
+ action.actionKey.getDigest().getHash()
+ ": expected output "
+ prettyPrint(output)
+ " does not exist.");
Path localPath = execRoot.getRelative(output.getExecPath());
if (!metadata.files.containsKey(localPath)
&& !metadata.directories.containsKey(localPath)
&& !metadata.symlinks.containsKey(localPath)) {
throw new IOException(
"Invalid action cache entry "
+ action.actionKey.getDigest().getHash()
+ ": expected output "
+ prettyPrint(output)
+ " does not exist.");
}
}
}
}
Expand Down
19 changes: 19 additions & 0 deletions src/test/shell/bazel/remote/remote_execution_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3579,4 +3579,23 @@ EOF
[[ "$remote_ac_files" == 0 ]] || fail "Expected 0 remote action cache entries, not $remote_ac_files"
}

function test_failed_action_dont_check_declared_outputs() {
# Test that if action failed, outputs are not checked

mkdir -p a
cat > a/BUILD <<EOF
genrule(
name = 'foo',
outs = ["foo.txt"],
cmd = "exit 1",
)
EOF

bazel build \
--remote_executor=grpc://localhost:${worker_port} \
//a:foo >& $TEST_log && fail "Should failed to build"

expect_log "Executing genrule .* failed: (Exit 1):"
}

run_suite "Remote execution and remote cache tests"

0 comments on commit d418245

Please sign in to comment.