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 27132d528c1961..346182c4df39cf 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 @@ -147,6 +147,9 @@ import java.util.concurrent.Executor; import java.util.concurrent.Phaser; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Predicate; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import javax.annotation.Nullable; /** @@ -174,6 +177,8 @@ public class RemoteExecutionService { private final AtomicBoolean shutdown = new AtomicBoolean(false); private final AtomicBoolean buildInterrupted = new AtomicBoolean(false); + private final boolean shouldForceDownloads; + private final Predicate shouldForceDownloadPredicate; public RemoteExecutionService( Executor executor, @@ -215,6 +220,19 @@ public RemoteExecutionService( this.captureCorruptedOutputsDir = captureCorruptedOutputsDir; this.scheduler = Schedulers.from(executor, /*interruptibleWorker=*/ true); + + String regex = remoteOptions.remoteDownloadRegex; + // TODO(bazel-team): Consider adding a warning or more validation if the remoteDownloadRegex is used + // without RemoteOutputsMode.MINIMAL + this.shouldForceDownloads = + !regex.isEmpty() + && (remoteOptions.remoteOutputsMode == RemoteOutputsMode.MINIMAL + || remoteOptions.remoteOutputsMode == RemoteOutputsMode.TOPLEVEL); + Pattern pattern = Pattern.compile(regex); + this.shouldForceDownloadPredicate = path -> { + Matcher m = pattern.matcher(path); + return m.matches(); + }; } static Command buildCommand( @@ -1021,24 +1039,10 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re /* exitCode = */ result.getExitCode(), hasFilesToDownload(action.getSpawn().getOutputFiles(), filesToDownload)); - if (downloadOutputs) { - HashSet queuedFilePaths = new HashSet<>(); - - for (FileMetadata file : metadata.files()) { - PathFragment filePath = file.path().asFragment(); - if (queuedFilePaths.add(filePath)) { - downloadsBuilder.add(downloadFile(action, file)); - } - } + ImmutableList.Builder> forcedDownloadsBuilder = ImmutableList.builder(); - for (Map.Entry entry : metadata.directories()) { - for (FileMetadata file : entry.getValue().files()) { - PathFragment filePath = file.path().asFragment(); - if (queuedFilePaths.add(filePath)) { - downloadsBuilder.add(downloadFile(action, file)); - } - } - } + if (downloadOutputs) { + downloadsBuilder.addAll(buildFilesToDownload(metadata, action)); } else { checkState( result.getExitCode() == 0, @@ -1049,6 +1053,9 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re "Symlinks in action outputs are not yet supported by " + "--experimental_remote_download_outputs=minimal"); } + if (shouldForceDownloads) { + forcedDownloadsBuilder.addAll(buildFilesToDownloadWithPredicate(metadata, action, shouldForceDownloadPredicate)); + } } FileOutErr tmpOutErr = outErr.childOutErr(); @@ -1063,6 +1070,18 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re try (SilentCloseable c = Profiler.instance().profile("Remote.download")) { waitForBulkTransfer(downloads, /* cancelRemainingOnInterrupt= */ true); } catch (Exception e) { + // TODO(bazel-team): Consider adding better case-by-case exception handling instead of just rethrowing + captureCorruptedOutputs(e); + deletePartialDownloadedOutputs(result, tmpOutErr, e); + throw e; + } + + ImmutableList> forcedDownloads = forcedDownloadsBuilder.build(); + // TODO(bazel-team): Unify this block with the equivalent block above. + try (SilentCloseable c = Profiler.instance().profile("Remote.forcedDownload")) { + waitForBulkTransfer(forcedDownloads, /* cancelRemainingOnInterrupt= */ true); + } catch (Exception e) { + // TODO(bazel-team): Consider adding better case-by-case exception handling instead of just rethrowing captureCorruptedOutputs(e); deletePartialDownloadedOutputs(result, tmpOutErr, e); throw e; @@ -1096,6 +1115,13 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re // might not be supported on all platforms createSymlinks(symlinks); } else { + // TODO(bazel-team): We should unify this if-block to rely on downloadOutputs above but, as of 2022-07-05, + // downloadOuputs' semantics isn't exactly the same as build-without-the-bytes which is necessary for using + // remoteDownloadRegex. + if (!forcedDownloads.isEmpty()) { + moveOutputsToFinalLocation(forcedDownloads); + } + ActionInput inMemoryOutput = null; Digest inMemoryOutputDigest = null; PathFragment inMemoryOutputPath = getInMemoryOutputPath(action.getSpawn()); @@ -1133,6 +1159,35 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re return null; } + private ImmutableList> buildFilesToDownload( + ActionResultMetadata metadata, RemoteAction action) { + Predicate alwaysTrue = unused -> true; + return buildFilesToDownloadWithPredicate(metadata, action, alwaysTrue); + } + + private ImmutableList> buildFilesToDownloadWithPredicate( + ActionResultMetadata metadata, RemoteAction action, Predicate predicate) { + HashSet queuedFilePaths = new HashSet<>(); + ImmutableList.Builder> builder = new ImmutableList.Builder<>(); + for (FileMetadata file : metadata.files()) { + PathFragment filePath = file.path().asFragment(); + if (queuedFilePaths.add(filePath) && predicate.test(file.path.toString())) { + builder.add(downloadFile(action, file)); + } + } + + for (Map.Entry entry : metadata.directories()) { + for (FileMetadata file : entry.getValue().files()) { + PathFragment filePath = file.path().asFragment(); + if (queuedFilePaths.add(filePath) && predicate.test(file.path.toString())) { + builder.add(downloadFile(action, file)); + } + } + } + + return builder.build(); + } + private static String prettyPrint(ActionInput actionInput) { if (actionInput instanceof Artifact) { return ((Artifact) actionInput).prettyPrint(); 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 841df554e66088..6d3ea161eb6976 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 @@ -587,6 +587,16 @@ public RemoteOutputsStrategyConverter() { + "`all` to print always.") public ExecutionMessagePrintMode remotePrintExecutionMessages; + @Option( + name = "experimental_remote_download_regex", + defaultValue = "", + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = {OptionEffectTag.AFFECTS_OUTPUTS}, + help = "Force Bazel to download the artifacts that match the given regexp. To be used in conjunction with " + + "--remote_download_minimal or --remote_download_toplevel to allow the client to request certain " + + "artifacts that might be needed locally (e.g. IDE support)") + public String remoteDownloadRegex; + // The below options are not configurable by users, only tests. // This is part of the effort to reduce the overall number of flags. 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 4cdecf01ca71d7..4528ab18e3d167 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 @@ -1354,4 +1354,61 @@ EOF expect_log "START.*: \[.*\] Executing genrule //a:dep" } +function test_remote_download_regex() { + mkdir -p a + + cat > a/BUILD <<'EOF' +java_library( + name = "lib", + srcs = ["Library.java"], +) +java_test( + name = "test", + srcs = ["JavaTest.java"], + test_class = "JavaTest", + deps = [":lib"], +) +EOF + + cat > a/Library.java <<'EOF' +public class Library { + public static boolean TEST = true; +} +EOF + + cat > a/JavaTest.java <<'EOF' +import org.junit.Assert; +import org.junit.Test; +public class JavaTest { + @Test + public void test() { Assert.assertTrue(Library.TEST); } +} +EOF + bazel test \ + --remote_executor=grpc://localhost:${worker_port} \ + --remote_download_minimal \ + //a:test >& $TEST_log || fail "Failed to build" + + [[ ! -e "bazel-bin/a/liblib.jar" ]] || fail "bazel-bin/a/liblib.jar shouldn't exist" + [[ ! -e "bazel-bin/a/liblib.jdeps" ]] || fail "bazel-bin/a/liblib.jdeps shouldn't exist" + + bazel clean && bazel test \ + --remote_executor=grpc://localhost:${worker_port} \ + --remote_download_minimal \ + --experimental_remote_download_regex=".*" \ + //a:test >& $TEST_log || fail "Failed to build" + + [[ -e "bazel-bin/a/liblib.jar" ]] || fail "bazel-bin/a/liblib.jar file does not exist!" + [[ -e "bazel-bin/a/liblib.jdeps" ]] || fail "bazel-bin/a/liblib.jdeps file does not exist!" + + bazel clean && bazel test \ + --remote_executor=grpc://localhost:${worker_port} \ + --remote_download_minimal \ + --experimental_remote_download_regex=".*jar$" \ + //a:test >& $TEST_log || fail "Failed to build" + + [[ -e "bazel-bin/a/liblib.jar" ]] || fail "bazel-bin/a/liblib.jar file does not exist!" + [[ ! -e "bazel-bin/a/liblib.jdeps" ]] || fail "bazel-bin/a/liblib.jdeps shouldn't exist" +} + run_suite "Build without the Bytes tests"