Separate Fileset-symlink errors out from I/O exceptions, and propagate them everywhere.
Because remaining I/O exceptions should all be environmental, made them 36 instead of 1.
Out of respect for ulfjack@, I left "strict" mode as a legitimate exception, even though it's completely dead in production.
PiperOrigin-RevId: 374474696
diff --git a/src/main/java/com/google/devtools/build/lib/actions/CompletionContext.java b/src/main/java/com/google/devtools/build/lib/actions/CompletionContext.java
index 7fb70e8..531aca3 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/CompletionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/CompletionContext.java
@@ -13,16 +13,12 @@
// limitations under the License.
package com.google.devtools.build.lib.actions;
-import static com.google.devtools.build.lib.actions.FilesetManifest.RelativeSymlinkBehavior.RESOLVE;
-import static com.google.devtools.build.lib.actions.FilesetManifest.RelativeSymlinkBehavior.RESOLVE_FULLY;
-
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.actions.FilesetManifest.RelativeSymlinkBehavior;
+import com.google.devtools.build.lib.actions.FilesetManifest.RelativeSymlinkBehaviorWithoutError;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
-import java.io.IOException;
import java.util.Map;
/**
@@ -77,8 +73,7 @@
ActionInputMap inputMap,
PathResolverFactory pathResolverFactory,
Path execRoot,
- String workspaceName)
- throws IOException {
+ String workspaceName) {
ArtifactPathResolver pathResolver =
pathResolverFactory.shouldCreatePathResolverForArtifactValues()
? pathResolverFactory.createPathResolverForArtifactValues(
@@ -116,7 +111,12 @@
continue;
} else if (artifact.isFileset()) {
if (expandFilesets) {
- visitFileset(artifact, receiver, fullyResolveFilesetLinks ? RESOLVE_FULLY : RESOLVE);
+ visitFileset(
+ artifact,
+ receiver,
+ fullyResolveFilesetLinks
+ ? RelativeSymlinkBehaviorWithoutError.RESOLVE_FULLY
+ : RelativeSymlinkBehaviorWithoutError.RESOLVE);
}
} else if (artifact.isTreeArtifact()) {
ImmutableCollection<? extends Artifact> expandedArtifacts =
@@ -133,17 +133,11 @@
private void visitFileset(
Artifact filesetArtifact,
ArtifactReceiver receiver,
- RelativeSymlinkBehavior relativeSymlinkBehavior) {
+ RelativeSymlinkBehaviorWithoutError relativeSymlinkBehavior) {
ImmutableList<FilesetOutputSymlink> links = expandedFilesets.get(filesetArtifact);
- FilesetManifest filesetManifest;
- try {
- filesetManifest =
- FilesetManifest.constructFilesetManifest(
- links, PathFragment.EMPTY_FRAGMENT, relativeSymlinkBehavior);
- } catch (IOException e) {
- // Unexpected: RelativeSymlinkBehavior.RESOLVE should not throw.
- throw new IllegalStateException(e);
- }
+ FilesetManifest filesetManifest =
+ FilesetManifest.constructFilesetManifestWithoutError(
+ links, PathFragment.EMPTY_FRAGMENT, relativeSymlinkBehavior);
for (Map.Entry<PathFragment, String> mapping : filesetManifest.getEntries().entrySet()) {
String targetFile = mapping.getValue();
@@ -165,8 +159,7 @@
ActionInputMap actionInputMap,
Map<Artifact, ImmutableCollection<? extends Artifact>> expandedArtifacts,
Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesets,
- String workspaceName)
- throws IOException;
+ String workspaceName);
boolean shouldCreatePathResolverForArtifactValues();
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/FilesetManifest.java b/src/main/java/com/google/devtools/build/lib/actions/FilesetManifest.java
index 66ffc85..6fdef5e 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/FilesetManifest.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/FilesetManifest.java
@@ -16,7 +16,6 @@
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.google.common.flogger.GoogleLogger;
-import com.google.devtools.build.lib.bugreport.BugReport;
import com.google.devtools.build.lib.vfs.DigestHashFunction;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
@@ -38,9 +37,7 @@
private static final int MAX_SYMLINK_TRAVERSALS = 256;
private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
- /**
- * Mode that determines how to handle relative target paths.
- */
+ /** Mode that determines how to handle relative target paths. */
public enum RelativeSymlinkBehavior {
/** Ignore any relative target paths. */
IGNORE,
@@ -52,14 +49,58 @@
RESOLVE,
/** Fully resolve all relative paths, even those pointing to internal directories. */
- RESOLVE_FULLY;
+ RESOLVE_FULLY
+ }
+
+ /**
+ * Shadow of {@link RelativeSymlinkBehavior} without the {@link RelativeSymlinkBehavior#ERROR}
+ * value for callers who know there won't be an error thrown when constructing the manifest.
+ */
+ public enum RelativeSymlinkBehaviorWithoutError {
+ /** Ignore any relative target paths. */
+ IGNORE(RelativeSymlinkBehavior.IGNORE),
+
+ /** Resolve all relative target paths. */
+ RESOLVE(RelativeSymlinkBehavior.RESOLVE),
+
+ /** Fully resolve all relative paths, even those pointing to internal directories. */
+ RESOLVE_FULLY(RelativeSymlinkBehavior.RESOLVE_FULLY);
+
+ private final RelativeSymlinkBehavior target;
+
+ RelativeSymlinkBehaviorWithoutError(RelativeSymlinkBehavior target) {
+ this.target = target;
+ }
+ }
+
+ /**
+ * Constructs a FilesetManifest from the given {@code outputSymlinks}, processing relative
+ * symlinks according to {@code relSymlinkBehavior}. Use when {@link
+ * RelativeSymlinkBehavior#ERROR} is guaranteed not to be the behavior.
+ */
+ public static FilesetManifest constructFilesetManifestWithoutError(
+ List<FilesetOutputSymlink> outputSymlinks,
+ PathFragment targetPrefix,
+ RelativeSymlinkBehaviorWithoutError relSymlinkBehavior) {
+ try {
+ return constructFilesetManifest(outputSymlinks, targetPrefix, relSymlinkBehavior.target);
+ } catch (ForbiddenRelativeSymlinkException e) {
+ throw new IllegalStateException(
+ "Can't throw forbidden symlink exception unless behavior is ERROR: "
+ + relSymlinkBehavior
+ + ", "
+ + targetPrefix
+ + ", "
+ + outputSymlinks,
+ e);
+ }
}
public static FilesetManifest constructFilesetManifest(
List<FilesetOutputSymlink> outputSymlinks,
PathFragment targetPrefix,
RelativeSymlinkBehavior relSymlinkBehavior)
- throws IOException {
+ throws ForbiddenRelativeSymlinkException {
LinkedHashMap<PathFragment, String> entries = new LinkedHashMap<>();
Map<PathFragment, String> relativeLinks = new HashMap<>();
Map<String, FileArtifactValue> artifactValues = new HashMap<>();
@@ -91,12 +132,10 @@
PathFragment fullLocation,
RelativeSymlinkBehavior relSymlinkBehavior,
Map<PathFragment, String> relativeLinks)
- throws IOException {
+ throws ForbiddenRelativeSymlinkException {
switch (relSymlinkBehavior) {
case ERROR:
- IOException ioException = new IOException("runfiles target is not absolute: " + artifact);
- BugReport.sendBugReport(ioException);
- throw ioException;
+ throw new ForbiddenRelativeSymlinkException(artifact);
case RESOLVE:
case RESOLVE_FULLY:
if (!relativeLinks.containsKey(fullLocation)) { // Keep consistent behavior: no overwriting.
@@ -110,8 +149,9 @@
/** Fully resolve relative symlinks including internal directory symlinks. */
private static void fullyResolveRelativeSymlinks(
- Map<PathFragment, String> entries, Map<PathFragment, String> relativeLinks, boolean absolute)
- throws IOException {
+ Map<PathFragment, String> entries,
+ Map<PathFragment, String> relativeLinks,
+ boolean absolute) {
try {
// Construct an in-memory Filesystem containing all the non-relative-symlink entries in the
// Fileset. Treat these as regular files in the filesystem whose contents are the "real"
@@ -141,15 +181,7 @@
addSymlinks(root, entries, absolute);
} catch (IOException e) {
- // TODO(janakr): make this crash hard if there are no bug reports.
- BugReport.sendBugReport(
- new IllegalStateException(
- "Unexpected IOException from InMemoryFileSystem operations for "
- + entries
- + " with "
- + relativeLinks,
- e));
- throw e;
+ throw new IllegalStateException("InMemoryFileSystem can't throw", e);
}
}
@@ -181,8 +213,7 @@
Map<PathFragment, String> entries,
Map<PathFragment, String> relativeLinks,
boolean absolute,
- RelativeSymlinkBehavior relSymlinkBehavior)
- throws IOException {
+ RelativeSymlinkBehavior relSymlinkBehavior) {
if (relSymlinkBehavior == RelativeSymlinkBehavior.RESOLVE_FULLY && !relativeLinks.isEmpty()) {
fullyResolveRelativeSymlinks(entries, relativeLinks, absolute);
} else if (relSymlinkBehavior == RelativeSymlinkBehavior.RESOLVE) {
@@ -258,4 +289,12 @@
public Map<String, FileArtifactValue> getArtifactValues() {
return artifactValues;
}
+
+ /** Exception indicating that a relative symlink was encountered but not permitted. */
+ public static final class ForbiddenRelativeSymlinkException
+ extends ForbiddenActionInputException {
+ private ForbiddenRelativeSymlinkException(String symlinkTarget) {
+ super("Fileset symlink " + symlinkTarget + " is not absolute");
+ }
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ForbiddenActionInputException.java b/src/main/java/com/google/devtools/build/lib/actions/ForbiddenActionInputException.java
new file mode 100644
index 0000000..1464e37
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/actions/ForbiddenActionInputException.java
@@ -0,0 +1,26 @@
+// 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.actions;
+
+/**
+ * Ancestor for exceptions thrown because of a bad input: an input that is not allowed for the given
+ * spawn/execution platform (like a relative symlink in a Fileset or a directory for a platform that
+ * does not support directory inputs). Indicates a user error, rather than an I/O error.
+ */
+public class ForbiddenActionInputException extends Exception {
+ protected ForbiddenActionInputException(String message) {
+ super(message);
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkCustomCommandLine.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkCustomCommandLine.java
index a5ad079..33e2701 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkCustomCommandLine.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkCustomCommandLine.java
@@ -27,7 +27,7 @@
import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.actions.CommandLineItem;
import com.google.devtools.build.lib.actions.FilesetManifest;
-import com.google.devtools.build.lib.actions.FilesetManifest.RelativeSymlinkBehavior;
+import com.google.devtools.build.lib.actions.FilesetManifest.RelativeSymlinkBehaviorWithoutError;
import com.google.devtools.build.lib.actions.FilesetOutputSymlink;
import com.google.devtools.build.lib.actions.SingleStringArgFormatter;
import com.google.devtools.build.lib.cmdline.Label;
@@ -39,7 +39,6 @@
import com.google.devtools.build.lib.starlarkbuildapi.FileRootApi;
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.vfs.PathFragment;
-import java.io.IOException;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.IllegalFormatException;
@@ -354,15 +353,11 @@
fileset),
e);
}
- try {
- FilesetManifest filesetManifest =
- FilesetManifest.constructFilesetManifest(
- expandedFileSet, fileset.getExecPath(), RelativeSymlinkBehavior.IGNORE);
- for (PathFragment relativePath : filesetManifest.getEntries().keySet()) {
- expandedValues.add(new FilesetSymlinkFile(fileset, relativePath));
- }
- } catch (IOException e) {
- throw new CommandLineExpansionException("Could not expand fileset: " + e.getMessage());
+ FilesetManifest filesetManifest =
+ FilesetManifest.constructFilesetManifestWithoutError(
+ expandedFileSet, fileset.getExecPath(), RelativeSymlinkBehaviorWithoutError.IGNORE);
+ for (PathFragment relativePath : filesetManifest.getEntries().keySet()) {
+ expandedValues.add(new FilesetSymlinkFile(fileset, relativePath));
}
}
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 079c54a..0cfb2b7 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
@@ -25,6 +25,7 @@
import com.google.devtools.build.lib.actions.ArtifactPathResolver;
import com.google.devtools.build.lib.actions.EnvironmentalExecException;
import com.google.devtools.build.lib.actions.ExecException;
+import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
import com.google.devtools.build.lib.actions.LostInputsActionExecutionException;
import com.google.devtools.build.lib.actions.LostInputsExecException;
import com.google.devtools.build.lib.actions.MetadataProvider;
@@ -36,6 +37,7 @@
import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.actions.SpawnResult.Status;
import com.google.devtools.build.lib.actions.Spawns;
+import com.google.devtools.build.lib.actions.UserExecException;
import com.google.devtools.build.lib.actions.cache.MetadataHandler;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
@@ -164,6 +166,13 @@
ex = e;
spawnResult = e.getSpawnResult();
// Log the Spawn and re-throw.
+ } catch (ForbiddenActionInputException e) {
+ throw new UserExecException(
+ e,
+ FailureDetail.newBuilder()
+ .setMessage("Exec failed due to forbidden input")
+ .setSpawn(FailureDetails.Spawn.newBuilder().setCode(Code.FORBIDDEN_INPUT))
+ .build());
}
SpawnLogContext spawnLogContext = actionExecutionContext.getContext(SpawnLogContext.class);
@@ -175,7 +184,7 @@
context.getInputMapping(PathFragment.EMPTY_FRAGMENT),
context.getTimeout(),
spawnResult);
- } catch (IOException e) {
+ } catch (IOException | ForbiddenActionInputException e) {
actionExecutionContext
.getEventHandler()
.handle(
@@ -232,7 +241,8 @@
}
@Override
- public void prefetchInputs() throws IOException, InterruptedException {
+ public void prefetchInputs()
+ throws IOException, ForbiddenActionInputException, InterruptedException {
if (Spawns.shouldPrefetchInputsForLocalExecution(spawn)) {
actionExecutionContext
.getActionInputPrefetcher()
@@ -290,7 +300,7 @@
@Override
public SortedMap<PathFragment, ActionInput> getInputMapping(PathFragment baseDirectory)
- throws IOException {
+ throws IOException, ForbiddenActionInputException {
if (lazyInputMapping == null || !inputMappingBaseDirectory.equals(baseDirectory)) {
try (SilentCloseable c =
Profiler.instance().profile("AbstractSpawnStrategy.getInputMapping")) {
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnCache.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnCache.java
index b47c867..46429eb4 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/SpawnCache.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnCache.java
@@ -15,6 +15,7 @@
import com.google.devtools.build.lib.actions.ActionContext;
import com.google.devtools.build.lib.actions.ExecException;
+import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext;
@@ -164,7 +165,7 @@
* instance's {@link CacheHandle#store} is a no-op.
*/
CacheHandle lookup(Spawn spawn, SpawnExecutionContext context)
- throws ExecException, IOException, InterruptedException;
+ throws ExecException, IOException, InterruptedException, ForbiddenActionInputException;
/**
* Returns whether this cache implementation makes sense to use together with dynamic execution.
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java
index c3c8783..545e952 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java
@@ -25,8 +25,10 @@
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.actions.FileArtifactValue;
import com.google.devtools.build.lib.actions.FilesetManifest;
+import com.google.devtools.build.lib.actions.FilesetManifest.ForbiddenRelativeSymlinkException;
import com.google.devtools.build.lib.actions.FilesetManifest.RelativeSymlinkBehavior;
import com.google.devtools.build.lib.actions.FilesetOutputSymlink;
+import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
import com.google.devtools.build.lib.actions.MetadataProvider;
import com.google.devtools.build.lib.actions.RunfilesSupplier;
import com.google.devtools.build.lib.actions.Spawn;
@@ -110,7 +112,7 @@
MetadataProvider actionFileCache,
ArtifactExpander artifactExpander,
PathFragment baseDirectory)
- throws IOException {
+ throws IOException, ForbiddenActionInputException {
Map<PathFragment, Map<PathFragment, Artifact>> rootsAndMappings =
runfilesSupplier.getMappings();
@@ -161,7 +163,7 @@
MetadataProvider actionFileCache,
ArtifactExpander artifactExpander,
PathFragment baseDirectory)
- throws IOException {
+ throws IOException, ForbiddenActionInputException {
Map<PathFragment, ActionInput> inputMap = new HashMap<>();
addRunfilesToInputs(
inputMap, runfilesSupplier, actionFileCache, artifactExpander, baseDirectory);
@@ -169,10 +171,10 @@
}
private static void failIfDirectory(MetadataProvider actionFileCache, ActionInput input)
- throws IOException {
+ throws IOException, ForbiddenActionInputException {
FileArtifactValue metadata = actionFileCache.getMetadata(input);
if (metadata != null && !metadata.getType().isFile()) {
- throw new IOException("Not a file: " + input.getExecPathString());
+ throw new ForbiddenNonFileException(input);
}
}
@@ -181,7 +183,7 @@
Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesetMappings,
Map<PathFragment, ActionInput> inputMappings,
PathFragment baseDirectory)
- throws IOException {
+ throws ForbiddenRelativeSymlinkException {
for (Artifact fileset : filesetMappings.keySet()) {
addFilesetManifest(
fileset.getExecPath(),
@@ -198,7 +200,7 @@
ImmutableList<FilesetOutputSymlink> filesetLinks,
Map<PathFragment, ActionInput> inputMappings,
PathFragment baseDirectory)
- throws IOException {
+ throws ForbiddenRelativeSymlinkException {
Preconditions.checkState(filesetArtifact.isFileset(), filesetArtifact);
FilesetManifest filesetManifest =
FilesetManifest.constructFilesetManifest(filesetLinks, location, relSymlinkBehavior);
@@ -230,9 +232,7 @@
* {@link ActionInput}s. The returned map does not contain tree artifacts as they are expanded to
* file artifacts.
*
- * <p>The returned map never contains {@code null} values; it uses {@link #EMPTY_FILE} for empty
- * files, which is an instance of {@link
- * com.google.devtools.build.lib.actions.cache.VirtualActionInput}.
+ * <p>The returned map never contains {@code null} values.
*
* <p>The returned map contains all runfiles, but not the {@code MANIFEST}.
*/
@@ -241,7 +241,7 @@
ArtifactExpander artifactExpander,
PathFragment baseDirectory,
MetadataProvider actionInputFileCache)
- throws IOException {
+ throws IOException, ForbiddenActionInputException {
TreeMap<PathFragment, ActionInput> inputMap = new TreeMap<>();
addInputs(inputMap, spawn, artifactExpander, baseDirectory);
addRunfilesToInputs(
@@ -253,4 +253,14 @@
addFilesetManifests(spawn.getFilesetMappings(), inputMap, baseDirectory);
return inputMap;
}
+
+ /**
+ * Exception signaling that an input was not a regular file: most likely a directory. This
+ * exception is currently never thrown in practice since we do not enforce "strict" mode.
+ */
+ private static final class ForbiddenNonFileException extends ForbiddenActionInputException {
+ ForbiddenNonFileException(ActionInput input) {
+ super("Not a file: " + input.getExecPathString());
+ }
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java
index 7b79837..185cfcd 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java
@@ -18,6 +18,7 @@
import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
import com.google.devtools.build.lib.actions.ArtifactPathResolver;
import com.google.devtools.build.lib.actions.ExecException;
+import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
import com.google.devtools.build.lib.actions.FutureSpawn;
import com.google.devtools.build.lib.actions.LostInputsExecException;
import com.google.devtools.build.lib.actions.MetadataProvider;
@@ -159,7 +160,7 @@
* again. I suppose we could require implementations to memoize getInputMapping (but not compute
* it eagerly), and that may change in the future.
*/
- void prefetchInputs() throws IOException, InterruptedException;
+ void prefetchInputs() throws IOException, InterruptedException, ForbiddenActionInputException;
/**
* The input file metadata cache for this specific spawn, which can be used to efficiently
@@ -210,7 +211,7 @@
* is not the same as the execroot.
*/
SortedMap<PathFragment, ActionInput> getInputMapping(PathFragment baseDirectory)
- throws IOException;
+ throws IOException, ForbiddenActionInputException;
/** Reports a progress update to the Spawn strategy. */
void report(ProgressStatus state, String name);
@@ -247,7 +248,7 @@
* @throws ExecException if the request is malformed
*/
default FutureSpawn execAsync(Spawn spawn, SpawnExecutionContext context)
- throws InterruptedException, IOException, ExecException {
+ throws InterruptedException, IOException, ExecException, ForbiddenActionInputException {
// TODO(ulfjack): Remove this default implementation. [exec-async]
return FutureSpawn.immediate(exec(spawn, context));
}
@@ -264,7 +265,7 @@
* @throws ExecException if the request is malformed
*/
SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
- throws InterruptedException, IOException, ExecException;
+ throws InterruptedException, IOException, ExecException, ForbiddenActionInputException;
/** Returns whether this SpawnRunner supports executing the given Spawn. */
boolean canExec(Spawn spawn);
diff --git a/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java
index 2ad58e1..613c3dd 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java
@@ -28,6 +28,7 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.CommandLines.ParamFileActionInput;
import com.google.devtools.build.lib.actions.ExecException;
+import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
import com.google.devtools.build.lib.actions.ResourceManager;
import com.google.devtools.build.lib.actions.ResourceManager.ResourceHandle;
import com.google.devtools.build.lib.actions.Spawn;
@@ -118,7 +119,7 @@
@Override
public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
- throws IOException, InterruptedException, ExecException {
+ throws IOException, InterruptedException, ExecException, ForbiddenActionInputException {
runfilesTreeUpdater.updateRunfilesDirectory(
execRoot,
@@ -186,7 +187,8 @@
setState(State.PARSING);
}
- public SpawnResult run() throws InterruptedException, IOException {
+ public SpawnResult run()
+ throws InterruptedException, IOException, ForbiddenActionInputException {
if (localExecutionOptions.localRetriesOnCrash == 0) {
return runOnce();
} else {
@@ -212,7 +214,8 @@
}
}
- private SpawnResult runOnce() throws InterruptedException, IOException {
+ private SpawnResult runOnce()
+ throws InterruptedException, IOException, ForbiddenActionInputException {
try {
return start();
} catch (InterruptedException | InterruptedIOException e) {
@@ -224,6 +227,9 @@
} catch (Error e) {
stepLog(SEVERE, e, UNHANDLED_EXCEPTION_MSG);
throw e;
+ } catch (ForbiddenActionInputException e) {
+ stepLog(WARNING, e, "Bad input file");
+ throw e;
} catch (IOException e) {
stepLog(SEVERE, e, "Local I/O error");
throw e;
@@ -275,7 +281,8 @@
}
/** Parse the request and run it locally. */
- private SpawnResult start() throws InterruptedException, IOException {
+ private SpawnResult start()
+ throws InterruptedException, IOException, ForbiddenActionInputException {
logger.atInfo().log("starting local subprocess #%d, argv: %s", id, debugCmdString());
FileOutErr outErr = context.getFileOutErr();
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
index aecd293..060e20b 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
@@ -38,6 +38,7 @@
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ExecException;
+import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.Spawns;
import com.google.devtools.build.lib.actions.UserExecException;
@@ -215,7 +216,7 @@
/** Creates a new {@link RemoteAction} instance from spawn. */
public RemoteAction buildRemoteAction(Spawn spawn, SpawnExecutionContext context)
- throws IOException, UserExecException {
+ throws IOException, UserExecException, ForbiddenActionInputException {
SortedMap<PathFragment, ActionInput> inputMap = remotePathResolver.getInputMapping(context);
final MerkleTree merkleTree =
MerkleTree.build(inputMap, context.getMetadataProvider(), execRoot, digestUtil);
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
index d6b3e1c..340210b 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
@@ -22,6 +22,7 @@
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.FileArtifactValue;
+import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnMetrics;
import com.google.devtools.build.lib.actions.SpawnResult;
@@ -76,7 +77,7 @@
@Override
public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context)
- throws InterruptedException, IOException, ExecException {
+ throws InterruptedException, IOException, ExecException, ForbiddenActionInputException {
if (!Spawns.mayBeCached(spawn)
|| (!Spawns.mayBeCachedRemotely(spawn) && useRemoteCache(options))) {
// returning SpawnCache.NO_RESULT_NO_STORE in case the caching is disabled or in case
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
index 0dc4c8a..ea31f5d 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
@@ -32,6 +32,7 @@
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.CommandLines.ParamFileActionInput;
import com.google.devtools.build.lib.actions.ExecException;
+import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnMetrics;
import com.google.devtools.build.lib.actions.SpawnResult;
@@ -155,7 +156,7 @@
@Override
public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
- throws ExecException, InterruptedException, IOException {
+ throws ExecException, InterruptedException, IOException, ForbiddenActionInputException {
Preconditions.checkArgument(
Spawns.mayBeExecutedRemotely(spawn), "Spawn can't be executed remotely. This is a bug.");
@@ -395,7 +396,7 @@
}
private SpawnResult execLocally(Spawn spawn, SpawnExecutionContext context)
- throws ExecException, InterruptedException, IOException {
+ throws ExecException, InterruptedException, IOException, ForbiddenActionInputException {
RemoteLocalFallbackRegistry localFallbackRegistry =
context.getContext(RemoteLocalFallbackRegistry.class);
checkNotNull(localFallbackRegistry, "Expected a RemoteLocalFallbackRegistry to be registered");
@@ -413,7 +414,7 @@
SpawnExecutionContext context,
boolean uploadLocalResults,
IOException cause)
- throws ExecException, InterruptedException, IOException {
+ throws ExecException, InterruptedException, IOException, ForbiddenActionInputException {
// Regardless of cause, if we are interrupted, we should stop without displaying a user-visible
// failure/stack trace.
if (Thread.currentThread().isInterrupted()) {
@@ -520,7 +521,7 @@
@VisibleForTesting
SpawnResult execLocallyAndUpload(
RemoteAction action, Spawn spawn, SpawnExecutionContext context, boolean uploadLocalResults)
- throws ExecException, IOException, InterruptedException {
+ throws ExecException, IOException, ForbiddenActionInputException, InterruptedException {
Map<Path, Long> ctimesBefore = getInputCtimes(action.getInputMap());
SpawnResult result = execLocally(spawn, context);
Map<Path, Long> ctimesAfter = getInputCtimes(action.getInputMap());
diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/RemotePathResolver.java b/src/main/java/com/google/devtools/build/lib/remote/common/RemotePathResolver.java
index d067820..5b063c6 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/common/RemotePathResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/common/RemotePathResolver.java
@@ -17,6 +17,7 @@
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.ActionInputHelper;
+import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -40,7 +41,7 @@
* ActionInput}.
*/
SortedMap<PathFragment, ActionInput> getInputMapping(SpawnExecutionContext context)
- throws IOException;
+ throws IOException, ForbiddenActionInputException;
/** Resolves the output path relative to input root for the given {@link Path}. */
String localPathToOutputPath(Path path);
@@ -94,7 +95,7 @@
@Override
public SortedMap<PathFragment, ActionInput> getInputMapping(SpawnExecutionContext context)
- throws IOException {
+ throws IOException, ForbiddenActionInputException {
return context.getInputMapping(PathFragment.EMPTY_FRAGMENT);
}
@@ -151,7 +152,7 @@
@Override
public SortedMap<PathFragment, ActionInput> getInputMapping(SpawnExecutionContext context)
- throws IOException {
+ throws IOException, ForbiddenActionInputException {
// The "root directory" of the action from the point of view of RBE is the parent directory of
// the execroot locally. This is so that paths of artifacts in external repositories don't
// start with an uplevel reference.
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 3d14c22..f7e03f0 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
@@ -21,6 +21,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ActionExecutionMetadata;
import com.google.devtools.build.lib.actions.ExecException;
+import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
import com.google.devtools.build.lib.actions.ResourceManager;
import com.google.devtools.build.lib.actions.ResourceManager.ResourceHandle;
import com.google.devtools.build.lib.actions.Spawn;
@@ -91,6 +92,11 @@
createFailureDetail(
"I/O exception during sandboxed execution", Code.EXECUTION_IO_EXCEPTION);
throw new UserExecException(e, failureDetail);
+ } catch (ForbiddenActionInputException e) {
+ FailureDetail failureDetail =
+ createFailureDetail(
+ "Forbidden input found during sandboxed execution", Code.FORBIDDEN_INPUT);
+ throw new UserExecException(e, failureDetail);
}
}
@@ -105,11 +111,11 @@
}
protected abstract SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context)
- throws IOException, ExecException, InterruptedException;
+ throws IOException, ExecException, InterruptedException, ForbiddenActionInputException;
private SpawnResult runSpawn(
Spawn originalSpawn, SandboxedSpawn sandbox, SpawnExecutionContext context)
- throws IOException, InterruptedException {
+ throws IOException, ForbiddenActionInputException, InterruptedException {
try {
try (SilentCloseable c = Profiler.instance().profile("sandbox.createFileSystem")) {
sandbox.createFileSystem();
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java
index c32638e..f510d35 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java
@@ -21,6 +21,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.io.ByteStreams;
+import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.Spawns;
import com.google.devtools.build.lib.exec.TreeDeleter;
@@ -206,7 +207,7 @@
@Override
protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context)
- throws IOException, InterruptedException {
+ throws IOException, ForbiddenActionInputException, InterruptedException {
// Each invocation of "exec" gets its own sandbox base.
// Note that the value returned by context.getId() is only unique inside one given SpawnRunner,
// so we have to prefix our name to turn it into a globally unique value.
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DockerSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/DockerSandboxedSpawnRunner.java
index 13f09e5..2fda26a 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/DockerSandboxedSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/DockerSandboxedSpawnRunner.java
@@ -23,6 +23,7 @@
import com.google.common.eventbus.Subscribe;
import com.google.common.io.ByteStreams;
import com.google.devtools.build.lib.actions.ExecException;
+import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.Spawns;
import com.google.devtools.build.lib.actions.UserExecException;
@@ -203,7 +204,7 @@
@Override
protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context)
- throws IOException, ExecException, InterruptedException {
+ throws IOException, ExecException, InterruptedException, ForbiddenActionInputException {
// Each invocation of "exec" gets its own sandbox base, execroot and temporary directory.
Path sandboxPath =
sandboxBase.getRelative(getName()).getRelative(Integer.toString(context.getId()));
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java
index 96158ed..79ce0e0 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java
@@ -21,6 +21,7 @@
import com.google.common.io.ByteStreams;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.ExecutionRequirements;
+import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.Spawns;
import com.google.devtools.build.lib.actions.UserExecException;
@@ -154,7 +155,7 @@
@Override
protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context)
- throws IOException, ExecException, InterruptedException {
+ throws IOException, ForbiddenActionInputException, ExecException, InterruptedException {
// Each invocation of "exec" gets its own sandbox base.
// Note that the value returned by context.getId() is only unique inside one given SpawnRunner,
// so we have to prefix our name to turn it into a globally unique value.
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java
index dff0d99..356fc67 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java
@@ -16,6 +16,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.exec.TreeDeleter;
import com.google.devtools.build.lib.exec.local.LocalEnvProvider;
@@ -76,7 +77,7 @@
@Override
protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context)
- throws IOException, InterruptedException {
+ throws IOException, ForbiddenActionInputException, InterruptedException {
// Each invocation of "exec" gets its own sandbox base.
// Note that the value returned by context.getId() is only unique inside one given SpawnRunner,
// so we have to prefix our name to turn it into a globally unique value.
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java
index 87542fb..d64ebcd 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java
@@ -22,6 +22,7 @@
import com.google.common.eventbus.Subscribe;
import com.google.common.flogger.GoogleLogger;
import com.google.devtools.build.lib.actions.ExecException;
+import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnExecutedEvent;
import com.google.devtools.build.lib.actions.SpawnResult;
@@ -482,7 +483,7 @@
@Override
public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
- throws InterruptedException, IOException, ExecException {
+ throws InterruptedException, IOException, ExecException, ForbiddenActionInputException {
Instant spawnExecutionStartInstant = Instant.now();
SpawnResult spawnResult;
if (sandboxSpawnRunner.canExec(spawn)) {
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxedSpawnRunner.java
index 846090f..f602fab 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxedSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxedSpawnRunner.java
@@ -18,6 +18,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ActionInput;
+import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.exec.local.LocalEnvProvider;
import com.google.devtools.build.lib.exec.local.WindowsLocalEnvProvider;
@@ -60,7 +61,7 @@
@Override
protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context)
- throws IOException, InterruptedException {
+ throws IOException, ForbiddenActionInputException, InterruptedException {
Path tmpDir = createActionTemp(execRoot);
Path commandTmpDir = tmpDir.getRelative("work");
commandTmpDir.createDirectory();
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
index 5bc72d2..39293ca 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
@@ -36,7 +36,7 @@
import com.google.devtools.build.lib.actions.FileStateType;
import com.google.devtools.build.lib.actions.FileStateValue;
import com.google.devtools.build.lib.actions.FilesetManifest;
-import com.google.devtools.build.lib.actions.FilesetManifest.RelativeSymlinkBehavior;
+import com.google.devtools.build.lib.actions.FilesetManifest.RelativeSymlinkBehaviorWithoutError;
import com.google.devtools.build.lib.actions.FilesetOutputSymlink;
import com.google.devtools.build.lib.actions.cache.MetadataHandler;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
@@ -203,22 +203,15 @@
Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesets, PathFragment execRoot) {
Map<PathFragment, FileArtifactValue> filesetMap = new HashMap<>();
for (Map.Entry<Artifact, ImmutableList<FilesetOutputSymlink>> entry : filesets.entrySet()) {
- try {
- FilesetManifest fileset =
- FilesetManifest.constructFilesetManifest(
- entry.getValue(), execRoot, RelativeSymlinkBehavior.RESOLVE);
+ FilesetManifest fileset =
+ FilesetManifest.constructFilesetManifestWithoutError(
+ entry.getValue(), execRoot, RelativeSymlinkBehaviorWithoutError.RESOLVE);
for (Map.Entry<String, FileArtifactValue> favEntry :
fileset.getArtifactValues().entrySet()) {
if (favEntry.getValue().getDigest() != null) {
filesetMap.put(PathFragment.create(favEntry.getKey()), favEntry.getValue());
}
}
- } catch (IOException e) {
- // If we cannot get the FileArtifactValue, then we will make a FileSystem call to get the
- // digest, so it is okay to skip and continue here.
- logger.atWarning().withCause(e).log(
- "Could not properly get digest for %s", entry.getKey().getExecPath());
- }
}
return ImmutableMap.copyOf(filesetMap);
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
index 251f442..6632349 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
@@ -54,7 +54,6 @@
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.build.skyframe.ValueOrException2;
-import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
import javax.annotation.Nullable;
@@ -240,20 +239,16 @@
}
final CompletionContext ctx;
- try {
- ctx =
- CompletionContext.create(
- expandedArtifacts,
- expandedFilesets,
- key.topLevelArtifactContext().expandFilesets(),
- key.topLevelArtifactContext().fullyResolveFilesetSymlinks(),
- inputMap,
- pathResolverFactory,
- skyframeActionExecutor.getExecRoot(),
- workspaceNameValue.getName());
- } catch (IOException e) {
- throw new CompletionFunctionException(e);
- }
+ ctx =
+ CompletionContext.create(
+ expandedArtifacts,
+ expandedFilesets,
+ key.topLevelArtifactContext().expandFilesets(),
+ key.topLevelArtifactContext().fullyResolveFilesetSymlinks(),
+ inputMap,
+ pathResolverFactory,
+ skyframeActionExecutor.getExecRoot(),
+ workspaceNameValue.getName());
if (!rootCauses.isEmpty()) {
ImmutableMap<String, ArtifactsInOutputGroup> builtOutputs =
@@ -336,7 +331,6 @@
}
private static final class CompletionFunctionException extends SkyFunctionException {
-
private final ActionExecutionException actionException;
CompletionFunctionException(ActionExecutionException e) {
@@ -350,11 +344,6 @@
this.actionException = null;
}
- CompletionFunctionException(IOException e) {
- super(e, Transience.TRANSIENT);
- this.actionException = null;
- }
-
@Override
public boolean isCatastrophic() {
return actionException != null && actionException.isCatastrophe();
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
index 810784b..1a024fc 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
@@ -356,21 +356,11 @@
}
private void updateActionFileSystemContext(
- Action action,
FileSystem actionFileSystem,
Environment env,
MetadataInjector metadataInjector,
- ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> filesets)
- throws ActionExecutionException {
- try {
- outputService.updateActionFileSystemContext(
- actionFileSystem, env, metadataInjector, filesets);
- } catch (IOException e) {
- String message = "Failed to update filesystem context: " + e.getMessage();
- DetailedExitCode code =
- createDetailedExitCode(message, Code.FILESYSTEM_CONTEXT_UPDATE_FAILURE);
- throw new ActionExecutionException(message, e, action, /*catastrophe=*/ false, code);
- }
+ ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> filesets) {
+ outputService.updateActionFileSystemContext(actionFileSystem, env, metadataInjector, filesets);
}
void executionOver() {
@@ -466,8 +456,7 @@
boolean hasDiscoveredInputs)
throws ActionExecutionException, InterruptedException {
if (actionFileSystem != null) {
- updateActionFileSystemContext(
- action, actionFileSystem, env, metadataHandler, expandedFilesets);
+ updateActionFileSystemContext(actionFileSystem, env, metadataHandler, expandedFilesets);
}
ActionExecutionContext actionExecutionContext =
@@ -787,7 +776,6 @@
syscalls.get());
if (actionFileSystem != null) {
updateActionFileSystemContext(
- action,
actionFileSystem,
env,
THROWING_METADATA_INJECTOR_FOR_ACTIONFS,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index ea71094..6a2f6f5 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -208,7 +208,6 @@
import com.google.devtools.common.options.OptionsParsingException;
import com.google.devtools.common.options.OptionsProvider;
import com.google.errorprone.annotations.ForOverride;
-import java.io.IOException;
import java.io.PrintStream;
import java.util.ArrayList;
import java.util.Collection;
@@ -388,8 +387,7 @@
ActionInputMap actionInputMap,
Map<Artifact, ImmutableCollection<? extends Artifact>> expandedArtifacts,
Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesets,
- String workspaceName)
- throws IOException {
+ String workspaceName) {
Preconditions.checkState(shouldCreatePathResolverForArtifactValues());
return outputService.createPathResolverForArtifactValues(
directories.getExecRoot(workspaceName).asFragment(),
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java b/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java
index 4c9f255..5155be8 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java
@@ -183,8 +183,7 @@
FileSystem actionFileSystem,
Environment env,
MetadataInjector injector,
- ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> filesets)
- throws IOException {}
+ ImmutableMap<Artifact, ImmutableList<FilesetOutputSymlink>> filesets) {}
/**
* Checks the filesystem returned by {@link #createActionFileSystem} for errors attributable to
@@ -204,8 +203,7 @@
ImmutableList<Root> pathEntries,
ActionInputMap actionInputMap,
Map<Artifact, ImmutableCollection<? extends Artifact>> expandedArtifacts,
- Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesets)
- throws IOException {
+ Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesets) {
throw new IllegalStateException("Path resolver not supported by this class");
}
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java
index ca6a3b0..7729b5d 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java
@@ -28,6 +28,7 @@
import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.ExecutionRequirements.WorkerProtocolFormat;
+import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
import com.google.devtools.build.lib.actions.MetadataProvider;
import com.google.devtools.build.lib.actions.ResourceManager;
import com.google.devtools.build.lib.actions.ResourceManager.ResourceHandle;
@@ -139,7 +140,7 @@
@Override
public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
- throws ExecException, IOException, InterruptedException {
+ throws ExecException, IOException, InterruptedException, ForbiddenActionInputException {
context.report(
ProgressStatus.SCHEDULING,
WorkerKey.makeWorkerTypeName(
@@ -400,6 +401,9 @@
restoreInterrupt(e);
String message = "IOException while prefetching for worker:";
throw createUserExecException(e, message, Code.PREFETCH_FAILURE);
+ } catch (ForbiddenActionInputException e) {
+ throw createUserExecException(
+ e, "Forbidden input found while prefetching for worker:", Code.FORBIDDEN_INPUT);
}
Duration setupInputsTime = setupInputsStopwatch.elapsed();
@@ -599,6 +603,12 @@
ErrorMessage.builder().message(message).exception(e).build().toString(), detailedCode);
}
+ private static UserExecException createUserExecException(
+ ForbiddenActionInputException e, String message, Code detailedCode) {
+ return createUserExecException(
+ ErrorMessage.builder().message(message).exception(e).build().toString(), detailedCode);
+ }
+
private static UserExecException createUserExecException(String message, Code detailedCode) {
return new UserExecException(
FailureDetail.newBuilder()
diff --git a/src/main/protobuf/failure_details.proto b/src/main/protobuf/failure_details.proto
index d4deeda..ab5580d 100644
--- a/src/main/protobuf/failure_details.proto
+++ b/src/main/protobuf/failure_details.proto
@@ -221,6 +221,7 @@
// TODO(b/138456686): this code should be deprecated when SpawnResult is
// refactored to prohibit undetailed failures
UNSPECIFIED_EXECUTION_FAILURE = 12 [(metadata) = { exit_code: 1 }];
+ FORBIDDEN_INPUT = 13 [(metadata) = { exit_code: 1 }];
}
Code code = 1;
@@ -713,7 +714,7 @@
enum Code {
SANDBOX_FAILURE_UNKNOWN = 0 [(metadata) = { exit_code: 37 }];
INITIALIZATION_FAILURE = 1 [(metadata) = { exit_code: 36 }];
- EXECUTION_IO_EXCEPTION = 2 [(metadata) = { exit_code: 1 }];
+ EXECUTION_IO_EXCEPTION = 2 [(metadata) = { exit_code: 36 }];
DOCKER_COMMAND_FAILURE = 3 [(metadata) = { exit_code: 1 }];
NO_DOCKER_IMAGE = 4 [(metadata) = { exit_code: 1 }];
DOCKER_IMAGE_PREPARATION_FAILURE = 5 [(metadata) = { exit_code: 1 }];
@@ -722,6 +723,7 @@
MOUNT_SOURCE_TARGET_TYPE_MISMATCH = 8 [(metadata) = { exit_code: 1 }];
MOUNT_TARGET_DOES_NOT_EXIST = 9 [(metadata) = { exit_code: 1 }];
SUBPROCESS_START_FAILED = 10 [(metadata) = { exit_code: 36 }];
+ FORBIDDEN_INPUT = 11 [(metadata) = { exit_code: 1 }];
}
Code code = 1;
@@ -1123,12 +1125,13 @@
NO_FLAGFILE = 4 [(metadata) = { exit_code: 1 }];
VIRTUAL_INPUT_MATERIALIZATION_FAILURE = 5 [(metadata) = { exit_code: 1 }];
BORROW_FAILURE = 6 [(metadata) = { exit_code: 1 }];
- PREFETCH_FAILURE = 7 [(metadata) = { exit_code: 1 }];
+ PREFETCH_FAILURE = 7 [(metadata) = { exit_code: 36 }];
PREPARE_FAILURE = 8 [(metadata) = { exit_code: 1 }];
REQUEST_FAILURE = 9 [(metadata) = { exit_code: 1 }];
PARSE_RESPONSE_FAILURE = 10 [(metadata) = { exit_code: 1 }];
NO_RESPONSE = 11 [(metadata) = { exit_code: 1 }];
FINISH_FAILURE = 12 [(metadata) = { exit_code: 1 }];
+ FORBIDDEN_INPUT = 13 [(metadata) = { exit_code: 1 }];
}
Code code = 1;
diff --git a/src/test/java/com/google/devtools/build/lib/exec/FilesetManifestTest.java b/src/test/java/com/google/devtools/build/lib/exec/FilesetManifestTest.java
index e8fbf18..782395f 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/FilesetManifestTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/FilesetManifestTest.java
@@ -29,7 +29,6 @@
import com.google.devtools.build.lib.exec.FilesetManifestTest.OneOffManifestTests;
import com.google.devtools.build.lib.exec.FilesetManifestTest.ResolvingManifestTests;
import com.google.devtools.build.lib.vfs.PathFragment;
-import java.io.IOException;
import java.util.List;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -170,18 +169,13 @@
List<FilesetOutputSymlink> symlinks =
ImmutableList.of(filesetSymlink("bar", "foo"), filesetSymlink("foo", "/foo/bar"));
- // Because BugReport throws in tests, we catch the wrapped exception.
- IllegalStateException e =
+ FilesetManifest.ForbiddenRelativeSymlinkException e =
assertThrows(
- IllegalStateException.class,
+ FilesetManifest.ForbiddenRelativeSymlinkException.class,
() ->
FilesetManifest.constructFilesetManifest(
symlinks, PathFragment.create("out/foo"), ERROR));
- assertThat(e).hasCauseThat().isInstanceOf(IOException.class);
- assertThat(e)
- .hasCauseThat()
- .hasMessageThat()
- .contains("runfiles target is not absolute: foo");
+ assertThat(e).hasMessageThat().contains("Fileset symlink foo is not absolute");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java b/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java
index 17cb13a..0ca931f 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java
@@ -33,7 +33,9 @@
import com.google.devtools.build.lib.actions.ArtifactRoot.RootType;
import com.google.devtools.build.lib.actions.EmptyRunfilesSupplier;
import com.google.devtools.build.lib.actions.FileArtifactValue;
+import com.google.devtools.build.lib.actions.FilesetManifest;
import com.google.devtools.build.lib.actions.FilesetOutputSymlink;
+import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
import com.google.devtools.build.lib.actions.RunfilesSupplier;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.cache.VirtualActionInput;
@@ -154,9 +156,9 @@
FakeActionInputFileCache mockCache = new FakeActionInputFileCache();
mockCache.put(artifact, FileArtifactValue.createForDirectoryWithMtime(-1));
- IOException expected =
+ ForbiddenActionInputException expected =
assertThrows(
- IOException.class,
+ ForbiddenActionInputException.class,
() ->
expander.addRunfilesToInputs(
inputMappings,
@@ -463,15 +465,13 @@
@Test
public void testManifestWithErrorOnRelativeSymlink() {
expander = new SpawnInputExpander(execRoot, /*strict=*/ true, ERROR);
- // Because BugReport throws in tests, we catch the wrapped exception.
- IllegalStateException e =
+ FilesetManifest.ForbiddenRelativeSymlinkException e =
assertThrows(
- IllegalStateException.class,
+ FilesetManifest.ForbiddenRelativeSymlinkException.class,
() ->
expander.addFilesetManifests(
simpleFilesetManifest(), inputMappings, PathFragment.EMPTY_FRAGMENT));
- assertThat(e).hasCauseThat().isInstanceOf(IOException.class);
- assertThat(e).hasCauseThat().hasMessageThat().contains("runfiles target is not absolute: foo");
+ assertThat(e).hasMessageThat().contains("Fileset symlink foo is not absolute");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java
index 69b54bd..9935cfd 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java
@@ -38,6 +38,7 @@
import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
import com.google.devtools.build.lib.actions.ExecutionRequirements;
+import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
import com.google.devtools.build.lib.actions.MetadataProvider;
import com.google.devtools.build.lib.actions.ResourceSet;
import com.google.devtools.build.lib.actions.SimpleSpawn;
@@ -158,7 +159,7 @@
@Override
public SortedMap<PathFragment, ActionInput> getInputMapping(PathFragment baseDirectory)
- throws IOException {
+ throws IOException, ForbiddenActionInputException {
return new SpawnInputExpander(execRoot, /*strict*/ false)
.getInputMapping(simpleSpawn, SIMPLE_ARTIFACT_EXPANDER, baseDirectory, fakeFileCache);
}
diff --git a/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java b/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java
index 28d70a4..d7cbf99 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java
@@ -19,6 +19,7 @@
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
+import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
import com.google.devtools.build.lib.actions.MetadataProvider;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.cache.MetadataInjector;
@@ -113,7 +114,7 @@
@Override
public SortedMap<PathFragment, ActionInput> getInputMapping(PathFragment baseDirectory)
- throws IOException {
+ throws IOException, ForbiddenActionInputException {
return new SpawnInputExpander(execRoot, /*strict*/ false)
.getInputMapping(spawn, this::artifactExpander, baseDirectory, metadataProvider);
}
