Skip to content

Commit

Permalink
SymlinkAction: Avoid using Artifact.prettyPrint in error messages.
Browse files Browse the repository at this point in the history
As a comment in Artifact.prettyPrint notes: https://github.com/bazelbuild/bazel/blob/5d9333540a3ee9fe270e0e44eb4cc9fed8049bab/src/main/java/com/google/devtools/build/lib/actions/Artifact.java#L841-L842 prettyPrint can be confusing; in particular, it hides the artifact's root.

Closes bazelbuild#15262.

PiperOrigin-RevId: 443271532
  • Loading branch information
benjaminp authored and copybara-github committed Apr 21, 2022
1 parent 092884b commit 4ea7d69
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.AbstractAction;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionExecutionException;
Expand Down Expand Up @@ -187,7 +186,7 @@ public ActionResult execute(ActionExecutionContext actionExecutionContext)
String message =
String.format(
"failed to create symbolic link '%s' to '%s' due to I/O error: %s",
Iterables.getOnlyElement(getOutputs()).prettyPrint(), printInputs(), e.getMessage());
getPrimaryOutput().getExecPathString(), printInputs(), e.getMessage());
DetailedExitCode code = createDetailedExitCode(message, Code.LINK_CREATION_IO_EXCEPTION);
throw new ActionExecutionException(message, e, this, false, code);
}
Expand All @@ -206,26 +205,24 @@ private void maybeVerifyTargetIsExecutable(ActionExecutionContext actionExecutio
try {
// Validate that input path is a file with the executable bit set.
if (!inputPath.isFile()) {
String message =
String.format("'%s' is not a file", getInputs().getSingleton().prettyPrint());
String message = String.format("'%s' is not a file", getPrimaryInput().getExecPathString());
throw new ActionExecutionException(
message, this, false, createDetailedExitCode(message, Code.EXECUTABLE_INPUT_NOT_FILE));
}
if (!inputPath.isExecutable()) {
String message =
String.format(
"failed to create symbolic link '%s': file '%s' is not executable",
Iterables.getOnlyElement(getOutputs()).prettyPrint(),
getInputs().getSingleton().prettyPrint());
getPrimaryOutput().getExecPathString(), getPrimaryInput().getExecPathString());
throw new ActionExecutionException(
message, this, false, createDetailedExitCode(message, Code.EXECUTABLE_INPUT_IS_NOT));
}
} catch (IOException e) {
String message =
String.format(
"failed to create symbolic link '%s' to the '%s' due to I/O error: %s",
Iterables.getOnlyElement(getOutputs()).prettyPrint(),
getInputs().getSingleton().prettyPrint(),
getPrimaryOutput().getExecPathString(),
getPrimaryInput().getExecPathString(),
e.getMessage());
DetailedExitCode detailedExitCode =
createDetailedExitCode(message, Code.EXECUTABLE_INPUT_CHECK_IO_EXCEPTION);
Expand Down Expand Up @@ -255,8 +252,8 @@ private void updateInputMtimeIfNeeded(ActionExecutionContext actionExecutionCont
String message =
String.format(
"failed to touch symbolic link '%s' to the '%s' due to I/O error: %s",
Iterables.getOnlyElement(getOutputs()).prettyPrint(),
getInputs().getSingleton().prettyPrint(),
getPrimaryOutput().getExecPathString(),
getPrimaryInput().getExecPathString(),
e.getMessage());
DetailedExitCode code = createDetailedExitCode(message, Code.LINK_TOUCH_IO_EXCEPTION);
throw new ActionExecutionException(message, e, this, false, code);
Expand All @@ -267,7 +264,7 @@ private String printInputs() {
if (getInputs().isEmpty()) {
return inputPath.getPathString();
} else if (getInputs().isSingleton()) {
return getInputs().getSingleton().prettyPrint();
return getPrimaryInput().getExecPathString();
} else {
throw new IllegalStateException(
"Inputs unexpectedly contains more than 1 element: " + getInputs());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public void testFailIfInputIsNotAFile() throws Exception {
SymlinkAction action = SymlinkAction.toExecutable(NULL_ACTION_OWNER, input, output, "progress");
ActionExecutionException e =
assertThrows(ActionExecutionException.class, () -> action.execute(createContext()));
assertThat(e).hasMessageThat().contains("'some-dir' is not a file");
assertThat(e).hasMessageThat().contains("'in/some-dir' is not a file");
}

@Test
Expand All @@ -125,7 +125,7 @@ public void testFailIfInputIsNotExecutable() throws Exception {
SymlinkAction action = SymlinkAction.toExecutable(NULL_ACTION_OWNER, input, output, "progress");
ActionExecutionException e =
assertThrows(ActionExecutionException.class, () -> action.execute(createContext()));
String want = "'some-file' is not executable";
String want = "'in/some-file' is not executable";
String got = e.getMessage();
assertWithMessage(String.format("got %s, want %s", got, want))
.that(got.contains(want))
Expand Down
2 changes: 1 addition & 1 deletion src/test/shell/bazel/bazel_symlink_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ Hello, World!
EOF

bazel build //a:a >& $TEST_log && fail "build succeeded"
expect_log "failed to create symbolic link 'a/a.link': file 'a/foo.txt' is not executable"
expect_log "failed to create symbolic link 'bazel-out/[^/]*/bin/a/a.link': file 'a/foo.txt' is not executable"
}

function test_symlink_cycle() {
Expand Down

0 comments on commit 4ea7d69

Please sign in to comment.