Automated rollback of commit a76c94be7c56b93fc5a2f9ececfba7ac1f61f69c.

*** Reason for rollback ***

Caused memory regression.

*** Original change description ***

CustomCommandLine.Builder: clean up its interface

In this commit:
- remove unused methods and classes
- turn CustomCommandLine.ArgvFragment into an
  interface
- remove the
  CustomCommandLine.TreeFileArtifactArgvFragment
  abstract class; it only had one remaining
  subclass
- add @Nullable annotations where nulls are fine
- add Precondition checks for non-nullable args
- simplify the interface by removing add* methods
  that can be composed of other add* methods; this
  makes it easier to see...

***

RELNOTES: none
PiperOrigin-RevId: 162320031

2 files changed, 66 insertions, 112 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/actions/CustomCommandLineTest.java b/src/test/java/com/google/devtools/build/lib/actions/CustomCommandLineTest.java
index f7995b42d3..ea061beb2d 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/CustomCommandLineTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/CustomCommandLineTest.java
@@ -18,7 +18,6 @@ import static org.junit.Assert.fail;
 
 import com.google.common.base.Function;
 import com.google.common.collect.ImmutableList;
-import com.google.common.testing.NullPointerTester;
 import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
 import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
 import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType;
@@ -33,6 +32,7 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.testutil.Scratch;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.Collection;
+import javax.annotation.Nullable;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -71,18 +71,15 @@ public class CustomCommandLineTest {
 
   @Test
   public void testStringsArgs() {
-    CustomCommandLine cl =
-        CustomCommandLine.builder().add("--arg").add(ImmutableList.of("a", "b")).build();
+    CustomCommandLine cl = CustomCommandLine.builder().add("--arg",
+        ImmutableList.of("a", "b")).build();
     assertThat(cl.arguments()).isEqualTo(ImmutableList.of("--arg", "a", "b"));
   }
 
   @Test
   public void testArtifactJoinStringArgs() {
-    CustomCommandLine cl =
-        CustomCommandLine.builder()
-            .add("--path")
-            .addJoinStrings(":", ImmutableList.of("foo", "bar"))
-            .build();
+    CustomCommandLine cl = CustomCommandLine.builder().addJoinStrings("--path", ":",
+        ImmutableList.of("foo", "bar")).build();
     assertThat(cl.arguments()).isEqualTo(ImmutableList.of("--path", "foo:bar"));
   }
 
@@ -90,8 +87,17 @@ public class CustomCommandLineTest {
   public void testJoinValues() {
     CustomCommandLine cl =
         CustomCommandLine.builder()
-            .add("--path")
-            .addJoinValues(":", ImmutableList.of("foo", "bar", "baz"), String::toUpperCase)
+            .addJoinValues(
+                "--path",
+                ":",
+                ImmutableList.of("foo", "bar", "baz"),
+                new Function<String, String>() {
+                  @Nullable
+                  @Override
+                  public String apply(@Nullable String s) {
+                    return s.toUpperCase();
+                  }
+                })
             .build();
     assertThat(cl.arguments()).isEqualTo(ImmutableList.of("--path", "FOO:BAR:BAZ"));
   }
@@ -104,11 +110,8 @@ public class CustomCommandLineTest {
 
   @Test
   public void testArtifactExecPathsArgs() {
-    CustomCommandLine cl =
-        CustomCommandLine.builder()
-            .add("--path")
-            .addExecPaths(ImmutableList.of(artifact1, artifact2))
-            .build();
+    CustomCommandLine cl = CustomCommandLine.builder().addExecPaths("--path",
+        ImmutableList.of(artifact1, artifact2)).build();
     assertThat(cl.arguments())
         .isEqualTo(ImmutableList.of("--path", "dir/file1.txt", "dir/file2.txt"));
   }
@@ -122,11 +125,8 @@ public class CustomCommandLineTest {
 
   @Test
   public void testArtifactJoinExecPathArgs() {
-    CustomCommandLine cl =
-        CustomCommandLine.builder()
-            .add("--path")
-            .addJoinExecPaths(":", ImmutableList.of(artifact1, artifact2))
-            .build();
+    CustomCommandLine cl = CustomCommandLine.builder().addJoinExecPaths("--path", ":",
+        ImmutableList.of(artifact1, artifact2)).build();
     assertThat(cl.arguments()).isEqualTo(ImmutableList.of("--path", "dir/file1.txt:dir/file2.txt"));
   }
 
@@ -137,6 +137,13 @@ public class CustomCommandLineTest {
   }
 
   @Test
+  public void testJoinPathArgs() {
+    CustomCommandLine cl = CustomCommandLine.builder().addJoinPaths(":",
+        ImmutableList.of(artifact1.getExecPath(), artifact2.getExecPath())).build();
+    assertThat(cl.arguments()).isEqualTo(ImmutableList.of("dir/file1.txt:dir/file2.txt"));
+  }
+
+  @Test
   public void testPathsArgs() {
     CustomCommandLine cl = CustomCommandLine.builder().addPaths("%s:%s",
         artifact1.getExecPath(), artifact1.getRootRelativePath()).build();
@@ -167,15 +174,12 @@ public class CustomCommandLineTest {
 
   @Test
   public void testCombinedArgs() {
-    CustomCommandLine cl =
-        CustomCommandLine.builder()
-            .add("--arg")
-            .add("--args")
-            .add(ImmutableList.of("abc"))
-            .add("--path1")
-            .addExecPaths(ImmutableList.of(artifact1))
-            .addExecPath("--path2", artifact2)
-            .build();
+    CustomCommandLine cl = CustomCommandLine.builder()
+        .add("--arg")
+        .add("--args", ImmutableList.of("abc"))
+        .addExecPaths("--path1", ImmutableList.of(artifact1))
+        .addExecPath("--path2", artifact2)
+        .build();
     assertThat(cl.arguments())
         .isEqualTo(
             ImmutableList.of(
@@ -183,73 +187,13 @@ public class CustomCommandLineTest {
   }
 
   @Test
-  public void testAddNulls() throws Exception {
-    Artifact treeArtifact = createTreeArtifact("myTreeArtifact");
-    assertThat(treeArtifact).isNotNull();
-
-    CustomCommandLine cl =
-        CustomCommandLine.builder()
-            .add((CharSequence) null)
-            .add((Label) null)
-            .add((Iterable<String>) null)
-            .add(ImmutableList.<String>of())
-            .addExecPaths(null)
-            .addExecPaths(ImmutableList.of())
-            .addExecPath("foo", null)
-            .addPlaceholderTreeArtifactExecPath(null)
-            .addJoinStrings("foo", null)
-            .addJoinStrings("foo", ImmutableList.of())
-            .addJoinValues("foo", null, String::toString)
-            .addJoinValues("foo", ImmutableList.of(), String::toString)
-            .addJoinExecPaths("foo", null)
-            .addJoinExecPaths("foo", ImmutableList.of())
-            .addPath(null)
-            .addPaths("foo")
-            .addPaths("foo", (PathFragment[]) null)
-            .addPaths("foo", new PathFragment[0])
-            .addBeforeEachPath("foo", null)
-            .addBeforeEachPath("foo", ImmutableList.of())
-            .addBeforeEach("foo", null)
-            .addBeforeEach("foo", ImmutableList.of())
-            .addBeforeEachExecPath("foo", null)
-            .addBeforeEachExecPath("foo", ImmutableList.of())
-            .addFormatEach("%s", null)
-            .addFormatEach("%s", ImmutableList.of())
-            .add((CustomArgv) null)
-            .add((CustomMultiArgv) null)
-            .build();
+  public void testAddNulls() {
+    CustomCommandLine cl = CustomCommandLine.builder()
+        .add("--args", null)
+        .addExecPaths(null, ImmutableList.of(artifact1))
+        .addExecPath(null, null)
+        .build();
     assertThat(cl.arguments()).isEqualTo(ImmutableList.of());
-
-    CustomCommandLine.Builder obj = CustomCommandLine.builder();
-    Class<CustomCommandLine.Builder> clazz = CustomCommandLine.Builder.class;
-    NullPointerTester npt =
-        new NullPointerTester()
-            .setDefault(Artifact.class, artifact1)
-            .setDefault(String.class, "foo")
-            .setDefault(PathFragment[].class, new PathFragment[] {PathFragment.create("foo")});
-
-    npt.testMethod(obj, clazz.getMethod("addExecPath", String.class, Artifact.class));
-    npt.testMethod(obj, clazz.getMethod("addParamFile", String.class, Artifact.class));
-    npt.testMethod(obj, clazz.getMethod("addPaths", String.class, PathFragment[].class));
-
-    npt.setDefault(Iterable.class, ImmutableList.of("foo"));
-    npt.testMethod(obj, clazz.getMethod("addJoinStrings", String.class, Iterable.class));
-    npt.testMethod(
-        obj, clazz.getMethod("addJoinValues", String.class, Iterable.class, Function.class));
-    npt.testMethod(obj, clazz.getMethod("addBeforeEach", String.class, Iterable.class));
-    npt.testMethod(obj, clazz.getMethod("addFormatEach", String.class, Iterable.class));
-
-    npt.setDefault(Iterable.class, ImmutableList.of(artifact1));
-    npt.testMethod(obj, clazz.getMethod("addJoinExecPaths", String.class, Iterable.class));
-    npt.testMethod(obj, clazz.getMethod("addBeforeEachExecPath", String.class, Iterable.class));
-
-    npt.setDefault(Iterable.class, ImmutableList.of(PathFragment.create("foo")));
-    npt.testMethod(obj, clazz.getMethod("addBeforeEachPath", String.class, Iterable.class));
-
-    npt.setDefault(Artifact.class, treeArtifact);
-    npt.testMethod(
-        obj, clazz.getMethod("addJoinExpandedTreeArtifactExecPath", String.class, Artifact.class));
-    npt.testMethod(obj, clazz.getMethod("addExpandedTreeArtifactExecPaths", Artifact.class));
   }
 
   @Test
@@ -257,13 +201,10 @@ public class CustomCommandLineTest {
     Artifact treeArtifactOne = createTreeArtifact("myArtifact/treeArtifact1");
     Artifact treeArtifactTwo = createTreeArtifact("myArtifact/treeArtifact2");
 
-    CustomCommandLine commandLineTemplate =
-        CustomCommandLine.builder()
-            .add("--argOne")
-            .addPlaceholderTreeArtifactExecPath(treeArtifactOne)
-            .add("--argTwo")
-            .addPlaceholderTreeArtifactExecPath(treeArtifactTwo)
-            .build();
+    CustomCommandLine commandLineTemplate = CustomCommandLine.builder()
+        .addPlaceholderTreeArtifactExecPath("--argOne", treeArtifactOne)
+        .addPlaceholderTreeArtifactExecPath("--argTwo", treeArtifactTwo)
+        .build();
 
     TreeFileArtifact treeFileArtifactOne = createTreeFileArtifact(
         treeArtifactOne, "children/child1");
@@ -283,17 +224,32 @@ public class CustomCommandLineTest {
   }
 
   @Test
+  public void testTreeFileArtifactExecPathWithTemplateArgs() {
+    Artifact treeArtifact = createTreeArtifact("myArtifact/treeArtifact1");
+
+    CustomCommandLine commandLineTemplate = CustomCommandLine.builder()
+        .addPlaceholderTreeArtifactFormattedExecPath("path:%s", treeArtifact)
+        .build();
+
+    TreeFileArtifact treeFileArtifact = createTreeFileArtifact(
+        treeArtifact, "children/child1");
+
+    CustomCommandLine commandLine = commandLineTemplate.evaluateTreeFileArtifacts(
+        ImmutableList.of(treeFileArtifact));
+
+    assertThat(commandLine.arguments()).containsExactly(
+        "path:myArtifact/treeArtifact1/children/child1");
+  }
+
+  @Test
   public void testTreeFileArtifactArgThrowWithoutSubstitution() {
     Artifact treeArtifactOne = createTreeArtifact("myArtifact/treeArtifact1");
     Artifact treeArtifactTwo = createTreeArtifact("myArtifact/treeArtifact2");
 
-    CustomCommandLine commandLineTemplate =
-        CustomCommandLine.builder()
-            .add("--argOne")
-            .addPlaceholderTreeArtifactExecPath(treeArtifactOne)
-            .add("--argTwo")
-            .addPlaceholderTreeArtifactExecPath(treeArtifactTwo)
-            .build();
+    CustomCommandLine commandLineTemplate = CustomCommandLine.builder()
+        .addPlaceholderTreeArtifactExecPath("--argOne", treeArtifactOne)
+        .addPlaceholderTreeArtifactExecPath("--argTwo", treeArtifactTwo)
+        .build();
 
     try {
       commandLineTemplate.arguments();
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java
index 6e5de2fce5..8711b47a4b 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java
@@ -132,9 +132,7 @@ public class ParamFileWriteActionTest extends BuildViewTestCase {
     return CustomCommandLine.builder()
         .add("--flag1")
         .add("--flag2")
-        .add("--flag3")
-        .add("value1")
-        .add("value2")
+        .add("--flag3", ImmutableList.of("value1", "value2"))
         .build();
   }