Adds an action-scoped filesystem.
PiperOrigin-RevId: 195973862
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java
index c67645d..5f1e051 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java
@@ -29,6 +29,7 @@
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunction.Environment;
import com.google.devtools.common.options.OptionsClassProvider;
@@ -51,9 +52,10 @@
private final ImmutableMap<String, String> clientEnv;
private final ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>>
inputFilesetMappings;
- private final ArtifactExpander artifactExpander;
- @Nullable
- private final Environment env;
+ @Nullable private final ArtifactExpander artifactExpander;
+ @Nullable private final Environment env;
+
+ @Nullable private final FileSystem actionFileSystem;
@Nullable private ImmutableList<FilesetOutputSymlink> outputSymlinks;
@@ -67,7 +69,8 @@
Map<String, String> clientEnv,
ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> inputFilesetMappings,
@Nullable ArtifactExpander artifactExpander,
- @Nullable SkyFunction.Environment env) {
+ @Nullable SkyFunction.Environment env,
+ @Nullable FileSystem actionFileSystem) {
this.actionInputFileCache = actionInputFileCache;
this.actionInputPrefetcher = actionInputPrefetcher;
this.actionKeyContext = actionKeyContext;
@@ -78,6 +81,7 @@
this.executor = executor;
this.artifactExpander = artifactExpander;
this.env = env;
+ this.actionFileSystem = actionFileSystem;
}
public ActionExecutionContext(
@@ -89,7 +93,8 @@
FileOutErr fileOutErr,
Map<String, String> clientEnv,
ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> inputFilesetMappings,
- ArtifactExpander artifactExpander) {
+ ArtifactExpander artifactExpander,
+ @Nullable FileSystem actionFileSystem) {
this(
executor,
actionInputFileCache,
@@ -100,7 +105,8 @@
clientEnv,
inputFilesetMappings,
artifactExpander,
- null);
+ /*env=*/ null,
+ actionFileSystem);
}
public static ActionExecutionContext forInputDiscovery(
@@ -111,7 +117,8 @@
MetadataHandler metadataHandler,
FileOutErr fileOutErr,
Map<String, String> clientEnv,
- Environment env) {
+ Environment env,
+ @Nullable FileSystem actionFileSystem) {
return new ActionExecutionContext(
executor,
actionInputFileCache,
@@ -121,8 +128,9 @@
fileOutErr,
clientEnv,
ImmutableMap.of(),
- null,
- env);
+ /*artifactExpander=*/ null,
+ env,
+ actionFileSystem);
}
public ActionInputPrefetcher getActionInputPrefetcher() {
@@ -151,16 +159,28 @@
* <p>Notably, in the future, we want any action-scoped artifacts to resolve paths using this
* method instead of {@link Artifact#getPath} because that does not allow filesystem injection.
*
- * <p>TODO(shahan): cleanup {@link Action}-scoped references to {@link Artifact.getPath}.
+ * <p>TODO(shahan): cleanup {@link Action}-scoped references to {@link Artifact.getPath} and
+ * {@link Artifact.getRoot}.
*/
public Path getInputPath(ActionInput input) {
if (input instanceof Artifact) {
- // TODO(shahan): replace this with actual logic once we understand what it is.
- return ((Artifact) input).getPath();
+ Artifact artifact = (Artifact) input;
+ if (actionFileSystem != null) {
+ return actionFileSystem.getPath(artifact.getPath().getPathString());
+ }
+ return artifact.getPath();
}
return executor.getExecRoot().getRelative(input.getExecPath());
}
+ public Root getRoot(Artifact artifact) {
+ if (actionFileSystem != null) {
+ return Root.fromPath(
+ actionFileSystem.getPath(artifact.getRoot().getRoot().asPath().getPathString()));
+ }
+ return artifact.getRoot().getRoot();
+ }
+
/**
* Returns whether failures should have verbose error messages.
*/
@@ -290,6 +310,7 @@
clientEnv,
inputFilesetMappings,
artifactExpander,
- env);
+ env,
+ actionFileSystem);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContextFactory.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContextFactory.java
deleted file mode 100644
index c3bcb09..0000000
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContextFactory.java
+++ /dev/null
@@ -1,38 +0,0 @@
-// Copyright 2014 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;
-
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.actions.cache.MetadataHandler;
-import com.google.devtools.build.lib.vfs.PathFragment;
-import java.util.Collection;
-import java.util.Map;
-
-/**
- * Interface that provides an {@link ActionExecutionContext} on demand. Used to limit the surface
- * area available to callers that need to execute an action but do not need the full framework
- * normally provided.
- */
-public interface ActionExecutionContextFactory {
- /**
- * Returns an action execution context. This involves creating a new FileOutErr, and it is the
- * caller's responsibility to close that, e.g. by calling {@link ActionExecutionContext#close}.
- */
- ActionExecutionContext getContext(
- ActionInputFileCache graphFileCache,
- MetadataHandler metadataHandler,
- Map<Artifact, Collection<Artifact>> expandedInputs,
- ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> inputFilesetMappings);
-}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
index 3cd4a4b..8b43ac1 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
@@ -971,7 +971,7 @@
}
}
// Still not found: see if it is in a subdir of a declared package.
- Root root = input.getRoot().getRoot();
+ Root root = actionExecutionContext.getRoot(input);
for (Path dir = actionExecutionContext.getInputPath(input).getParentDirectory(); ; ) {
if (dir.getRelative(BUILD_PATH_FRAGMENT).exists()) {
return false; // Bad: this is a sub-package, not a subdir of a declared package.
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
index aca1c5c..283f0d9 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
@@ -189,6 +189,17 @@
Preconditions.checkState(!state.hasArtifactData(), "%s %s", state, action);
state.inputArtifactData = checkedInputs.first;
state.expandedArtifacts = checkedInputs.second;
+ if (skyframeActionExecutor.usesActionFileSystem()) {
+ state.actionFileSystem =
+ new ActionFileSystem(
+ checkedInputs.first,
+ // TODO(shahan): based on experimentation, it suffices to include only {@link
+ // com.google.devtools.build.lib.rules.cpp.IncludeScannable#getDeclaredIncludeSrcs}.
+ // That may be a smaller set and more efficient but makes this class have an
+ // awkward dependency on something in the cpp package.
+ action.discoversInputs() ? action.getAllowedDerivedInputs() : ImmutableList.of(),
+ action.getOutputs());
+ }
}
ActionExecutionValue result;
@@ -391,14 +402,18 @@
metadataHandler.discardOutputMetadata();
// This may be recreated if we discover inputs.
+ // TODO(shahan): this isn't used when using ActionFileSystem so we can avoid creating some
+ // unused objects.
PerActionFileCache perActionFileCache =
new PerActionFileCache(
state.inputArtifactData, /*missingArtifactsAllowed=*/ action.discoversInputs());
if (action.discoversInputs()) {
if (state.discoveredInputs == null) {
try {
- state.discoveredInputs = skyframeActionExecutor.discoverInputs(action,
- perActionFileCache, metadataHandler, env);
+ state.updateFileSystemContext(env, metadataHandler);
+ state.discoveredInputs =
+ skyframeActionExecutor.discoverInputs(
+ action, perActionFileCache, metadataHandler, env, state.actionFileSystem);
Preconditions.checkState(state.discoveredInputs != null,
"discoverInputs() returned null on action %s", action);
} catch (MissingDepException e) {
@@ -413,6 +428,7 @@
}
perActionFileCache =
new PerActionFileCache(state.inputArtifactData, /*missingArtifactsAllowed=*/ false);
+ state.updateFileSystemInputData();
// Stage 1 finished, let's do stage 2. The stage 1 of input discovery will have added some
// files with addDiscoveredInputs() and then have waited for those files to be available
@@ -430,6 +446,7 @@
}
perActionFileCache =
new PerActionFileCache(state.inputArtifactData, /*missingArtifactsAllowed=*/ false);
+ state.updateFileSystemInputData();
}
metadataHandler =
new ActionMetadataHandler(state.inputArtifactData, action.getOutputs(), tsgm.get());
@@ -460,12 +477,14 @@
filesetMappings.put(actionInput.getExecPath(), filesetValue.getOutputSymlinks());
}
+ state.updateFileSystemContext(env, metadataHandler);
try (ActionExecutionContext actionExecutionContext =
skyframeActionExecutor.getContext(
perActionFileCache,
metadataHandler,
Collections.unmodifiableMap(state.expandedArtifacts),
- filesetMappings.build())) {
+ filesetMappings.build(),
+ state.actionFileSystem)) {
if (!state.hasExecutedAction()) {
state.value =
skyframeActionExecutor.executeAction(
@@ -772,6 +791,7 @@
Iterable<Artifact> discoveredInputs = null;
Iterable<Artifact> discoveredInputsStage2 = null;
ActionExecutionValue value = null;
+ ActionFileSystem actionFileSystem = null;
boolean hasCollectedInputs() {
return allInputs != null;
@@ -793,6 +813,21 @@
return value != null;
}
+ /** Must be called to assign values to the given variables as they change. */
+ void updateFileSystemContext(
+ SkyFunction.Environment env, ActionMetadataHandler metadataHandler) {
+ if (actionFileSystem != null) {
+ actionFileSystem.updateContext(env, metadataHandler::injectOutputData);
+ }
+ }
+
+ /** Propagates {@link inputArtifactData} changes due to input discovery. */
+ void updateFileSystemInputData() {
+ if (actionFileSystem != null) {
+ actionFileSystem.updateInputData(inputArtifactData);
+ }
+ }
+
@Override
public String toString() {
return token + ", " + value + ", " + allInputs + ", " + inputArtifactData + ", "
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionFileSystem.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionFileSystem.java
new file mode 100644
index 0000000..62e9761
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionFileSystem.java
@@ -0,0 +1,658 @@
+// Copyright 2018 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.skyframe;
+
+import static java.nio.charset.StandardCharsets.US_ASCII;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Streams;
+import com.google.common.io.BaseEncoding;
+import com.google.devtools.build.lib.actions.ActionInput;
+import com.google.devtools.build.lib.actions.ActionInputFileCache;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.FileStateType;
+import com.google.devtools.build.lib.vfs.AbstractFileSystem;
+import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.build.skyframe.SkyFunction;
+import com.google.protobuf.ByteString;
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InterruptedIOException;
+import java.io.OutputStream;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Function;
+import java.util.function.Supplier;
+import java.util.logging.Logger;
+import javax.annotation.Nullable;
+
+/**
+ * File system for actions.
+ *
+ * <p>This class is thread-safe except that
+ *
+ * <ul>
+ * <li>{@link updateContext} and {@link updateInputData} must be called exclusively of any other
+ * methods.
+ * <li>This class relies on synchronized access to {@link env}. If there are other threads, that
+ * access {@link env}, they must also used synchronized access.
+ * </ul>
+ */
+final class ActionFileSystem extends AbstractFileSystem implements ActionInputFileCache {
+ private static final Logger LOGGER = Logger.getLogger(ActionFileSystem.class.getName());
+
+ /**
+ * Exec root and source roots.
+ *
+ * <p>First entry is exec root. Used to convert paths into exec paths.
+ */
+ private final LinkedHashSet<PathFragment> roots = new LinkedHashSet<>();
+
+ /** exec path → artifact and metadata */
+ private final Map<PathFragment, ArtifactAndMetadata> inputs;
+
+ /** exec path → artifact and metadata */
+ private final ImmutableMap<PathFragment, ArtifactAndMutableMetadata> outputs;
+
+ /** digest → artifacts in {@link inputs} */
+ private final ConcurrentHashMap<ByteString, Artifact> reverseMap;
+
+ /** Used to lookup metadata for optional inputs. */
+ private SkyFunction.Environment env = null;
+
+ /**
+ * Called whenever there is new metadata for an output.
+ *
+ * <p>This is backed by injection into an {@link ActionMetadataHandler} instance so should only be
+ * called once per artifact.
+ */
+ private MetadataConsumer metadataConsumer = null;
+
+ ActionFileSystem(
+ Map<Artifact, FileArtifactValue> inputData,
+ Iterable<Artifact> allowedInputs,
+ Iterable<Artifact> outputArtifacts) {
+ roots.add(computeExecRoot(outputArtifacts));
+
+ // TODO(shahan): Underestimates because this doesn't account for discovered inputs. Improve
+ // this estimate using data.
+ this.reverseMap = new ConcurrentHashMap<>(inputData.size());
+
+ HashMap<PathFragment, ArtifactAndMetadata> inputs = new HashMap<>();
+ for (Map.Entry<Artifact, FileArtifactValue> entry : inputData.entrySet()) {
+ Artifact input = entry.getKey();
+ updateRootsIfSource(input);
+ inputs.put(input.getExecPath(), new SimpleArtifactAndMetadata(input, entry.getValue()));
+ updateReverseMapIfDigestExists(entry.getValue(), entry.getKey());
+ }
+ for (Artifact input : allowedInputs) {
+ PathFragment execPath = input.getExecPath();
+ inputs.computeIfAbsent(execPath, unused -> new OptionalInputArtifactAndMetadata(input));
+ updateRootsIfSource(input);
+ }
+ this.inputs = inputs;
+
+ validateRoots();
+
+ this.outputs =
+ Streams.stream(outputArtifacts)
+ .collect(
+ ImmutableMap.toImmutableMap(
+ a -> a.getExecPath(), a -> new ArtifactAndMutableMetadata(a)));
+ }
+
+ /**
+ * Must be called prior to access and updated as needed.
+ *
+ * <p>These cannot be passed into the constructor because while {@link ActionFileSystem} is
+ * action-scoped, the environment and metadata consumer change multiple times, at well defined
+ * points, during the lifetime of an action.
+ */
+ public void updateContext(SkyFunction.Environment env, MetadataConsumer metadataConsumer) {
+ this.env = env;
+ this.metadataConsumer = metadataConsumer;
+ }
+
+ /** Input discovery changes the values of the input data map so it must be updated accordingly. */
+ public void updateInputData(Map<Artifact, FileArtifactValue> inputData) {
+ boolean foundNewRoots = false;
+ for (Map.Entry<Artifact, FileArtifactValue> entry : inputData.entrySet()) {
+ ArtifactAndMetadata current = inputs.get(entry.getKey().getExecPath());
+ if (current == null || isUnsetOptional(current)) {
+ Artifact input = entry.getKey();
+ inputs.put(input.getExecPath(), new SimpleArtifactAndMetadata(input, entry.getValue()));
+ foundNewRoots = updateRootsIfSource(entry.getKey()) || foundNewRoots;
+ updateReverseMapIfDigestExists(entry.getValue(), entry.getKey());
+ }
+ }
+ if (foundNewRoots) {
+ validateRoots();
+ }
+ }
+
+ // -------------------- ActionInputFileCache implementation --------------------
+
+ @Override
+ @Nullable
+ public FileArtifactValue getMetadata(ActionInput actionInput) {
+ return apply(
+ actionInput.getExecPath(),
+ input -> {
+ try {
+ return input.getMetadata();
+ } catch (IOException e) {
+ // TODO(shahan): improve the handling of this error by propagating it correctly
+ // through MetadataHandler.getMetadata().
+ throw new IllegalStateException(e);
+ }
+ },
+ output -> output.getMetadata(),
+ () -> null);
+ }
+
+ @Override
+ public boolean contentsAvailableLocally(ByteString digest) {
+ // TODO(shahan): we assume this is never true, though the digests might be present. Should
+ // this be relaxed for locally available source files?
+ return false;
+ }
+
+ @Override
+ @Nullable
+ public Artifact getInputFromDigest(ByteString digest) {
+ return reverseMap.get(digest);
+ }
+
+ @Override
+ public Path getInputPath(ActionInput actionInput) {
+ ArtifactAndMetadata input = inputs.get(actionInput.getExecPath());
+ if (input != null) {
+ return getPath(input.getArtifact().getPath().getPathString());
+ }
+ ArtifactAndMutableMetadata output = outputs.get(actionInput.getExecPath());
+ if (output != null) {
+ return getPath(output.getArtifact().getPath().getPathString());
+ }
+ // TODO(shahan): this might need to be relaxed
+ throw new IllegalStateException(actionInput + " not found");
+ }
+
+ // -------------------- FileSystem implementation --------------------
+
+ @Override
+ public boolean supportsModifications(Path path) {
+ return isOutput(path);
+ }
+
+ @Override
+ public boolean supportsSymbolicLinksNatively(Path path) {
+ return isOutput(path);
+ }
+
+ @Override
+ protected boolean supportsHardLinksNatively(Path path) {
+ return isOutput(path);
+ }
+
+ @Override
+ public boolean isFilePathCaseSensitive() {
+ return true;
+ }
+
+ /** ActionFileSystem currently doesn't track directories. */
+ @Override
+ public boolean createDirectory(Path path) throws IOException {
+ return true;
+ }
+
+ @Override
+ public void createDirectoryAndParents(Path path) throws IOException {}
+
+ @Override
+ protected long getFileSize(Path path, boolean followSymlinks) throws IOException {
+ Preconditions.checkArgument(
+ followSymlinks, "ActionFileSystem doesn't support no-follow: %s", path);
+ return getMetadataOrThrowFileNotFound(path).getSize();
+ }
+
+ @Override
+ public boolean delete(Path path) throws IOException {
+ throw new UnsupportedOperationException(path.getPathString());
+ }
+
+ @Override
+ protected long getLastModifiedTime(Path path, boolean followSymlinks) throws IOException {
+ Preconditions.checkArgument(
+ followSymlinks, "ActionFileSystem doesn't support no-follow: %s", path);
+ return getMetadataOrThrowFileNotFound(path).getModifiedTime();
+ }
+
+ @Override
+ public void setLastModifiedTime(Path path, long newTime) throws IOException {
+ throw new UnsupportedOperationException(path.getPathString());
+ }
+
+ @Override
+ protected byte[] getFastDigest(Path path, HashFunction hash) throws IOException {
+ if (hash != HashFunction.MD5) {
+ return null;
+ }
+ return getMetadataOrThrowFileNotFound(path).getDigest();
+ }
+
+ @Override
+ protected boolean isSymbolicLink(Path path) {
+ throw new UnsupportedOperationException(path.getPathString());
+ }
+
+ @Override
+ protected boolean isDirectory(Path path, boolean followSymlinks) {
+ Preconditions.checkArgument(
+ followSymlinks, "ActionFileSystem doesn't support no-follow: %s", path);
+ FileArtifactValue metadata = getMetadataUnchecked(path);
+ return metadata == null ? false : metadata.getType() == FileStateType.DIRECTORY;
+ }
+
+ @Override
+ protected boolean isFile(Path path, boolean followSymlinks) {
+ Preconditions.checkArgument(
+ followSymlinks, "ActionFileSystem doesn't support no-follow: %s", path);
+ FileArtifactValue metadata = getMetadataUnchecked(path);
+ return metadata == null ? false : metadata.getType() == FileStateType.REGULAR_FILE;
+ }
+
+ @Override
+ protected boolean isSpecialFile(Path path, boolean followSymlinks) {
+ Preconditions.checkArgument(
+ followSymlinks, "ActionFileSystem doesn't support no-follow: %s", path);
+ FileArtifactValue metadata = getMetadataUnchecked(path);
+ return metadata == null ? false : metadata.getType() == FileStateType.SPECIAL_FILE;
+ }
+
+ private static String createSymbolicLinkErrorMessage(
+ Path linkPath, PathFragment targetFragment, String message) {
+ return "createSymbolicLink(" + linkPath + ", " + targetFragment + "): " + message;
+ }
+
+ @Override
+ protected void createSymbolicLink(Path linkPath, PathFragment targetFragment) throws IOException {
+ ArtifactAndMetadata input = inputs.get(asExecPath(targetFragment));
+ if (input == null) {
+ throw new FileNotFoundException(
+ createSymbolicLinkErrorMessage(
+ linkPath, targetFragment, targetFragment + " is not an input."));
+ }
+ ArtifactAndMutableMetadata output = outputs.get(asExecPath(linkPath));
+ if (output == null) {
+ throw new FileNotFoundException(
+ createSymbolicLinkErrorMessage(
+ linkPath, targetFragment, linkPath + " is not an output."));
+ }
+ output.setMetadata(input.getMetadata());
+ }
+
+ @Override
+ protected PathFragment readSymbolicLink(Path path) throws IOException {
+ throw new UnsupportedOperationException(path.getPathString());
+ }
+
+ @Override
+ protected boolean exists(Path path, boolean followSymlinks) {
+ Preconditions.checkArgument(
+ followSymlinks, "ActionFileSystem doesn't support no-follow: %s", path);
+ return apply(path, input -> true, output -> output.getMetadata() != null, () -> false);
+ }
+
+ @Override
+ protected Collection<String> getDirectoryEntries(Path path) throws IOException {
+ throw new UnsupportedOperationException(path.getPathString());
+ }
+
+ @Override
+ protected boolean isReadable(Path path) throws IOException {
+ return exists(path, true);
+ }
+
+ @Override
+ protected void setReadable(Path path, boolean readable) throws IOException {}
+
+ @Override
+ protected boolean isWritable(Path path) throws IOException {
+ return isOutput(path);
+ }
+
+ @Override
+ public void setWritable(Path path, boolean writable) throws IOException {}
+
+ @Override
+ protected boolean isExecutable(Path path) throws IOException {
+ return true;
+ }
+
+ @Override
+ protected void setExecutable(Path path, boolean executable) throws IOException {}
+
+ @Override
+ protected InputStream getInputStream(Path path) throws IOException {
+ // TODO(shahan): cleanup callers of this method and disable or maybe figure out a reasonable
+ // implementation.
+ LOGGER.severe("Raw read of path: " + path);
+ return super.getInputStream(path);
+ }
+
+ @Override
+ protected OutputStream getOutputStream(Path path, boolean append) throws IOException {
+ // TODO(shahan): cleanup callers of this method and disable or maybe figure out a reasonable
+ // implementation.
+ LOGGER.severe("Raw write of path: " + path);
+ return super.getOutputStream(path, append);
+ }
+
+ @Override
+ public void renameTo(Path sourcePath, Path targetPath) throws IOException {
+ throw new UnsupportedOperationException("renameTo(" + sourcePath + ", " + targetPath + ")");
+ }
+
+ @Override
+ protected void createFSDependentHardLink(Path linkPath, Path originalPath) throws IOException {
+ throw new UnsupportedOperationException(
+ "createFSDependendHardLink(" + linkPath + ", " + originalPath + ")");
+ }
+
+ // -------------------- Implementation Helpers --------------------
+
+ private PathFragment asExecPath(Path path) {
+ return asExecPath(path.asFragment());
+ }
+
+ private PathFragment asExecPath(PathFragment fragment) {
+ for (PathFragment root : roots) {
+ if (fragment.startsWith(root)) {
+ return fragment.relativeTo(root);
+ }
+ }
+ throw new IllegalArgumentException(fragment + " was not found under any known root: " + roots);
+ }
+
+ private boolean isOutput(Path path) {
+ return outputs.containsKey(asExecPath(path));
+ }
+
+ /**
+ * Lambda-based case implementation.
+ *
+ * <p>One of {@code inputOp} or {@code outputOp} will be called depending on whether {@code path}
+ * is an input or output.
+ */
+ private <T> T apply(Path path, InputFileOperator<T> inputOp, OutputFileOperator<T> outputOp)
+ throws IOException {
+ PathFragment execPath = asExecPath(path);
+ ArtifactAndMetadata input = inputs.get(execPath);
+ if (input != null) {
+ return inputOp.apply(input);
+ }
+ ArtifactAndMutableMetadata output = outputs.get(execPath);
+ if (output != null) {
+ return outputOp.apply(output);
+ }
+ throw new FileNotFoundException(path.getPathString());
+ }
+
+ /**
+ * Apply variant that doesn't throw exceptions.
+ *
+ * <p>Useful for implementing existence-type methods.
+ */
+ private <T> T apply(
+ Path path,
+ Function<ArtifactAndMetadata, T> inputOp,
+ Function<ArtifactAndMutableMetadata, T> outputOp,
+ Supplier<T> notFoundOp) {
+ return apply(asExecPath(path), inputOp, outputOp, notFoundOp);
+ }
+
+ private <T> T apply(
+ PathFragment execPath,
+ Function<ArtifactAndMetadata, T> inputOp,
+ Function<ArtifactAndMutableMetadata, T> outputOp,
+ Supplier<T> notFoundOp) {
+ ArtifactAndMetadata input = inputs.get(execPath);
+ if (input != null) {
+ return inputOp.apply(input);
+ }
+ ArtifactAndMutableMetadata output = outputs.get(execPath);
+ if (output != null) {
+ return outputOp.apply(output);
+ }
+ return notFoundOp.get();
+ }
+
+ private boolean updateRootsIfSource(Artifact input) {
+ if (input.isSourceArtifact()) {
+ return roots.add(input.getRoot().getRoot().asPath().asFragment());
+ }
+ return false;
+ }
+
+ /**
+ * The execution root is globally unique for a build so can be derived from any output.
+ *
+ * <p>Outputs must be nonempty.
+ */
+ private static PathFragment computeExecRoot(Iterable<Artifact> outputs) {
+ Artifact derived = outputs.iterator().next();
+ Preconditions.checkArgument(!derived.isSourceArtifact(), derived);
+ PathFragment rootFragment = derived.getRoot().getRoot().asPath().asFragment();
+ int rootSegments = rootFragment.segmentCount();
+ int execSegments = derived.getRoot().getExecPath().segmentCount();
+ return rootFragment.subFragment(0, rootSegments - execSegments);
+ }
+
+ /**
+ * Verifies that no root is the prefix of any other root.
+ *
+ * <p>TODO(shahan): if this is insufficiently general, we can topologically order on the prefix
+ * relation between roots.
+ */
+ private void validateRoots() {
+ for (PathFragment root1 : roots) {
+ for (PathFragment root2 : roots) {
+ if (root1 == root2) {
+ continue;
+ }
+ Preconditions.checkState(!root1.startsWith(root2), "%s starts with %s", root1, root2);
+ }
+ }
+ }
+
+ private static ByteString toByteString(byte[] digest) {
+ return ByteString.copyFrom(BaseEncoding.base16().lowerCase().encode(digest).getBytes(US_ASCII));
+ }
+
+ private static boolean isUnsetOptional(ArtifactAndMetadata input) {
+ if (input instanceof OptionalInputArtifactAndMetadata) {
+ OptionalInputArtifactAndMetadata optional = (OptionalInputArtifactAndMetadata) input;
+ return !optional.hasMetadata();
+ }
+ return false;
+ }
+
+ private void updateReverseMapIfDigestExists(FileArtifactValue metadata, Artifact artifact) {
+ if (metadata.getDigest() != null) {
+ reverseMap.put(toByteString(metadata.getDigest()), artifact);
+ }
+ }
+
+ private FileArtifactValue getMetadataOrThrowFileNotFound(Path path) throws IOException {
+ return apply(
+ path,
+ input -> input.getMetadata(),
+ output -> {
+ if (output.getMetadata() == null) {
+ throw new FileNotFoundException(path.getPathString());
+ }
+ return output.getMetadata();
+ });
+ }
+
+ @Nullable
+ private FileArtifactValue getMetadataUnchecked(Path path) {
+ return apply(
+ path,
+ input -> {
+ try {
+ return input.getMetadata();
+ } catch (IOException e) {
+ // TODO(shahan): propagate this error correctly through higher level APIs.
+ throw new IllegalStateException(e);
+ }
+ },
+ output -> output.getMetadata(),
+ () -> null);
+ }
+
+ @FunctionalInterface
+ private static interface InputFileOperator<T> {
+ T apply(ArtifactAndMetadata entry) throws IOException;
+ }
+
+ @FunctionalInterface
+ private static interface OutputFileOperator<T> {
+ T apply(ArtifactAndMutableMetadata entry) throws IOException;
+ }
+
+ @FunctionalInterface
+ public static interface MetadataConsumer {
+ void accept(Artifact artifact, FileArtifactValue value) throws IOException;
+ }
+
+ private abstract static class ArtifactAndMetadata {
+ public abstract Artifact getArtifact();
+
+ public abstract FileArtifactValue getMetadata() throws IOException;
+
+ @Override
+ public String toString() {
+ String metadataText = null;
+ try {
+ metadataText = "" + getMetadata();
+ } catch (IOException e) {
+ metadataText = "Error getting metadata(" + e.getMessage() + ")";
+ }
+ return getArtifact() + ": " + metadataText;
+ }
+ }
+
+ private static class SimpleArtifactAndMetadata extends ArtifactAndMetadata {
+ private final Artifact artifact;
+ private final FileArtifactValue metadata;
+
+ private SimpleArtifactAndMetadata(Artifact artifact, FileArtifactValue metadata) {
+ this.artifact = artifact;
+ this.metadata = metadata;
+ }
+
+ @Override
+ public Artifact getArtifact() {
+ return artifact;
+ }
+
+ @Override
+ public FileArtifactValue getMetadata() {
+ return metadata;
+ }
+ }
+
+ private class OptionalInputArtifactAndMetadata extends ArtifactAndMetadata {
+ private final Artifact artifact;
+ private volatile FileArtifactValue metadata = null;
+
+ private OptionalInputArtifactAndMetadata(Artifact artifact) {
+ this.artifact = artifact;
+ }
+
+ @Override
+ public Artifact getArtifact() {
+ return artifact;
+ }
+
+ @Override
+ public FileArtifactValue getMetadata() throws IOException {
+ if (metadata == null) {
+ synchronized (this) {
+ if (metadata == null) {
+ try {
+ // TODO(shahan): {@link SkyFunction.Environment} requires single-threaded access so
+ // we enforce that here by making these (multithreaded) calls synchronized. It might
+ // be better to make the underlying methods synchronized to avoid having another
+ // caller unintentionally calling into the environment without locking.
+ //
+ // This is currently known to be reached from the distributor during remote include
+ // scanning. It might make sense to instead of bubbling this error out all the way
+ // from within the distributor, to ensure that this metadata value exists when
+ // creating the spawn from the include parser, which will require slightly fewer
+ // layers of error propagation and there is some batching opportunity (across the
+ // parallel expansion of the include scanner).
+ synchronized (env) {
+ metadata = (FileArtifactValue) env.getValue(ArtifactSkyKey.key(artifact, false));
+ }
+ } catch (InterruptedException e) {
+ throw new InterruptedIOException(e.getMessage());
+ }
+ if (metadata == null) {
+ throw new ActionExecutionFunction.MissingDepException();
+ }
+ updateReverseMapIfDigestExists(metadata, artifact);
+ }
+ }
+ }
+ return metadata;
+ }
+
+ public boolean hasMetadata() {
+ return metadata != null;
+ }
+ }
+
+ private class ArtifactAndMutableMetadata extends ArtifactAndMetadata {
+ private final Artifact artifact;
+ @Nullable private volatile FileArtifactValue metadata = null;
+
+ @Override
+ public Artifact getArtifact() {
+ return artifact;
+ }
+
+ @Override
+ @Nullable
+ public FileArtifactValue getMetadata() {
+ return metadata;
+ }
+
+ public void setMetadata(FileArtifactValue metadata) throws IOException {
+ metadataConsumer.accept(artifact, metadata);
+ this.metadata = metadata;
+ }
+
+ private ArtifactAndMutableMetadata(Artifact artifact) {
+ this.artifact = artifact;
+ }
+ }
+}
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 fe6e8af..2b001d9 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
@@ -167,7 +167,7 @@
return null;
}
- return Preconditions.checkNotNull(inputArtifactData.get(input), input);
+ return inputArtifactData.get(input);
}
@Override
@@ -478,36 +478,31 @@
size,
locationIndex);
- Preconditions.checkState(injectedFiles.add(output), output);
-
// TODO(shahan): there are a couple of things that could reduce memory usage
// 1. We might be able to skip creating an entry in `outputArtifactData` and only create
// the `FileArtifactValue`, but there are likely downstream consumers that expect it that
// would need to be cleaned up.
// 2. Instead of creating an `additionalOutputData` entry, we could add the extra
// `locationIndex` to `FileStateValue`.
- FileStateValue fileStateValue =
- new FileStateValue.RegularFileStateValue(size, digest, /*contentsProxy=*/ null);
- RootedPath rootedPath =
- RootedPath.toRootedPath(output.getRoot().getRoot(), output.getRootRelativePath());
- FileValue value = FileValue.value(rootedPath, fileStateValue, rootedPath, fileStateValue);
- FileValue oldFsValue = outputArtifactData.putIfAbsent(output, value);
try {
- checkInconsistentData(output, oldFsValue, value);
+ injectOutputData(
+ output,
+ new FileArtifactValue.RemoteFileArtifactValue(digest, size, modifiedTime, locationIndex));
} catch (IOException e) {
- // Should never happen.
- throw new IllegalStateException("Inconsistent FileValues for " + output, e);
+ throw new IllegalStateException(e); // Should never happen.
}
+ }
- FileArtifactValue artifactValue =
- new FileArtifactValue.RemoteFileArtifactValue(digest, size, modifiedTime, locationIndex);
+ public void injectOutputData(Artifact output, FileArtifactValue artifactValue)
+ throws IOException {
+ Preconditions.checkState(injectedFiles.add(output), output);
+ // While `artifactValue` carries the important information, the control flow of `getMetadata`
+ // requires an entry in `outputArtifactData` to access `additionalOutputData`, so a
+ // `PLACEHOLDER` is added to `outputArtifactData`.
+ FileValue oldFileValue = outputArtifactData.putIfAbsent(output, FileValue.PLACEHOLDER);
+ checkInconsistentData(output, oldFileValue, FileValue.PLACEHOLDER);
FileArtifactValue oldArtifactValue = additionalOutputData.putIfAbsent(output, artifactValue);
- try {
- checkInconsistentData(output, oldArtifactValue, artifactValue);
- } catch (IOException e) {
- // Should never happen.
- throw new IllegalStateException("Inconsistent FileArtifactValues for " + output, e);
- }
+ checkInconsistentData(output, oldArtifactValue, artifactValue);
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileValue.java
index 46e75ce..eeb9fc6 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FileValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileValue.java
@@ -51,6 +51,16 @@
@ThreadSafe
public abstract class FileValue implements SkyValue {
+ /**
+ * Exists to accommodate the control flow of {@link ActionMetadataHandler#getMetadata}.
+ *
+ * <p>{@link ActionMetadataHandler#getMetadata} always checks {@link
+ * ActionMetadataHandler#outputArtifactData} before checking {@link
+ * ActionMetadataHandler#additionalOutputData} so some placeholder value is needed to allow an
+ * injected {@link FileArtifactValue} to be returned.
+ */
+ @AutoCodec public static final FileValue PLACEHOLDER = new PlaceholderFileValue();
+
public boolean exists() {
return realFileStateValue().getType() != FileStateType.NONEXISTENT;
}
@@ -315,4 +325,18 @@
realRootedPath, realFileStateValue, linkTarget);
}
}
+
+ private static final class PlaceholderFileValue extends FileValue {
+ private PlaceholderFileValue() {}
+
+ @Override
+ public RootedPath realRootedPath() {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public FileStateValue realFileStateValue() {
+ throw new UnsupportedOperationException();
+ }
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
index c02ca2c..e63ae09 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
@@ -103,6 +103,7 @@
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.Callable;
+import java.util.function.BooleanSupplier;
import java.util.logging.Logger;
import javax.annotation.Nullable;
@@ -151,7 +152,8 @@
List<BuildFileName> buildFilesByPriority,
ActionOnIOExceptionReadingBuildFile actionOnIOExceptionReadingBuildFile,
BuildOptions defaultBuildOptions,
- MutableArtifactFactorySupplier mutableArtifactFactorySupplier) {
+ MutableArtifactFactorySupplier mutableArtifactFactorySupplier,
+ BooleanSupplier usesActionFileSystem) {
super(
evaluatorSupplier,
pkgFactory,
@@ -170,7 +172,8 @@
/*shouldUnblockCpuWorkWhenFetchingDeps=*/ false,
defaultBuildOptions,
new PackageProgressReceiver(),
- mutableArtifactFactorySupplier);
+ mutableArtifactFactorySupplier,
+ usesActionFileSystem);
this.diffAwarenessManager = new DiffAwarenessManager(diffAwarenessFactories);
this.customDirtinessCheckers = customDirtinessCheckers;
}
@@ -207,7 +210,8 @@
buildFilesByPriority,
actionOnIOExceptionReadingBuildFile,
defaultBuildOptions,
- new MutableArtifactFactorySupplier());
+ new MutableArtifactFactorySupplier(),
+ /*usesActionFileSystem=*/ () -> false);
}
public static SequencedSkyframeExecutor create(
@@ -226,7 +230,8 @@
List<BuildFileName> buildFilesByPriority,
ActionOnIOExceptionReadingBuildFile actionOnIOExceptionReadingBuildFile,
BuildOptions defaultBuildOptions,
- MutableArtifactFactorySupplier mutableArtifactFactorySupplier) {
+ MutableArtifactFactorySupplier mutableArtifactFactorySupplier,
+ BooleanSupplier usesActionFileSystem) {
SequencedSkyframeExecutor skyframeExecutor =
new SequencedSkyframeExecutor(
InMemoryMemoizingEvaluator.SUPPLIER,
@@ -245,7 +250,8 @@
buildFilesByPriority,
actionOnIOExceptionReadingBuildFile,
defaultBuildOptions,
- mutableArtifactFactorySupplier);
+ mutableArtifactFactorySupplier,
+ usesActionFileSystem);
skyframeExecutor.init();
return skyframeExecutor;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java
index 27f8301..49aea6d 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java
@@ -64,6 +64,7 @@
BazelSkyframeExecutorConstants.BUILD_FILES_BY_PRIORITY,
BazelSkyframeExecutorConstants.ACTION_ON_IO_EXCEPTION_READING_BUILD_FILE,
defaultBuildOptions,
- new MutableArtifactFactorySupplier());
+ new MutableArtifactFactorySupplier(),
+ /*usesActionFileSystem=*/ () -> false);
}
}
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 221fc92..37683f3 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
@@ -32,7 +32,6 @@
import com.google.devtools.build.lib.actions.ActionExecutedEvent;
import com.google.devtools.build.lib.actions.ActionExecutedEvent.ErrorTiming;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
-import com.google.devtools.build.lib.actions.ActionExecutionContextFactory;
import com.google.devtools.build.lib.actions.ActionExecutionException;
import com.google.devtools.build.lib.actions.ActionExecutionStatusReporter;
import com.google.devtools.build.lib.actions.ActionGraph;
@@ -104,6 +103,7 @@
import java.util.concurrent.FutureTask;
import java.util.concurrent.atomic.AtomicReference;
import java.util.concurrent.locks.Lock;
+import java.util.function.BooleanSupplier;
import java.util.logging.Logger;
import javax.annotation.Nullable;
@@ -111,7 +111,7 @@
* Action executor: takes care of preparing an action for execution, executing it, validating that
* all output artifacts were created, error reporting, etc.
*/
-public final class SkyframeActionExecutor implements ActionExecutionContextFactory {
+public final class SkyframeActionExecutor {
private static final Logger logger = Logger.getLogger(SkyframeActionExecutor.class.getName());
// Used to prevent check-then-act races in #createOutputDirectories. See the comment there for
@@ -157,12 +157,15 @@
private final AtomicReference<ActionExecutionStatusReporter> statusReporterRef;
private OutputService outputService;
+ private final BooleanSupplier usesActionFileSystem;
SkyframeActionExecutor(
ActionKeyContext actionKeyContext,
- AtomicReference<ActionExecutionStatusReporter> statusReporterRef) {
+ AtomicReference<ActionExecutionStatusReporter> statusReporterRef,
+ BooleanSupplier usesActionFileSystem) {
this.actionKeyContext = actionKeyContext;
this.statusReporterRef = statusReporterRef;
+ this.usesActionFileSystem = usesActionFileSystem;
}
/**
@@ -356,6 +359,10 @@
this.clientEnv = clientEnv;
}
+ public boolean usesActionFileSystem() {
+ return usesActionFileSystem.getAsBoolean();
+ }
+
void executionOver() {
this.reporter = null;
// This transitively holds a bunch of heavy objects, so it's important to clear it at the
@@ -481,23 +488,24 @@
* pass the returned context to {@link #executeAction}, and any other method that needs to execute
* tasks related to that action.
*/
- @Override
public ActionExecutionContext getContext(
ActionInputFileCache graphFileCache,
MetadataHandler metadataHandler,
Map<Artifact, Collection<Artifact>> expandedInputs,
- ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> inputFilesetMappings) {
+ ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> inputFilesetMappings,
+ @Nullable ActionFileSystem actionFileSystem) {
FileOutErr fileOutErr = actionLogBufferPathGenerator.generate();
return new ActionExecutionContext(
executorEngine,
- new DelegatingPairFileCache(graphFileCache, perBuildFileCache),
+ createFileCache(graphFileCache, actionFileSystem),
actionInputPrefetcher,
actionKeyContext,
metadataHandler,
fileOutErr,
clientEnv,
inputFilesetMappings,
- new ArtifactExpanderImpl(expandedInputs));
+ new ArtifactExpanderImpl(expandedInputs),
+ actionFileSystem);
}
/**
@@ -606,18 +614,21 @@
Action action,
PerActionFileCache graphFileCache,
MetadataHandler metadataHandler,
- Environment env)
+ Environment env,
+ @Nullable ActionFileSystem actionFileSystem)
throws ActionExecutionException, InterruptedException {
+ ActionInputFileCache cache;
ActionExecutionContext actionExecutionContext =
ActionExecutionContext.forInputDiscovery(
executorEngine,
- new DelegatingPairFileCache(graphFileCache, perBuildFileCache),
+ createFileCache(graphFileCache, actionFileSystem),
actionInputPrefetcher,
actionKeyContext,
metadataHandler,
actionLogBufferPathGenerator.generate(),
clientEnv,
- env);
+ env,
+ actionFileSystem);
try {
actionExecutionContext.getEventBus().post(ActionStatusMessage.analysisStrategy(action));
return action.discoverInputs(actionExecutionContext);
@@ -631,6 +642,14 @@
}
}
+ private ActionInputFileCache createFileCache(
+ ActionInputFileCache graphFileCache, @Nullable ActionFileSystem actionFileSystem) {
+ if (actionFileSystem != null) {
+ return actionFileSystem;
+ }
+ return new DelegatingPairFileCache(graphFileCache, perBuildFileCache);
+ }
+
/**
* This method should be called if the builder encounters an error during
* execution. This allows the builder to record that it encountered at
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 d05b137..2da54f5 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
@@ -38,7 +38,6 @@
import com.google.common.eventbus.EventBus;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.ActionCacheChecker;
-import com.google.devtools.build.lib.actions.ActionExecutionContextFactory;
import com.google.devtools.build.lib.actions.ActionExecutionStatusReporter;
import com.google.devtools.build.lib.actions.ActionGraph;
import com.google.devtools.build.lib.actions.ActionInputFileCache;
@@ -180,6 +179,7 @@
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.BooleanSupplier;
import java.util.logging.Logger;
import java.util.stream.Collectors;
import java.util.stream.Stream;
@@ -342,7 +342,8 @@
boolean shouldUnblockCpuWorkWhenFetchingDeps,
BuildOptions defaultBuildOptions,
@Nullable PackageProgressReceiver packageProgress,
- MutableArtifactFactorySupplier artifactResolverSupplier) {
+ MutableArtifactFactorySupplier artifactResolverSupplier,
+ BooleanSupplier usesActionFileSystem) {
// Strictly speaking, these arguments are not required for initialization, but all current
// callsites have them at hand, so we might as well set them during construction.
this.evaluatorSupplier = evaluatorSupplier;
@@ -354,7 +355,8 @@
new SkyframePackageLoader(), new SkyframeTransitivePackageLoader(),
syscalls, cyclesReporter, pkgLocator, numPackagesLoaded, this);
this.resourceManager = ResourceManager.instance();
- this.skyframeActionExecutor = new SkyframeActionExecutor(actionKeyContext, statusReporterRef);
+ this.skyframeActionExecutor =
+ new SkyframeActionExecutor(actionKeyContext, statusReporterRef, usesActionFileSystem);
this.fileSystem = fileSystem;
this.directories = Preconditions.checkNotNull(directories);
this.actionKeyContext = Preconditions.checkNotNull(actionKeyContext);
@@ -977,10 +979,6 @@
return eventBus.get();
}
- public ActionExecutionContextFactory getActionExecutionContextFactory() {
- return skyframeActionExecutor;
- }
-
@VisibleForTesting
ImmutableList<Root> getPathEntries() {
return pkgLocator.get().getPathEntries();
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystem.java
index 4d47205..3159c21 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystem.java
@@ -25,7 +25,7 @@
/** This class implements the FileSystem interface using direct calls to the UNIX filesystem. */
@ThreadSafe
-abstract class AbstractFileSystem extends FileSystem {
+public abstract class AbstractFileSystem extends FileSystem {
protected static final String ERR_PERMISSION_DENIED = " (Permission denied)";
protected static final Profiler profiler = Profiler.instance();
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 6124b91..9f233b3 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
@@ -65,6 +65,7 @@
outErr,
ImmutableMap.<String, String>of(),
ImmutableMap.of(),
+ null,
null);
}
diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
index 29e9c23..45a0cb1 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
@@ -146,7 +146,8 @@
ImmutableMap.of(),
actionGraph == null
? createDummyArtifactExpander()
- : ActionInputHelper.actionGraphArtifactExpander(actionGraph));
+ : ActionInputHelper.actionGraphArtifactExpander(actionGraph),
+ /*actionFileSystem=*/ null);
}
public static ActionExecutionContext createContextForInputDiscovery(
@@ -165,7 +166,8 @@
fileOutErr,
ImmutableMap.of(),
new BlockingSkyFunctionEnvironment(
- buildDriver, executor == null ? null : executor.getEventHandler()));
+ buildDriver, executor == null ? null : executor.getEventHandler()),
+ /*actionFileSystem=*/ null);
}
public static ActionExecutionContext createContext(ExtendedEventHandler eventHandler) {
@@ -179,7 +181,8 @@
null,
ImmutableMap.of(),
ImmutableMap.of(),
- createDummyArtifactExpander());
+ createDummyArtifactExpander(),
+ /*actionFileSystem=*/ null);
}
private static ArtifactExpander createDummyArtifactExpander() {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionTestCase.java
index 147c692..12cbfb4 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionTestCase.java
@@ -67,6 +67,7 @@
new FileOutErr(),
ImmutableMap.<String, String>of(),
ImmutableMap.of(),
+ null,
null);
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java
index a212893..11ced4c 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java
@@ -179,7 +179,8 @@
new FileOutErr(),
ImmutableMap.<String, String>of(),
ImmutableMap.of(),
- artifactExpander);
+ artifactExpander,
+ /*actionFileSystem=*/ null);
}
private enum KeyAttributes {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkActionTest.java
index 7544613..c3e2561 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkActionTest.java
@@ -89,7 +89,8 @@
null,
ImmutableMap.<String, String>of(),
ImmutableMap.of(),
- null));
+ null,
+ /*actionFileSystem=*/ null));
assertThat(actionResult.spawnResults()).isEmpty();
assertThat(output.isSymbolicLink()).isTrue();
assertThat(output.resolveSymbolicLinks()).isEqualTo(input);
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionActionTest.java
index 9f2cd5e..81dc77b 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionActionTest.java
@@ -192,7 +192,8 @@
new FileOutErr(),
ImmutableMap.<String, String>of(),
ImmutableMap.of(),
- null);
+ null,
+ /*actionFileSystem=*/ null);
}
private void executeTemplateExpansion(String expected) throws Exception {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index eaca063..b917b32 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -2091,7 +2091,8 @@
actionLogBufferPathGenerator.generate(),
clientEnv,
ImmutableMap.of(),
- artifactExpander);
+ artifactExpander,
+ /*actionFileSystem=*/ null);
}
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkActionTest.java
index 9408eda..2f4a8e5 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkActionTest.java
@@ -115,7 +115,7 @@
private ActionExecutionContext makeDummyContext() {
DummyExecutor executor = new DummyExecutor(fileSystem, rootDirectory);
return new ActionExecutionContext(
- executor, null, null, null, null, null, ImmutableMap.of(), ImmutableMap.of(), null);
+ executor, null, null, null, null, null, ImmutableMap.of(), ImmutableMap.of(), null, null);
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java
index 59fd9bf..3e0d629 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java
@@ -88,7 +88,8 @@
new FileOutErr(),
ImmutableMap.<String, String>of(),
ImmutableMap.of(),
- null);
+ null,
+ /*actionFileSystem=*/ null);
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java
index 9b77e71..0120bfc 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java
@@ -784,7 +784,8 @@
null,
ImmutableMap.<String, String>of(),
ImmutableMap.of(),
- DUMMY_ARTIFACT_EXPANDER);
+ DUMMY_ARTIFACT_EXPANDER,
+ null);
ByteArrayOutputStream moduleMapStream = new ByteArrayOutputStream();
ByteArrayOutputStream umbrellaHeaderStream = new ByteArrayOutputStream();
moduleMapAction.newDeterministicWriter(dummyActionExecutionContext)
@@ -834,7 +835,8 @@
null,
ImmutableMap.<String, String>of(),
ImmutableMap.of(),
- DUMMY_ARTIFACT_EXPANDER);
+ DUMMY_ARTIFACT_EXPANDER,
+ null);
ByteArrayOutputStream moduleMapStream = new ByteArrayOutputStream();
ByteArrayOutputStream umbrellaHeaderStream = new ByteArrayOutputStream();
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
index b327612..e80ec31 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
@@ -185,7 +185,10 @@
ActionExecutionStatusReporter statusReporter =
ActionExecutionStatusReporter.create(new StoredEventHandler());
final SkyframeActionExecutor skyframeActionExecutor =
- new SkyframeActionExecutor(actionKeyContext, new AtomicReference<>(statusReporter));
+ new SkyframeActionExecutor(
+ actionKeyContext,
+ new AtomicReference<>(statusReporter),
+ /*usesActionFileSystem=*/ () -> false);
Path actionOutputBase = scratch.dir("/usr/local/google/_blaze_jrluser/FAKEMD5/action_out/");
skyframeActionExecutor.setActionLogBufferPathGenerator(
diff --git a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
index 04d416be..5ef553f 100644
--- a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
@@ -192,7 +192,8 @@
outErr,
ImmutableMap.<String, String>of(),
ImmutableMap.of(),
- SIMPLE_ARTIFACT_EXPANDER);
+ SIMPLE_ARTIFACT_EXPANDER,
+ /*actionFileSystem=*/ null);
}
@Test
