AndroidBusyBox: deprecate path-list-type flags

This commit:
- deprecates PathListConverter
- removes ExistingPathListConverter because it was
  not used in production, only tests
- deprecated List<Path> type flags that use
  PathListConverter
- introduces new List<Path> type flags next to the
  deprecated ones that use @Options.allowMultiple
  and convert with PathConverter; the new and old
  lists are concatenated, yielding the flag value

PathListConverter and all of its occurrences
should be removed after 2018-01-31 (about 6 months
from now, which is a safe enough timeframe for
everyone to upgrade Bazel so it uses the new-style
flags).

Reason for deprecation is that colon-separated
path lists don't work on Windows because paths
have colons in them.

Since the Android BusyBox is not intended to be
executed by users but by Bazel only, there's no
release notes necessary.

See https://github.com/bazelbuild/bazel/issues/3264

RELNOTES: none
PiperOrigin-RevId: 162193998

3 files changed, 31 insertions, 44 deletions

diff --git a/src/test/java/com/google/devtools/build/android/ConvertersTest.java b/src/test/java/com/google/devtools/build/android/ConvertersTest.java
index 92c9cf2f22..0bb8366575 100644
--- a/src/test/java/com/google/devtools/build/android/ConvertersTest.java
+++ b/src/test/java/com/google/devtools/build/android/ConvertersTest.java
@@ -15,18 +15,13 @@ package com.google.devtools.build.android;
 
 import static com.google.common.truth.Truth.assertThat;
 
-import com.google.common.base.Joiner;
 import com.google.devtools.build.android.Converters.ExistingPathConverter;
-import com.google.devtools.build.android.Converters.ExistingPathListConverter;
 import com.google.devtools.build.android.Converters.ExistingPathStringDictionaryConverter;
 import com.google.devtools.build.android.Converters.PathConverter;
-import com.google.devtools.build.android.Converters.PathListConverter;
 import com.google.devtools.build.android.Converters.StringDictionaryConverter;
 import com.google.devtools.common.options.OptionsParsingException;
-import java.io.File;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.List;
 import java.util.Map;
 import org.junit.Rule;
 import org.junit.Test;
