Add DescribableExecutionUnit interface to allow centralizing command failure descriptions. PiperOrigin-RevId: 398323043
diff --git a/src/main/java/com/google/devtools/build/lib/actions/BUILD b/src/main/java/com/google/devtools/build/lib/actions/BUILD index b859a4a..6d292df 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/BUILD +++ b/src/main/java/com/google/devtools/build/lib/actions/BUILD
@@ -101,6 +101,7 @@ "//src/main/java/com/google/devtools/build/lib/unsafe:string", "//src/main/java/com/google/devtools/build/lib/util", "//src/main/java/com/google/devtools/build/lib/util:command", + "//src/main/java/com/google/devtools/build/lib/util:describable_execution_unit", "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code", "//src/main/java/com/google/devtools/build/lib/util:exit_code", "//src/main/java/com/google/devtools/build/lib/util:filetype",
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java index 479a5a8..faa1213 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java
@@ -18,16 +18,16 @@ import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.analysis.platform.PlatformInfo; import com.google.devtools.build.lib.collect.nestedset.NestedSet; +import com.google.devtools.build.lib.util.DescribableExecutionUnit; import java.util.Collection; import javax.annotation.Nullable; /** - * An object representing a subprocess to be invoked, including its command and - * arguments, its working directory, its environment, a boolean indicating - * whether remote execution is appropriate for this command, and if so, the set - * of files it is expected to read and write. + * An object representing a subprocess to be invoked, including its command and arguments, its + * working directory, its environment, a boolean indicating whether remote execution is appropriate + * for this command, and if so, the set of files it is expected to read and write. */ -public interface Spawn { +public interface Spawn extends DescribableExecutionUnit { /** * Out-of-band data for this spawn. This can be used to signal hints (hardware requirements, local * vs. remote) to the execution subsystem. This data can come from multiple places e.g. tags, hard
diff --git a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java index a56b839..5b9df88 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java
@@ -199,12 +199,7 @@ String message = !Strings.isNullOrEmpty(resultMessage) ? resultMessage - : CommandFailureUtils.describeCommandFailure( - verboseFailures, - spawn.getArguments(), - spawn.getEnvironment(), - cwd, - spawn.getExecutionPlatform()); + : CommandFailureUtils.describeCommandFailure(verboseFailures, cwd, spawn); throw new SpawnExecException(message, spawnResult, /*forciblyRunRemotely=*/false); } return ImmutableList.of(spawnResult);
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java index 7272343..1769a81 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java
@@ -159,18 +159,10 @@ private String makeFailureMessage(Spawn originalSpawn, SandboxedSpawn sandbox) { if (sandboxOptions.sandboxDebug) { return CommandFailureUtils.describeCommandFailure( - true, - sandbox.getArguments(), - sandbox.getEnvironment(), - sandbox.getSandboxExecRoot().getPathString(), - null); + true, sandbox.getSandboxExecRoot().getPathString(), sandbox); } else { return CommandFailureUtils.describeCommandFailure( - verboseFailures, - originalSpawn.getArguments(), - originalSpawn.getEnvironment(), - sandbox.getSandboxExecRoot().getPathString(), - originalSpawn.getExecutionPlatform()) + verboseFailures, sandbox.getSandboxExecRoot().getPathString(), originalSpawn) + SANDBOX_DEBUG_SUGGESTION; } }
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/BUILD b/src/main/java/com/google/devtools/build/lib/sandbox/BUILD index 8701c24..4ceba33 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/BUILD +++ b/src/main/java/com/google/devtools/build/lib/sandbox/BUILD
@@ -41,6 +41,7 @@ "//src/main/java/com/google/devtools/build/lib/util", "//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception", "//src/main/java/com/google/devtools/build/lib/util:command", + "//src/main/java/com/google/devtools/build/lib/util:describable_execution_unit", "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code", "//src/main/java/com/google/devtools/build/lib/util:os", "//src/main/java/com/google/devtools/build/lib/util:process",
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxedSpawn.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxedSpawn.java index 088924f..8c58418 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxedSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxedSpawn.java
@@ -14,8 +14,7 @@ package com.google.devtools.build.lib.sandbox; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.util.DescribableExecutionUnit; import com.google.devtools.build.lib.vfs.Path; import java.io.IOException; import javax.annotation.Nullable; @@ -27,16 +26,10 @@ * so that a process running inside the directory can access the files. It also handles moving the * output files generated by the process out of the directory into a destination directory. */ -interface SandboxedSpawn { +interface SandboxedSpawn extends DescribableExecutionUnit { /** The path in which to execute the subprocess. */ Path getSandboxExecRoot(); - /** The command-line of the subprocess. */ - ImmutableList<String> getArguments(); - - /** The environment variables to be set for the subprocess. */ - ImmutableMap<String, String> getEnvironment(); - /** Returns {@code true}, if the runner should use the Subprocess timeout feature. */ default boolean useSubprocessTimeout() { return false;
diff --git a/src/main/java/com/google/devtools/build/lib/util/BUILD b/src/main/java/com/google/devtools/build/lib/util/BUILD index 4d26a69..497deb8 100644 --- a/src/main/java/com/google/devtools/build/lib/util/BUILD +++ b/src/main/java/com/google/devtools/build/lib/util/BUILD
@@ -77,6 +77,16 @@ ) java_library( + name = "describable_execution_unit", + srcs = ["DescribableExecutionUnit.java"], + deps = [ + "//src/main/java/com/google/devtools/build/lib/analysis/platform", + "//third_party:guava", + "//third_party:jsr305", + ], +) + +java_library( name = "command", srcs = [ "CommandBuilder.java", @@ -85,6 +95,7 @@ "CommandUtils.java", ], deps = [ + ":describable_execution_unit", ":os", ":shell_escaper", "//src/main/java/com/google/devtools/build/lib/analysis/platform",
diff --git a/src/main/java/com/google/devtools/build/lib/util/CommandFailureUtils.java b/src/main/java/com/google/devtools/build/lib/util/CommandFailureUtils.java index 00c4244..18e791b 100644 --- a/src/main/java/com/google/devtools/build/lib/util/CommandFailureUtils.java +++ b/src/main/java/com/google/devtools/build/lib/util/CommandFailureUtils.java
@@ -280,7 +280,7 @@ boolean verbose, Collection<String> commandLineElements, Map<String, String> env, - String cwd, + @Nullable String cwd, @Nullable PlatformInfo executionPlatform) { String commandName = commandLineElements.iterator().next(); @@ -290,4 +290,14 @@ + " failed: " + describeCommandError(verbose, commandLineElements, env, cwd, executionPlatform); } + + public static String describeCommandFailure( + boolean verboseFailures, @Nullable String cwd, DescribableExecutionUnit command) { + return describeCommandFailure( + verboseFailures, + command.getArguments(), + command.getEnvironment(), + cwd, + command.getExecutionPlatform()); + } }
diff --git a/src/main/java/com/google/devtools/build/lib/util/DescribableExecutionUnit.java b/src/main/java/com/google/devtools/build/lib/util/DescribableExecutionUnit.java new file mode 100644 index 0000000..e54cbd1 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/util/DescribableExecutionUnit.java
@@ -0,0 +1,40 @@ +// Copyright 2021 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.util; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.analysis.platform.PlatformInfo; +import javax.annotation.Nullable; + +/** + * Something executable that can be described by {@link CommandFailureUtils#describeCommandFailure}. + */ +public interface DescribableExecutionUnit { + + /** Returns the command (the first element) and its arguments. */ + ImmutableList<String> getArguments(); + + /** + * Returns the initial environment of the process. If null, the environment is inherited from the + * parent process. + */ + ImmutableMap<String, String> getEnvironment(); + + /** Returns the execution platform for the command, if any. */ + @Nullable + default PlatformInfo getExecutionPlatform() { + return null; + } +}