Stop using CustomMultiArgv in ProtoCompileActionBuilder.

CustomMultiArgv is hard to serialise. This CL still uses mapping functions, which are equally hard to serialise, but it moves one step closer to being structural.

PiperOrigin-RevId: 166230153

1 files changed, 41 insertions, 29 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java
index 2cf4e1b36b..44cc3f8418 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java
@@ -29,12 +29,13 @@ import com.google.devtools.build.lib.analysis.FilesToRunProvider;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
-import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder.ProtoCommandLineArgv;
 import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder.ToolchainInvocation;
 import com.google.devtools.build.lib.util.LazyString;
 import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
+import javax.annotation.Nullable;
 import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -164,7 +165,8 @@ public class ProtoCompileActionBuilderTest {
             "--java_out=param1,param2:foo.srcjar",
             "-Iimport1.proto=import1.proto",
             "-Iimport2.proto=import2.proto",
-            "--direct_dependencies=import1.proto",
+            "--direct_dependencies",
+            "import1.proto",
             String.format(ProtoCompileActionBuilder.STRICT_DEPS_FLAG_TEMPLATE, "//foo:bar"),
             "source_file.proto")
         .inOrder();
@@ -289,35 +291,32 @@ public class ProtoCompileActionBuilderTest {
   @Test
   public void testProtoCommandLineArgv() throws Exception {
     assertThat(
-            new ProtoCommandLineArgv(
-                    null /* directDependencies */,
-                    ImmutableList.of(derivedArtifact("//:dont-care", "foo.proto")))
-                .argv())
+            protoArgv(
+                null /* directDependencies */,
+                ImmutableList.of(derivedArtifact("//:dont-care", "foo.proto"))))
         .containsExactly("-Ifoo.proto=out/foo.proto");
 
     assertThat(
-            new ProtoCommandLineArgv(
-                    ImmutableList.<Artifact>of() /* directDependencies */,
-                    ImmutableList.of(derivedArtifact("//:dont-care", "foo.proto")))
-                .argv())
+            protoArgv(
+                ImmutableList.of() /* directDependencies */,
+                ImmutableList.of(derivedArtifact("//:dont-care", "foo.proto"))))
         .containsExactly("-Ifoo.proto=out/foo.proto", "--direct_dependencies=");
 
     assertThat(
-            new ProtoCommandLineArgv(
-                    ImmutableList.of(
-                        derivedArtifact("//:dont-care", "foo.proto")) /* directDependencies */,
-                    ImmutableList.of(derivedArtifact("//:dont-care", "foo.proto")))
-                .argv())
-        .containsExactly("-Ifoo.proto=out/foo.proto", "--direct_dependencies=foo.proto");
+            protoArgv(
+                ImmutableList.of(
+                    derivedArtifact("//:dont-care", "foo.proto")) /* directDependencies */,
+                ImmutableList.of(derivedArtifact("//:dont-care", "foo.proto"))))
+        .containsExactly("-Ifoo.proto=out/foo.proto", "--direct_dependencies", "foo.proto");
 
     assertThat(
-            new ProtoCommandLineArgv(
-                    ImmutableList.of(
-                        derivedArtifact("//:dont-care", "foo.proto"),
-                        derivedArtifact("//:dont-care", "bar.proto")) /* directDependencies */,
-                    ImmutableList.of(derivedArtifact("//:dont-care", "foo.proto")))
-                .argv())
-        .containsExactly("-Ifoo.proto=out/foo.proto", "--direct_dependencies=foo.proto:bar.proto");
+            protoArgv(
+                ImmutableList.of(
+                    derivedArtifact("//:dont-care", "foo.proto"),
+                    derivedArtifact("//:dont-care", "bar.proto")) /* directDependencies */,
+                ImmutableList.of(derivedArtifact("//:dont-care", "foo.proto"))))
+        .containsExactly(
+            "-Ifoo.proto=out/foo.proto", "--direct_dependencies", "foo.proto:bar.proto");
   }
 
   /**
@@ -329,10 +328,9 @@ public class ProtoCompileActionBuilderTest {
   @Test
   public void testIncludeMapsOfExternalFiles() throws Exception {
     assertThat(
-            new ProtoCommandLineArgv(
-                    null /* protosInDirectoDependencies */,
-                    ImmutableList.of(artifact("@bla//foo:bar", "external/bla/foo/bar.proto")))
-                .argv())
+            protoArgv(
+                null /* protosInDirectoDependencies */,
+                ImmutableList.of(artifact("@bla//foo:bar", "external/bla/foo/bar.proto"))))
         .containsExactly("-Ifoo/bar.proto=external/bla/foo/bar.proto");
   }
 
@@ -342,8 +340,7 @@ public class ProtoCompileActionBuilderTest {
   public void directDependenciesOnExternalFiles() throws Exception {
     ImmutableList<Artifact> protos =
         ImmutableList.of(artifact("@bla//foo:bar", "external/bla/foo/bar.proto"));
-    assertThat(new ProtoCommandLineArgv(protos, protos).argv())
-        .containsExactly("--direct_dependencies=foo/bar.proto");
+    assertThat(protoArgv(protos, protos)).containsExactly("--direct_dependencies", "foo/bar.proto");
   }
 
   private Artifact artifact(String ownerLabel, String path) {
@@ -362,4 +359,19 @@ public class ProtoCompileActionBuilderTest {
         derivedRoot.getExecPath().getRelative(path),
         new LabelArtifactOwner(Label.parseAbsoluteUnchecked(ownerLabel)));
   }
+
+  private static Iterable<String> protoArgv(
+      @Nullable Iterable<Artifact> protosInDirectDependencies,
+      Iterable<Artifact> transitiveImports) {
+    CustomCommandLine.Builder commandLine = CustomCommandLine.builder();
+    NestedSet<Artifact> protosInDirectDependenciesBuilder =
+        protosInDirectDependencies != null
+            ? NestedSetBuilder.wrap(STABLE_ORDER, protosInDirectDependencies)
+            : null;
+    NestedSet<Artifact> transitiveImportsNestedSet =
+        NestedSetBuilder.wrap(STABLE_ORDER, transitiveImports);
+    ProtoCompileActionBuilder.addIncludeMapArguments(
+        commandLine, protosInDirectDependenciesBuilder, transitiveImportsNestedSet);
+    return commandLine.build().arguments();
+  }
 }