Skip to content

Commit

Permalink
Add a flag to force Bazel to download certain artifacts when using --…
Browse files Browse the repository at this point in the history 
…remote_download_minimal

When using --remote_download_minimal Bazel only downloads to the client the minimum number of artifacts necessary for the build to succeed. This can be sometimes problematic when local development (i.e. IDE) requires some artifacts to do local work (e.g. syntax completion). We add a flag --experimental_remote_download_regex that allow specifying a regular expression that will force certain artifacts to be forced to downloaded.

Tested locally that syntax errors disappear in IntelliJ when compiling Bazel itself with --remote_download_minimal and --experimental_remote_download_regex=".*jar$", also included an integration test with several cases.

Signed-off-by: Luis Fernando Pino Duque <luis@engflow.com>

Closes #15638.

PiperOrigin-RevId: 460672954
Change-Id: I3a4f18d046d2d91603a276f16173ad2a253480dd
  • Loading branch information
@lfpino @copybara-github
lfpino authored and copybara-github committed Jul 13, 2022
1 parent 7801ac0 commit 3eeae23
Show file tree
Hide file tree
Showing 3 changed files with 145 additions and 17 deletions.
93 changes: 76 additions & 17 deletions src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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<String> shouldForceDownloadPredicate;

public RemoteExecutionService(
Executor executor,
Expand Down Expand Up @@ -215,6 +220,20 @@ 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(
Expand Down Expand Up @@ -1021,24 +1040,10 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re
/* exitCode = */ result.getExitCode(),
hasFilesToDownload(action.getSpawn().getOutputFiles(), filesToDownload));

if (downloadOutputs) {
HashSet<PathFragment> queuedFilePaths = new HashSet<>();

for (FileMetadata file : metadata.files()) {
PathFragment filePath = file.path().asFragment();
if (queuedFilePaths.add(filePath)) {
downloadsBuilder.add(downloadFile(action, file));
}
}
ImmutableList<ListenableFuture<FileMetadata>> forcedDownloads = ImmutableList.of();

for (Map.Entry<Path, DirectoryMetadata> 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,
Expand All @@ -1049,6 +1054,10 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re
"Symlinks in action outputs are not yet supported by "
+ "--experimental_remote_download_outputs=minimal");
}
if (shouldForceDownloads) {
forcedDownloads =
buildFilesToDownloadWithPredicate(metadata, action, shouldForceDownloadPredicate);
}
}

FileOutErr tmpOutErr = outErr.childOutErr();
Expand All @@ -1063,6 +1072,19 @@ 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;
}

// 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;
Expand Down Expand Up @@ -1096,6 +1118,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());
Expand Down Expand Up @@ -1133,6 +1162,36 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re
return null;
}

private ImmutableList<ListenableFuture<FileMetadata>> buildFilesToDownload(
ActionResultMetadata metadata, RemoteAction action) {
Predicate<String> alwaysTrue = unused -> true;
return buildFilesToDownloadWithPredicate(metadata, action, alwaysTrue);
}

private ImmutableList<ListenableFuture<FileMetadata>> buildFilesToDownloadWithPredicate(
ActionResultMetadata metadata, RemoteAction action, Predicate<String> predicate) {
HashSet<PathFragment> queuedFilePaths = new HashSet<>();
ImmutableList.Builder<ListenableFuture<FileMetadata>> 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<Path, DirectoryMetadata> 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();
Expand Down
12 changes: 12 additions & 0 deletions src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,18 @@ 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.

Expand Down
57 changes: 57 additions & 0 deletions src/test/shell/bazel/remote/build_without_the_bytes_test.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"

0 comments on commit 3eeae23

Please sign in to comment.