diff --git a/src/main/java/com/google/devtools/build/lib/actions/BUILD b/src/main/java/com/google/devtools/build/lib/actions/BUILD
index ea010f7a542104..ee221e3da92764 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/actions/BUILD
@@ -457,6 +457,7 @@ java_library(
deps = [
":artifacts",
":has_digest",
+ "//src/main/java/com/google/devtools/build/lib/analysis:actions/deterministic_writer",
"//src/main/java/com/google/devtools/build/lib/concurrent",
"//src/main/java/com/google/devtools/build/lib/io:inconsistent_filesystem_exception",
"//src/main/java/com/google/devtools/build/lib/skyframe:filesystem_keys",
diff --git a/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java b/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java
index 01be1c2c711034..df47be1a30d306 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java
@@ -21,12 +21,16 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.MoreObjects;
import com.google.common.hash.HashFunction;
+import com.google.common.hash.HashingOutputStream;
import com.google.common.io.BaseEncoding;
+import com.google.common.io.CountingOutputStream;
+import com.google.devtools.build.lib.analysis.actions.DeterministicWriter;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant;
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.util.HashCodes;
+import com.google.devtools.build.lib.util.StreamWriter;
import com.google.devtools.build.lib.vfs.DigestUtils;
import com.google.devtools.build.lib.vfs.FileStatus;
import com.google.devtools.build.lib.vfs.Path;
@@ -36,8 +40,10 @@
import com.google.devtools.build.lib.vfs.XattrProvider;
import com.google.devtools.build.skyframe.SkyValue;
import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
+import java.io.OutputStream;
import java.time.Instant;
import java.util.Arrays;
import java.util.Objects;
@@ -67,7 +73,7 @@
*/
@Immutable
@ThreadSafe
-public abstract class FileArtifactValue implements SkyValue, HasDigest {
+public abstract class FileArtifactValue implements SkyValue, HasDigest, StreamWriter {
/**
* The type of the underlying file system object. If it is a regular file, then it is guaranteed
* to have a digest. Otherwise it does not have a digest.
@@ -148,11 +154,30 @@ public InputStream getInputStream() {
throw new UnsupportedOperationException();
}
+ /**
+ * Writes the inline file contents to the given {@link OutputStream}.
+ *
+ * @throws UnsupportedOperationException if the file contents are not inline.
+ */
+ public void writeTo(OutputStream out) throws IOException {
+ try (var in = getInputStream()) {
+ in.transferTo(out);
+ }
+ }
+
/** Returns whether the file contents exist remotely. */
public boolean isRemote() {
return false;
}
+ /**
+ * Returns whether the file contents are materialized lazily, for example because they exist
+ * remotely.
+ */
+ public final boolean isLazy() {
+ return isRemote() || isInline();
+ }
+
/** Returns the location index for remote files. For non-remote files, returns 0. */
public int getLocationIndex() {
return 0;
@@ -398,6 +423,20 @@ public static FileArtifactValue createForRemoteFileWithMaterializationData(
digest, size, locationIndex, expirationTime);
}
+ public static FileArtifactValue createForFileWriteActionOutput(
+ DeterministicWriter writer, HashFunction hashFunction) throws IOException {
+ long size;
+ byte[] digest;
+ try (CountingOutputStream countingOut =
+ new CountingOutputStream(OutputStream.nullOutputStream());
+ HashingOutputStream hashingOut = new HashingOutputStream(hashFunction, countingOut)) {
+ writer.writeOutputFile(hashingOut);
+ size = countingOut.getCount();
+ digest = hashingOut.hash().asBytes();
+ }
+ return new FileWriteOutputArtifactValue(writer, size, digest);
+ }
+
public static FileArtifactValue createFromExistingWithResolvedPath(
FileArtifactValue delegate, PathFragment resolvedPath) {
return new ResolvedSymlinkArtifactValue(delegate, resolvedPath);
@@ -1004,6 +1043,105 @@ public boolean wasModifiedSinceDigest(Path path) {
}
}
+ private static final class FileWriteOutputArtifactValue extends FileArtifactValue {
+ private final DeterministicWriter writer;
+ private final long size;
+ private final byte[] digest;
+ @Nullable private FileContentsProxy proxy;
+
+ private FileWriteOutputArtifactValue(DeterministicWriter writer, long size, byte[] digest) {
+ this.writer = writer;
+ this.size = size;
+ this.digest = digest;
+ }
+
+ @Override
+ public FileStateType getType() {
+ return FileStateType.REGULAR_FILE;
+ }
+
+ @Override
+ public byte[] getDigest() {
+ return digest;
+ }
+
+ @Override
+ public long getSize() {
+ return size;
+ }
+
+ @Override
+ public boolean isInline() {
+ return true;
+ }
+
+ @Override
+ public InputStream getInputStream() {
+ // TODO: Avoid materializing the full content in memory by using a variant of
+ // Piped{Input,Output}Stream that works well with virtual threads.
+ var out = new ByteArrayOutputStream(Math.clamp(getSize(), 0, Integer.MAX_VALUE));
+ try {
+ writeTo(out);
+ } catch (IOException e) {
+ // writer is not expected to throw if out doesn't.
+ throw new IllegalStateException(e);
+ }
+ return new ByteArrayInputStream(out.toByteArray());
+ }
+
+ @Override
+ public void writeTo(OutputStream out) throws IOException {
+ writer.writeOutputFile(out);
+ }
+
+ @Override
+ public long getModifiedTime() {
+ throw new UnsupportedOperationException();
+ }
+
+ /**
+ * Sets the {@link FileContentsProxy} if the output backed by this metadata is materialized
+ * later.
+ */
+ @Override
+ public void setContentsProxy(FileContentsProxy proxy) {
+ this.proxy = proxy;
+ }
+
+ /**
+ * Returns a non-null {@link FileContentsProxy} if this metadata is backed by a local file, e.g.
+ * the file is materialized after action execution.
+ */
+ @Override
+ @Nullable
+ public FileContentsProxy getContentsProxy() {
+ return proxy;
+ }
+
+ @Override
+ public boolean wasModifiedSinceDigest(Path path) {
+ return false;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (!(o instanceof FileWriteOutputArtifactValue that)) {
+ return false;
+ }
+ return Objects.equals(writer, that.writer)
+ && size == that.size
+ && Arrays.equals(digest, that.digest);
+ }
+
+ @Override
+ public int hashCode() {
+ return HashCodes.hashObjects(writer, size, Arrays.hashCode(digest));
+ }
+ }
+
/** Metadata for files whose contents are available in memory. */
private static final class InlineFileArtifactValue extends FileArtifactValue {
private final byte[] data;
diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/VirtualActionInput.java b/src/main/java/com/google/devtools/build/lib/actions/cache/VirtualActionInput.java
index e292666e0abb40..6accda1c16813d 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/cache/VirtualActionInput.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/cache/VirtualActionInput.java
@@ -114,19 +114,6 @@ protected byte[] writeTo(Path target) throws IOException {
return digest;
}
- /**
- * Gets a {@link ByteString} representation of the fake file. Used to avoid copying if the fake
- * file is internally represented as a {@link ByteString}.
- *
- * <p>Prefer {@link #writeTo} to this method to avoid materializing the entire file in memory. The
- * return value should not be retained.
- */
- public ByteString getBytes() throws IOException {
- ByteString.Output out = ByteString.newOutput();
- writeTo(out);
- return out.toByteString();
- }
-
/**
* Returns the metadata for this input if available. Null otherwise.
*
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
index fa8112519e115f..4e64465e337aa7 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
@@ -1663,6 +1663,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/collect/nestedset",
"//src/main/java/com/google/devtools/build/lib/concurrent",
+ "//src/main/java/com/google/devtools/build/lib/unsafe:string",
"//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
"//src/main/java/com/google/devtools/build/lib/util:string",
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/DeterministicWriter.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/DeterministicWriter.java
index 301860b9812f19..0323cb2d8a9ccb 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/DeterministicWriter.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/DeterministicWriter.java
@@ -22,6 +22,9 @@
* invocation of writeOutputFile().
*/
public interface DeterministicWriter {
+ /**
+ * @throws IOException only if out throws an IOException
+ */
void writeOutputFile(OutputStream out) throws IOException;
/**
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionContext.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionContext.java
index e780716e979fbd..4bc456899903d4 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionContext.java
@@ -56,4 +56,12 @@ default ImmutableList<SpawnResult> writeOutputToFile(
isRemotable,
Iterables.getOnlyElement(action.getOutputs()));
}
+
+ /**
+ * Returns whether the {@link DeterministicWriter} may be retained after {@link
+ * #writeOutputToFile} returns.
+ */
+ default boolean mayRetainWriter() {
+ return false;
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/LocalTemplateExpansionStrategy.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/LocalTemplateExpansionStrategy.java
index 93396ad2923fb9..cbac36de6a826c 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/LocalTemplateExpansionStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/LocalTemplateExpansionStrategy.java
@@ -14,7 +14,6 @@
package com.google.devtools.build.lib.analysis.actions;
-import static java.nio.charset.StandardCharsets.ISO_8859_1;
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.AbstractAction;
@@ -25,6 +24,7 @@
import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.server.FailureDetails.Execution;
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
+import com.google.devtools.build.lib.unsafe.StringUnsafe;
import com.google.devtools.build.lib.util.StringUtilities;
import java.io.IOException;
import java.util.List;
@@ -44,18 +44,43 @@ public ImmutableList<SpawnResult> expandTemplate(
TemplateExpansionContext.TemplateMetadata templateMetadata)
throws InterruptedException, ExecException {
try {
+ // If writeOutputToFile may retain the writer, make sure that it doesn't capture the
+ // expanded template string. Since expansion may throw and the writer must not, expand the
+ // template once before calling writeOutputToFile. It is assumed to be deterministic, so if
+ // it doesn't throw once, it won't throw again.
final String expandedTemplate =
getExpandedTemplateUnsafe(
templateMetadata.template(), templateMetadata.substitutions(), ctx.getPathResolver());
- DeterministicWriter deterministicWriter =
- out -> out.write(expandedTemplate.getBytes(ISO_8859_1));
- return ctx.getContext(FileWriteActionContext.class)
- .writeOutputToFile(
- action,
- ctx,
- deterministicWriter,
- templateMetadata.makeExecutable(),
- /* isRemotable= */ true);
+ FileWriteActionContext fileWriteActionContext = ctx.getContext(FileWriteActionContext.class);
+ DeterministicWriter deterministicWriter;
+ if (fileWriteActionContext.mayRetainWriter()) {
+ ArtifactPathResolver pathResolver = ctx.getPathResolver();
+ deterministicWriter =
+ out -> {
+ try {
+ out.write(
+ StringUnsafe.getInternalStringBytes(
+ getExpandedTemplateUnsafe(
+ templateMetadata.template(),
+ templateMetadata.substitutions(),
+ pathResolver)));
+ } catch (EvalException e) {
+ throw new IllegalStateException(
+ "Template expansion is not deterministic, first succeeded and then failed with: "
+ + e.getMessage(),
+ e);
+ }
+ };
+ } else {
+ deterministicWriter =
+ out -> out.write(StringUnsafe.getInternalStringBytes(expandedTemplate));
+ }
+ return fileWriteActionContext.writeOutputToFile(
+ action,
+ ctx,
+ deterministicWriter,
+ templateMetadata.makeExecutable(),
+ /* isRemotable= */ true);
} catch (IOException | EvalException e) {
throw new EnvironmentalExecException(
e,
diff --git a/src/main/java/com/google/devtools/build/lib/exec/BUILD b/src/main/java/com/google/devtools/build/lib/exec/BUILD
index 156a5ae39d161b..26e92ed9687f9f 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/exec/BUILD
@@ -141,10 +141,14 @@ java_library(
java_library(
name = "file_write_strategy",
- srcs = ["FileWriteStrategy.java"],
+ srcs = [
+ "EagerFileWriteStrategy.java",
+ "LazyFileWriteStrategy.java",
+ ],
deps = [
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
+ "//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
"//src/main/java/com/google/devtools/build/lib/analysis:actions/deterministic_writer",
"//src/main/java/com/google/devtools/build/lib/analysis:actions/file_write_action_context",
"//src/main/java/com/google/devtools/build/lib/profiler",
diff --git a/src/main/java/com/google/devtools/build/lib/exec/FileWriteStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/EagerFileWriteStrategy.java
similarity index 95%
rename from src/main/java/com/google/devtools/build/lib/exec/FileWriteStrategy.java
rename to src/main/java/com/google/devtools/build/lib/exec/EagerFileWriteStrategy.java
index dce4893d9345df..f7dc585a9284ad 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/FileWriteStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/EagerFileWriteStrategy.java
@@ -35,9 +35,10 @@
/**
* A strategy for executing an {@link
- * com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction}.
+ * com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction} that writes the file to
+ * disk eagerly.
*/
-public final class FileWriteStrategy implements FileWriteActionContext {
+public class EagerFileWriteStrategy implements FileWriteActionContext {
private static final Duration MIN_LOGGING = Duration.ofMillis(100);
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java
index 811567aea62c74..d73b7bfb06454d 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java
@@ -562,6 +562,29 @@ public boolean usingLocalTestJobs() {
+ " code 39.")
public int remoteRetryOnTransientCacheError;
+ /** An enum for specifying different file write strategies. */
+ public enum FileWriteStrategy {
+ EAGER,
+ LAZY;
+
+ /** Converts to {@link FileWriteStrategy}. */
+ public static class Converter extends EnumConverter<FileWriteStrategy> {
+ public Converter() {
+ super(FileWriteStrategy.class, "file write strategy");
+ }
+ }
+ }
+
+ @Option(
+ name = "file_write_strategy",
+ defaultValue = "eager",
+ converter = FileWriteStrategy.Converter.class,
+ documentationCategory = OptionDocumentationCategory.REMOTE,
+ effectTags = {OptionEffectTag.EXECUTION},
+ help =
+ "If set to 'eager', Bazel will write the output of file write actions to disk. If set to 'lazy', their output will be kept in-memory if possible and is only written out if needed or requested, depending on the value of --remote_download_outputs.")
+ public FileWriteStrategy fileWriteStrategy;
+
/** An enum for specifying different formats of test output. */
public enum TestOutputFormat {
SUMMARY, // Provide summary output only.
diff --git a/src/main/java/com/google/devtools/build/lib/exec/LazyFileWriteStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/LazyFileWriteStrategy.java
new file mode 100644
index 00000000000000..15d2f0be2e7179
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/exec/LazyFileWriteStrategy.java
@@ -0,0 +1,83 @@
+// Copyright 2025 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.exec;
+
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.actions.AbstractAction;
+import com.google.devtools.build.lib.actions.ActionExecutionContext;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.EnvironmentalExecException;
+import com.google.devtools.build.lib.actions.ExecException;
+import com.google.devtools.build.lib.actions.FileArtifactValue;
+import com.google.devtools.build.lib.actions.RunningActionEvent;
+import com.google.devtools.build.lib.actions.SpawnResult;
+import com.google.devtools.build.lib.analysis.actions.DeterministicWriter;
+import com.google.devtools.build.lib.profiler.AutoProfiler;
+import com.google.devtools.build.lib.profiler.GoogleAutoProfilerUtils;
+import com.google.devtools.build.lib.server.FailureDetails;
+import java.io.IOException;
+import java.time.Duration;
+
+/**
+ * A strategy for executing an {@link
+ * com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction} that avoids writing the
+ * file to disk if possible.
+ */
+public final class LazyFileWriteStrategy extends EagerFileWriteStrategy {
+ private static final Duration MIN_LOGGING = Duration.ofMillis(100);
+
+ @Override
+ public ImmutableList<SpawnResult> writeOutputToFile(
+ AbstractAction action,
+ ActionExecutionContext actionExecutionContext,
+ DeterministicWriter deterministicWriter,
+ boolean makeExecutable,
+ boolean isRemotable,
+ Artifact output)
+ throws ExecException {
+ if (!isRemotable || actionExecutionContext.getActionFileSystem() == null) {
+ return super.writeOutputToFile(
+ action, actionExecutionContext, deterministicWriter, makeExecutable, isRemotable, output);
+ }
+ actionExecutionContext.getEventHandler().post(new RunningActionEvent(action, "local"));
+ try (AutoProfiler p =
+ GoogleAutoProfilerUtils.logged("hashing output of " + action.prettyPrint(), MIN_LOGGING)) {
+ // TODO: Bazel currently marks all output files as executable after local execution and stages
+ // all files as executable for remote execution, so we don't keep track of the executable
+ // bit yet.
+ try {
+ actionExecutionContext
+ .getOutputMetadataStore()
+ .injectFile(
+ output,
+ FileArtifactValue.createForFileWriteActionOutput(
+ deterministicWriter,
+ actionExecutionContext
+ .getActionFileSystem()
+ .getDigestFunction()
+ .getHashFunction()));
+ } catch (IOException e) {
+ throw new EnvironmentalExecException(
+ e, FailureDetails.Execution.Code.FILE_WRITE_IO_EXCEPTION);
+ }
+ }
+ return ImmutableList.of();
+ }
+
+ @Override
+ public boolean mayRetainWriter() {
+ return true;
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java b/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java
index a330456005cbd2..b5a0df2f394d3f 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java
@@ -26,6 +26,7 @@
import static java.util.Objects.requireNonNull;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.google.common.flogger.GoogleLogger;
@@ -58,6 +59,7 @@
import com.google.devtools.build.lib.vfs.PathFragment;
import io.reactivex.rxjava3.core.Completable;
import java.io.IOException;
+import java.io.OutputStream;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
@@ -562,33 +564,53 @@ private Completable downloadFileNoCheckRx(
Path finalPath = path;
- Completable download =
- usingTempPath(
- (tempPath, alreadyDeleted) ->
- toCompletable(
- () ->
- doDownloadFile(
- action,
- reporter,
- tempPath,
- finalPath.relativeTo(execRoot),
- metadata,
- priority,
- reason),
- directExecutor())
- .doOnComplete(
- () -> {
- finalizeDownload(
- metadata, tempPath, finalPath, dirsWithOutputPermissions);
- alreadyDeleted.set(true);
- })
- .doOnError(
- error -> {
- if (error instanceof CacheNotFoundException cacheNotFoundException) {
- reporter.post(
- new LostInputsEvent(cacheNotFoundException.getMissingDigest()));
- }
- }));
+ Completable download;
+ if (metadata.isInline()) {
+ download =
+ usingTempPath(
+ (tempPath, alreadyDeleted) -> {
+ try {
+ tempPath.getParentDirectory().createDirectoryAndParents();
+ try (OutputStream out = tempPath.getOutputStream()) {
+ metadata.writeTo(out);
+ }
+ finalizeDownload(metadata, tempPath, finalPath, dirsWithOutputPermissions);
+ alreadyDeleted.set(true);
+ return Completable.complete();
+ } catch (IOException e) {
+ return Completable.error(e);
+ }
+ });
+ } else {
+ Preconditions.checkState(metadata.isRemote());
+ download =
+ usingTempPath(
+ (tempPath, alreadyDeleted) ->
+ toCompletable(
+ () ->
+ doDownloadFile(
+ action,
+ reporter,
+ tempPath,
+ finalPath.relativeTo(execRoot),
+ metadata,
+ priority,
+ reason),
+ directExecutor())
+ .doOnComplete(
+ () -> {
+ finalizeDownload(
+ metadata, tempPath, finalPath, dirsWithOutputPermissions);
+ alreadyDeleted.set(true);
+ })
+ .doOnError(
+ error -> {
+ if (error instanceof CacheNotFoundException cacheNotFoundException) {
+ reporter.post(
+ new LostInputsEvent(cacheNotFoundException.getMissingDigest()));
+ }
+ }));
+ }
return downloadCache.executeIfNot(
finalPath,
@@ -719,7 +741,7 @@ public void finalizeAction(Action action, OutputMetadataStore outputMetadataStor
}
var metadata = outputMetadataStore.getOutputMetadata(output);
- if (!metadata.isRemote()) {
+ if (!metadata.isLazy()) {
continue;
}
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java
index f70600344b8191..cc053621b9af9f 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java
@@ -285,12 +285,8 @@ protected FileSystem getLocalFileSystem() {
/** Returns whether a path is stored remotely. Follows symlinks. */
boolean isRemote(Path path) throws IOException {
- return isRemote(path.asFragment());
- }
-
- private boolean isRemote(PathFragment path) throws IOException {
// Files in the local filesystem are non-remote by definition, so stat only in-memory sources.
- var status = statInternal(path, FollowMode.FOLLOW_ALL, StatSources.IN_MEMORY_ONLY);
+ var status = statInternal(path.asFragment(), FollowMode.FOLLOW_ALL, StatSources.IN_MEMORY_ONLY);
return status instanceof FileStatusWithMetadata fileStatusWithMetadata
&& fileStatusWithMetadata.getMetadata().isRemote();
}
@@ -369,7 +365,15 @@ protected boolean delete(PathFragment path) throws IOException {
@Override
protected InputStream getInputStream(PathFragment path) throws IOException {
- downloadFileIfRemote(path);
+ var inMemoryStatus = statInternal(path, FollowMode.FOLLOW_ALL, StatSources.IN_MEMORY_ONLY);
+ if (inMemoryStatus instanceof FileStatusWithMetadata fileStatusWithMetadata) {
+ if (fileStatusWithMetadata.getMetadata().isInline()) {
+ return fileStatusWithMetadata.getMetadata().getInputStream();
+ }
+ if (fileStatusWithMetadata.getMetadata().isRemote()) {
+ downloadRemoteFile(path);
+ }
+ }
// TODO(tjgq): Consider only falling back to the local filesystem for source (non-output) files.
// See getMetadata() for why this isn't currently possible.
return localFs.getPath(path).getInputStream();
@@ -748,10 +752,7 @@ FileArtifactValue getInputMetadata(ActionInput input) {
return inputArtifactData.getInputMetadata(input);
}
- private void downloadFileIfRemote(PathFragment path) throws IOException {
- if (!isRemote(path)) {
- return;
- }
+ private void downloadRemoteFile(PathFragment path) throws IOException {
PathFragment execPath = path.relativeTo(execRoot);
try {
ActionInput input = getInput(execPath.getPathString());
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java
index 251791bdab499d..a2f5eee3b231d5 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java
@@ -74,7 +74,7 @@ protected void prefetchVirtualActionInput(VirtualActionInput input) throws IOExc
@Override
protected boolean canDownloadFile(Path path, FileArtifactValue metadata) {
- return metadata.isRemote();
+ return metadata.isLazy();
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionCache.java
index fc90073279294b..d810835d6da694 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionCache.java
@@ -34,7 +34,6 @@
import com.google.common.collect.Iterables;
import com.google.common.flogger.GoogleLogger;
import com.google.common.util.concurrent.ListenableFuture;
-import com.google.devtools.build.lib.actions.cache.VirtualActionInput;
import com.google.devtools.build.lib.events.Reporter;
import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.profiler.SilentCloseable;
@@ -49,6 +48,7 @@
import com.google.devtools.build.lib.remote.options.RemoteOptions;
import com.google.devtools.build.lib.remote.util.DigestUtil;
import com.google.devtools.build.lib.remote.util.RxUtils.TransferResult;
+import com.google.devtools.build.lib.util.StreamWriter;
import com.google.devtools.build.lib.vfs.Path;
import com.google.protobuf.ByteString;
import com.google.protobuf.Message;
@@ -177,14 +177,14 @@ public void ensureInputsPresent(
}
}
- private static final class VirtualActionInputBlob implements Blob {
- private VirtualActionInput virtualActionInput;
+ private static final class StreamWriterBlob implements Blob {
+ private StreamWriter streamWriter;
// Can be large compared to the retained size of the VirtualActionInput and thus shouldn't be
// kept in memory for an extended period of time.
private volatile ByteString data;
- VirtualActionInputBlob(VirtualActionInput virtualActionInput) {
- this.virtualActionInput = Preconditions.checkNotNull(virtualActionInput);
+ StreamWriterBlob(StreamWriter streamWriter) {
+ this.streamWriter = Preconditions.checkNotNull(streamWriter);
}
@Override
@@ -192,7 +192,7 @@ public InputStream get() throws IOException {
if (data == null) {
synchronized (this) {
if (data == null) {
- data = Preconditions.checkNotNull(virtualActionInput, "used after close()").getBytes();
+ data = Preconditions.checkNotNull(streamWriter, "used after close()").getBytes();
}
}
}
@@ -201,7 +201,7 @@ public InputStream get() throws IOException {
@Override
public void close() {
- virtualActionInput = null;
+ streamWriter = null;
data = null;
}
}
@@ -220,9 +220,8 @@ private ListenableFuture<Void> uploadBlob(
ContentSource file = merkleTree.getFileByDigest(digest);
if (file != null) {
return switch (file) {
- case ContentSource.VirtualActionInputSource(VirtualActionInput virtualActionInput) ->
- remoteCacheClient.uploadBlob(
- context, digest, new VirtualActionInputBlob(virtualActionInput));
+ case ContentSource.InMemorySource(StreamWriter streamWriter) ->
+ remoteCacheClient.uploadBlob(context, digest, new StreamWriterBlob(streamWriter));
case ContentSource.PathSource(Path path) -> {
try {
if (remotePathChecker.isRemote(context, path)) {
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputChecker.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputChecker.java
index f5f44b3d729f8d..60f361bfdb486d 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputChecker.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputChecker.java
@@ -279,7 +279,6 @@ public boolean shouldDownloadOutput(ActionInput output) {
return shouldDownloadOutput(output.getExecPath());
}
- /** Returns whether an {@link ActionInput} with the given path should be downloaded. */
public boolean shouldDownloadOutput(PathFragment execPath) {
return outputsMode == RemoteOutputsMode.ALL
|| pathsToDownload.contains(execPath)
@@ -288,8 +287,8 @@ public boolean shouldDownloadOutput(PathFragment execPath) {
@Override
public boolean shouldTrustArtifact(ActionInput file, FileArtifactValue metadata) {
- // Local metadata is always trusted.
- if (!metadata.isRemote()) {
+ // Eager metadata is always trusted.
+ if (!metadata.isLazy()) {
return true;
}
diff --git a/src/main/java/com/google/devtools/build/lib/remote/merkletree/BUILD b/src/main/java/com/google/devtools/build/lib/remote/merkletree/BUILD
index 00f5b343ae3cf4..027857705eabb1 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/merkletree/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/remote/merkletree/BUILD
@@ -22,6 +22,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/profiler",
"//src/main/java/com/google/devtools/build/lib/remote:scrubber",
"//src/main/java/com/google/devtools/build/lib/remote/util:digest_utils",
+ "//src/main/java/com/google/devtools/build/lib/util:stream_writer",
"//src/main/java/com/google/devtools/build/lib/util:string_encoding",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
diff --git a/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTree.java b/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTree.java
index c24763b7d0f8a8..56169f74750fcc 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTree.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTree.java
@@ -16,7 +16,7 @@
import build.bazel.remote.execution.v2.Digest;
import com.google.common.base.Preconditions;
import com.google.common.collect.Iterables;
-import com.google.devtools.build.lib.actions.cache.VirtualActionInput;
+import com.google.devtools.build.lib.util.StreamWriter;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
@@ -73,7 +73,7 @@ public boolean equals(Object o) {
static class FileNode extends Node {
private final Path path;
- private final VirtualActionInput virtualActionInput;
+ private final StreamWriter data;
private final Digest digest;
private final boolean isExecutable;
private final boolean toolInput;
@@ -97,7 +97,7 @@ static FileNode createExecutable(
static FileNode createExecutable(
String pathSegment,
- VirtualActionInput virtualActionInput,
+ StreamWriter virtualActionInput,
Digest digest,
boolean toolInput) {
return new FileNode(
@@ -108,7 +108,7 @@ private FileNode(
String pathSegment, Path path, Digest digest, boolean isExecutable, boolean toolInput) {
super(pathSegment);
this.path = Preconditions.checkNotNull(path, "path");
- this.virtualActionInput = null;
+ this.data = null;
this.digest = Preconditions.checkNotNull(digest, "digest");
this.isExecutable = isExecutable;
this.toolInput = toolInput;
@@ -116,13 +116,13 @@ private FileNode(
private FileNode(
String pathSegment,
- VirtualActionInput input,
+ StreamWriter input,
Digest digest,
boolean isExecutable,
boolean toolInput) {
super(pathSegment);
this.path = null;
- this.virtualActionInput = Preconditions.checkNotNull(input, "data");
+ this.data = Preconditions.checkNotNull(input, "data");
this.digest = Preconditions.checkNotNull(digest, "digest");
this.isExecutable = isExecutable;
this.toolInput = toolInput;
@@ -136,8 +136,8 @@ Path getPath() {
return path;
}
- VirtualActionInput getVirtualActionInput() {
- return virtualActionInput;
+ StreamWriter getData() {
+ return data;
}
public boolean isExecutable() {
@@ -151,7 +151,7 @@ boolean isToolInput() {
@Override
public int hashCode() {
return Objects.hash(
- super.hashCode(), path, virtualActionInput, digest, toolInput, isExecutable);
+ super.hashCode(), path, data, digest, toolInput, isExecutable);
}
@Override
@@ -159,7 +159,7 @@ public boolean equals(Object o) {
if (o instanceof FileNode other) {
return super.equals(other)
&& Objects.equals(path, other.path)
- && Objects.equals(virtualActionInput, other.virtualActionInput)
+ && Objects.equals(data, other.data)
&& Objects.equals(digest, other.digest)
&& toolInput == other.toolInput
&& isExecutable == other.isExecutable;
diff --git a/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java b/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java
index 38f46a177a3004..8e915e03d92f82 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java
@@ -169,11 +169,21 @@ private static int buildFromActionInputs(
switch (metadata.getType()) {
case REGULAR_FILE -> {
Digest d = DigestUtil.buildDigest(metadata.getDigest(), metadata.getSize());
- Path inputPath = artifactPathResolver.toPath(input);
- boolean childAdded =
- currDir.addChild(
- FileNode.createExecutable(
- path.getBaseName(), inputPath, d, toolInputs.contains(path)));
+ boolean childAdded;
+ if (metadata.isInline()) {
+ childAdded =
+ currDir.addChild(
+ FileNode.createExecutable(
+ path.getBaseName(), metadata, d, toolInputs.contains(path)));
+ } else {
+ childAdded =
+ currDir.addChild(
+ FileNode.createExecutable(
+ path.getBaseName(),
+ artifactPathResolver.toPath(input),
+ d,
+ toolInputs.contains(path)));
+ }
return childAdded ? 1 : 0;
}
case DIRECTORY -> {
diff --git a/src/main/java/com/google/devtools/build/lib/remote/merkletree/MerkleTree.java b/src/main/java/com/google/devtools/build/lib/remote/merkletree/MerkleTree.java
index db6f3b90c276e4..669c569023a3dc 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/merkletree/MerkleTree.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/merkletree/MerkleTree.java
@@ -32,13 +32,13 @@
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.ArtifactPathResolver;
import com.google.devtools.build.lib.actions.InputMetadataProvider;
-import com.google.devtools.build.lib.actions.cache.VirtualActionInput;
import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.profiler.SilentCloseable;
import com.google.devtools.build.lib.remote.Scrubber.SpawnScrubber;
+import com.google.devtools.build.lib.remote.merkletree.MerkleTree.ContentSource.InMemorySource;
import com.google.devtools.build.lib.remote.merkletree.MerkleTree.ContentSource.PathSource;
-import com.google.devtools.build.lib.remote.merkletree.MerkleTree.ContentSource.VirtualActionInputSource;
import com.google.devtools.build.lib.remote.util.DigestUtil;
+import com.google.devtools.build.lib.util.StreamWriter;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.io.IOException;
@@ -63,8 +63,7 @@ public sealed interface ContentSource {
record PathSource(Path path) implements ContentSource {}
/** Content provided by a virtual action input. */
- record VirtualActionInputSource(VirtualActionInput virtualActionInput)
- implements ContentSource {}
+ record InMemorySource(StreamWriter streamWriter) implements ContentSource {}
}
private interface MerkleTreeDirectoryVisitor {
@@ -172,7 +171,7 @@ private Map<Digest, Object> getDigestFileMap() {
if (file.getPath() != null) {
newDigestMap.put(file.getDigest(), file.getPath());
} else {
- newDigestMap.put(file.getDigest(), file.getVirtualActionInput());
+ newDigestMap.put(file.getDigest(), file.getData());
}
}
});
@@ -190,8 +189,7 @@ public Directory getDirectoryByDigest(Digest digest) {
public ContentSource getFileByDigest(Digest digest) {
return switch (getDigestFileMap().get(digest)) {
case Path path -> new PathSource(path);
- case VirtualActionInput virtualActionInput ->
- new VirtualActionInputSource(virtualActionInput);
+ case StreamWriter streamWriter -> new InMemorySource(streamWriter);
case null -> null;
default ->
throw new IllegalStateException("Unexpected value: " + getDigestFileMap().get(digest));
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
index d1f0e1ba531640..218410f4af30a0 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
@@ -1091,11 +1091,13 @@ private static BlazeCommandResult fullyValidateTarget(
Path executablePath = executable.getPath();
try {
- if (!executablePath.exists() || !executablePath.isExecutable()) {
+ if (!executablePath.exists()) {
return reportAndCreateFailureResult(
- env,
- "Non-existent or non-executable " + executablePath,
- Code.TARGET_BUILT_BUT_PATH_NOT_EXECUTABLE);
+ env, "Non-existent " + executablePath, Code.TARGET_BUILT_BUT_PATH_NOT_EXECUTABLE);
+ }
+ if (!executablePath.isExecutable()) {
+ return reportAndCreateFailureResult(
+ env, "Non-executable " + executablePath, Code.TARGET_BUILT_BUT_PATH_NOT_EXECUTABLE);
}
} catch (IOException e) {
return reportAndCreateFailureResult(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java b/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java
index d48071791a8f8e..cdf22a4d104a30 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java
@@ -524,11 +524,11 @@ private boolean artifactIsDirtyWithDirectSystemCalls(
FileArtifactValue fileMetadata =
ActionOutputMetadataStore.fileArtifactValueFromArtifact(
file, null, xattrProviderOverrider.getXattrProvider(syscallCache), tsgm);
- boolean isTrustedRemoteValue =
+ boolean isTrustedLazyValue =
fileMetadata.getType() == FileStateType.NONEXISTENT
- && lastKnownData.isRemote()
+ && lastKnownData.isLazy()
&& outputChecker.shouldTrustArtifact(file, lastKnownData);
- if (!isTrustedRemoteValue && fileMetadata.couldBeModifiedSince(lastKnownData)) {
+ if (!isTrustedLazyValue && fileMetadata.couldBeModifiedSince(lastKnownData)) {
modifiedOutputsReceiver.reportModifiedOutputFile(
fileMetadata.getType() != FileStateType.NONEXISTENT
? file.getPath().getLastModifiedTime(Symlinks.FOLLOW)
diff --git a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneModule.java b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneModule.java
index 6535cc94671ee9..d4e75e7e6a29cb 100644
--- a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneModule.java
+++ b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneModule.java
@@ -22,8 +22,9 @@
import com.google.devtools.build.lib.analysis.test.TestActionContext;
import com.google.devtools.build.lib.analysis.test.TestStrategy;
import com.google.devtools.build.lib.buildtool.BuildRequest;
+import com.google.devtools.build.lib.exec.EagerFileWriteStrategy;
import com.google.devtools.build.lib.exec.ExecutionOptions;
-import com.google.devtools.build.lib.exec.FileWriteStrategy;
+import com.google.devtools.build.lib.exec.LazyFileWriteStrategy;
import com.google.devtools.build.lib.exec.ModuleActionContextRegistry;
import com.google.devtools.build.lib.exec.RunfilesTreeUpdater;
import com.google.devtools.build.lib.exec.SpawnRunner;
@@ -69,7 +70,13 @@ public void registerActionContexts(
registryBuilder.register(TestActionContext.class, testStrategy, "standalone");
registryBuilder.register(
TestActionContext.class, new ExclusiveTestStrategy(testStrategy), "exclusive");
- registryBuilder.register(FileWriteActionContext.class, new FileWriteStrategy(), "local");
+ registryBuilder.register(
+ FileWriteActionContext.class,
+ switch (executionOptions.fileWriteStrategy) {
+ case LAZY -> new LazyFileWriteStrategy();
+ case EAGER -> new EagerFileWriteStrategy();
+ },
+ "local");
registryBuilder.register(
TemplateExpansionContext.class, new LocalTemplateExpansionStrategy(), "local");
}
diff --git a/src/main/java/com/google/devtools/build/lib/util/BUILD b/src/main/java/com/google/devtools/build/lib/util/BUILD
index e1651d83476157..fe328ea6be02f8 100644
--- a/src/main/java/com/google/devtools/build/lib/util/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/util/BUILD
@@ -271,6 +271,9 @@ java_library(
srcs = [
"StreamWriter.java",
],
+ deps = [
+ "@com_google_protobuf//:protobuf_java",
+ ],
)
java_library(
diff --git a/src/main/java/com/google/devtools/build/lib/util/StreamWriter.java b/src/main/java/com/google/devtools/build/lib/util/StreamWriter.java
index 2a501d323ae36b..e1e87c37cfa4d4 100644
--- a/src/main/java/com/google/devtools/build/lib/util/StreamWriter.java
+++ b/src/main/java/com/google/devtools/build/lib/util/StreamWriter.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.util;
+import com.google.protobuf.ByteString;
import java.io.IOException;
import java.io.OutputStream;
@@ -27,4 +28,22 @@ public interface StreamWriter {
* @throws IOException only if out throws an IOException
*/
void writeTo(OutputStream out) throws IOException;
+
+ /**
+ * Gets a {@link ByteString} representation of the content to write. Used to avoid copying if the
+ * stream is internally represented as a {@link ByteString}.
+ *
+ * <p>Prefer {@link #writeTo} to this method to avoid materializing the entire file in memory. The
+ * return value should not be retained.
+ */
+ default ByteString getBytes() {
+ ByteString.Output out = ByteString.newOutput();
+ try {
+ writeTo(out);
+ } catch (IOException e) {
+ // ByteString.Output doesn't throw IOExceptions.
+ throw new IllegalStateException(e);
+ }
+ return out.toByteString();
+ }
}
diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/ConvenienceSymlinkTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/ConvenienceSymlinkTest.java
index 0e4623b297b035..391ed95d11928c 100644
--- a/src/test/java/com/google/devtools/build/lib/buildtool/ConvenienceSymlinkTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildtool/ConvenienceSymlinkTest.java
@@ -37,7 +37,7 @@
import com.google.devtools.build.lib.buildtool.util.TestRuleModule;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.events.EventHandler;
-import com.google.devtools.build.lib.exec.FileWriteStrategy;
+import com.google.devtools.build.lib.exec.EagerFileWriteStrategy;
import com.google.devtools.build.lib.exec.ModuleActionContextRegistry;
import com.google.devtools.build.lib.packages.AttributeTransitionData;
import com.google.devtools.build.lib.packages.ImplicitOutputsFunction;
@@ -195,7 +195,7 @@ public void registerActionContexts(
CommandEnvironment env,
BuildRequest buildRequest) {
// we need an implementation of FileWriteActionContext to get our file writes to succeed
- registryBuilder.register(FileWriteActionContext.class, new FileWriteStrategy());
+ registryBuilder.register(FileWriteActionContext.class, new EagerFileWriteStrategy());
// we need something to consume FileWriteActionContext or the registration will have no effect
registryBuilder.restrictTo(FileWriteActionContext.class, "local");
}
diff --git a/src/test/java/com/google/devtools/build/lib/exec/BUILD b/src/test/java/com/google/devtools/build/lib/exec/BUILD
index bd7978c336e65c..8532fa6d9e4e4c 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/exec/BUILD
@@ -46,6 +46,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
"//src/main/java/com/google/devtools/build/lib/analysis:server_directories",
"//src/main/java/com/google/devtools/build/lib/bazel/rules/python",
+ "//src/main/java/com/google/devtools/build/lib/bugreport",
"//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto",
"//src/main/java/com/google/devtools/build/lib/clock",
"//src/main/java/com/google/devtools/build/lib/cmdline",
@@ -71,6 +72,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/exec:symlink_tree_helper",
"//src/main/java/com/google/devtools/build/lib/exec:symlink_tree_strategy",
"//src/main/java/com/google/devtools/build/lib/exec:test_log_helper",
+ "//src/main/java/com/google/devtools/build/lib/remote",
"//src/main/java/com/google/devtools/build/lib/remote/options",
"//src/main/java/com/google/devtools/build/lib/shell",
"//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value",
diff --git a/src/test/java/com/google/devtools/build/lib/exec/FileWriteStrategyTest.java b/src/test/java/com/google/devtools/build/lib/exec/EagerFileWriteStrategyTest.java
similarity index 95%
rename from src/test/java/com/google/devtools/build/lib/exec/FileWriteStrategyTest.java
rename to src/test/java/com/google/devtools/build/lib/exec/EagerFileWriteStrategyTest.java
index 31abcda73ac388..e5e207d1ccd6a3 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/FileWriteStrategyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/EagerFileWriteStrategyTest.java
@@ -48,12 +48,12 @@
import org.junit.Test;
import org.junit.runner.RunWith;
-/** Tests for {@link FileWriteStrategy}. */
+/** Tests for {@link EagerFileWriteStrategy}. */
@RunWith(TestParameterInjector.class)
-public final class FileWriteStrategyTest {
+public final class EagerFileWriteStrategyTest {
private static final IOException INJECTED_EXCEPTION = new IOException("oh no!");
- private final FileWriteStrategy fileWriteStrategy = new FileWriteStrategy();
+ private final EagerFileWriteStrategy eagerFileWriteStrategy = new EagerFileWriteStrategy();
private final SpiedFileSystem fileSystem = SpiedFileSystem.createInMemorySpy();
private final Scratch scratch = new Scratch(fileSystem);
@@ -73,7 +73,7 @@ public void writeOutputToFile_writesCorrectOutput(
AbstractAction action = createAction("file");
var unused =
- fileWriteStrategy.writeOutputToFile(
+ eagerFileWriteStrategy.writeOutputToFile(
action,
createActionExecutionContext(),
out -> out.write(content.getBytes(UTF_8)),
@@ -124,7 +124,7 @@ public void writeOutputToFile_errorInWriter_returnsFailure(@TestParameter Failur
assertThrows(
EnvironmentalExecException.class,
() -> {
- fileWriteStrategy.writeOutputToFile(
+ eagerFileWriteStrategy.writeOutputToFile(
action,
createActionExecutionContext(),
failureMode,
diff --git a/src/test/java/com/google/devtools/build/lib/exec/LazyFileWriteStrategyTest.java b/src/test/java/com/google/devtools/build/lib/exec/LazyFileWriteStrategyTest.java
new file mode 100644
index 00000000000000..2bef191fc922ca
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/exec/LazyFileWriteStrategyTest.java
@@ -0,0 +1,190 @@
+// Copyright 2022 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.exec;
+
+import static com.google.common.truth.Truth.assertThat;
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.Assert.assertThrows;
+import static org.mockito.Mockito.mock;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSortedSet;
+import com.google.devtools.build.lib.actions.AbstractAction;
+import com.google.devtools.build.lib.actions.ActionExecutionContext;
+import com.google.devtools.build.lib.actions.ActionExecutionException;
+import com.google.devtools.build.lib.actions.ActionInput;
+import com.google.devtools.build.lib.actions.ActionInputMap;
+import com.google.devtools.build.lib.actions.ActionInputPrefetcher;
+import com.google.devtools.build.lib.actions.ActionKeyContext;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.ArtifactRoot;
+import com.google.devtools.build.lib.actions.ArtifactRoot.RootType;
+import com.google.devtools.build.lib.actions.DiscoveredModulesPruner;
+import com.google.devtools.build.lib.actions.EnvironmentalExecException;
+import com.google.devtools.build.lib.actions.ExecException;
+import com.google.devtools.build.lib.actions.Executor;
+import com.google.devtools.build.lib.actions.FileArtifactValue;
+import com.google.devtools.build.lib.actions.ThreadStateReceiver;
+import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
+import com.google.devtools.build.lib.actions.util.ActionsTestUtil.FakeInputMetadataHandlerBase;
+import com.google.devtools.build.lib.actions.util.ActionsTestUtil.NullAction;
+import com.google.devtools.build.lib.actions.util.DummyExecutor;
+import com.google.devtools.build.lib.bugreport.BugReporter;
+import com.google.devtools.build.lib.events.NullEventHandler;
+import com.google.devtools.build.lib.remote.RemoteActionFileSystem;
+import com.google.devtools.build.lib.remote.RemoteActionInputFetcher;
+import com.google.devtools.build.lib.server.FailureDetails.Execution.Code;
+import com.google.devtools.build.lib.testutil.Scratch;
+import com.google.devtools.build.lib.util.DetailedExitCode;
+import com.google.devtools.build.lib.util.ExitCode;
+import com.google.devtools.build.lib.vfs.DigestHashFunction;
+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.SyscallCache;
+import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
+import com.google.testing.junit.testparameterinjector.TestParameter;
+import com.google.testing.junit.testparameterinjector.TestParameterInjector;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+import javax.annotation.Nullable;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+/** Tests for {@link LazyFileWriteStrategy}. */
+@RunWith(TestParameterInjector.class)
+public final class LazyFileWriteStrategyTest {
+ private static final IOException INJECTED_EXCEPTION = new IOException("oh no!");
+
+ private final LazyFileWriteStrategy lazyFileWriteStrategy = new LazyFileWriteStrategy();
+
+ private final FileSystem fileSystem = new InMemoryFileSystem(DigestHashFunction.SHA256);
+ private final Scratch scratch = new Scratch(fileSystem);
+ private Path execRoot;
+ private ArtifactRoot outputRoot;
+ private RemoteActionFileSystem actionFileSystem;
+ private FakeInputMetadataHandlerBase metadataHandler;
+ private AbstractAction action;
+
+ @Before
+ public void createOutputRoot() throws IOException {
+ execRoot = scratch.dir("/execroot");
+ outputRoot = ArtifactRoot.asDerivedRoot(execRoot, RootType.Output, "bazel-out");
+ outputRoot.getRoot().asPath().createDirectory();
+ metadataHandler =
+ new FakeInputMetadataHandlerBase() {
+ private Map<ActionInput, FileArtifactValue> injectedMetadata = new HashMap<>();
+
+ @Override
+ public void injectFile(Artifact output, FileArtifactValue metadata) {
+ injectedMetadata.put(output, metadata);
+ }
+
+ @Nullable
+ @Override
+ public FileArtifactValue getOutputMetadata(ActionInput input) {
+ return injectedMetadata.get(input);
+ }
+ };
+ action = createAction("file");
+ actionFileSystem =
+ new RemoteActionFileSystem(
+ fileSystem,
+ execRoot.asFragment(),
+ outputRoot.getExecPathString(),
+ new ActionInputMap(BugReporter.defaultInstance(), 0),
+ action.getOutputs(),
+ metadataHandler,
+ mock(RemoteActionInputFetcher.class));
+ actionFileSystem.createDirectoryAndParents(
+ execRoot
+ .getRelative(action.getPrimaryOutput().getExecPath().getParentDirectory())
+ .asFragment());
+ }
+
+ @Test
+ public void writeOutputToFile_writesCorrectOutput(
+ @TestParameter({"", "hello", "hello there"}) String content) throws Exception {
+ var unused =
+ lazyFileWriteStrategy.writeOutputToFile(
+ action,
+ createActionExecutionContext(actionFileSystem, metadataHandler),
+ out -> out.write(content.getBytes(UTF_8)),
+ /* makeExecutable= */ false,
+ /* isRemotable= */ true);
+
+ FileArtifactValue metadata = metadataHandler.getOutputMetadata(action.getPrimaryOutput());
+ assertThat(metadata.isInline()).isTrue();
+ assertThat(metadata.getInputStream().readAllBytes()).isEqualTo(content.getBytes(UTF_8));
+ }
+
+ @Test
+ public void writeOutputToFile_errorInWriter_returnsFailure() throws Exception {
+ AbstractAction action = createAction("file");
+
+ ExecException e =
+ assertThrows(
+ EnvironmentalExecException.class,
+ () ->
+ lazyFileWriteStrategy.writeOutputToFile(
+ action,
+ createActionExecutionContext(actionFileSystem, metadataHandler),
+ out -> {
+ throw INJECTED_EXCEPTION;
+ },
+ /* makeExecutable= */ false,
+ /* isRemotable= */ true));
+
+ assertThat(e).hasCauseThat().isSameInstanceAs(INJECTED_EXCEPTION);
+ var detailExitCode = getDetailExitCode(e);
+ assertThat(detailExitCode.getExitCode()).isEqualTo(ExitCode.LOCAL_ENVIRONMENTAL_ERROR);
+ assertThat(detailExitCode.getFailureDetail().getExecution().getCode())
+ .isEqualTo(Code.FILE_WRITE_IO_EXCEPTION);
+ }
+
+ private DetailedExitCode getDetailExitCode(ExecException e) {
+ return ActionExecutionException.fromExecException(e, new NullAction()).getDetailedExitCode();
+ }
+
+ private ActionExecutionContext createActionExecutionContext(
+ FileSystem actionFileSystem, FakeInputMetadataHandlerBase metadataHandler) {
+ Executor executor = new DummyExecutor(fileSystem, execRoot);
+ return new ActionExecutionContext(
+ executor,
+ /* inputMetadataProvider= */ metadataHandler,
+ ActionInputPrefetcher.NONE,
+ new ActionKeyContext(),
+ /* outputMetadataStore= */ metadataHandler,
+ /* rewindingEnabled= */ false,
+ ActionExecutionContext.LostInputsCheck.NONE,
+ /* fileOutErr= */ null,
+ NullEventHandler.INSTANCE,
+ /* clientEnv= */ ImmutableMap.of(),
+ /* topLevelFilesets= */ ImmutableMap.of(),
+ treeArtifact -> ImmutableSortedSet.of(),
+ /* actionFileSystem= */ actionFileSystem,
+ /* skyframeDepsResult= */ null,
+ DiscoveredModulesPruner.DEFAULT,
+ SyscallCache.NO_CACHE,
+ ThreadStateReceiver.NULL_INSTANCE);
+ }
+
+ private AbstractAction createAction(String outputRelativePath) {
+ return new NullAction(
+ ActionsTestUtil.createArtifactWithRootRelativePath(
+ outputRoot, PathFragment.create(outputRelativePath)));
+ }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/exec/util/TestExecutorBuilder.java b/src/test/java/com/google/devtools/build/lib/exec/util/TestExecutorBuilder.java
index 7cd1a44c8ce9ee..a66e9557d2720c 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/util/TestExecutorBuilder.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/util/TestExecutorBuilder.java
@@ -29,7 +29,7 @@
import com.google.devtools.build.lib.exec.BinTools;
import com.google.devtools.build.lib.exec.BlazeExecutor;
import com.google.devtools.build.lib.exec.ExecutionOptions;
-import com.google.devtools.build.lib.exec.FileWriteStrategy;
+import com.google.devtools.build.lib.exec.EagerFileWriteStrategy;
import com.google.devtools.build.lib.exec.ModuleActionContextRegistry;
import com.google.devtools.build.lib.exec.SpawnStrategyRegistry;
import com.google.devtools.build.lib.exec.SpawnStrategyResolver;
@@ -69,7 +69,7 @@ public TestExecutorBuilder(
public TestExecutorBuilder(FileSystem fileSystem, Path execRoot, BinTools binTools) {
this.fileSystem = fileSystem;
this.execRoot = execRoot;
- addContext(FileWriteActionContext.class, new FileWriteStrategy());
+ addContext(FileWriteActionContext.class, new EagerFileWriteStrategy());
addContext(TemplateExpansionContext.class, new LocalTemplateExpansionStrategy());
addContext(
SymlinkTreeActionContext.class,
diff --git a/src/test/java/com/google/devtools/build/lib/remote/BUILD b/src/test/java/com/google/devtools/build/lib/remote/BUILD
index 0afa13bb3aca7a..1b9c95c82eb06b 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/remote/BUILD
@@ -212,6 +212,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
+ "//src/main/java/com/google/devtools/build/lib/exec:execution_options",
"//src/main/java/com/google/devtools/build/lib/skyframe:action_execution_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value",
"//src/main/java/com/google/devtools/build/lib/util:command",
@@ -223,6 +224,7 @@ java_library(
"//third_party:guava",
"//third_party:junit4",
"//third_party:truth",
+ "@maven//:com_google_testparameterinjector_test_parameter_injector",
],
)
@@ -252,6 +254,7 @@ java_test(
"//third_party:guava",
"//third_party:junit4",
"//third_party:truth",
+ "@maven//:com_google_testparameterinjector_test_parameter_injector",
],
)
diff --git a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java
index 04294c160cf0c0..c44bc21bff58e0 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java
@@ -39,6 +39,7 @@
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Symlinks;
+import com.google.testing.junit.testparameterinjector.TestParameterInjector;
import java.io.IOException;
import org.junit.After;
import org.junit.Test;
@@ -46,7 +47,7 @@
import org.junit.runners.JUnit4;
/** Integration tests for Build without the Bytes. */
-@RunWith(JUnit4.class)
+@RunWith(TestParameterInjector.class)
public class BuildWithoutTheBytesIntegrationTest extends BuildWithoutTheBytesIntegrationTestBase {
private WorkerInstance worker;
@@ -190,6 +191,8 @@ def _substitute_username_impl(ctx):
)
""");
+ addOptions("--file_write_strategy=eager");
+
buildTarget("//a:substitute-buchgr");
// The genrule //a:generate-template should run remotely and //a:substitute-buchgr should be a
diff --git a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java
index f0b7f0216e9f14..43910599e416e6 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java
@@ -31,13 +31,16 @@
import com.google.devtools.build.lib.actions.FileArtifactValue;
import com.google.devtools.build.lib.analysis.TargetCompleteEvent;
import com.google.devtools.build.lib.buildtool.util.BuildIntegrationTestCase;
+import com.google.devtools.build.lib.exec.ExecutionOptions.FileWriteStrategy;
import com.google.devtools.build.lib.skyframe.ActionExecutionValue;
import com.google.devtools.build.lib.skyframe.TreeArtifactValue;
import com.google.devtools.build.lib.util.CommandBuilder;
import com.google.devtools.build.lib.util.OS;
import com.google.devtools.build.lib.util.io.RecordingOutErr;
+import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.testing.junit.testparameterinjector.TestParameter;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
@@ -1029,6 +1032,54 @@ public void downloadToplevel_symlinkToDirectory() throws Exception {
assertValidOutputFile("foo-link/file-3", "3");
}
+ @Test
+ public void downloadToplevel_symlinkToWrittenFile(
+ @TestParameter FileWriteStrategy fileWriteStrategy,
+ @TestParameter({"expand_template", "write_file"}) String fileWriteRule)
+ throws Exception {
+ setDownloadToplevel();
+ writeSymlinkRule();
+ writeFileWriteRules();
+ write(
+ "BUILD",
+ """
+ load(':symlink.bzl', 'symlink')
+ load('//rules:%1$s.bzl', '%1$s')
+ %1$s(
+ name = 'foo',
+ content = 'hello',
+ # assertValidOutputFile checks for the executable bit.
+ executable = True,
+ )
+ symlink(
+ name = 'foo-link',
+ target_artifact = ':foo',
+ )
+ """
+ .formatted(fileWriteRule));
+
+ addOptions("--file_write_strategy=" + fileWriteStrategy);
+
+ buildTarget("//:foo-link");
+
+ assertSymlink("foo-link", getOutputPath("foo").asFragment());
+ assertValidOutputFile("foo-link", "hello");
+
+ // Delete link, re-plant symlink
+ getOutputPath("foo-link").delete();
+ buildTarget("//:foo-link");
+
+ assertSymlink("foo-link", getOutputPath("foo").asFragment());
+ assertValidOutputFile("foo-link", "hello");
+
+ // Delete target, re-download it
+ getOutputPath("foo").delete();
+ buildTarget("//:foo-link");
+
+ assertSymlink("foo-link", getOutputPath("foo").asFragment());
+ assertValidOutputFile("foo-link", "hello");
+ }
+
@Test
public void downloadToplevel_unresolvedSymlink() throws Exception {
// Dangling symlink would require developer mode to be enabled in the CI environment.
@@ -1055,6 +1106,89 @@ public void downloadToplevel_unresolvedSymlink() throws Exception {
assertSymlink("foo-link", PathFragment.create("/some/path"));
}
+ @Test
+ public void downloadMinimal_fileWrite(
+ @TestParameter boolean isExecutable,
+ @TestParameter FileWriteStrategy fileWriteStrategy,
+ @TestParameter({"expand_template", "write_file"}) String fileWriteRule)
+ throws Exception {
+ writeFileWriteRules();
+ writeSymlinkRule();
+ // Remote execution stages all files as executable.
+ write(
+ "BUILD",
+ """
+ load('//rules:%1$s.bzl', '%1$s')
+ %1$s(
+ name = 'foo',
+ content = 'hello',
+ executable = %2$s,
+ )
+ genrule(
+ name = 'gen',
+ srcs = [':foo'],
+ outs = ['out/gen.txt'],
+ cmd = \"""
+ %3$s
+ cat $(location :foo) $(location :foo) > $@
+ \""",
+ )
+ """
+ .formatted(
+ fileWriteRule,
+ isExecutable ? "True" : "False",
+ OS.getCurrent() == OS.WINDOWS
+ ? ""
+ : """
+ [ -x $(location :foo) ] || { echo "unexpectedly not executable"; exit 1; }
+ """));
+
+ addOptions("--file_write_strategy=" + fileWriteStrategy);
+
+ buildTarget("//:gen");
+ if (fileWriteStrategy == FileWriteStrategy.LAZY) {
+ assertOutputsDoNotExist("//:foo");
+ } else {
+ assertValidOutputFile("foo", "hello");
+ }
+ assertOutputsDoNotExist("//:gen");
+ }
+
+ @Test
+ public void downloadToplevel_fileWrite(
+ @TestParameter boolean isExecutable,
+ @TestParameter FileWriteStrategy fileWriteStrategy,
+ @TestParameter({"expand_template", "write_file"}) String fileWriteRule)
+ throws Exception {
+ setDownloadToplevel();
+ writeFileWriteRules();
+ write(
+ "BUILD",
+ """
+ load('//rules:%1$s.bzl', '%1$s')
+ %1$s(
+ name = 'foo',
+ content = 'hello',
+ executable = %2$s,
+ )
+ """
+ .formatted(fileWriteRule, isExecutable ? "True" : "False"));
+
+ addOptions("--file_write_strategy=" + fileWriteStrategy);
+
+ buildTarget("//:foo");
+
+ assertOnlyOutputContent("//:foo", "foo", "hello");
+ // TODO: Bazel doesn't honor the executable bit.
+ assertThat(getOutputPath("foo").isExecutable()).isTrue();
+
+ // Delete file, re-create it
+ getOutputPath("foo-link").delete();
+ buildTarget("//:foo");
+
+ assertOnlyOutputContent("//:foo", "foo", "hello");
+ }
+
@Test
public void treeOutputsFromLocalFileSystem_works() throws Exception {
// Test that tree artifact generated locally can be consumed by other actions.
@@ -1372,6 +1506,57 @@ public void incrementalBuild_fileOutputIsPrefetched_noRuns() throws Exception {
assertThat(metadata.getContentsProxy()).isNotNull();
}
+ @Test
+ public void incrementalBuild_writeFileOutputIsPrefetched_noRuns(
+ @TestParameter({"expand_template", "write_file"}) String fileWriteRule) throws Exception {
+ // We need to download the intermediate output
+ if (!hasAccessToRemoteOutputs()) {
+ return;
+ }
+
+ // Arrange: Prepare workspace and run a clean build
+ writeFileWriteRules();
+ write(
+ "BUILD",
+ """
+ load('//rules:%1$s.bzl', '%1$s')
+ %1$s(
+ name = 'foo',
+ content = 'foo',
+ executable = True,
+ )
+ genrule(
+ name = 'foobar',
+ srcs = [':foo'],
+ outs = ['out/foobar.txt'],
+ cmd = 'cat $(location :foo) > $@ && echo bar >> $@',
+ tags = ['no-remote'],
+ )
+ """
+ .formatted(fileWriteRule));
+
+ addOptions("--file_write_strategy=lazy");
+
+ buildTarget("//:foobar");
+ assertValidOutputFile("foo", "foo");
+ assertValidOutputFile("out/foobar.txt", "foobar\n");
+ assertThat(getOnlyElement(getMetadata("//:foo").values()).isLazy()).isTrue();
+
+ // Act: Do an incremental build without any modifications
+ ActionEventCollector actionEventCollector = new ActionEventCollector();
+ getRuntimeWrapper().registerSubscriber(actionEventCollector);
+ buildTarget("//:foobar");
+
+ // Assert: remote file metadata has contents proxy and action node is not marked as dirty.
+ assertValidOutputFile("foo", "foo");
+ assertValidOutputFile("out/foobar.txt", "foobar\n");
+ assertThat(actionEventCollector.getActionExecutedEvents()).isEmpty();
+ assertThat(actionEventCollector.getCachedActionEvents()).isEmpty();
+ var metadata = getOnlyElement(getMetadata("//:foo").values());
+ assertThat(metadata.isLazy()).isTrue();
+ assertThat(metadata.getContentsProxy()).isNotNull();
+ }
+
@Test
public void incrementalBuild_treeOutputIsPrefetched_noRuns() throws Exception {
// We need to download the intermediate output
@@ -1995,6 +2180,66 @@ protected void writeCopyAspectRule(boolean aggregate) throws IOException {
write("rules.bzl", lines.build().toArray(new String[0]));
}
+ protected void writeFileWriteRules() throws IOException {
+ write(
+ "rules/write_file.bzl",
+ """
+ def _write_file_impl(ctx):
+ out = ctx.actions.declare_file(ctx.label.name)
+ ctx.actions.write(output = out, content = ctx.attr.content, is_executable = ctx.attr.executable)
+ return DefaultInfo(files = depset([out]))
+
+ write_file = rule(
+ implementation = _write_file_impl,
+ attrs = {
+ "content": attr.string(),
+ "executable": attr.bool(default = False),
+ },
+ )
+ """);
+ write(
+ "rules/expand_template.bzl",
+ """
+ def identity(x):
+ return x
+
+ def _expand_template_impl(ctx):
+ out = ctx.actions.declare_file(ctx.label.name)
+ substitutions = ctx.actions.template_dict()
+ substitutions.add_joined(
+ "%CONTENT%",
+ depset([ctx.attr.content]),
+ join_with = "",
+ map_each = identity)
+ ctx.actions.expand_template(
+ output = out,
+ template = ctx.file._template,
+ computed_substitutions = substitutions,
+ is_executable = ctx.attr.executable,
+ )
+ return DefaultInfo(files = depset([out]))
+
+ expand_template = rule(
+ implementation = _expand_template_impl,
+ attrs = {
+ "content": attr.string(),
+ "executable": attr.bool(default = False),
+ "_template": attr.label(
+ default = "template.txt",
+ allow_single_file = True,
+ ),
+ },
+ )
+ """);
+ write(
+ "rules/BUILD",
+ """
+ exports_files(["template.txt"])
+ """);
+ FileSystemUtils.writeContentAsLatin1(
+ getWorkspace().getRelative("rules/template.txt"), "%CONTENT%");
+ }
+
protected static class ActionEventCollector {
private final List<ActionExecutedEvent> actionExecutedEvents = new ArrayList<>();
private final List<CachedActionEvent> cachedActionEvents = new ArrayList<>();
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java
index 4af76aa711eb24..3ada6ece663fc5 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java
@@ -53,6 +53,7 @@
import com.google.devtools.build.lib.remote.options.RemoteOutputsMode;
import com.google.devtools.build.lib.skyframe.TreeArtifactValue;
import com.google.devtools.build.lib.testing.vfs.SpiedFileSystem;
+import com.google.devtools.build.lib.vfs.DigestHashFunction;
import com.google.devtools.build.lib.vfs.Dirent;
import com.google.devtools.build.lib.vfs.FileStatus;
import com.google.devtools.build.lib.vfs.FileStatusWithDigest;
@@ -273,6 +274,23 @@ public void getInputStream_fromLocalFilesystem_forOutputFile() throws Exception
assertThat(contents).isEqualTo("local contents");
}
+ @Test
+ public void testGetInputStream_fromInlineMetadata() throws Exception {
+ // arrange
+ ActionInputMap inputs = new ActionInputMap(1);
+ Artifact artifact = createInlineArtifact("inline-file", "inline contents", inputs);
+ FileSystem actionFs = createActionFileSystem(inputs);
+
+ // act
+ Path actionFsPath = actionFs.getPath(artifact.getPath().asFragment());
+ String contents = FileSystemUtils.readContent(actionFsPath, UTF_8);
+
+ // assert
+ assertThat(actionFsPath.getFileSystem()).isSameInstanceAs(actionFs);
+ assertThat(contents).isEqualTo("inline contents");
+ verifyNoMoreInteractions(inputFetcher);
+ }
+
@Test
public void getInput_fromInputArtifactData_forLocalArtifact() throws Exception {
ActionInputMap inputs = new ActionInputMap(1);
@@ -1430,6 +1448,17 @@ private TreeArtifactValue createLocalTreeArtifactValue(
return builder.build();
}
+ /** Returns an inline artifact and puts its metadata into the action input map. */
+ private Artifact createInlineArtifact(String pathFragment, String content, ActionInputMap inputs)
+ throws IOException {
+ Artifact a = ActionsTestUtil.createArtifact(outputRoot, pathFragment);
+ FileArtifactValue f =
+ FileArtifactValue.createForFileWriteActionOutput(
+ out -> out.write(content.getBytes(UTF_8)), DigestHashFunction.SHA256.getHashFunction());
+ inputs.putWithNoDepOwner(a, f);
+ return a;
+ }
+
private byte[] getDigest(String content) {
return fs.getDigestFunction().getHashFunction().hashString(content, UTF_8).asBytes();
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
index 68bbde3cddabb2..b9cee597ab9027 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
@@ -1402,11 +1402,30 @@ private FileArtifactValue createRemoteMetadata(String contents, Instant expirati
hash.asBytes(), data.length, -1, expirationTime);
}
+ private FileArtifactValue createInlineMetadata(String contents) throws IOException {
+ return FileArtifactValue.createForFileWriteActionOutput(
+ out -> out.write(contents.getBytes()), fs.getDigestFunction().getHashFunction());
+ }
+
+ enum LazyMetadataType {
+ INLINE,
+ REMOTE;
+ }
+
+ private FileArtifactValue createMetadata(LazyMetadataType type, String contents)
+ throws IOException {
+ return switch (type) {
+ case INLINE -> createInlineMetadata(contents);
+ case REMOTE -> createRemoteMetadata(contents);
+ };
+ }
+
@Test
- public void testRemoteAndLocalArtifacts(@TestParameter boolean setContentsProxy)
+ public void testLazyAndLocalArtifacts(
+ @TestParameter LazyMetadataType metadataType, @TestParameter boolean setContentsProxy)
throws Exception {
- // Test that injected remote artifacts are trusted by the FileSystemValueChecker if it is
- // configured to trust remote artifacts, and that local files always take precedence over remote
+ // Test that injected lazy artifacts are trusted by the FileSystemValueChecker if it is
+ // configured to trust lazy artifacts, and that local files always take precedence over remote
// files if they are different.
SkyKey actionKey1 = ActionLookupData.create(ACTION_LOOKUP_KEY, 0);
SkyKey actionKey2 = ActionLookupData.create(ACTION_LOOKUP_KEY, 1);
@@ -1414,10 +1433,10 @@ public void testRemoteAndLocalArtifacts(@TestParameter boolean setContentsProxy)
Artifact out1 = createDerivedArtifact("foo");
Artifact out2 = createDerivedArtifact("bar");
Map<SkyKey, Delta> metadataToInject = new HashMap<>();
- var out1Metadata = createRemoteMetadata("foo-content");
+ var out1Metadata = createMetadata(metadataType, "foo-content");
metadataToInject.put(actionKey1, actionValueWithMetadata(out1, out1Metadata));
metadataToInject.put(
- actionKey2, actionValueWithMetadata(out2, createRemoteMetadata("bar-content")));
+ actionKey2, actionValueWithMetadata(out2, createMetadata(metadataType, "bar-content")));
differencer.inject(metadataToInject);
EvaluationContext evaluationContext =
@@ -1469,9 +1488,10 @@ public void testRemoteAndLocalArtifacts(@TestParameter boolean setContentsProxy)
}
@Test
- public void testRemoteAndLocalArtifacts_identicalContent(@TestParameter boolean setContentsProxy)
+ public void testLazyAndLocalArtifacts_identicalContent(
+ @TestParameter LazyMetadataType metadataType, @TestParameter boolean setContentsProxy)
throws Exception {
- // Test that if injected remote artifacts and local files are identical, the generating actions
+ // Test that if injected lazy artifacts and local files are identical, the generating actions
// are not marked as dirty if it has contents proxy.
SkyKey actionKey1 = ActionLookupData.create(ACTION_LOOKUP_KEY, 0);
SkyKey actionKey2 = ActionLookupData.create(ACTION_LOOKUP_KEY, 1);
@@ -1479,10 +1499,10 @@ public void testRemoteAndLocalArtifacts_identicalContent(@TestParameter boolean
Artifact out1 = createDerivedArtifact("foo");
Artifact out2 = createDerivedArtifact("bar");
Map<SkyKey, Delta> metadataToInject = new HashMap<>();
- var out1Metadata = createRemoteMetadata("foo-content");
+ var out1Metadata = createMetadata(metadataType, "foo-content");
metadataToInject.put(actionKey1, actionValueWithMetadata(out1, out1Metadata));
metadataToInject.put(
- actionKey2, actionValueWithMetadata(out2, createRemoteMetadata("bar-content")));
+ actionKey2, actionValueWithMetadata(out2, createMetadata(metadataType, "bar-content")));
differencer.inject(metadataToInject);
EvaluationContext evaluationContext =