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 3c3b0230ec9516..73d4fd788fc57e 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 @@ -31,6 +31,7 @@ import com.google.devtools.build.lib.events.ExtendedEventHandler; 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.options.RemoteBuildEventUploadMode; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; import com.google.devtools.build.lib.vfs.Path; @@ -52,12 +53,14 @@ import java.util.concurrent.CancellationException; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.regex.Pattern; import java.util.stream.Collectors; import javax.annotation.Nullable; /** A {@link BuildEventArtifactUploader} backed by {@link RemoteCache}. */ class ByteStreamBuildEventArtifactUploader extends AbstractReferenceCounted implements BuildEventArtifactUploader { + private static final Pattern TEST_LOG_PATTERN = Pattern.compile(".*/bazel-out/[^/]*/testlogs/.*"); private final Executor executor; private final ExtendedEventHandler reporter; @@ -73,6 +76,8 @@ class ByteStreamBuildEventArtifactUploader extends AbstractReferenceCounted private final Set omittedFiles = Sets.newConcurrentHashSet(); private final Set omittedTreeRoots = Sets.newConcurrentHashSet(); private final XattrProvider xattrProvider; + private final RemoteBuildEventUploadMode remoteBuildEventUploadMode; + private final Path profilePath; ByteStreamBuildEventArtifactUploader( Executor executor, @@ -82,7 +87,9 @@ class ByteStreamBuildEventArtifactUploader extends AbstractReferenceCounted String remoteServerInstanceName, String buildRequestId, String commandId, - XattrProvider xattrProvider) { + XattrProvider xattrProvider, + RemoteBuildEventUploadMode remoteBuildEventUploadMode, + Path profilePath) { this.executor = executor; this.reporter = reporter; this.verboseFailures = verboseFailures; @@ -92,6 +99,8 @@ class ByteStreamBuildEventArtifactUploader extends AbstractReferenceCounted this.remoteServerInstanceName = remoteServerInstanceName; this.scheduler = Schedulers.from(executor); this.xattrProvider = xattrProvider; + this.remoteBuildEventUploadMode = remoteBuildEventUploadMode; + this.profilePath = profilePath; } public void omitFile(Path file) { @@ -197,8 +206,23 @@ private static void processQueryResult( } } - private static boolean shouldUpload(PathMetadata path) { - return path.getDigest() != null && !path.isRemote() && !path.isDirectory() && !path.isOmitted(); + private boolean shouldUpload(PathMetadata path) { + boolean result = + path.getDigest() != null && !path.isRemote() && !path.isDirectory() && !path.isOmitted(); + + if (remoteBuildEventUploadMode == RemoteBuildEventUploadMode.MINIMAL) { + result = result && (isTestLog(path) || isProfile(path)); + } + + return result; + } + + private boolean isTestLog(PathMetadata path) { + return TEST_LOG_PATTERN.matcher(path.getPath().getPathString()).matches(); + } + + private boolean isProfile(PathMetadata path) { + return path.getPath().equals(profilePath); } private Single> queryRemoteCache( @@ -207,8 +231,7 @@ private Single> queryRemoteCache( List filesToQuery = new ArrayList<>(); Set digestsToQuery = new HashSet<>(); for (PathMetadata path : paths) { - // Query remote cache for files even if omitted from uploading - if (shouldUpload(path) || path.isOmitted()) { + if (path.getDigest() != null && !path.isRemote() && !path.isDirectory()) { filesToQuery.add(path); digestsToQuery.add(path.getDigest()); } else { @@ -256,17 +279,18 @@ private Single> uploadLocalFiles( return toCompletable( () -> remoteCache.uploadFile(context, path.getDigest(), path.getPath()), executor) - .toSingleDefault(path) + .toSingle( + () -> + new PathMetadata( + path.getPath(), + path.getDigest(), + path.isDirectory(), + /*remote=*/ true, + path.isOmitted())) .onErrorResumeNext( error -> { reporterUploadError(error); - return Single.just( - new PathMetadata( - path.getPath(), - /*digest=*/ null, - path.isDirectory(), - path.isRemote(), - path.isOmitted())); + return Single.just(path); }); }) .collect(Collectors.toList()); @@ -347,7 +371,7 @@ private static class PathConverterImpl implements PathConverter { for (PathMetadata pair : uploads) { Path path = pair.getPath(); Digest digest = pair.getDigest(); - if (!pair.isRemote() && pair.isOmitted()) { + if (!pair.isRemote()) { localPaths.add(path); } else if (digest != null) { pathToDigest.put(path, digest); 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 3eec1eeb8aa8eb..eddb91a0dca977 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 @@ -17,8 +17,11 @@ import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader; import com.google.devtools.build.lib.events.ExtendedEventHandler; +import com.google.devtools.build.lib.remote.options.RemoteBuildEventUploadMode; import com.google.devtools.build.lib.runtime.BuildEventArtifactUploaderFactory; import com.google.devtools.build.lib.runtime.CommandEnvironment; +import com.google.devtools.build.lib.runtime.CommonCommandOptions; +import com.google.devtools.build.lib.vfs.Path; import java.util.concurrent.Executor; import javax.annotation.Nullable; @@ -32,6 +35,7 @@ class ByteStreamBuildEventArtifactUploaderFactory implements BuildEventArtifactU private final String remoteServerInstanceName; private final String buildRequestId; private final String commandId; + private final RemoteBuildEventUploadMode remoteBuildEventUploadMode; @Nullable private ByteStreamBuildEventArtifactUploader uploader; @@ -42,7 +46,8 @@ class ByteStreamBuildEventArtifactUploaderFactory implements BuildEventArtifactU RemoteCache remoteCache, String remoteServerInstanceName, String buildRequestId, - String commandId) { + String commandId, + RemoteBuildEventUploadMode remoteBuildEventUploadMode) { this.executor = executor; this.reporter = reporter; this.verboseFailures = verboseFailures; @@ -50,11 +55,20 @@ class ByteStreamBuildEventArtifactUploaderFactory implements BuildEventArtifactU this.remoteServerInstanceName = remoteServerInstanceName; this.buildRequestId = buildRequestId; this.commandId = commandId; + this.remoteBuildEventUploadMode = remoteBuildEventUploadMode; } @Override public BuildEventArtifactUploader create(CommandEnvironment env) { checkState(uploader == null, "Already created"); + + Path profilePath; + CommonCommandOptions options = env.getOptions().getOptions(CommonCommandOptions.class); + if (options != null && options.profilePath != null) { + profilePath = env.getWorkspace().getRelative(options.profilePath); + } else { + profilePath = env.getDirectories().getOutputBase().getRelative("command.profile.gz"); + } uploader = new ByteStreamBuildEventArtifactUploader( executor, @@ -64,7 +78,9 @@ public BuildEventArtifactUploader create(CommandEnvironment env) { remoteServerInstanceName, buildRequestId, commandId, - env.getXattrProvider()); + env.getXattrProvider(), + remoteBuildEventUploadMode, + profilePath); return uploader; } 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 4ee34b50a2ccc1..88518e2a611e58 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 @@ -66,6 +66,7 @@ import com.google.devtools.build.lib.remote.common.RemoteExecutionClient; import com.google.devtools.build.lib.remote.downloader.GrpcRemoteDownloader; import com.google.devtools.build.lib.remote.logging.LoggingInterceptor; +import com.google.devtools.build.lib.remote.options.RemoteBuildEventUploadMode; import com.google.devtools.build.lib.remote.options.RemoteOptions; import com.google.devtools.build.lib.remote.options.RemoteOutputsMode; import com.google.devtools.build.lib.remote.util.DigestUtil; @@ -634,7 +635,8 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { actionContextProvider.getRemoteCache(), remoteBytestreamUriPrefix, buildRequestId, - invocationId)); + invocationId, + remoteOptions.remoteBuildEventUploadMode)); if (enableRemoteDownloader) { remoteDownloaderSupplier.set( @@ -732,7 +734,9 @@ public void afterAnalysis( actionContextProvider.setFilesToDownload(ImmutableSet.copyOf(filesToDownload)); } - if (remoteOptions != null && remoteOptions.incompatibleRemoteBuildEventUploadRespectNoCache) { + if (remoteOptions != null + && remoteOptions.remoteBuildEventUploadMode == RemoteBuildEventUploadMode.ALL + && remoteOptions.incompatibleRemoteBuildEventUploadRespectNoCache) { parseNoCacheOutputs(analysisResult); } } diff --git a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteBuildEventUploadMode.java b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteBuildEventUploadMode.java new file mode 100644 index 00000000000000..d621e3683af402 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteBuildEventUploadMode.java @@ -0,0 +1,6 @@ +package com.google.devtools.build.lib.remote.options; + +public enum RemoteBuildEventUploadMode { + ALL, + MINIMAL, +} 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 6acdaddbccd273..30b7a90275e6e2 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 @@ -268,16 +268,44 @@ public String getTypeDescription() { help = "Whether to upload locally executed action results to the remote cache.") public boolean remoteUploadLocalResults; + @Deprecated @Option( name = "incompatible_remote_build_event_upload_respect_no_cache", defaultValue = "false", documentationCategory = OptionDocumentationCategory.REMOTE, effectTags = {OptionEffectTag.UNKNOWN}, + deprecationWarning = + "--incompatible_remote_build_event_upload_respect_no_cache has been deprecated in favor" + + " of --experimental_remote_build_event_upload=minimal.", 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 = "experimental_remote_build_event_upload", + defaultValue = "all", + documentationCategory = OptionDocumentationCategory.REMOTE, + effectTags = {OptionEffectTag.UNKNOWN}, + converter = RemoteBuildEventUploadModeConverter.class, + help = + "If set to 'all', all local outputs referenced by BEP are uploaded to remote cache.\n" + + "If set to 'minimal', local outputs referenced by BEP are not uploaded to the" + + " remote cache, except for files that are important to the consumers of BEP (e.g." + + " test logs and timing profile).\n" + + "file:// scheme is used for the paths of local files and bytestream:// scheme is" + + " used for the paths of (already) uploaded files.\n" + + "Default to 'all'.") + public RemoteBuildEventUploadMode remoteBuildEventUploadMode; + + /** Build event upload mode flag parser */ + public static class RemoteBuildEventUploadModeConverter + extends EnumConverter { + public RemoteBuildEventUploadModeConverter() { + super(RemoteBuildEventUploadMode.class, "remote build event upload"); + } + } + @Option( name = "incompatible_remote_results_ignore_disk", defaultValue = "false", diff --git a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java index 8234fe5161c761..ccce01597e040a 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java @@ -53,6 +53,7 @@ import com.google.devtools.build.lib.remote.common.MissingDigestsFinder; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; import com.google.devtools.build.lib.remote.grpc.ChannelConnectionFactory; +import com.google.devtools.build.lib.remote.options.RemoteBuildEventUploadMode; import com.google.devtools.build.lib.remote.options.RemoteOptions; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.remote.util.RxNoGlobalErrorsRule; @@ -481,7 +482,9 @@ private ByteStreamBuildEventArtifactUploader newArtifactUploader(RemoteCache rem /*remoteServerInstanceName=*/ "localhost/instance", /*buildRequestId=*/ "none", /*commandId=*/ "none", - SyscallCache.NO_CACHE); + SyscallCache.NO_CACHE, + RemoteBuildEventUploadMode.ALL, + /*profilePath=*/ fs.getPath("command.profile.gz")); } private static class StaticMissingDigestsFinder implements MissingDigestsFinder { diff --git a/src/test/shell/bazel/remote/BUILD b/src/test/shell/bazel/remote/BUILD index f32b3165c839e4..7be428ac0d1c9d 100644 --- a/src/test/shell/bazel/remote/BUILD +++ b/src/test/shell/bazel/remote/BUILD @@ -117,3 +117,14 @@ sh_test( "@bazel_tools//tools/bash/runfiles", ], ) + +sh_test( + name = "remote_build_event_uploader_test", + srcs = ["remote_build_event_uploader_test.sh"], + data = [ + ":remote_utils", + "//src/test/shell/bazel:test-deps", + "//src/tools/remote:worker", + "@bazel_tools//tools/bash/runfiles", + ], +) diff --git a/src/test/shell/bazel/remote/remote_build_event_uploader_test.sh b/src/test/shell/bazel/remote/remote_build_event_uploader_test.sh new file mode 100755 index 00000000000000..60a055d6e571f6 --- /dev/null +++ b/src/test/shell/bazel/remote/remote_build_event_uploader_test.sh @@ -0,0 +1,271 @@ +#!/bin/bash +# +# Copyright 2022 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# Tests remote build event uploader. + +set -euo pipefail + +# --- begin runfiles.bash initialization --- +if [[ ! -d "${RUNFILES_DIR:-/dev/null}" && ! -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then + if [[ -f "$0.runfiles_manifest" ]]; then + export RUNFILES_MANIFEST_FILE="$0.runfiles_manifest" + elif [[ -f "$0.runfiles/MANIFEST" ]]; then + export RUNFILES_MANIFEST_FILE="$0.runfiles/MANIFEST" + elif [[ -f "$0.runfiles/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then + export RUNFILES_DIR="$0.runfiles" + fi +fi +if [[ -f "${RUNFILES_DIR:-/dev/null}/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then + source "${RUNFILES_DIR}/bazel_tools/tools/bash/runfiles/runfiles.bash" +elif [[ -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then + source "$(grep -m1 "^bazel_tools/tools/bash/runfiles/runfiles.bash " \ + "$RUNFILES_MANIFEST_FILE" | cut -d ' ' -f 2-)" +else + echo >&2 "ERROR: cannot find @bazel_tools//tools/bash/runfiles:runfiles.bash" + exit 1 +fi +# --- end runfiles.bash initialization --- + +source "$(rlocation "io_bazel/src/test/shell/integration_test_setup.sh")" \ + || { echo "integration_test_setup.sh not found!" >&2; exit 1; } +source "$(rlocation "io_bazel/src/test/shell/bazel/remote/remote_utils.sh")" \ + || { echo "remote_utils.sh not found!" >&2; exit 1; } + +function set_up() { + start_worker \ + --incompatible_remote_symlinks +} + +function tear_down() { + bazel clean >& $TEST_log + stop_worker +} + +case "$(uname -s | tr [:upper:] [:lower:])" in +msys*|mingw*|cygwin*) + declare -r is_windows=true + ;; +*) + declare -r is_windows=false + ;; +esac + +if "$is_windows"; then + export MSYS_NO_PATHCONV=1 + export MSYS2_ARG_CONV_EXCL="*" + declare -r EXE_EXT=".exe" +else + declare -r EXE_EXT="" +fi + +function test_upload_minimal_convert_paths_for_existed_blobs() { + mkdir -p a + cat > a/BUILD < \$@", +) +EOF + + bazel build \ + --remote_executor=grpc://localhost:${worker_port} \ + --experimental_remote_build_event_upload=minimal \ + --build_event_json_file=bep.json \ + //a:foo >& $TEST_log || fail "Failed to build" + + cat bep.json > $TEST_log + expect_log "a:foo.*bytestream://" || fail "paths for existed blobs should be converted" + expect_log "command.profile.gz.*bytestream://" || fail "should upload profile data" +} + +function test_upload_minimal_doesnt_upload_missing_blobs() { + mkdir -p a + cat > a/BUILD < \$@", + tags = ["no-remote"], +) +EOF + + bazel build \ + --remote_executor=grpc://localhost:${worker_port} \ + --experimental_remote_build_event_upload=minimal \ + --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 uploaded" + expect_log "command.profile.gz.*bytestream://" || fail "should upload profile data" +} + +function test_upload_minimal_respect_no_upload_results() { + mkdir -p a + cat > a/BUILD < \$@", +) +EOF + + bazel build \ + --remote_cache=grpc://localhost:${worker_port} \ + --remote_upload_local_results=false \ + --experimental_remote_build_event_upload=minimal \ + --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 uploaded" + expect_log "command.profile.gz.*bytestream://" || fail "should upload profile data" +} + +function test_upload_minimal_respect_no_upload_results_combined_cache() { + local cache_dir="${TEST_TMPDIR}/disk_cache" + mkdir -p a + cat > a/BUILD < \$@", +) +EOF + + rm -rf $cache_dir + bazel build \ + --remote_cache=grpc://localhost:${worker_port} \ + --disk_cache=$cache_dir \ + --remote_upload_local_results=false \ + --experimental_remote_build_event_upload=minimal \ + --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 uploaded" + 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 cas entries, not $remote_cas_files" + disk_cas_files="$(count_disk_cas_files $cache_dir)" + # foo.txt, stdout and stderr for action 'foo' + [[ "$disk_cas_files" == 3 ]] || fail "Expected 3 disk cas entries, not $disk_cas_files" +} + + +function test_upload_minimal_alias_action_doesnt_upload_missing_blobs() { + mkdir -p a + cat > a/BUILD < \$@", + tags = ["no-remote"], +) + +alias( + name = 'foo-alias', + actual = '//a:foo', +) +EOF + + bazel build \ + --remote_executor=grpc://localhost:${worker_port} \ + --experimental_remote_build_event_upload=minimal \ + --build_event_json_file=bep.json \ + //a:foo-alias >& $TEST_log || fail "Failed to build" + + cat bep.json > $TEST_log + expect_not_log "a:foo.*bytestream://" + expect_log "command.profile.gz.*bytestream://" +} + +function test_upload_minimal_trees_doesnt_upload_missing_blobs() { + 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-remote": ""}, + ) + 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 uploaded" + expect_not_log "a/dir/.*bytestream://" || fail "local tree files are uploaded" + expect_log "command.profile.gz.*bytestream://" || fail "should upload profile data" +} + +function test_upload_minimal_upload_testlogs() { + mkdir -p a + cat > a/BUILD < a/test.sh <& $TEST_log || fail "Failed to build" + + cat bep.json > $TEST_log + expect_not_log "test.sh.*bytestream://" || fail "test script is uploaded" + expect_log "test.log.*bytestream://" || fail "should upload test.log" + expect_log "test.xml.*bytestream://" || fail "should upload test.xml" + expect_log "command.profile.gz.*bytestream://" || fail "should upload profile data" +} + +run_suite "Remote build event uploader tests" \ No newline at end of file