@@ -41,8 +36,6 @@ import org.junit.runners.JUnit4;
 @RunWith(JUnit4.class)
 public final class ConvertersTest {
 
-  private static final String SEPARATOR = File.pathSeparator;
-
   @Rule public final TemporaryFolder tmp = new TemporaryFolder();
 
   @Rule
@@ -58,9 +51,9 @@ public final class ConvertersTest {
   @Test
   public void testPathConverter_invalid() throws Exception {
     String arg = "\u0000";
+    PathConverter converter = new PathConverter();
     expected.expect(OptionsParsingException.class);
     expected.expectMessage(String.format("%s is not a valid path:", arg));
-    PathConverter converter = new PathConverter();
     converter.convert(arg);
   }
 
@@ -74,9 +67,9 @@ public final class ConvertersTest {
   @Test
   public void testExistingPathConverter_nonExisting() throws Exception {
     String arg = "test_file";
+    ExistingPathConverter converter = new ExistingPathConverter();
     expected.expect(OptionsParsingException.class);
     expected.expectMessage(String.format("%s is not a valid path: it does not exist.", arg));
-    ExistingPathConverter converter = new ExistingPathConverter();
     converter.convert(arg);
   }
 
@@ -89,50 +82,30 @@ public final class ConvertersTest {
   }
 
   @Test
-  public void testPathListConverter() throws Exception {
-    PathListConverter converter = new PathListConverter();
-    List<Path> result =
-        converter.convert("foo" + SEPARATOR + "bar" + SEPARATOR + SEPARATOR + "baz" + SEPARATOR);
-    assertThat(result)
-        .containsAllOf(Paths.get("foo"), Paths.get("bar"), Paths.get("baz"))
-        .inOrder();
-  }
-
-  @Test
-  public void testExisingPathListConverter() throws Exception {
-    String arg = "non-existing";
-    Path existingFile = tmp.newFile("existing").toPath();
-    expected.expect(OptionsParsingException.class);
-    expected.expectMessage(String.format("%s is not a valid path: it does not exist.", arg));
-    ExistingPathListConverter converter = new ExistingPathListConverter();
-    converter.convert(Joiner.on(SEPARATOR).join(existingFile.toString(), arg));
-  }
-
-  @Test
   public void testStringDictionaryConverter_emptyEntry() throws Exception {
+    StringDictionaryConverter converter = new StringDictionaryConverter();
     expected.expect(OptionsParsingException.class);
     expected.expectMessage("Dictionary entry [] does not contain both a key and a value.");
-    StringDictionaryConverter converter = new StringDictionaryConverter();
     converter.convert("foo:bar,,baz:bar");
   }
 
   @Test
   public void testStringDictionaryConverter_missingKeyOrValue() throws Exception {
     String badEntry = "foo";
+    StringDictionaryConverter converter = new StringDictionaryConverter();
     expected.expect(OptionsParsingException.class);
     expected.expectMessage(String.format(
         "Dictionary entry [%s] does not contain both a key and a value.", badEntry));
-    StringDictionaryConverter converter = new StringDictionaryConverter();
     converter.convert(badEntry);
   }
 
   @Test
   public void testStringDictionaryConverter_extraFields() throws Exception {
     String badEntry = "foo:bar:baz";
+    StringDictionaryConverter converter = new StringDictionaryConverter();
     expected.expect(OptionsParsingException.class);
     expected.expectMessage(String.format(
         "Dictionary entry [%s] contains too many fields.", badEntry));
-    StringDictionaryConverter converter = new StringDictionaryConverter();
     converter.convert(badEntry);
   }
 
@@ -140,10 +113,10 @@ public final class ConvertersTest {
   public void testStringDictionaryConverter_duplicateKey() throws Exception {
     String key = "foo";
     String arg = String.format("%s:%s,%s:%s", key, "bar", key, "baz");
+    StringDictionaryConverter converter = new StringDictionaryConverter();
     expected.expect(OptionsParsingException.class);
     expected.expectMessage(String.format(
         "Dictionary already contains the key [%s].", key));
-    StringDictionaryConverter converter = new StringDictionaryConverter();
     converter.convert(arg);
   }
 
diff --git a/src/test/java/com/google/devtools/build/android/ideinfo/JarFilterTest.java b/src/test/java/com/google/devtools/build/android/ideinfo/JarFilterTest.java
index a17048d9e0..2bb779f088 100644
--- a/src/test/java/com/google/devtools/build/android/ideinfo/JarFilterTest.java
+++ b/src/test/java/com/google/devtools/build/android/ideinfo/JarFilterTest.java
@@ -17,7 +17,6 @@ package com.google.devtools.build.android.ideinfo;
 import static com.google.common.truth.Truth.assertThat;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
-import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import com.google.common.io.Files;
@@ -106,7 +105,7 @@ public class JarFilterTest {
 
     String[] args =
         new String[] {
-          "--jars",
+          "--jar",
           filterJar.getPath(),
           "--output",
           outputJar.getPath(),
@@ -198,13 +197,17 @@ public class JarFilterTest {
 
     String[] args =
         new String[] {
-          "--keep_java_files",
-          fooJava.getPath() + File.pathSeparator + barJava.getPath(),
-          "--keep_source_jars",
-          Joiner.on(File.pathSeparator).join(srcJar.getPath(), src3Jar.getPath()),
-          "--filter_jars",
+          "--keep_java_file",
+          fooJava.getPath(),
+          "--keep_java_file",
+          barJava.getPath(),
+          "--keep_source_jar",
+          srcJar.getPath(),
+          "--keep_source_jar",
+          src3Jar.getPath(),
+          "--filter_jar",
           filterJar.getPath(),
-          "--filter_source_jars",
+          "--filter_source_jar",
           filterSrcJar.getPath(),
           "--filtered_jar",
           filteredJar.getPath(),
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..ea24af9324 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
@@ -103,14 +103,22 @@ public class CustomCommandLineTest {
   }
 
   @Test
-  public void testArtifactExecPathsArgs() {
+  public void testArtifactExecPaths() {
     CustomCommandLine cl =
         CustomCommandLine.builder()
-            .add("--path")
             .addExecPaths(ImmutableList.of(artifact1, artifact2))
             .build();
+    assertThat(cl.arguments()).isEqualTo(ImmutableList.of("dir/file1.txt", "dir/file2.txt"));
+  }
+
+  @Test
+  public void testArtifactExecPathsArgs() {
+    CustomCommandLine cl =
+        CustomCommandLine.builder()
+            .addExecPaths("--path", ImmutableList.of(artifact1, artifact2))
+            .build();
     assertThat(cl.arguments())
-        .isEqualTo(ImmutableList.of("--path", "dir/file1.txt", "dir/file2.txt"));
+        .isEqualTo(ImmutableList.of("--path", "dir/file1.txt", "--path", "dir/file2.txt"));
   }
 
   @Test
@@ -195,6 +203,8 @@ public class CustomCommandLineTest {
             .add(ImmutableList.<String>of())
             .addExecPaths(null)
             .addExecPaths(ImmutableList.of())
+            .addExecPaths("foo", null)
+            .addExecPaths("foo", ImmutableList.of())
             .addExecPath("foo", null)
             .addPlaceholderTreeArtifactExecPath(null)
             .addJoinStrings("foo", null)
@@ -240,6 +250,7 @@ public class CustomCommandLineTest {
     npt.testMethod(obj, clazz.getMethod("addFormatEach", String.class, Iterable.class));
 
     npt.setDefault(Iterable.class, ImmutableList.of(artifact1));
+    npt.testMethod(obj, clazz.getMethod("addExecPaths", String.class, Iterable.class));
     npt.testMethod(obj, clazz.getMethod("addJoinExecPaths", String.class, Iterable.class));
     npt.testMethod(obj, clazz.getMethod("addBeforeEachExecPath", String.class, Iterable.class));