From 4ea7d697527f00bf6fb7eeb953a568e3505dd62c Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Wed, 20 Apr 2022 21:09:01 -0700 Subject: [PATCH] SymlinkAction: Avoid using Artifact.prettyPrint in error messages. 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 #15262. PiperOrigin-RevId: 443271532 --- .../lib/analysis/actions/SymlinkAction.java | 19 ++++++++----------- .../actions/ExecutableSymlinkActionTest.java | 4 ++-- src/test/shell/bazel/bazel_symlink_test.sh | 2 +- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkAction.java index 7834538d640967..076b9567528e0f 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkAction.java @@ -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; @@ -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); } @@ -206,8 +205,7 @@ 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)); } @@ -215,8 +213,7 @@ private void maybeVerifyTargetIsExecutable(ActionExecutionContext actionExecutio 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)); } @@ -224,8 +221,8 @@ private void maybeVerifyTargetIsExecutable(ActionExecutionContext actionExecutio 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); @@ -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); @@ -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()); diff --git a/src/test/java/com/google/devtools/build/lib/actions/ExecutableSymlinkActionTest.java b/src/test/java/com/google/devtools/build/lib/actions/ExecutableSymlinkActionTest.java index a71afcb6297f96..1415a950d0b9b0 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/ExecutableSymlinkActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/ExecutableSymlinkActionTest.java @@ -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 @@ -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)) diff --git a/src/test/shell/bazel/bazel_symlink_test.sh b/src/test/shell/bazel/bazel_symlink_test.sh index c8e2ba7fcb71fa..5e0772c247781d 100755 --- a/src/test/shell/bazel/bazel_symlink_test.sh +++ b/src/test/shell/bazel/bazel_symlink_test.sh @@ -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() {