[10.0.0] Add `--file_write_strategy=remote` and `--template_expansion_strategy=remote` to keep file write outputs off disk (#31402) (#32284) ### Description With these flags, `ctx.actions.write` and `ctx.actions.expand_template` stream their outputs to the disk and/or remote cache instead of writing them to disk. These outputs are then injected as metadata-only files with BwoB, honoring `--remote_download_outputs` and `--remote_download_regex` just like regular action outputs. The write falls back to the local strategy if the action isn't remotable, BwoB isn't enabled or the output is requested for download anyway. A failed upload is reported as a warning and also falls back to a local write. With uploads to the remote cache disabled (`--noremote_upload_local_results` without remote execution), the strategy still checks whether the contents are already present remotely and only writes to disk if they are missing, so that builds against a read-only cache populated by CI benefit as well. ### Motivation ### Build API Changes No ### Release Notes RELNOTES[NEW]: `--file_write_strategy=remote` and `--template_expansion_strategy=remote` keep the outputs of `ctx.actions.write` and `ctx.actions.expand_template` in the remote and/or disk cache instead of writing them to disk when building without the bytes. Closes #31402. COPYBARA_INTEGRATE_REVIEW=https://github.com/bazelbuild/bazel/pull/31402 from fmeum:remote-file-write 9f040bef2857ba996cd087a3cc3726cab0689cfe PiperOrigin-RevId: 1009655482 Change-Id: I3c903dbbbed8a8cbef9351078421da0fbcdfc10d Commit https://github.com/bazelbuild/bazel/commit/05dc4b777db1372d07bb6b0dcdb7a226ce15eeb6 --------- Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im> Co-authored-by: iancha1992 <heec@google.com>
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 334ff80..588958a 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
@@ -31,12 +31,23 @@ import java.util.List; import net.starlark.java.eval.EvalException; -/** Strategy to perform template expansion locally. */ +/** + * Strategy to perform template expansion locally and write the result with a given {@link + * FileWriteActionContext}. + */ public class LocalTemplateExpansionStrategy implements TemplateExpansionContext { public static final Class<LocalTemplateExpansionStrategy> TYPE = LocalTemplateExpansionStrategy.class; - public static LocalTemplateExpansionStrategy INSTANCE = new LocalTemplateExpansionStrategy(); + private final FileWriteActionContext fileWriteStrategy; + + /** + * @param fileWriteStrategy the strategy with which to write the expanded template, independent of + * the {@link FileWriteActionContext} selected for file write actions + */ + public LocalTemplateExpansionStrategy(FileWriteActionContext fileWriteStrategy) { + this.fileWriteStrategy = fileWriteStrategy; + } @Override public ImmutableList<SpawnResult> expandTemplate( @@ -50,13 +61,12 @@ 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); + return fileWriteStrategy.writeOutputToFile( + action, + ctx, + deterministicWriter, + templateMetadata.makeExecutable(), + /* isRemotable= */ true); } catch (IOException | EvalException e) { throw new EnvironmentalExecException( e, @@ -72,7 +82,7 @@ * public access to this method as it's unhealthy to evaluate the action result without the action * being executed. */ - public String getExpandedTemplateUnsafe( + public static String getExpandedTemplateUnsafe( Template template, List<Substitution> substitutions, ArtifactPathResolver resolver) throws EvalException, IOException, InterruptedException { String templateString;
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionAction.java index 634b450..cab17f6 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionAction.java
@@ -170,7 +170,7 @@ @VisibleForTesting public String getFileContents() throws IOException, EvalException, InterruptedException { - return LocalTemplateExpansionStrategy.INSTANCE.getExpandedTemplateUnsafe( + return LocalTemplateExpansionStrategy.getExpandedTemplateUnsafe( template, substitutions, ArtifactPathResolver.IDENTITY); }
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelStrategyModule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelStrategyModule.java index 6a3c176..1b28118 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelStrategyModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelStrategyModule.java
@@ -31,17 +31,52 @@ import com.google.devtools.build.lib.runtime.CommandEnvironment; import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.util.RegexFilter; +import com.google.devtools.common.options.Option; +import com.google.devtools.common.options.OptionDocumentationCategory; +import com.google.devtools.common.options.OptionEffectTag; import com.google.devtools.common.options.OptionsBase; +import com.google.devtools.common.options.OptionsClass; import java.util.ArrayList; import java.util.List; import java.util.Map; /** Module which registers the strategy options for Bazel. */ public class BazelStrategyModule extends BlazeModule { + /** Strategy options that only exist in Bazel. */ + @OptionsClass + public abstract static class Options extends OptionsBase { + @Option( + name = "file_write_strategy", + defaultValue = "local", + documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY, + effectTags = {OptionEffectTag.EXECUTION}, + help = + "Specifies which strategy to use for file write actions such as ctx.actions.write." + + " 'local' writes the file to disk. 'remote' requires a disk or remote cache and" + + " stores the contents there, recording them as a remote output when building" + + " without the bytes, so that the file is only written to disk if it is needed by" + + " a local action or requested via --remote_download_outputs or" + + " --remote_download_regex. If there is no disk cache and Bazel isn't allowed to" + + " upload to the remote cache, the file is only kept off disk if the remote cache" + + " already contains its contents.") + public abstract String getFileWriteStrategy(); + + @Option( + name = "template_expansion_strategy", + defaultValue = "local", + documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY, + effectTags = {OptionEffectTag.EXECUTION}, + help = + "Specifies which strategy to use for template expansion actions such as" + + " ctx.actions.expand_template. 'local' writes the expanded template to disk." + + " 'remote' keeps it off disk in the same way as --file_write_strategy=remote.") + public abstract String getTemplateExpansionStrategy(); + } + @Override public Iterable<Class<? extends OptionsBase>> getCommandOptions(String commandName) { return commandName.equals("build") - ? ImmutableList.of(ExecutionOptions.class, RemoteOptions.class) + ? ImmutableList.of(ExecutionOptions.class, RemoteOptions.class, Options.class) : ImmutableList.of(); } @@ -50,11 +85,12 @@ ModuleActionContextRegistry.Builder registryBuilder, CommandEnvironment env, BuildRequest buildRequest) { + Options options = env.getOptions().getOptions(Options.class); registryBuilder .restrictTo(CppIncludeExtractionContext.class, "") .restrictTo(CppIncludeScanningContext.class, "") - .restrictTo(FileWriteActionContext.class, "") - .restrictTo(TemplateExpansionContext.class, "") + .restrictTo(FileWriteActionContext.class, options.getFileWriteStrategy()) + .restrictTo(TemplateExpansionContext.class, options.getTemplateExpansionStrategy()) .restrictTo(SpawnCache.class, ""); }
diff --git a/src/main/java/com/google/devtools/build/lib/remote/BUILD b/src/main/java/com/google/devtools/build/lib/remote/BUILD index e27e248..63c4a89 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/BUILD
@@ -213,8 +213,6 @@ "//third_party/java/guava:collect", "//third_party/java/guava:concurrent", "//third_party/java/jsr305_annotations", - "//third_party/java/netty4:codec", - "//third_party/java/netty4:codec_http", "//third_party/java/rxjava3", "@remoteapis//:build_bazel_remote_execution_v2_remote_execution_java_proto", ], @@ -586,15 +584,18 @@ ":combined_cache", ":remote_action_input_fetcher", ":remote_execution_cache", + ":remote_file_write_strategy", ":remote_output_checker", ":remote_spawn_cache", ":remote_spawn_runner", ":remote_spawn_strategy", + "//src/main/java/com/google/devtools/build/lib/analysis/actions:file_write_action_context", + "//src/main/java/com/google/devtools/build/lib/analysis/actions:template_expansion_action", "//src/main/java/com/google/devtools/build/lib/exec:execution_options", + "//src/main/java/com/google/devtools/build/lib/exec:file_write_strategy", "//src/main/java/com/google/devtools/build/lib/exec:module_action_context_registry", "//src/main/java/com/google/devtools/build/lib/exec:spawn_cache", "//src/main/java/com/google/devtools/build/lib/exec:spawn_strategy_registry", - "//src/main/java/com/google/devtools/build/lib/packages/semantics", "//src/main/java/com/google/devtools/build/lib/remote/common", "//src/main/java/com/google/devtools/build/lib/remote/options", "//src/main/java/com/google/devtools/build/lib/remote/util:digest_util", @@ -976,6 +977,31 @@ ) java_library( + name = "remote_file_write_strategy", + srcs = ["RemoteFileWriteStrategy.java"], + deps = [ + ":chunker", + ":combined_cache", + ":remote_action_file_system", + ":remote_output_checker", + "//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/analysis/actions:file_write_action_context", + "//src/main/java/com/google/devtools/build/lib/events", + "//src/main/java/com/google/devtools/build/lib/profiler", + "//src/main/java/com/google/devtools/build/lib/remote/common", + "//src/main/java/com/google/devtools/build/lib/remote/util:digest_util", + "//src/main/java/com/google/devtools/build/lib/remote/util:futures", + "//src/main/java/com/google/devtools/build/lib/remote/util:tracing_metadata_utils", + "//src/main/java/com/google/devtools/build/lib/remote/util:utils", + "//src/main/java/com/google/devtools/build/lib/util:deterministic_writer", + "//src/main/protobuf:failure_details_java_proto", + "//third_party/java/guava:collect", + "@remoteapis//:build_bazel_remote_execution_v2_remote_execution_java_proto", + ], +) + +java_library( name = "remote_spawn_cache", srcs = ["RemoteSpawnCache.java"], visibility = [
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java index 9398242..3a85f3b 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
@@ -18,7 +18,11 @@ import build.bazel.remote.execution.v2.Digest; import com.google.common.base.Preconditions; import com.google.common.util.concurrent.ListeningScheduledExecutorService; +import com.google.devtools.build.lib.analysis.actions.FileWriteActionContext; +import com.google.devtools.build.lib.analysis.actions.LocalTemplateExpansionStrategy; +import com.google.devtools.build.lib.analysis.actions.TemplateExpansionContext; import com.google.devtools.build.lib.exec.ExecutionOptions; +import com.google.devtools.build.lib.exec.FileWriteStrategy; import com.google.devtools.build.lib.exec.ModuleActionContextRegistry; import com.google.devtools.build.lib.exec.SpawnCache; import com.google.devtools.build.lib.exec.SpawnStrategyRegistry; @@ -218,6 +222,49 @@ registryBuilder.register(SpawnCache.class, spawnCache, "remote-cache"); } + /** + * Registers file write and template expansion strategies that store file contents in the disk + * and/or remote cache under the {@code remote} identifier if this instance was created with a + * cache, otherwise does nothing. Whether they are used is up to {@code --file_write_strategy} and + * {@code --template_expansion_strategy}. + * + * @param registryBuilder builder with which to register the strategies + */ + public void registerFileWriteStrategies(ModuleActionContextRegistry.Builder registryBuilder) { + if (combinedCache == null || remoteOutputChecker == null) { + return; + } + ExecutionOptions executionOptions = + checkNotNull(env.getOptions().getOptions(ExecutionOptions.class)); + RemoteOptions remoteOptions = checkNotNull(env.getOptions().getOptions(RemoteOptions.class)); + + var localFileWriteStrategy = new FileWriteStrategy(); + var remoteFileWriteStrategy = + new RemoteFileWriteStrategy( + new FileWriteStrategy(), + combinedCache, + remoteOutputChecker, + digestUtil, + env.getBuildRequestId(), + env.getCommandId().toString(), + remoteOptions.getRemoteCacheTtl(), + // Remote execution uploads action inputs regardless of the setting for local results. + /* remoteUploadEnabled= */ remoteExecutor != null + || remoteOptions.getRemoteUploadLocalResults(), + executionOptions.getVerboseFailures()); + + registryBuilder.register(FileWriteActionContext.class, remoteFileWriteStrategy, "remote"); + registryBuilder.register(FileWriteActionContext.class, localFileWriteStrategy, "local"); + registryBuilder.register( + TemplateExpansionContext.class, + new LocalTemplateExpansionStrategy(remoteFileWriteStrategy), + "remote"); + registryBuilder.register( + TemplateExpansionContext.class, + new LocalTemplateExpansionStrategy(localFileWriteStrategy), + "local"); + } + CombinedCache getCombinedCache() { return combinedCache; }
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteFileWriteStrategy.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteFileWriteStrategy.java new file mode 100644 index 0000000..854c3e6 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteFileWriteStrategy.java
@@ -0,0 +1,224 @@ +// Copyright 2026 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.remote; + +import static com.google.devtools.build.lib.remote.util.Futures.getFromFuture; + +import build.bazel.remote.execution.v2.Digest; +import build.bazel.remote.execution.v2.RequestMetadata; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.actions.AbstractAction; +import com.google.devtools.build.lib.actions.ActionExecutionContext; +import com.google.devtools.build.lib.actions.ActionOwner; +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.RunningActionEvent; +import com.google.devtools.build.lib.actions.SpawnResult; +import com.google.devtools.build.lib.analysis.actions.FileWriteActionContext; +import com.google.devtools.build.lib.events.Event; +import com.google.devtools.build.lib.profiler.Profiler; +import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; +import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext.CachePolicy; +import com.google.devtools.build.lib.remote.common.RemoteCacheClient.Blob; +import com.google.devtools.build.lib.remote.util.DigestUtil; +import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; +import com.google.devtools.build.lib.remote.util.Utils; +import com.google.devtools.build.lib.server.FailureDetails.Execution.Code; +import com.google.devtools.build.lib.util.DeterministicWriter; +import java.io.IOException; +import java.io.InputStream; +import java.time.Duration; +import java.time.Instant; + +/** + * A {@link FileWriteActionContext} that stores the contents of the file in the disk and/or remote + * cache and records them as a remote output instead of writing them to disk. + * + * <p>The file is written to disk instead if the file write isn't remotable, if the action doesn't + * run on a {@link RemoteActionFileSystem} or if the output has to be downloaded anyway according to + * {@code --remote_download_outputs} or {@code --remote_download_regex}. + * + * <p>The contents are always stored in the disk cache if there is one. They are uploaded to the + * remote cache if it doesn't have them yet and uploads are enabled. If there is no disk cache and + * uploads to the remote cache are disabled, the contents are only recorded as a remote output if + * the remote cache already has them and the file is written to disk otherwise. If the caches lose + * the contents later, action rewinding re-executes the action to store them again. + */ +public final class RemoteFileWriteStrategy implements FileWriteActionContext { + private final FileWriteActionContext localStrategy; + private final CombinedCache combinedCache; + private final RemoteOutputChecker remoteOutputChecker; + private final DigestUtil digestUtil; + private final String buildRequestId; + private final String commandId; + private final Duration remoteCacheTtl; + private final boolean remoteUploadEnabled; + private final boolean verboseFailures; + + public RemoteFileWriteStrategy( + FileWriteActionContext localStrategy, + CombinedCache combinedCache, + RemoteOutputChecker remoteOutputChecker, + DigestUtil digestUtil, + String buildRequestId, + String commandId, + Duration remoteCacheTtl, + boolean remoteUploadEnabled, + boolean verboseFailures) { + this.localStrategy = localStrategy; + this.combinedCache = combinedCache; + this.remoteOutputChecker = remoteOutputChecker; + this.digestUtil = digestUtil; + this.buildRequestId = buildRequestId; + this.commandId = commandId; + this.remoteCacheTtl = remoteCacheTtl; + this.remoteUploadEnabled = remoteUploadEnabled; + this.verboseFailures = verboseFailures; + } + + @Override + public ImmutableList<SpawnResult> writeOutputToFile( + AbstractAction action, + ActionExecutionContext actionExecutionContext, + DeterministicWriter deterministicWriter, + boolean makeExecutable, + boolean isRemotable, + Artifact output) + throws InterruptedException, ExecException { + // Non-remotable file writes are consumed by Bazel itself and thus have to exist locally. + // Outputs that are requested for download would be materialized right after the action anyway. + if (!isRemotable + || !(actionExecutionContext.getActionFileSystem() + instanceof RemoteActionFileSystem remoteActionFileSystem) + || remoteOutputChecker.shouldDownloadOutput( + output.getExecPath(), /* treeRootExecPath= */ null)) { + return localStrategy.writeOutputToFile( + action, actionExecutionContext, deterministicWriter, makeExecutable, isRemotable, output); + } + + actionExecutionContext.getEventHandler().post(new RunningActionEvent(action, "remote")); + try (var _ = Profiler.instance().profile("RemoteFileWriteStrategy.writeOutputToFile")) { + Digest digest; + try { + digest = digestUtil.compute(deterministicWriter); + } catch (IOException e) { + throw new EnvironmentalExecException(e, Code.FILE_WRITE_IO_EXCEPTION); + } + + var context = RemoteActionExecutionContext.create(buildRequestMetadata(action)); + try { + if (!storeInCaches(context, digest, deterministicWriter, output)) { + // No cache has the contents and Bazel isn't allowed to store them in any, so the file + // has to exist locally for consumers to be able to use it. + return localStrategy.writeOutputToFile( + action, + actionExecutionContext, + deterministicWriter, + makeExecutable, + isRemotable, + output); + } + } catch (IOException e) { + // Cache failures shouldn't fail the build, so write the file to disk instead. + actionExecutionContext + .getEventHandler() + .handle(Event.warn("Remote Cache: " + Utils.grpcAwareErrorMessage(e, verboseFailures))); + return localStrategy.writeOutputToFile( + action, + actionExecutionContext, + deterministicWriter, + makeExecutable, + isRemotable, + output); + } + + // TODO: Bazel currently marks all output files as executable after local execution and + // stages all files as executable for remote execution, so the executable bit isn't tracked + // in the metadata yet. + try { + remoteActionFileSystem.injectRemoteFile( + actionExecutionContext.getInputPath(output).asFragment(), + DigestUtil.toBinaryDigest(digest), + digest.getSizeBytes(), + Instant.now().plus(remoteCacheTtl), + /* inMemoryOutput= */ false); + } catch (IOException e) { + throw new EnvironmentalExecException(e, Code.FILE_WRITE_IO_EXCEPTION); + } + } + return ImmutableList.of(); + } + + /** + * Stores the contents in every cache that Bazel is allowed to write to and that doesn't have them + * yet. + * + * @return whether at least one cache has the contents afterwards + */ + private boolean storeInCaches( + RemoteActionExecutionContext context, + Digest digest, + DeterministicWriter deterministicWriter, + Artifact output) + throws IOException, InterruptedException { + // The disk cache is always writable and skips the write if it already has the contents. + boolean writeToDiskCache = combinedCache.hasDiskCache(); + boolean presentInRemoteCache = false; + boolean uploadToRemoteCache = false; + if (combinedCache.hasRemoteCache()) { + ImmutableSet<Digest> missingDigests = + getFromFuture(combinedCache.findMissingDigests(context, ImmutableList.of(digest))); + presentInRemoteCache = missingDigests.isEmpty(); + uploadToRemoteCache = !presentInRemoteCache && remoteUploadEnabled; + } + if (writeToDiskCache || uploadToRemoteCache) { + getFromFuture( + combinedCache.uploadBlob( + context.withWriteCachePolicy( + CachePolicy.create(uploadToRemoteCache, writeToDiskCache)), + digest, + new DeterministicWriterBlob(deterministicWriter, output), + // The remote cache just reported the blob as missing. + /* force= */ true)); + } + return writeToDiskCache || presentInRemoteCache || uploadToRemoteCache; + } + + private RequestMetadata buildRequestMetadata(AbstractAction action) { + ActionOwner owner = action.getOwner(); + return TracingMetadataUtils.buildMetadata( + buildRequestId, + commandId, + action.getPrimaryOutput().getExecPathString(), + action.getMnemonic(), + owner.getLabel() != null ? owner.getLabel().getCanonicalForm() : null, + owner.getConfigurationChecksum()); + } + + /** Streams the contents of a {@link DeterministicWriter} through a bounded pipe. */ + private record DeterministicWriterBlob(DeterministicWriter writer, Artifact output) + implements Blob { + @Override + public InputStream get() { + return writer.getInputStream(Chunker.getDefaultChunkSize()); + } + + @Override + public String description() { + return output.getExecPathString(); + } + } +}
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java index 794ceff..280765a 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
@@ -1193,6 +1193,7 @@ return; } actionContextProvider.registerSpawnCache(registryBuilder); + actionContextProvider.registerFileWriteStrategies(registryBuilder); // For skymeld, a non-toplevel target might become a toplevel after it has been executed. This // is the last chance to download the missing toplevel outputs in this case before sending out
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 25efdce1..1d7454f 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
@@ -208,6 +208,10 @@ if (runfilesSupport == null) { return; } + var repoMappingManifest = runfilesSupport.getRepoMappingManifest(); + if (repoMappingManifest != null) { + addOutputToDownload(repoMappingManifest); + } var runfiles = runfilesSupport.getRunfiles(); for (Artifact runfile : runfiles.getArtifacts().toList()) { if (mayBeRemote(runfile)) {
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 ecec2e8..67911fb 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
@@ -70,9 +70,12 @@ registryBuilder.register( TestActionContext.class, new ExclusiveTestStrategy(testStrategy), "exclusive"); registryBuilder.register(TestActionContext.class, testStrategy, "standalone"); - registryBuilder.register(FileWriteActionContext.class, new FileWriteStrategy(), "local"); + var fileWriteStrategy = new FileWriteStrategy(); + registryBuilder.register(FileWriteActionContext.class, fileWriteStrategy, "local"); registryBuilder.register( - TemplateExpansionContext.class, new LocalTemplateExpansionStrategy(), "local"); + TemplateExpansionContext.class, + new LocalTemplateExpansionStrategy(fileWriteStrategy), + "local"); } @Override
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 e59a0e2..79bb790 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
@@ -65,8 +65,10 @@ public TestExecutorBuilder(FileSystem fileSystem, Path execRoot) { this.fileSystem = fileSystem; this.execRoot = execRoot; - addContext(FileWriteActionContext.class, new FileWriteStrategy()); - addContext(TemplateExpansionContext.class, new LocalTemplateExpansionStrategy()); + var fileWriteStrategy = new FileWriteStrategy(); + addContext(FileWriteActionContext.class, fileWriteStrategy); + addContext( + TemplateExpansionContext.class, new LocalTemplateExpansionStrategy(fileWriteStrategy)); addContext( SymlinkTreeActionContext.class, new SymlinkTreeStrategy(null, TestConstants.WORKSPACE_NAME));
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 64c2b6d2c..980e779 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/test/java/com/google/devtools/build/lib/remote/BUILD
@@ -99,6 +99,7 @@ "//src/main/java/com/google/devtools/build/lib/actions:parameter_file", "//src/main/java/com/google/devtools/build/lib/actions:runfiles_metadata", "//src/main/java/com/google/devtools/build/lib/actions:runfiles_tree", + "//src/main/java/com/google/devtools/build/lib/actions:thread_state_receiver", "//src/main/java/com/google/devtools/build/lib/actions:virtual_action_input", "//src/main/java/com/google/devtools/build/lib/analysis:analysis_core", "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories", @@ -124,6 +125,7 @@ "//src/main/java/com/google/devtools/build/lib/exec:abstract_spawn_strategy", "//src/main/java/com/google/devtools/build/lib/exec:bin_tools", "//src/main/java/com/google/devtools/build/lib/exec:execution_options", + "//src/main/java/com/google/devtools/build/lib/exec:file_write_strategy", "//src/main/java/com/google/devtools/build/lib/exec:remote_local_fallback_registry", "//src/main/java/com/google/devtools/build/lib/exec:spawn_cache", "//src/main/java/com/google/devtools/build/lib/exec:spawn_input_expander", @@ -152,6 +154,7 @@ "//src/main/java/com/google/devtools/build/lib/remote:remote_download_idle_timeout_interceptor", "//src/main/java/com/google/devtools/build/lib/remote:remote_execution_cache", "//src/main/java/com/google/devtools/build/lib/remote:remote_external_overlay_file_system", + "//src/main/java/com/google/devtools/build/lib/remote:remote_file_write_strategy", "//src/main/java/com/google/devtools/build/lib/remote:remote_grpc_service_config", "//src/main/java/com/google/devtools/build/lib/remote:remote_important_output_handler", "//src/main/java/com/google/devtools/build/lib/remote:remote_module", @@ -197,6 +200,7 @@ "//src/main/java/com/google/devtools/build/lib/testing/vfs:spied_filesystem", "//src/main/java/com/google/devtools/build/lib/util", "//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception", + "//src/main/java/com/google/devtools/build/lib/util:deterministic_writer", "//src/main/java/com/google/devtools/build/lib/util:exit_code", "//src/main/java/com/google/devtools/build/lib/util:os", "//src/main/java/com/google/devtools/build/lib/util:string_encoding", @@ -380,6 +384,7 @@ "//src/main/java/com/google/devtools/build/lib/runtime:block_waiting_module", "//src/main/java/com/google/devtools/build/lib/runtime:build_summary_stats_module", "//src/main/java/com/google/devtools/build/lib/standalone", + "//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception", "//src/main/java/com/google/devtools/build/lib/util:os", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
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 e33eb04..c740356 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.skyframe.rewinding.RewindingTestsHelper; import com.google.devtools.build.lib.standalone.StandaloneModule; import com.google.devtools.build.lib.testutil.ActionEventRecorder; +import com.google.devtools.build.lib.util.AbruptExitException; import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; @@ -118,6 +119,142 @@ } } + @Test + public void remoteFileWrite_switchToLocalBuild_materializesOutput() throws Exception { + writeFileWriteRules(); + write( + "BUILD", + """ + load('//rules:write_file.bzl', 'write_file') + write_file(name = 'foo', content = 'hello') + """); + addOptions("--file_write_strategy=remote"); + buildTarget("//:foo"); + assertOutputsDoNotExist("//:foo"); + + addOptions( + "--file_write_strategy=local", "--remote_executor=", "--remote_cache=", "--disk_cache="); + buildTarget("//:foo"); + assertOnlyOutputContent("//:foo", "foo", "hello"); + } + + @Test + public void remoteFileWrite_withoutCache_isRejected() throws Exception { + writeFileWriteRules(); + write( + "BUILD", + """ + load('//rules:write_file.bzl', 'write_file') + write_file(name = 'foo', content = 'hello') + """); + addOptions( + "--file_write_strategy=remote", "--remote_executor=", "--remote_cache=", "--disk_cache="); + + var e = assertThrows(AbruptExitException.class, () -> buildTarget("//:foo")); + + assertThat(e).hasMessageThat().contains("FileWriteActionContext"); + assertThat(e).hasMessageThat().contains("'remote'"); + } + + @Test + public void remoteFileWrite_strategiesAreIndependent( + @TestParameter({"expand_template", "write_file"}) String remoteFileWriteRule) + throws Exception { + writeFileWriteRules(); + write( + "BUILD", + """ + load('//rules:expand_template.bzl', 'expand_template') + load('//rules:write_file.bzl', 'write_file') + expand_template(name = 'expand_template', content = 'hello') + write_file(name = 'write_file', content = 'hello') + """); + setFileWriteStrategy(remoteFileWriteRule, "remote"); + + buildTarget("//:expand_template", "//:write_file"); + + for (var fileWriteRule : ImmutableList.of("expand_template", "write_file")) { + if (fileWriteRule.equals(remoteFileWriteRule)) { + assertOutputsDoNotExist("//:" + fileWriteRule); + } else { + assertOnlyOutputContent("//:" + fileWriteRule, fileWriteRule, "hello"); + } + } + } + + @Test + public void remoteFileWrite_readOnlyRemoteCache() throws Exception { + writeFileWriteRules(); + write( + "BUILD", + """ + load('//rules:write_file.bzl', 'write_file') + write_file(name = 'foo', content = 'hello') + """); + addOptions("--file_write_strategy=remote"); + + // Populate the caches with the contents of the file. + buildTarget("//:foo"); + assertOutputsDoNotExist("//:foo"); + + // A read-only remote cache that already has the contents doesn't require a local write. + getOutputBase().getRelative("action_cache").deleteTreesBelow(); + restartServer(); + addOptions( + "--file_write_strategy=remote", + "--remote_executor=", + "--remote_cache=grpc://localhost:" + worker.getPort(), + "--noremote_upload_local_results"); + buildTarget("//:foo"); + assertOutputsDoNotExist("//:foo"); + + // Without the contents in any cache, they can only be kept off disk if there is a disk cache + // to store them in. Otherwise, the file has to be written locally. + evictAllBlobs(); + getOutputBase().getRelative("action_cache").deleteTreesBelow(); + restartServer(); + addOptions( + "--file_write_strategy=remote", + "--remote_executor=", + "--remote_cache=grpc://localhost:" + worker.getPort(), + "--noremote_upload_local_results"); + buildTarget("//:foo"); + if (useDiskCache) { + assertOutputsDoNotExist("//:foo"); + } else { + assertOnlyOutputContent("//:foo", "foo", "hello"); + } + } + + @Test + public void remoteFileWrite_diskCacheOnly_keepsOutputOffDisk() throws Exception { + writeFileWriteRules(); + write( + "BUILD", + """ + load('//rules:write_file.bzl', 'write_file') + write_file(name = 'foo', content = 'hello') + genrule( + name = 'bar', + srcs = [':foo'], + outs = ['bar.txt'], + cmd = 'cat $(location :foo) > $@', + ) + """); + addOptions( + "--file_write_strategy=remote", + "--remote_executor=", + "--remote_cache=", + "--disk_cache=" + getWorkspace().getRelative(UUID.randomUUID().toString())); + + buildTarget("//:foo"); + assertOutputsDoNotExist("//:foo"); + + // A local consumer fetches the contents from the disk cache. + buildTarget("//:bar"); + assertOnlyOutputContent("//:bar", "bar.txt", "hello"); + } + @Override protected void setDownloadToplevel() { addOptions("--remote_download_outputs=toplevel"); @@ -137,6 +274,26 @@ } @Override + protected String getLocalFileWriteStrategyFlag() { + return "--file_write_strategy=local"; + } + + @Override + protected String getRemoteFileWriteStrategyFlag() { + return "--file_write_strategy=remote"; + } + + @Override + protected String getLocalTemplateExpansionStrategyFlag() { + return "--template_expansion_strategy=local"; + } + + @Override + protected String getRemoteTemplateExpansionStrategyFlag() { + return "--template_expansion_strategy=remote"; + } + + @Override protected BlazeRuntime.Builder getRuntimeBuilder() throws Exception { return super.getRuntimeBuilder() .addBlazeModule(new RemoteModule())
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 87b1eb2..d6ba197 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
@@ -80,6 +80,14 @@ addOptions("--norewind_lost_inputs"); } + protected abstract String getLocalFileWriteStrategyFlag(); + + protected abstract String getRemoteFileWriteStrategyFlag(); + + protected abstract String getLocalTemplateExpansionStrategyFlag(); + + protected abstract String getRemoteTemplateExpansionStrategyFlag(); + protected abstract void assertOutputEquals(Path path, String expectedContent) throws Exception; protected abstract void assertOutputContains(String content, String contains) throws Exception; @@ -1290,6 +1298,54 @@ } @Test + public void downloadToplevel_symlinkToWrittenFile( + @TestParameter({"local", "remote"}) String 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)); + + setFileWriteStrategy(fileWriteRule, 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(@TestParameter SymlinkTargetType targetType) throws Exception { Path targetPath = TestUtils.createUniqueTmpDir(null).getChild("target"); @@ -1340,6 +1396,88 @@ } @Test + public void downloadMinimal_fileWrite( + @TestParameter boolean isExecutable, + @TestParameter({"local", "remote"}) String fileWriteStrategy, + @TestParameter({"expand_template", "write_file"}) String fileWriteRule) + throws Exception { + writeFileWriteRules(); + // 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; } + """)); + + setFileWriteStrategy(fileWriteRule, fileWriteStrategy); + + buildTarget("//:gen"); + if (fileWriteStrategy.equals("remote")) { + assertOutputsDoNotExist("//:foo"); + } else { + assertValidOutputFile("foo", "hello"); + } + assertOutputsDoNotExist("//:gen"); + } + + @Test + public void downloadToplevel_fileWrite( + @TestParameter boolean isExecutable, + @TestParameter({"local", "remote"}) String 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")); + + setFileWriteStrategy(fileWriteRule, 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 + assertThat(getOutputPath("foo").delete()).isTrue(); + buildTarget("//:foo"); + + assertOnlyOutputContent("//:foo", "foo", "hello"); + } + + @Test public void treeOutputsFromLocalFileSystem_works( @TestParameter({"no-remote-exec", "local"}) String executionInfo) throws Exception { // Test that tree artifact generated locally can be consumed by other actions. @@ -1784,6 +1922,55 @@ } @Test + public void incrementalBuild_writeFileOutputIsPrefetched_noRuns() 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:write_file.bzl', 'write_file') + write_file( + name = 'foo', + content = 'foo', + executable = True, + ) + genrule( + name = 'foobar', + srcs = [':foo'], + outs = ['out/foobar.txt'], + cmd = 'cat $(location :foo) > $@ && echo bar >> $@', + tags = ['no-remote'], + ) + """); + + addOptions(getRemoteFileWriteStrategyFlag()); + + buildTarget("//:foobar"); + assertValidOutputFile("foo", "foo"); + assertValidOutputFile("out/foobar.txt", "foobar\n"); + assertThat(getOnlyElement(getMetadata("//:foo").values()).isRemote()).isTrue(); + + // Act: Do an incremental build without any modifications + var 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.isRemote()).isTrue(); + assertThat(metadata.getContentsProxy()).isNotNull(); + } + + @Test public void incrementalBuild_treeOutputIsPrefetched_noRuns() throws Exception { // We need to download the intermediate output if (!hasAccessToRemoteOutputs()) { @@ -1973,6 +2160,68 @@ } @Test + public void remoteCacheEvictBlobs_whenPrefetchingRemoteFileWrite( + @TestParameter boolean actionRewinding) throws Exception { + // Arrange: Prepare workspace and populate remote cache + writeFileWriteRules(); + write( + "a/BUILD", + """ + load('//rules:write_file.bzl', 'write_file') + write_file( + name = "foo", + content = "foo\\n", + ) + + genrule( + name = "bar", + srcs = [ + ":foo", + "bar.in", + ], + outs = ["bar.out"], + cmd = "cat $(SRCS) > $@", + ) + """); + write("a/bar.in", "bar"); + addOptions(getRemoteFileWriteStrategyFlag()); + + // Populate remote cache + buildTarget("//a:bar"); + assertOutputDoesNotExist("a/foo"); + getOutputPath("a/bar.out").delete(); + getOutputBase().getRelative("action_cache").deleteTreesBelow(); + restartServer(); + addOptions(getRemoteFileWriteStrategyFlag()); + + // Clean build, foo is only stored remotely + buildTarget("//a:bar"); + assertOutputDoesNotExist("a/foo"); + + // Evict blobs from remote cache + evictAllBlobs(); + + write("a/bar.in", "updated bar"); + addOptions("--strategy_regexp=.*bar=local"); + if (actionRewinding) { + // The lost input's generating action is rewound within the next build and stores its + // contents remotely again. + enableActionRewinding(); + } else { + // The build fails because of remote cache eviction, but an incremental build without + // "clean" or "shutdown" can continue. + disableActionRewinding(); + assertThrows(BuildFailedException.class, () -> buildTarget("//a:bar")); + } + + // Act: Do an incremental build without "clean" or "shutdown" + buildTarget("//a:bar"); + + // Assert: target was successfully built + assertValidOutputFile("a/bar.out", "foo\nupdated bar\n"); + } + + @Test public void remoteCacheEvictBlobs_whenPrefetchingInputTree(@TestParameter boolean actionRewinding) throws Exception { // Arrange: Prepare workspace and populate remote cache @@ -2551,6 +2800,90 @@ } } + /** + * Selects the strategy for the file write performed by the given rule written by {@link + * #writeFileWriteRules}. + */ + protected void setFileWriteStrategy(String fileWriteRule, String strategy) { + var flag = + switch (fileWriteRule) { + case "expand_template" -> + switch (strategy) { + case "local" -> getLocalTemplateExpansionStrategyFlag(); + case "remote" -> getRemoteTemplateExpansionStrategyFlag(); + default -> throw new IllegalArgumentException(strategy); + }; + case "write_file" -> + switch (strategy) { + case "local" -> getLocalFileWriteStrategyFlag(); + case "remote" -> getRemoteFileWriteStrategyFlag(); + default -> throw new IllegalArgumentException(strategy); + }; + default -> throw new IllegalArgumentException(fileWriteRule); + }; + addOptions(flag); + } + + 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/RemoteFileWriteStrategyTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteFileWriteStrategyTest.java new file mode 100644 index 0000000..51a9138 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteFileWriteStrategyTest.java
@@ -0,0 +1,456 @@ +// Copyright 2026 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.remote; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.devtools.build.lib.remote.util.Futures.getFromFuture; +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; + +import build.bazel.remote.execution.v2.Digest; +import build.bazel.remote.execution.v2.RequestMetadata; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.util.concurrent.Futures; +import com.google.devtools.build.lib.actions.AbstractAction; +import com.google.devtools.build.lib.actions.ActionExecutionContext; +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.FileArtifactValue; +import com.google.devtools.build.lib.actions.FileStatusWithMetadata; +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.events.EventKind; +import com.google.devtools.build.lib.events.StoredEventHandler; +import com.google.devtools.build.lib.exec.FileWriteStrategy; +import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; +import com.google.devtools.build.lib.remote.disk.DiskCacheClient; +import com.google.devtools.build.lib.remote.options.RemoteOutputsMode; +import com.google.devtools.build.lib.remote.util.DigestUtil; +import com.google.devtools.build.lib.remote.util.InMemoryCacheClient; +import com.google.devtools.build.lib.testutil.Scratch; +import com.google.devtools.build.lib.util.DeterministicWriter; +import com.google.devtools.build.lib.vfs.DigestHashFunction; +import com.google.devtools.build.lib.vfs.FileSystem; +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.devtools.build.lib.vfs.SyscallCache; +import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; +import java.io.IOException; +import java.time.Duration; +import javax.annotation.Nullable; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link RemoteFileWriteStrategy}. */ +@RunWith(JUnit4.class) +public final class RemoteFileWriteStrategyTest { + private static final DigestUtil DIGEST_UTIL = + new DigestUtil(SyscallCache.NO_CACHE, DigestHashFunction.SHA256); + private static final String CONTENT = "hello"; + private static final Digest CONTENT_DIGEST = DIGEST_UTIL.compute(CONTENT.getBytes(UTF_8)); + + private final FileSystem fileSystem = new InMemoryFileSystem(DigestHashFunction.SHA256); + private final Scratch scratch = new Scratch(fileSystem); + private final InMemoryCacheClient cacheClient = spy(new InMemoryCacheClient()); + private final StoredEventHandler eventHandler = new StoredEventHandler(); + private Path execRoot; + private RemoteActionFileSystem actionFileSystem; + private AbstractAction action; + private Artifact output; + + @Before + public void setUp() throws IOException { + execRoot = scratch.dir("/execroot"); + var outputRoot = ArtifactRoot.asDerivedRoot(execRoot, RootType.OUTPUT, "bazel-out"); + outputRoot.getRoot().asPath().createDirectory(); + output = + ActionsTestUtil.createArtifactWithRootRelativePath( + outputRoot, PathFragment.create("pkg/file")); + action = new NullAction(output); + actionFileSystem = + new RemoteActionFileSystem( + fileSystem, + execRoot.asFragment(), + outputRoot.getExecPathString(), + new ActionInputMap(0), + mock(RemoteActionInputFetcher.class)); + actionFileSystem.createDirectoryAndParents(output.getPath().getParentDirectory().asFragment()); + } + + @Test + public void remotable_storesContentsRemotely() throws Exception { + var strategy = createStrategy(RemoteOutputsMode.MINIMAL, cacheClient); + + var unused = + strategy.writeOutputToFile( + action, + createActionExecutionContext(actionFileSystem), + writer(CONTENT), + /* makeExecutable= */ false, + /* isRemotable= */ true); + + assertStoredRemotely(cacheClient); + var metadata = getRemoteMetadata(); + assertThat(metadata.isRemote()).isTrue(); + assertThat(metadata.getDigest()).isEqualTo(DigestUtil.toBinaryDigest(CONTENT_DIGEST)); + assertThat(metadata.getSize()).isEqualTo(CONTENT.length()); + assertThat(output.getPath().exists()).isFalse(); + assertThat(eventHandler.getEvents()).isEmpty(); + } + + @Test + public void alreadyStoredRemotely_skipsUpload() throws Exception { + var prepopulatedCacheClient = + spy(new InMemoryCacheClient(ImmutableMap.of(CONTENT_DIGEST, CONTENT.getBytes(UTF_8)))); + var strategy = createStrategy(RemoteOutputsMode.MINIMAL, prepopulatedCacheClient); + + var unused = + strategy.writeOutputToFile( + action, + createActionExecutionContext(actionFileSystem), + writer(CONTENT), + /* makeExecutable= */ false, + /* isRemotable= */ true); + + verify(prepopulatedCacheClient, never()).uploadBlobImpl(any(), any(), any()); + assertThat(getRemoteMetadata().isRemote()).isTrue(); + assertThat(output.getPath().exists()).isFalse(); + } + + @Test + public void nonRemotable_writesLocally() throws Exception { + var strategy = createStrategy(RemoteOutputsMode.MINIMAL, cacheClient); + + var unused = + strategy.writeOutputToFile( + action, + createActionExecutionContext(actionFileSystem), + writer(CONTENT), + /* makeExecutable= */ false, + /* isRemotable= */ false); + + assertWrittenLocally(); + assertNotStoredRemotely(cacheClient); + } + + @Test + public void outputRequestedForDownload_writesLocally() throws Exception { + var strategy = createStrategy(RemoteOutputsMode.ALL, cacheClient); + + var unused = + strategy.writeOutputToFile( + action, + createActionExecutionContext(actionFileSystem), + writer(CONTENT), + /* makeExecutable= */ false, + /* isRemotable= */ true); + + assertWrittenLocally(); + assertNotStoredRemotely(cacheClient); + } + + @Test + public void remoteUploadDisabled_missingRemotely_writesLocally() throws Exception { + var strategy = + createStrategy(RemoteOutputsMode.MINIMAL, cacheClient, /* remoteUploadEnabled= */ false); + + var unused = + strategy.writeOutputToFile( + action, + createActionExecutionContext(actionFileSystem), + writer(CONTENT), + /* makeExecutable= */ false, + /* isRemotable= */ true); + + assertWrittenLocally(); + assertNotStoredRemotely(cacheClient); + assertThat(eventHandler.getEvents()).isEmpty(); + } + + @Test + public void remoteUploadDisabled_alreadyStoredRemotely_recordsRemoteOutput() throws Exception { + var prepopulatedCacheClient = + spy(new InMemoryCacheClient(ImmutableMap.of(CONTENT_DIGEST, CONTENT.getBytes(UTF_8)))); + var strategy = + createStrategy( + RemoteOutputsMode.MINIMAL, prepopulatedCacheClient, /* remoteUploadEnabled= */ false); + + var unused = + strategy.writeOutputToFile( + action, + createActionExecutionContext(actionFileSystem), + writer(CONTENT), + /* makeExecutable= */ false, + /* isRemotable= */ true); + + verify(prepopulatedCacheClient, never()).uploadBlobImpl(any(), any(), any()); + var metadata = getRemoteMetadata(); + assertThat(metadata.isRemote()).isTrue(); + assertThat(metadata.getDigest()).isEqualTo(DigestUtil.toBinaryDigest(CONTENT_DIGEST)); + assertThat(output.getPath().exists()).isFalse(); + assertThat(eventHandler.getEvents()).isEmpty(); + } + + @Test + public void noActionFileSystem_writesLocally() throws Exception { + var strategy = createStrategy(RemoteOutputsMode.MINIMAL, cacheClient); + + var unused = + strategy.writeOutputToFile( + action, + createActionExecutionContext(/* actionFileSystem= */ null), + writer(CONTENT), + /* makeExecutable= */ false, + /* isRemotable= */ true); + + assertWrittenLocally(); + assertNotStoredRemotely(cacheClient); + } + + @Test + public void diskCacheOnly_storesContentsInDiskCache() throws Exception { + var diskCacheClient = createDiskCacheClient(); + var strategy = + createStrategy( + RemoteOutputsMode.MINIMAL, + /* cacheClient= */ null, + diskCacheClient, + /* remoteUploadEnabled= */ false); + + var unused = + strategy.writeOutputToFile( + action, + createActionExecutionContext(actionFileSystem), + writer(CONTENT), + /* makeExecutable= */ false, + /* isRemotable= */ true); + + assertStoredInDiskCache(diskCacheClient); + var metadata = getRemoteMetadata(); + assertThat(metadata.isRemote()).isTrue(); + assertThat(metadata.getDigest()).isEqualTo(DigestUtil.toBinaryDigest(CONTENT_DIGEST)); + assertThat(output.getPath().exists()).isFalse(); + assertThat(eventHandler.getEvents()).isEmpty(); + } + + @Test + public void remoteUploadDisabled_missingRemotely_storesContentsInDiskCache() throws Exception { + var diskCacheClient = createDiskCacheClient(); + var strategy = + createStrategy( + RemoteOutputsMode.MINIMAL, + cacheClient, + diskCacheClient, + /* remoteUploadEnabled= */ false); + + var unused = + strategy.writeOutputToFile( + action, + createActionExecutionContext(actionFileSystem), + writer(CONTENT), + /* makeExecutable= */ false, + /* isRemotable= */ true); + + assertStoredInDiskCache(diskCacheClient); + assertNotStoredRemotely(cacheClient); + assertThat(getRemoteMetadata().isRemote()).isTrue(); + assertThat(output.getPath().exists()).isFalse(); + assertThat(eventHandler.getEvents()).isEmpty(); + } + + @Test + public void alreadyStoredRemotely_storesContentsInDiskCache() throws Exception { + var diskCacheClient = createDiskCacheClient(); + var prepopulatedCacheClient = + spy(new InMemoryCacheClient(ImmutableMap.of(CONTENT_DIGEST, CONTENT.getBytes(UTF_8)))); + var strategy = + createStrategy( + RemoteOutputsMode.MINIMAL, + prepopulatedCacheClient, + diskCacheClient, + /* remoteUploadEnabled= */ true); + + var unused = + strategy.writeOutputToFile( + action, + createActionExecutionContext(actionFileSystem), + writer(CONTENT), + /* makeExecutable= */ false, + /* isRemotable= */ true); + + verify(prepopulatedCacheClient, never()).uploadBlobImpl(any(), any(), any()); + assertStoredInDiskCache(diskCacheClient); + assertThat(getRemoteMetadata().isRemote()).isTrue(); + assertThat(output.getPath().exists()).isFalse(); + } + + @Test + public void remotable_storesContentsInBothCaches() throws Exception { + var diskCacheClient = createDiskCacheClient(); + var strategy = + createStrategy( + RemoteOutputsMode.MINIMAL, + cacheClient, + diskCacheClient, + /* remoteUploadEnabled= */ true); + + var unused = + strategy.writeOutputToFile( + action, + createActionExecutionContext(actionFileSystem), + writer(CONTENT), + /* makeExecutable= */ false, + /* isRemotable= */ true); + + assertStoredRemotely(cacheClient); + assertStoredInDiskCache(diskCacheClient); + assertThat(getRemoteMetadata().isRemote()).isTrue(); + assertThat(output.getPath().exists()).isFalse(); + } + + @Test + public void uploadFails_writesLocallyAndWarns() throws Exception { + doReturn(Futures.immediateFailedFuture(new IOException("upload failed"))) + .when(cacheClient) + .uploadBlobImpl(any(), any(), any()); + var strategy = createStrategy(RemoteOutputsMode.MINIMAL, cacheClient); + + var unused = + strategy.writeOutputToFile( + action, + createActionExecutionContext(actionFileSystem), + writer(CONTENT), + /* makeExecutable= */ false, + /* isRemotable= */ true); + + assertWrittenLocally(); + var warning = eventHandler.getEvents().stream().findFirst().orElseThrow(); + assertThat(warning.getKind()).isEqualTo(EventKind.WARNING); + assertThat(warning.getMessage()).contains("Remote Cache:"); + assertThat(warning.getMessage()).contains("upload failed"); + } + + private RemoteFileWriteStrategy createStrategy( + RemoteOutputsMode outputsMode, InMemoryCacheClient cacheClient) { + return createStrategy(outputsMode, cacheClient, /* remoteUploadEnabled= */ true); + } + + private RemoteFileWriteStrategy createStrategy( + RemoteOutputsMode outputsMode, InMemoryCacheClient cacheClient, boolean remoteUploadEnabled) { + return createStrategy( + outputsMode, cacheClient, /* diskCacheClient= */ null, remoteUploadEnabled); + } + + private RemoteFileWriteStrategy createStrategy( + RemoteOutputsMode outputsMode, + @Nullable InMemoryCacheClient cacheClient, + @Nullable DiskCacheClient diskCacheClient, + boolean remoteUploadEnabled) { + return new RemoteFileWriteStrategy( + new FileWriteStrategy(), + new CombinedCache( + cacheClient, + diskCacheClient, + /* symlinkTemplate= */ null, + DIGEST_UTIL, + /* chunkingFunction= */ null, + new ChunkLocationMap()), + new RemoteOutputChecker("build", outputsMode, ImmutableList.of()), + DIGEST_UTIL, + "build-request-id", + "command-id", + Duration.ofHours(1), + remoteUploadEnabled, + /* verboseFailures= */ false); + } + + private DiskCacheClient createDiskCacheClient() throws IOException { + return new DiskCacheClient( + scratch.dir("/disk_cache"), DIGEST_UTIL, /* checkActionResultIntegrity= */ false); + } + + private ActionExecutionContext createActionExecutionContext( + @Nullable FileSystem actionFileSystem) { + var metadataHandler = new FakeInputMetadataHandlerBase(); + return new ActionExecutionContext( + new DummyExecutor(fileSystem, execRoot), + /* inputMetadataProvider= */ metadataHandler, + ActionInputPrefetcher.NONE, + new ActionKeyContext(), + /* outputMetadataStore= */ metadataHandler, + /* rewindingEnabled= */ false, + ActionExecutionContext.LostInputsCheck.NONE, + /* fileOutErr= */ null, + eventHandler, + /* clientEnv= */ ImmutableMap.of(), + actionFileSystem, + DiscoveredModulesPruner.DEFAULT, + SyscallCache.NO_CACHE, + ThreadStateReceiver.NULL_INSTANCE); + } + + private static DeterministicWriter writer(String content) { + return out -> out.write(content.getBytes(UTF_8)); + } + + private FileArtifactValue getRemoteMetadata() throws IOException { + var status = actionFileSystem.getPath(output.getPath().asFragment()).stat(); + assertThat(status).isInstanceOf(FileStatusWithMetadata.class); + return ((FileStatusWithMetadata) status).getMetadata(); + } + + private void assertWrittenLocally() throws IOException { + assertThat(FileSystemUtils.readContent(output.getPath(), UTF_8)).isEqualTo(CONTENT); + } + + private static void assertStoredRemotely(InMemoryCacheClient cacheClient) throws Exception { + assertThat( + getFromFuture( + cacheClient.findMissingDigests( + RemoteActionExecutionContext.create(RequestMetadata.getDefaultInstance()), + ImmutableList.of(CONTENT_DIGEST)))) + .isEmpty(); + } + + private static void assertStoredInDiskCache(DiskCacheClient diskCacheClient) throws Exception { + assertThat(diskCacheClient.toPath(CONTENT_DIGEST, Store.CAS).exists()).isTrue(); + } + + private static void assertNotStoredRemotely(InMemoryCacheClient cacheClient) throws Exception { + verify(cacheClient, never()).uploadBlobImpl(any(), any(), any()); + assertThat( + getFromFuture( + cacheClient.findMissingDigests( + RemoteActionExecutionContext.create(RequestMetadata.getDefaultInstance()), + ImmutableList.of(CONTENT_DIGEST)))) + .containsExactly(CONTENT_DIGEST); + } +}
diff --git a/src/test/shell/bazel/remote/build_without_the_bytes_test.sh b/src/test/shell/bazel/remote/build_without_the_bytes_test.sh index 3716d77..ee8f441 100755 --- a/src/test/shell/bazel/remote/build_without_the_bytes_test.sh +++ b/src/test/shell/bazel/remote/build_without_the_bytes_test.sh
@@ -341,6 +341,25 @@ || fail "Expected toplevel output bazel-bin/a/foo.txt to be downloaded" } +function test_run_downloads_repo_mapping() { + add_rules_shell MODULE.bazel + cat > BUILD <<'EOF' +load("@rules_shell//shell:sh_binary.bzl", "sh_binary") +sh_binary(name = "foo", srcs = ["foo.sh"]) +EOF + cat > foo.sh <<'EOF' +#!/bin/sh +test -s "$0.runfiles/_repo_mapping" +EOF + chmod +x foo.sh + + bazel run \ + --remote_executor=grpc://localhost:${worker_port} \ + --remote_download_minimal \ + --file_write_strategy=remote \ + //:foo >& "$TEST_log" || fail "Repository mapping was not downloaded" +} + function test_downloads_toplevel_runfiles() { # Test that --remote_download_toplevel fetches only the top level binaries # and generated runfiles.