diff --git a/src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java b/src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java index dcdd4358b497c4..a21f1417860655 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java @@ -194,6 +194,9 @@ public enum WorkerProtocolFormat { /** Disables remote caching of a spawn. Note: does not disable remote execution */ public static final String NO_REMOTE_CACHE = "no-remote-cache"; + /** Disables upload part of remote caching of a spawn. Note: does not disable remote execution */ + public static final String NO_REMOTE_CACHE_UPLOAD = "no-remote-cache-upload"; + /** Disables remote execution of a spawn. Note: does not disable remote caching */ public static final String NO_REMOTE_EXEC = "no-remote-exec"; diff --git a/src/main/java/com/google/devtools/build/lib/actions/Spawns.java b/src/main/java/com/google/devtools/build/lib/actions/Spawns.java index 182fc0c6e6d448..70cc10a9ce19b0 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Spawns.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Spawns.java @@ -19,6 +19,7 @@ import com.google.devtools.build.lib.server.FailureDetails.Spawn.Code; import java.io.IOException; import java.time.Duration; +import java.util.Map; /** Helper methods relating to implementations of {@link Spawn}. */ public final class Spawns { @@ -28,8 +29,13 @@ private Spawns() {} * Returns {@code true} if the result of {@code spawn} may be cached. */ public static boolean mayBeCached(Spawn spawn) { - return !spawn.getExecutionInfo().containsKey(ExecutionRequirements.NO_CACHE) - && !spawn.getExecutionInfo().containsKey(ExecutionRequirements.LOCAL); + return mayBeCached(spawn.getExecutionInfo()); + } + + public static boolean mayBeCached(Map executionInfo) { + return !executionInfo.containsKey(ExecutionRequirements.NO_CACHE) + && !executionInfo.containsKey(ExecutionRequirements.LOCAL); + } /** Returns {@code true} if the result of {@code spawn} may be cached remotely. */ @@ -39,6 +45,13 @@ public static boolean mayBeCachedRemotely(Spawn spawn) { && !spawn.getExecutionInfo().containsKey(ExecutionRequirements.NO_REMOTE_CACHE); } + public static boolean mayBeCachedRemotely(Map executionInfo) { + return mayBeCached(executionInfo) + && !executionInfo.containsKey(ExecutionRequirements.NO_REMOTE) + && !executionInfo.containsKey(ExecutionRequirements.NO_REMOTE_CACHE); + + } + /** Returns {@code true} if {@code spawn} may be executed remotely. */ public static boolean mayBeExecutedRemotely(Spawn spawn) { return !spawn.getExecutionInfo().containsKey(ExecutionRequirements.LOCAL) diff --git a/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java b/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java index ffb8ee87ca71ff..d70c8d68303976 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java +++ b/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java @@ -22,6 +22,7 @@ import build.bazel.remote.execution.v2.RequestMetadata; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Sets; import com.google.common.util.concurrent.ListenableFuture; import com.google.devtools.build.lib.buildeventstream.BuildEvent.LocalFile; import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader; @@ -65,6 +66,9 @@ class ByteStreamBuildEventArtifactUploader extends AbstractReferenceCounted private final AtomicBoolean shutdown = new AtomicBoolean(); private final Scheduler scheduler; + private final Set omittedFiles = Sets.newConcurrentHashSet(); + private final Set omittedTreeRoots = Sets.newConcurrentHashSet(); + ByteStreamBuildEventArtifactUploader( Executor executor, ExtendedEventHandler reporter, @@ -83,6 +87,14 @@ class ByteStreamBuildEventArtifactUploader extends AbstractReferenceCounted this.scheduler = Schedulers.from(executor); } + public void omitFile(Path file) { + omittedFiles.add(file); + } + + public void omitTree(Path treeRoot) { + omittedTreeRoots.add(treeRoot); + } + /** Returns {@code true} if Bazel knows that the file is stored on a remote system. */ private static boolean isRemoteFile(Path file) { return file.getFileSystem() instanceof RemoteActionFileSystem @@ -124,10 +136,21 @@ public boolean isRemote() { * Collects metadata for {@code file}. Depending on the underlying filesystem used this method * might do I/O. */ - private static PathMetadata readPathMetadata(Path file) throws IOException { + private PathMetadata readPathMetadata(Path file) throws IOException { if (file.isDirectory()) { return new PathMetadata(file, /* digest= */ null, /* directory= */ true, /* remote= */ false); } + if (omittedFiles.contains(file)) { + return new PathMetadata(file, /*digest=*/ null, /*directory=*/ false, /*remote=*/ false); + } + + for (Path treeRoot : omittedTreeRoots) { + if (file.startsWith(treeRoot)) { + omittedFiles.add(file); + return new PathMetadata(file, /*digest=*/ null, /*directory=*/ false, /*remote=*/ false); + } + } + DigestUtil digestUtil = new DigestUtil(file.getFileSystem().getDigestFunction()); Digest digest = digestUtil.compute(file); return new PathMetadata(file, digest, /* directory= */ false, isRemoteFile(file)); @@ -248,7 +271,7 @@ private Single upload(Set files) { .collect(Collectors.toList()) .flatMap(paths -> queryRemoteCache(remoteCache, context, paths)) .flatMap(paths -> uploadLocalFiles(remoteCache, context, paths)) - .map(paths -> new PathConverterImpl(remoteServerInstanceName, paths)), + .map(paths -> new PathConverterImpl(remoteServerInstanceName, paths, omittedFiles)), RemoteCache::release); } @@ -280,9 +303,11 @@ private static class PathConverterImpl implements PathConverter { private final String remoteServerInstanceName; private final Map pathToDigest; private final Set skippedPaths; + private final Set localPaths; - PathConverterImpl(String remoteServerInstanceName, List uploads) { + PathConverterImpl(String remoteServerInstanceName, List uploads, Set localPaths) { Preconditions.checkNotNull(uploads); + this.localPaths = localPaths; this.remoteServerInstanceName = remoteServerInstanceName; pathToDigest = new HashMap<>(uploads.size()); ImmutableSet.Builder skippedPaths = ImmutableSet.builder(); @@ -301,6 +326,11 @@ private static class PathConverterImpl implements PathConverter { @Override public String apply(Path path) { Preconditions.checkNotNull(path); + + if (localPaths.contains(path)) { + return String.format("file://%s", path.getPathString()); + } + Digest digest = pathToDigest.get(path); if (digest == null) { if (skippedPaths.contains(path)) { diff --git a/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderFactory.java b/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderFactory.java index 31edf398e75653..7e757b43e9ff33 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderFactory.java +++ b/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderFactory.java @@ -13,11 +13,14 @@ // limitations under the License. package com.google.devtools.build.lib.remote; +import static com.google.common.base.Preconditions.checkState; + import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.runtime.BuildEventArtifactUploaderFactory; import com.google.devtools.build.lib.runtime.CommandEnvironment; import java.util.concurrent.Executor; +import javax.annotation.Nullable; /** A factory for {@link ByteStreamBuildEventArtifactUploader}. */ class ByteStreamBuildEventArtifactUploaderFactory implements BuildEventArtifactUploaderFactory { @@ -30,6 +33,9 @@ class ByteStreamBuildEventArtifactUploaderFactory implements BuildEventArtifactU private final String buildRequestId; private final String commandId; + @Nullable + private ByteStreamBuildEventArtifactUploader uploader; + ByteStreamBuildEventArtifactUploaderFactory( Executor executor, ExtendedEventHandler reporter, @@ -49,7 +55,8 @@ class ByteStreamBuildEventArtifactUploaderFactory implements BuildEventArtifactU @Override public BuildEventArtifactUploader create(CommandEnvironment env) { - return new ByteStreamBuildEventArtifactUploader( + checkState(uploader == null, "Already created"); + uploader = new ByteStreamBuildEventArtifactUploader( executor, reporter, verboseFailures, @@ -57,5 +64,11 @@ public BuildEventArtifactUploader create(CommandEnvironment env) { remoteServerInstanceName, buildRequestId, commandId); + return uploader; + } + + @Nullable + public ByteStreamBuildEventArtifactUploader get() { + return uploader; } } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java index 8cb9a9e8762716..5df843d5b102c3 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java @@ -356,6 +356,19 @@ public boolean shouldAcceptCachedResult(Spawn spawn) { } } + public static boolean shouldUploadLocalResults(RemoteOptions remoteOptions, + @Nullable Map executionInfo) { + if (useRemoteCache(remoteOptions)) { + if (useDiskCache(remoteOptions)) { + return shouldUploadLocalResultsToCombinedDisk(remoteOptions, executionInfo); + } else { + return shouldUploadLocalResultsToRemoteCache(remoteOptions, executionInfo); + } + } else { + return shouldUploadLocalResultsToDiskCache(remoteOptions, executionInfo); + } + } + /** * Returns {@code true} if the local results of the {@code spawn} should be uploaded to remote * cache. @@ -365,15 +378,7 @@ public boolean shouldUploadLocalResults(Spawn spawn) { return false; } - if (useRemoteCache(remoteOptions)) { - if (useDiskCache(remoteOptions)) { - return shouldUploadLocalResultsToCombinedDisk(remoteOptions, spawn); - } else { - return shouldUploadLocalResultsToRemoteCache(remoteOptions, spawn); - } - } else { - return shouldUploadLocalResultsToDiskCache(remoteOptions, spawn); - } + return shouldUploadLocalResults(remoteOptions, spawn.getExecutionInfo()); } /** Returns {@code true} if the spawn may be executed remotely. */ @@ -1140,6 +1145,7 @@ public void uploadOutputs(RemoteAction action, SpawnResult spawnResult) throws InterruptedException, ExecException { checkState(!shutdown.get(), "shutdown"); checkState(shouldUploadLocalResults(action.spawn), "spawn shouldn't upload local result"); + checkNotNull(remoteCache, "remoteCache can't be null"); checkState( SpawnResult.Status.SUCCESS.equals(spawnResult.status()) && spawnResult.exitCode() == 0, "shouldn't upload outputs of failed local action"); 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 5b857ca0eaced4..ddbc025a9b1fdf 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 @@ -30,6 +30,7 @@ import com.google.common.util.concurrent.ListeningScheduledExecutorService; import com.google.common.util.concurrent.MoreExecutors; import com.google.common.util.concurrent.ThreadFactoryBuilder; +import com.google.devtools.build.lib.actions.ActionAnalysisMetadata; import com.google.devtools.build.lib.actions.ActionExecutionMetadata; import com.google.devtools.build.lib.actions.ActionGraph; import com.google.devtools.build.lib.actions.ActionInput; @@ -111,6 +112,7 @@ import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.ThreadFactory; import java.util.concurrent.ThreadPoolExecutor; +import javax.annotation.Nullable; /** RemoteModule provides distributed cache and remote execution for Bazel. */ public final class RemoteModule extends BlazeModule { @@ -126,7 +128,7 @@ public final class RemoteModule extends BlazeModule { private RemoteActionContextProvider actionContextProvider; private RemoteActionInputFetcher actionInputFetcher; - private RemoteOutputsMode remoteOutputsMode; + private RemoteOptions remoteOptions; private RemoteOutputService remoteOutputService; private ChannelFactory channelFactory = @@ -244,7 +246,7 @@ private void initHttpAndDiskCache( public void beforeCommand(CommandEnvironment env) throws AbruptExitException { Preconditions.checkState(actionContextProvider == null, "actionContextProvider must be null"); Preconditions.checkState(actionInputFetcher == null, "actionInputFetcher must be null"); - Preconditions.checkState(remoteOutputsMode == null, "remoteOutputsMode must be null"); + Preconditions.checkState(remoteOptions == null, "remoteOptions must be null"); RemoteOptions remoteOptions = env.getOptions().getOptions(RemoteOptions.class); if (remoteOptions == null) { @@ -252,7 +254,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { return; } - remoteOutputsMode = remoteOptions.remoteOutputsMode; + this.remoteOptions = remoteOptions; AuthAndTLSOptions authAndTlsOptions = env.getOptions().getOptions(AuthAndTLSOptions.class); DigestHashFunction hashFn = env.getRuntime().getFileSystem().getDigestFunction(); @@ -705,8 +707,7 @@ public void afterAnalysis( AnalysisResult analysisResult) { // The actionContextProvider may be null if remote execution is disabled or if there was an // error during initialization. - if (remoteOutputsMode != null - && remoteOutputsMode.downloadToplevelOutputsOnly() + if (remoteOptions != null && remoteOptions.remoteOutputsMode.downloadToplevelOutputsOnly() && actionContextProvider != null) { boolean isTestCommand = env.getCommandName().equals("test"); TopLevelArtifactContext artifactContext = request.getTopLevelArtifactContext(); @@ -726,6 +727,40 @@ public void afterAnalysis( } actionContextProvider.setFilesToDownload(ImmutableSet.copyOf(filesToDownload)); } + + if (remoteOptions != null && remoteOptions.incompatibleRemoteBuildEventUploadRespectNoCache) { + parseNoCacheOutputs(analysisResult); + } + } + + private void parseNoCacheOutputs(AnalysisResult analysisResult) { + if (actionContextProvider == null || actionContextProvider.getRemoteCache() == null) { + return; + } + + ByteStreamBuildEventArtifactUploader uploader = buildEventArtifactUploaderFactoryDelegate.get(); + if (uploader == null) { + return; + } + + for (ConfiguredTarget configuredTarget : analysisResult.getTargetsToBuild()) { + if (configuredTarget instanceof RuleConfiguredTarget) { + RuleConfiguredTarget ruleConfiguredTarget = (RuleConfiguredTarget) configuredTarget; + for (ActionAnalysisMetadata action : ruleConfiguredTarget.getActions()) { + boolean uploadLocalResults = RemoteExecutionService.shouldUploadLocalResults( + remoteOptions, action.getExecutionInfo()); + if (!uploadLocalResults) { + for (Artifact output : action.getOutputs()) { + if (output.isTreeArtifact()) { + uploader.omitTree(output.getPath()); + } else { + uploader.omitFile(output.getPath()); + } + } + } + } + } + } } // This is a short-term fix for top-level outputs that are symlinks. Unfortunately, we cannot @@ -829,7 +864,7 @@ public void afterCommand() throws AbruptExitException { remoteDownloaderSupplier.set(null); actionContextProvider = null; actionInputFetcher = null; - remoteOutputsMode = null; + remoteOptions = null; remoteOutputService = null; if (failure != null) { @@ -873,7 +908,7 @@ public void registerActionContexts( @Override public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorBuilder builder) { Preconditions.checkState(actionInputFetcher == null, "actionInputFetcher must be null"); - Preconditions.checkNotNull(remoteOutputsMode, "remoteOutputsMode must not be null"); + Preconditions.checkNotNull(remoteOptions, "remoteOutputsMode must not be null"); if (actionContextProvider == null) { return; @@ -897,7 +932,7 @@ public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorB @Override public OutputService getOutputService() { Preconditions.checkState(remoteOutputService == null, "remoteOutputService must be null"); - if (remoteOutputsMode != null && !remoteOutputsMode.downloadAllOutputs()) { + if (remoteOptions != null && !remoteOptions.remoteOutputsMode.downloadAllOutputs()) { remoteOutputService = new RemoteOutputService(); } return remoteOutputService; @@ -913,13 +948,22 @@ public Iterable> getCommandOptions(Command command) private static class BuildEventArtifactUploaderFactoryDelegate implements BuildEventArtifactUploaderFactory { - private volatile BuildEventArtifactUploaderFactory uploaderFactory; + @Nullable + private ByteStreamBuildEventArtifactUploaderFactory uploaderFactory; - public void init(BuildEventArtifactUploaderFactory uploaderFactory) { + public void init(ByteStreamBuildEventArtifactUploaderFactory uploaderFactory) { Preconditions.checkState(this.uploaderFactory == null); this.uploaderFactory = uploaderFactory; } + @Nullable + public ByteStreamBuildEventArtifactUploader get() { + if (uploaderFactory == null) { + return null; + } + return uploaderFactory.get(); + } + public void reset() { this.uploaderFactory = null; } diff --git a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskAndRemoteCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskAndRemoteCacheClient.java index df2974c1e919ad..18ed99dd107a40 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskAndRemoteCacheClient.java +++ b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskAndRemoteCacheClient.java @@ -75,7 +75,11 @@ public void close() { public ListenableFuture uploadFile( RemoteActionExecutionContext context, Digest digest, Path file) { ListenableFuture future = diskCache.uploadFile(context, digest, file); - if (options.isRemoteExecutionEnabled() + + boolean uploadForSpawn = context.getSpawn() != null; + // If not upload for spawn e.g. for build event artifacts, we always upload files to remote + // cache. + if (!uploadForSpawn || options.isRemoteExecutionEnabled() || shouldUploadLocalResultsToRemoteCache(options, context.getSpawn())) { future = Futures.transformAsync( diff --git a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java index 5518672924d369..84ed3ad62493e5 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java +++ b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java @@ -269,6 +269,16 @@ public String getTypeDescription() { help = "Whether to upload locally executed action results to the remote cache.") public boolean remoteUploadLocalResults; + @Option( + name = "incompatible_remote_build_event_upload_respect_no_cache", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.REMOTE, + effectTags = {OptionEffectTag.UNKNOWN}, + help = "If set to true, outputs referenced by BEP are not uploaded to remote cache if the" + + " generating action cannot be cached remotely." + ) + public boolean incompatibleRemoteBuildEventUploadRespectNoCache; + @Option( name = "incompatible_remote_results_ignore_disk", defaultValue = "false", diff --git a/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java b/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java index a13faab04af198..95fe6b9135bb7c 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java +++ b/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java @@ -71,6 +71,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Locale; +import java.util.Map; import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; import java.util.function.BiFunction; @@ -596,31 +597,58 @@ public static boolean shouldAcceptCachedResultFromCombinedCache( } public static boolean shouldUploadLocalResultsToRemoteCache( - RemoteOptions remoteOptions, @Nullable Spawn spawn) { + RemoteOptions remoteOptions, @Nullable Map executionInfo) { return remoteOptions.remoteUploadLocalResults - && (spawn == null || Spawns.mayBeCachedRemotely(spawn)); + && (executionInfo == null || (Spawns.mayBeCachedRemotely(executionInfo) + && !executionInfo.containsKey(ExecutionRequirements.NO_REMOTE_CACHE_UPLOAD))); } - public static boolean shouldUploadLocalResultsToDiskCache( + public static boolean shouldUploadLocalResultsToRemoteCache( RemoteOptions remoteOptions, @Nullable Spawn spawn) { + Map executionInfo = null; + if (spawn != null) { + executionInfo = spawn.getExecutionInfo(); + } + return shouldUploadLocalResultsToRemoteCache(remoteOptions, executionInfo); + } + + public static boolean shouldUploadLocalResultsToDiskCache( + RemoteOptions remoteOptions, @Nullable Map executionInfo) { if (remoteOptions.incompatibleRemoteResultsIgnoreDisk) { - return spawn == null || Spawns.mayBeCached(spawn); + return executionInfo == null || Spawns.mayBeCached(executionInfo); } else { - return remoteOptions.remoteUploadLocalResults && (spawn == null || Spawns.mayBeCached(spawn)); + return remoteOptions.remoteUploadLocalResults && (executionInfo == null || Spawns.mayBeCached( + executionInfo)); } } - public static boolean shouldUploadLocalResultsToCombinedDisk( + public static boolean shouldUploadLocalResultsToDiskCache( RemoteOptions remoteOptions, @Nullable Spawn spawn) { + Map executionInfo = null; + if (spawn != null) { + executionInfo = spawn.getExecutionInfo(); + } + return shouldUploadLocalResultsToDiskCache(remoteOptions, executionInfo); + } + + public static boolean shouldUploadLocalResultsToCombinedDisk(RemoteOptions remoteOptions, + @Nullable Map executionInfo) { if (remoteOptions.incompatibleRemoteResultsIgnoreDisk) { // If --incompatible_remote_results_ignore_disk is set, we treat the disk cache part as local - // cache. Actions - // which are tagged with `no-remote-cache` can still hit the disk cache. - return spawn == null || Spawns.mayBeCached(spawn); + // cache. Actions which are tagged with `no-remote-cache` can still hit the disk cache. + return shouldUploadLocalResultsToDiskCache(remoteOptions, executionInfo); } else { // Otherwise, it's treated as a remote cache and disabled for `no-remote-cache`. - return remoteOptions.remoteUploadLocalResults - && (spawn == null || Spawns.mayBeCachedRemotely(spawn)); + return shouldUploadLocalResultsToRemoteCache(remoteOptions, executionInfo); + } + } + + public static boolean shouldUploadLocalResultsToCombinedDisk( + RemoteOptions remoteOptions, @Nullable Spawn spawn) { + Map executionInfo = null; + if (spawn != null) { + executionInfo = spawn.getExecutionInfo(); } + return shouldUploadLocalResultsToCombinedDisk(remoteOptions, executionInfo); } } diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java index 4171d01fe5b763..f7595f8257f8c7 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java @@ -206,7 +206,8 @@ private static SimpleSpawn simpleSpawnWithExecutionInfo( } private RemoteSpawnCache createRemoteSpawnCache() { - return remoteSpawnCacheWithOptions(Options.getDefaults(RemoteOptions.class)); + RemoteOptions remoteOptions = Options.getDefaults(RemoteOptions.class); + return remoteSpawnCacheWithOptions(remoteOptions); } private RemoteSpawnCache remoteSpawnCacheWithOptions(RemoteOptions options) { diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh index 04dfaa340f34c8..4281081b32c6ef 100755 --- a/src/test/shell/bazel/remote/remote_execution_test.sh +++ b/src/test/shell/bazel/remote/remote_execution_test.sh @@ -1900,6 +1900,42 @@ EOF expect_not_log "remote cache hit" } +function test_tag_no_remote_cache_upload() { + mkdir -p a + cat > a/BUILD <<'EOF' +genrule( + name = "foo", + srcs = [], + outs = ["foo.txt"], + cmd = "echo \"foo\" > \"$@\"", +) +EOF + + bazel build \ + --remote_cache=grpc://localhost:${worker_port} \ + --modify_execution_info=.*=+no-remote-cache-upload \ + //a:foo >& $TEST_log || "Failed to build //a:foo" + + remote_ac_files="$(count_remote_ac_files)" + [[ "$remote_ac_files" == 0 ]] || fail "Expected 0 remote action cache entries, not $remote_ac_files" + + # populate the cache + bazel clean + + bazel build \ + --remote_cache=grpc://localhost:${worker_port} \ + //a:foo >& $TEST_log || "Failed to build //a:foo" + + bazel clean + + bazel build \ + --remote_cache=grpc://localhost:${worker_port} \ + --modify_execution_info=.*=+no-remote-cache-upload \ + //a:foo >& $TEST_log || "Failed to build //a:foo" + + expect_log "remote cache hit" +} + function test_tag_no_remote_cache_for_disk_cache() { mkdir -p a cat > a/BUILD <<'EOF' @@ -3250,4 +3286,122 @@ EOF //a:consumer >& $TEST_log || fail "Failed to build without remote cache" } +function test_uploader_respsect_no_cache() { + mkdir -p a + cat > a/BUILD < \$@", + tags = ["no-cache"], +) +EOF + + bazel build \ + --remote_cache=grpc://localhost:${worker_port} \ + --incompatible_remote_build_event_upload_respect_no_cache \ + --build_event_json_file=bep.json \ + //a:foo >& $TEST_log || fail "Failed to build" + + cat bep.json > $TEST_log + expect_not_log "a:foo.*bytestream://" || fail "local files are converted" + expect_log "command.profile.gz.*bytestream://" || fail "should upload profile data" +} + +function test_uploader_respsect_no_cache_trees() { + mkdir -p a + cat > a/output_dir.bzl <<'EOF' +def _gen_output_dir_impl(ctx): + output_dir = ctx.actions.declare_directory(ctx.attr.outdir) + ctx.actions.run_shell( + outputs = [output_dir], + inputs = [], + command = """ + mkdir -p $1/sub; \ + index=0; while ((index<10)); do echo $index >$1/$index.txt; index=$(($index+1)); done + echo "Shuffle, duffle, muzzle, muff" > $1/sub/bar + """, + arguments = [output_dir.path], + execution_requirements = {"no-cache": ""}, + ) + return [ + DefaultInfo(files = depset(direct = [output_dir])), + ] + +gen_output_dir = rule( + implementation = _gen_output_dir_impl, + attrs = { + "outdir": attr.string(mandatory = True), + }, +) +EOF + + cat > a/BUILD <& $TEST_log || fail "Failed to build" + + cat bep.json > $TEST_log + expect_not_log "a:foo.*bytestream://" || fail "local tree files are converted" + expect_not_log "a/dir/.*bytestream://" || fail "local tree files are converted" + expect_log "command.profile.gz.*bytestream://" || fail "should upload profile data" +} + +function test_uploader_respsect_no_upload_results() { + mkdir -p a + cat > a/BUILD < \$@", +) +EOF + + bazel build \ + --remote_cache=grpc://localhost:${worker_port} \ + --remote_upload_local_results=false \ + --incompatible_remote_build_event_upload_respect_no_cache \ + --build_event_json_file=bep.json \ + //a:foo >& $TEST_log || fail "Failed to build" + + cat bep.json > $TEST_log + expect_not_log "a:foo.*bytestream://" || fail "local files are converted" + expect_log "command.profile.gz.*bytestream://" || fail "should upload profile data" +} + +function test_uploader_respsect_no_upload_results_combined_cache() { + mkdir -p a + cat > a/BUILD < \$@", +) +EOF + + bazel build \ + --remote_cache=grpc://localhost:${worker_port} \ + --disk_cache="${TEST_TMPDIR}/disk_cache" \ + --remote_upload_local_results=false \ + --incompatible_remote_build_event_upload_respect_no_cache \ + --build_event_json_file=bep.json \ + //a:foo >& $TEST_log || fail "Failed to build" + + cat bep.json > $TEST_log + expect_not_log "a:foo.*bytestream://" || fail "local files are converted" + expect_log "command.profile.gz.*bytestream://" || fail "should upload profile data" + remote_cas_files="$(count_remote_cas_files)" + [[ "$remote_cas_files" == 1 ]] || fail "Expected 1 remote action cache entries, not $remote_cas_files" +} + run_suite "Remote execution and remote cache tests" diff --git a/src/test/shell/bazel/remote/remote_utils.sh b/src/test/shell/bazel/remote/remote_utils.sh index 9a0191a9ced131..c24d4c71855e94 100644 --- a/src/test/shell/bazel/remote/remote_utils.sh +++ b/src/test/shell/bazel/remote/remote_utils.sh @@ -78,3 +78,11 @@ function count_remote_ac_files() { echo 0 fi } + +function count_remote_cas_files() { + if [ -d "$cas_path/cas" ]; then + expr $(find "$cas_path/cas" -type f | wc -l) + else + echo 0 + fi +} \ No newline at end of file