diff options
3 files changed, 65 insertions, 24 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnStrategy.java index c4f788b36c..c8a8643080 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnStrategy.java @@ -21,7 +21,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.ActionExecutionContext; import com.google.devtools.build.lib.actions.ActionInput; -import com.google.devtools.build.lib.actions.ActionInputHelper; import com.google.devtools.build.lib.actions.ActionStatusMessage; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ExecutionStrategy; @@ -33,6 +32,7 @@ import com.google.devtools.build.lib.actions.UserExecException; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; +import com.google.devtools.build.lib.exec.SpawnInputExpander; import com.google.devtools.build.lib.remote.ContentDigests.ActionKey; import com.google.devtools.build.lib.remote.RemoteProtocol.Action; import com.google.devtools.build.lib.remote.RemoteProtocol.ActionResult; @@ -43,9 +43,11 @@ import com.google.devtools.build.lib.remote.RemoteProtocol.ExecuteRequest; import com.google.devtools.build.lib.remote.RemoteProtocol.ExecutionStatus; import com.google.devtools.build.lib.remote.RemoteProtocol.Platform; import com.google.devtools.build.lib.remote.TreeNodeRepository.TreeNode; +import com.google.devtools.build.lib.rules.fileset.FilesetActionContext; import com.google.devtools.build.lib.standalone.StandaloneSpawnStrategy; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; import com.google.protobuf.TextFormat; import com.google.protobuf.TextFormat.ParseException; import io.grpc.StatusRuntimeException; @@ -54,6 +56,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.SortedMap; import java.util.TreeSet; /** @@ -71,6 +74,7 @@ final class RemoteSpawnStrategy implements SpawnActionContext { private final RemoteOptions options; // TODO(olaola): This will be set on a per-action basis instead. private final Platform platform; + private final SpawnInputExpander spawnInputExpander = new SpawnInputExpander(/*strict=*/false); RemoteSpawnStrategy( Map<String, String> clientEnv, @@ -135,7 +139,14 @@ final class RemoteSpawnStrategy implements SpawnActionContext { if (options.remoteLocalExecUploadResults && actionCache != null && actionKey != null) { ArrayList<Path> outputFiles = new ArrayList<>(); for (ActionInput output : spawn.getOutputFiles()) { - outputFiles.add(execRoot.getRelative(output.getExecPathString())); + Path outputFile = execRoot.getRelative(output.getExecPathString()); + // Ignore non-existent files. + // TODO(ulfjack): This is not ideal - in general, all spawn strategies should stat the + // output files and return a list of existing files. We shouldn't re-stat the files here. + if (!outputFile.exists()) { + continue; + } + outputFiles.add(outputFile); } try { ActionResult.Builder result = ActionResult.newBuilder(); @@ -223,10 +234,13 @@ final class RemoteSpawnStrategy implements SpawnActionContext { try { // Temporary hack: the TreeNodeRepository should be created and maintained upstream! TreeNodeRepository repository = new TreeNodeRepository(execRoot); - List<ActionInput> inputs = - ActionInputHelper.expandArtifacts( - spawn.getInputFiles(), actionExecutionContext.getArtifactExpander()); - TreeNode inputRoot = repository.buildFromActionInputs(inputs); + SortedMap<PathFragment, ActionInput> inputMap = + spawnInputExpander.getInputMapping( + spawn, + actionExecutionContext.getArtifactExpander(), + actionExecutionContext.getActionInputFileCache(), + actionExecutionContext.getExecutor().getContext(FilesetActionContext.class)); + TreeNode inputRoot = repository.buildFromActionInputs(inputMap); repository.computeMerkleDigests(inputRoot); Command command = buildCommand(spawn.getArguments(), spawn.getEnvironment()); Action action = @@ -261,7 +275,7 @@ final class RemoteSpawnStrategy implements SpawnActionContext { ExecuteRequest.newBuilder() .setAction(action) .setAcceptCached(acceptCachedResult) - .setTotalInputFileCount(inputs.size()) + .setTotalInputFileCount(inputMap.size()) .setTimeoutMillis(1000 * Spawns.getTimeoutSeconds(spawn, 120)); // TODO(olaola): set sensible local and remote timouts. ExecuteReply reply = workExecutor.executeRemotely(request.build()); diff --git a/src/main/java/com/google/devtools/build/lib/remote/TreeNodeRepository.java b/src/main/java/com/google/devtools/build/lib/remote/TreeNodeRepository.java index 3ecf6fe24b..8f66301986 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/TreeNodeRepository.java +++ b/src/main/java/com/google/devtools/build/lib/remote/TreeNodeRepository.java @@ -38,6 +38,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.SortedMap; import java.util.TreeMap; import javax.annotation.Nullable; @@ -217,16 +218,20 @@ public final class TreeNodeRepository extends TreeTraverser<TreeNodeRepository.T }); } + public TreeNode buildFromActionInputs(Iterable<? extends ActionInput> inputs) { + TreeMap<PathFragment, ActionInput> sortedMap = new TreeMap<>(); + for (ActionInput input : inputs) { + sortedMap.put(new PathFragment(input.getExecPathString()), input); + } + return buildFromActionInputs(sortedMap); + } + /** * This function is a temporary and highly inefficient hack! It builds the tree from a ready list * of input files. TODO(olaola): switch to creating and maintaining the TreeNodeRepository based * on the build graph structure. */ - public TreeNode buildFromActionInputs(Iterable<ActionInput> actionInputs) { - TreeMap<PathFragment, ActionInput> sortedMap = new TreeMap<>(); - for (ActionInput input : actionInputs) { - sortedMap.put(new PathFragment(input.getExecPathString()), input); - } + public TreeNode buildFromActionInputs(SortedMap<PathFragment, ActionInput> sortedMap) { ImmutableList.Builder<ImmutableList<String>> segments = ImmutableList.builder(); for (PathFragment path : sortedMap.keySet()) { segments.add(path.getSegments()); diff --git a/src/test/shell/bazel/remote_execution_test.sh b/src/test/shell/bazel/remote_execution_test.sh index a33805037f..9726ad07ed 100755 --- a/src/test/shell/bazel/remote_execution_test.sh +++ b/src/test/shell/bazel/remote_execution_test.sh @@ -23,18 +23,6 @@ source "${CURRENT_DIR}/../integration_test_setup.sh" \ || { echo "integration_test_setup.sh not found!" >&2; exit 1; } function set_up() { - mkdir -p a - cat > a/BUILD <<EOF -package(default_visibility = ["//visibility:public"]) -cc_binary( -name = 'test', -srcs = [ 'test.cc' ], -) -EOF - cat > a/test.cc <<EOF -#include <iostream> -int main() { std::cout << "Hello world!" << std::endl; return 0; } -EOF work_path=$(mktemp -d ${TEST_TMPDIR}/remote.XXXXXXXX) pid_file=$(mktemp -u ${TEST_TMPDIR}/remote.XXXXXXXX) worker_port=$(pick_random_unused_tcp_port) || fail "no port found" @@ -64,6 +52,18 @@ function tear_down() { } function test_cc_binary() { + mkdir -p a + cat > a/BUILD <<EOF +package(default_visibility = ["//visibility:public"]) +cc_binary( +name = 'test', +srcs = [ 'test.cc' ], +) +EOF + cat > a/test.cc <<EOF +#include <iostream> +int main() { std::cout << "Hello world!" << std::endl; return 0; } +EOF bazel build //a:test >& $TEST_log \ || fail "Failed to build //a:test without remote execution" cp bazel-bin/a/test ${TEST_TMPDIR}/test_expected @@ -79,6 +79,28 @@ function test_cc_binary() { || fail "Remote execution generated different result" } +function test_cc_test() { + mkdir -p a + cat > a/BUILD <<EOF +package(default_visibility = ["//visibility:public"]) +cc_test( +name = 'test', +srcs = [ 'test.cc' ], +) +EOF + cat > a/test.cc <<EOF +#include <iostream> +int main() { std::cout << "Hello test!" << std::endl; return 0; } +EOF + bazel --host_jvm_args=-Dbazel.DigestFunction=SHA1 test \ + --spawn_strategy=remote \ + --remote_worker=localhost:${worker_port} \ + --remote_cache=localhost:${worker_port} \ + --test_output=errors \ + //a:test >& $TEST_log \ + || fail "Failed to run //a:test with remote execution" +} + # TODO(alpha): Add a test that fails remote execution when remote worker # supports sandbox. |