Skip to content

Commit 4ea7d69

Browse files
benjaminpcopybara-github
authored andcommitted
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
1 parent 092884b commit 4ea7d69

File tree

3 files changed

+11
-14
lines changed

3 files changed

+11
-14
lines changed

src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkAction.java

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
import com.google.common.base.Preconditions;
1818
import com.google.common.collect.ImmutableSet;
19-
import com.google.common.collect.Iterables;
2019
import com.google.devtools.build.lib.actions.AbstractAction;
2120
import com.google.devtools.build.lib.actions.ActionExecutionContext;
2221
import com.google.devtools.build.lib.actions.ActionExecutionException;
@@ -187,7 +186,7 @@ public ActionResult execute(ActionExecutionContext actionExecutionContext)
187186
String message =
188187
String.format(
189188
"failed to create symbolic link '%s' to '%s' due to I/O error: %s",
190-
Iterables.getOnlyElement(getOutputs()).prettyPrint(), printInputs(), e.getMessage());
189+
getPrimaryOutput().getExecPathString(), printInputs(), e.getMessage());
191190
DetailedExitCode code = createDetailedExitCode(message, Code.LINK_CREATION_IO_EXCEPTION);
192191
throw new ActionExecutionException(message, e, this, false, code);
193192
}
@@ -206,26 +205,24 @@ private void maybeVerifyTargetIsExecutable(ActionExecutionContext actionExecutio
206205
try {
207206
// Validate that input path is a file with the executable bit set.
208207
if (!inputPath.isFile()) {
209-
String message =
210-
String.format("'%s' is not a file", getInputs().getSingleton().prettyPrint());
208+
String message = String.format("'%s' is not a file", getPrimaryInput().getExecPathString());
211209
throw new ActionExecutionException(
212210
message, this, false, createDetailedExitCode(message, Code.EXECUTABLE_INPUT_NOT_FILE));
213211
}
214212
if (!inputPath.isExecutable()) {
215213
String message =
216214
String.format(
217215
"failed to create symbolic link '%s': file '%s' is not executable",
218-
Iterables.getOnlyElement(getOutputs()).prettyPrint(),
219-
getInputs().getSingleton().prettyPrint());
216+
getPrimaryOutput().getExecPathString(), getPrimaryInput().getExecPathString());
220217
throw new ActionExecutionException(
221218
message, this, false, createDetailedExitCode(message, Code.EXECUTABLE_INPUT_IS_NOT));
222219
}
223220
} catch (IOException e) {
224221
String message =
225222
String.format(
226223
"failed to create symbolic link '%s' to the '%s' due to I/O error: %s",
227-
Iterables.getOnlyElement(getOutputs()).prettyPrint(),
228-
getInputs().getSingleton().prettyPrint(),
224+
getPrimaryOutput().getExecPathString(),
225+
getPrimaryInput().getExecPathString(),
229226
e.getMessage());
230227
DetailedExitCode detailedExitCode =
231228
createDetailedExitCode(message, Code.EXECUTABLE_INPUT_CHECK_IO_EXCEPTION);
@@ -255,8 +252,8 @@ private void updateInputMtimeIfNeeded(ActionExecutionContext actionExecutionCont
255252
String message =
256253
String.format(
257254
"failed to touch symbolic link '%s' to the '%s' due to I/O error: %s",
258-
Iterables.getOnlyElement(getOutputs()).prettyPrint(),
259-
getInputs().getSingleton().prettyPrint(),
255+
getPrimaryOutput().getExecPathString(),
256+
getPrimaryInput().getExecPathString(),
260257
e.getMessage());
261258
DetailedExitCode code = createDetailedExitCode(message, Code.LINK_TOUCH_IO_EXCEPTION);
262259
throw new ActionExecutionException(message, e, this, false, code);
@@ -267,7 +264,7 @@ private String printInputs() {
267264
if (getInputs().isEmpty()) {
268265
return inputPath.getPathString();
269266
} else if (getInputs().isSingleton()) {
270-
return getInputs().getSingleton().prettyPrint();
267+
return getPrimaryInput().getExecPathString();
271268
} else {
272269
throw new IllegalStateException(
273270
"Inputs unexpectedly contains more than 1 element: " + getInputs());

src/test/java/com/google/devtools/build/lib/actions/ExecutableSymlinkActionTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ public void testFailIfInputIsNotAFile() throws Exception {
111111
SymlinkAction action = SymlinkAction.toExecutable(NULL_ACTION_OWNER, input, output, "progress");
112112
ActionExecutionException e =
113113
assertThrows(ActionExecutionException.class, () -> action.execute(createContext()));
114-
assertThat(e).hasMessageThat().contains("'some-dir' is not a file");
114+
assertThat(e).hasMessageThat().contains("'in/some-dir' is not a file");
115115
}
116116

117117
@Test
@@ -125,7 +125,7 @@ public void testFailIfInputIsNotExecutable() throws Exception {
125125
SymlinkAction action = SymlinkAction.toExecutable(NULL_ACTION_OWNER, input, output, "progress");
126126
ActionExecutionException e =
127127
assertThrows(ActionExecutionException.class, () -> action.execute(createContext()));
128-
String want = "'some-file' is not executable";
128+
String want = "'in/some-file' is not executable";
129129
String got = e.getMessage();
130130
assertWithMessage(String.format("got %s, want %s", got, want))
131131
.that(got.contains(want))

src/test/shell/bazel/bazel_symlink_test.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -548,7 +548,7 @@ Hello, World!
548548
EOF
549549

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

554554
function test_symlink_cycle() {

0 commit comments

Comments
 (0)