Adds an AaptCommandLineBuilder to simplify the aapt line building process.

Part of this change makes the --no-version-vectors only be propagated if the sdk tools are >= 23.

--
MOS_MIGRATED_REVID=114351246

3 files changed, 171 insertions, 124 deletions

diff --git a/src/tools/android/java/com/google/devtools/build/android/AaptCommandBuilder.java b/src/tools/android/java/com/google/devtools/build/android/AaptCommandBuilder.java
new file mode 100644
index 0000000000..83591e73d5
--- /dev/null
+++ b/src/tools/android/java/com/google/devtools/build/android/AaptCommandBuilder.java
@@ -0,0 +1,116 @@
+// Copyright 2016 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.android;
+
+import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableList;
+
+import com.android.builder.core.VariantConfiguration.Type;
+import com.android.sdklib.repository.FullRevision;
+
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
+import javax.annotation.Nullable;
+
+class AaptCommandBuilder {
+  private final Path aapt;
+  private final String command;
+  private final List<String> flags = new ArrayList<>();
+  private final FullRevision buildToolsVersion;
+  private final Type variantType;
+
+  AaptCommandBuilder(
+      Path aapt, @Nullable FullRevision buildToolsVersion, Type variantType, String command) {
+    this.aapt = aapt;
+    this.buildToolsVersion = buildToolsVersion;
+    this.variantType = variantType;
+    this.command = command;
+  }
+
+  AaptCommandBuilder add(String flag) {
+    flags.add(flag);
+    return this;
+  }
+
+  AaptCommandBuilder add(String flag, @Nullable String value) {
+    if (!Strings.isNullOrEmpty(value)) {
+      flags.add(flag);
+      flags.add(value);
+    }
+    return this;
+  }
+
+  AaptCommandBuilder add(String flag, @Nullable Path path) {
+    if (path != null) {
+      add(flag, path.toString());
+    }
+    return this;
+  }
+
+  AaptCommandBuilder addRepeated(String flag, Collection<String> values) {
+    for (String value : values) {
+      add(flag, value);
+    }
+    return this;
+  }
+
+
+  AaptCommandBuilder maybeAdd(String flag, boolean condition) {
+    if (condition) {
+      add(flag);
+    }
+    return this;
+  }
+
+  AaptCommandBuilder maybeAdd(String flag, Path directory, boolean condition) {
+    if (condition) {
+      add(flag, directory);
+    }
+    return this;
+  }
+
+  AaptCommandBuilder maybeAdd(String flag, FullRevision requiredVersion) {
+    if (buildToolsVersion == null || buildToolsVersion.compareTo(requiredVersion) >= 0) {
+      add(flag);
+    }
+    return this;
+  }
+
+  AaptCommandBuilder maybeAdd(String flag, Type variant) {
+    if (variantType == variant) {
+      add(flag);
+    }
+    return this;
+  }
+
+  AaptCommandBuilder maybeAdd(String flag, @Nullable String value, Type variant) {
+    if (variantType == variant) {
+      add(flag, value);
+    }
+    return this;
+  }
+
+  List<String> build() {
+    return ImmutableList
+        .<String>builder()
+        .add(aapt.toString())
+        .add(command)
+        .addAll(flags)
+        .build();
+  }
+}
+
diff --git a/src/tools/android/java/com/google/devtools/build/android/AndroidResourceProcessingAction.java b/src/tools/android/java/com/google/devtools/build/android/AndroidResourceProcessingAction.java
index f468e8aa96..2d556f4109 100644
--- a/src/tools/android/java/com/google/devtools/build/android/AndroidResourceProcessingAction.java
+++ b/src/tools/android/java/com/google/devtools/build/android/AndroidResourceProcessingAction.java
@@ -81,14 +81,6 @@ public class AndroidResourceProcessingAction {
 
   /** Flag specifications for this action. */
   public static final class Options extends OptionsBase {
-    @Option(name = "apiVersion",
-        defaultValue = "21.0.0",
-        converter = FullRevisionConverter.class,
-        category = "config",
-        help = "ApiVersion indicates the version passed to the AndroidBuilder. ApiVersion must be"
-            + " > 19.10 when defined.")
-    // TODO(bazel-team): Determine what the API version changes in AndroidBuilder.
-    public FullRevision apiVersion;
 
     @Option(name = "buildToolsVersion",
         defaultValue = "null",
@@ -295,6 +287,7 @@ public class AndroidResourceProcessingAction {
 
     try {
       final Path tmp = Files.createTempDirectory("android_resources_tmp");
+      // Clean up the tmp file on exit to keep diskspace low.
       tmp.toFile().deleteOnExit();
 
       final Path expandedOut = tmp.resolve("tmp-expanded");
@@ -359,7 +352,8 @@ public class AndroidResourceProcessingAction {
           generatedSources,
           options.packagePath,
           options.proguardOutput,
-          options.manifestOutput);
+          options.manifestOutput,
+          options.buildToolsVersion);
       LOGGER.fine(String.format("appt finished at %sms", timer.elapsed(TimeUnit.MILLISECONDS)));
       if (options.srcJarOutput != null) {
         resourceProcessor.createSrcJar(generatedSources, options.srcJarOutput,
diff --git a/src/tools/android/java/com/google/devtools/build/android/AndroidResourceProcessor.java b/src/tools/android/java/com/google/devtools/build/android/AndroidResourceProcessor.java
index 5464b429a7..40cc4e740b 100644
--- a/src/tools/android/java/com/google/devtools/build/android/AndroidResourceProcessor.java
+++ b/src/tools/android/java/com/google/devtools/build/android/AndroidResourceProcessor.java
@@ -43,6 +43,7 @@ import com.android.manifmerger.ManifestMerger2.MergeFailureException;
 import com.android.manifmerger.ManifestMerger2.SystemProperty;
 import com.android.manifmerger.MergingReport;
 import com.android.manifmerger.XmlDocument;
+import com.android.sdklib.repository.FullRevision;
 import com.android.utils.StdLogger;
 
 import org.xml.sax.SAXException;
@@ -125,7 +126,8 @@ public class AndroidResourceProcessor {
       Throwables.propagate(e);
     }
   }
-
+  
+  // TODO(bazel-team): Clean up this method call -- 19 params is too many.
   /**
    * Processes resources for generated sources, configs and packaging resources.
    */
@@ -146,7 +148,9 @@ public class AndroidResourceProcessor {
       Path sourceOut,
       Path packageOut,
       Path proguardOut,
-      Path manifestOut) throws IOException, InterruptedException, LoggedErrorException {
+      Path manifestOut,
+      @Nullable FullRevision buildToolsVersion)
+      throws IOException, InterruptedException, LoggedErrorException {
     List<SymbolFileProvider> libraries = new ArrayList<>();
     List<String> packages = new ArrayList<>();
     for (DependencyAndroidData dataDep : dependencyData) {
@@ -156,7 +160,6 @@ public class AndroidResourceProcessor {
     }
 
     Path androidManifest = processManifest(
-
         variantType == VariantConfiguration.Type.DEFAULT ? applicationId : customPackageForR,
         versionCode,
         versionName,
@@ -165,114 +168,50 @@ public class AndroidResourceProcessor {
         variantType == VariantConfiguration.Type.DEFAULT
             ? ManifestMerger2.MergeType.APPLICATION : ManifestMerger2.MergeType.LIBRARY);
 
-    File resFolder = primaryData.getResourceDirFile();
-    File assetsDir = primaryData.getAssetDirFile();
-
-    List<String> command = new ArrayList<>();
-    
-    command.add(aapt.toString());
-    command.add("package");
-    
-    // Trigger the aapt logging level on the Logger.
-    if (stdLogger.getLevel() == StdLogger.Level.VERBOSE) {
-        command.add("-v");
-    }
-    
-    // Overwrite existing files, if they exist.
-    command.add("-f");
-    
-    // Resources are precrunched in the merge process.
-    command.add("--no-crunch");
-
-    // Do not automatically generate versioned copies of vector XML resources.
-    command.add("--no-version-vectors");
-
-    // Add the android.jar as a base input.
-    command.add("-I");
-    command.add(androidJar.toString());
-
-    // Add the manifest for validation.
-    command.add("-M");
-    command.add(androidManifest.toAbsolutePath().toString());
-
-    if (resFolder.isDirectory()) {
-      command.add("-S");
-      command.add(resFolder.getAbsolutePath());
-    }
-
-    if (assetsDir != null && assetsDir.isDirectory()) {
-      command.add("-A");
-      command.add(assetsDir.getAbsolutePath());
-    }
-
-    // Outputs
-    if (sourceOut != null) {
-      prepareOutputPath(sourceOut);
-      command.add("-m");
-      command.add("-J");
-      command.add(sourceOut.toString());
-      command.add("--output-text-symbols");
-      command.add(sourceOut.toString());
-    }
-
-    if (packageOut != null) {
-      command.add("-F");
-      command.add(packageOut.toString());
-    }
-
-    if (proguardOut != null) {
-      command.add("-G");
-      command.add(proguardOut.toString());
-    }
-    
-    // Additional options.
-    if (debug) {
-      command.add("--debug-mode");
-    }
-
-    if (customPackageForR != null) {
-      command.add("--custom-package");
-      command.add(customPackageForR);
-      stdLogger.verbose("Custom package for R class: '%s'", customPackageForR);
-    }
-
-    // If it is a library, do not generate final java ids.
-    if (variantType == VariantConfiguration.Type.LIBRARY) {
-      command.add("--non-constant-id");
-    }
-
-    if (variantType == VariantConfiguration.Type.DEFAULT  && !packages.isEmpty()) {
-      // Generate the dependent R and Manifest files.
-      command.add("--extra-packages");
-      command.add(Joiner.on(":").join(packages));
-    }
-
-    if (aaptOptions.getIgnoreAssets() != null) {
-      command.add("--ignore-assets");
-      command.add(aaptOptions.getIgnoreAssets());
-    }
-
-    if (aaptOptions.getFailOnMissingConfigEntry()) {
-      command.add("--error-on-missing-config-entry");
-    }
-
-    // Never compress apks.
-    command.add("-0");
-    command.add("apk");
-
-    // Add custom no-compress extensions.
-    for (String noCompress : aaptOptions.getNoCompress()) {
-      command.add("-0");
-      command.add(noCompress);
-    }
-
-    // Filter by resource configuration type.
-    if (!resourceConfigs.isEmpty()) {
-      command.add("-c");
-      command.add(Joiner.on(',').join(resourceConfigs));
-    }
-
-    new CommandLineRunner(stdLogger).runCmdLine(command, null);
+    Path resFolder = primaryData.getResourceDirFile().toPath();
+    Path assetsDir = primaryData.getAssetDirFile().toPath();
+
+    AaptCommandBuilder commandBuilder =
+        new AaptCommandBuilder(aapt, buildToolsVersion, variantType, "package")
+            // If the logger is verbose, set aapt to be verbose
+        .maybeAdd("-v", stdLogger.getLevel() == StdLogger.Level.VERBOSE)
+        // Overwrite existing files, if they exist.
+        .add("-f")
+        // Resources are precrunched in the merge process.
+        .add("--no-crunch")
+        // Do not automatically generate versioned copies of vector XML resources.
+        .maybeAdd("--no-version-vectors", new FullRevision(23))
+        // Add the android.jar as a base input.
+        .add("-I", androidJar)
+        // Add the manifest for validation.
+        .add("-M", androidManifest.toAbsolutePath())
+        // Maybe add the resources if they exist
+        .maybeAdd("-S", resFolder, Files.isDirectory(resFolder))
+        // Maybe add the assets if they exist
+        .maybeAdd("-A", assetsDir, Files.isDirectory(assetsDir))
+        // Outputs
+        .maybeAdd("-m", sourceOut != null)
+        .maybeAdd("-J", prepareOutputPath(sourceOut), sourceOut != null)
+        .maybeAdd("--output-text-symbols", prepareOutputPath(sourceOut), sourceOut != null)
+        .add("-F", packageOut)
+        .add("-G", proguardOut)
+        .maybeAdd("--debug-mode", debug)
+        .add("--custom-package", customPackageForR)
+        // If it is a library, do not generate final java ids.
+        .maybeAdd("--non-constant-id", VariantConfiguration.Type.LIBRARY)
+        // Generate the dependent R and Manifest files.
+        .maybeAdd("--extra-packages", Joiner.on(":").join(packages),
+            VariantConfiguration.Type.DEFAULT)
+        .add("--ignore-assets", aaptOptions.getIgnoreAssets())
+        .maybeAdd("--error-on-missing-config-entry", aaptOptions.getFailOnMissingConfigEntry())
+        // Never compress apks.
+        .add("-0", "apk")
+        // Add custom no-compress extensions.
+        .addRepeated("-0", aaptOptions.getNoCompress())
+        // Filter by resource configuration type.
+        .add("-c", Joiner.on(',').join(resourceConfigs));
+
+    new CommandLineRunner(stdLogger).runCmdLine(commandBuilder.build(), null);
 
     // The R needs to be created for each library in the dependencies,
     // but only if the current project is not a library.
@@ -288,7 +227,6 @@ public class AndroidResourceProcessor {
     }
     if (manifestOut != null) {
       Files.copy(androidManifest, manifestOut);
-
       Files.setLastModifiedTime(manifestOut, FileTime.fromMillis(0L));
     }
   }
@@ -349,7 +287,6 @@ public class AndroidResourceProcessor {
   }
 
   private Path processManifest(
-
       String newManifestPackage,
       int versionCode,
       String versionName,
@@ -489,11 +426,11 @@ public class AndroidResourceProcessor {
     assetSets.add(mainAssets);
   }
 
-  private String prepareOutputPath(@Nullable Path out) throws IOException {
+  @Nullable private Path prepareOutputPath(@Nullable Path out) throws IOException {
     if (out == null) {
       return null;
     }
-    return Files.createDirectories(out).toString();
+    return Files.createDirectories(out);
   }
 
   /**