Merge "AAPT2: Add flag to print multi APK artifact names."

5 files changed, 182 insertions, 77 deletions

diff --git a/tools/aapt2/cmd/Optimize.cpp b/tools/aapt2/cmd/Optimize.cpp
index 84b79274b9a6..887803e147da 100644
--- a/tools/aapt2/cmd/Optimize.cpp
+++ b/tools/aapt2/cmd/Optimize.cpp
@@ -323,11 +323,13 @@ int Optimize(const std::vector<StringPiece>& args) {
   std::vector<std::string> configs;
   std::vector<std::string> split_args;
   bool verbose = false;
+  bool print_only = false;
   Flags flags =
       Flags()
           .OptionalFlag("-o", "Path to the output APK.", &options.output_path)
           .OptionalFlag("-d", "Path to the output directory (for splits).", &options.output_dir)
           .OptionalFlag("-x", "Path to XML configuration file.", &config_path)
+          .OptionalSwitch("-p", "Print the multi APK artifacts and exit.", &print_only)
           .OptionalFlag(
               "--target-densities",
               "Comma separated list of the screen densities that the APK will be optimized for.\n"
@@ -372,12 +374,12 @@ int Optimize(const std::vector<StringPiece>& args) {
   }
 
   context.SetVerbose(verbose);
+  IDiagnostics* diag = context.GetDiagnostics();
 
   if (target_densities) {
     // Parse the target screen densities.
     for (const StringPiece& config_str : util::Tokenize(target_densities.value(), ',')) {
-      Maybe<uint16_t> target_density =
-          ParseTargetDensityParameter(config_str, context.GetDiagnostics());
+      Maybe<uint16_t> target_density = ParseTargetDensityParameter(config_str, diag);
       if (!target_density) {
         return 1;
       }
@@ -387,7 +389,7 @@ int Optimize(const std::vector<StringPiece>& args) {
 
   std::unique_ptr<IConfigFilter> filter;
   if (!configs.empty()) {
-    filter = ParseConfigFilterParameters(configs, context.GetDiagnostics());
+    filter = ParseConfigFilterParameters(configs, diag);
     if (filter == nullptr) {
       return 1;
     }
@@ -398,26 +400,45 @@ int Optimize(const std::vector<StringPiece>& args) {
   for (const std::string& split_arg : split_args) {
     options.split_paths.emplace_back();
     options.split_constraints.emplace_back();
-    if (!ParseSplitParameter(split_arg, context.GetDiagnostics(), &options.split_paths.back(),
+    if (!ParseSplitParameter(split_arg, diag, &options.split_paths.back(),
                              &options.split_constraints.back())) {
       return 1;
     }
   }
 
   if (config_path) {
-    if (!options.output_dir) {
-      context.GetDiagnostics()->Error(
-          DiagMessage() << "Output directory is required when using a configuration file");
-      return 1;
-    }
     std::string& path = config_path.value();
     Maybe<ConfigurationParser> for_path = ConfigurationParser::ForPath(path);
     if (for_path) {
-      options.configuration = for_path.value().WithDiagnostics(context.GetDiagnostics()).Parse();
+      options.configuration = for_path.value().WithDiagnostics(diag).Parse();
     } else {
-      context.GetDiagnostics()->Error(DiagMessage() << "Could not parse config file " << path);
+      diag->Error(DiagMessage() << "Could not parse config file " << path);
+      return 1;
+    }
+
+    if (print_only) {
+      std::vector<std::string> names;
+      const PostProcessingConfiguration& config = options.configuration.value();
+      if (!config.AllArtifactNames(file::GetFilename(apk_path), &names, diag)) {
+        diag->Error(DiagMessage() << "Failed to generate output artifact list");
+        return 1;
+      }
+
+      for (const auto& name : names) {
+        std::cout << name << std::endl;
+      }
+      return 0;
+    }
+
+    // Since we know that we are going to process the APK (not just print targets), make sure we
+    // have somewhere to write them to.
+    if (!options.output_dir) {
+      diag->Error(DiagMessage() << "Output directory is required when using a configuration file");
       return 1;
     }
+  } else if (print_only) {
+    diag->Error(DiagMessage() << "Asked to print artifacts without providing a configurations");
+    return 1;
   }
 
   if (!ExtractAppDataFromManifest(&context, apk.get(), &options)) {
diff --git a/tools/aapt2/configuration/ConfigurationParser.cpp b/tools/aapt2/configuration/ConfigurationParser.cpp
index 424e9be3ef09..1735a504e553 100644
--- a/tools/aapt2/configuration/ConfigurationParser.cpp
+++ b/tools/aapt2/configuration/ConfigurationParser.cpp
@@ -30,6 +30,7 @@
 #include "io/File.h"
 #include "io/FileSystem.h"
 #include "io/StringInputStream.h"
+#include "util/Files.h"
 #include "util/Maybe.h"
 #include "util/Util.h"
 #include "xml/XmlActionExecutor.h"
@@ -149,23 +150,48 @@ static bool ReplacePlaceholder(const StringPiece& placeholder, const Maybe<Strin
   return true;
 }
 
-Maybe<std::string> Artifact::ToArtifactName(const StringPiece& format, IDiagnostics* diag,
-                                            const StringPiece& base_name,
-                                            const StringPiece& ext) const {
-  std::string result = format.to_string();
+/**
+ * Returns the common artifact base name from a template string.
+ */
+Maybe<std::string> ToBaseName(std::string result, const StringPiece& apk_name, IDiagnostics* diag) {
+  const StringPiece ext = file::GetExtension(apk_name);
+  size_t end_index = apk_name.to_string().rfind(ext.to_string());
+  const std::string base_name =
+      (end_index != std::string::npos) ? std::string{apk_name.begin(), end_index} : "";
 
-  Maybe<StringPiece> maybe_base_name =
-      base_name.empty() ? Maybe<StringPiece>{} : Maybe<StringPiece>{base_name};
-  if (!ReplacePlaceholder("${basename}", maybe_base_name, &result, diag)) {
-    return {};
+  // Base name is optional.
+  if (result.find("${basename}") != std::string::npos) {
+    Maybe<StringPiece> maybe_base_name =
+        base_name.empty() ? Maybe<StringPiece>{} : Maybe<StringPiece>{base_name};
+    if (!ReplacePlaceholder("${basename}", maybe_base_name, &result, diag)) {
+      return {};
+    }
   }
 
   // Extension is optional.
   if (result.find("${ext}") != std::string::npos) {
-    if (!ReplacePlaceholder("${ext}", {ext}, &result, diag)) {
+    // Make sure we disregard the '.' in the extension when replacing the placeholder.
+    if (!ReplacePlaceholder("${ext}", {ext.substr(1)}, &result, diag)) {
       return {};
     }
+  } else {
+    // If no extension is specified, and the name template does not end in the current extension,
+    // add the existing extension.
+    if (!util::EndsWith(result, ext)) {
+      result.append(ext.to_string());
+    }
+  }
+
+  return result;
+}
+
+Maybe<std::string> Artifact::ToArtifactName(const StringPiece& format, const StringPiece& apk_name,
+                                            IDiagnostics* diag) const {
+  Maybe<std::string> base = ToBaseName(format.to_string(), apk_name, diag);
+  if (!base) {
+    return {};
   }
+  std::string result = std::move(base.value());
 
   if (!ReplacePlaceholder("${abi}", abi_group, &result, diag)) {
     return {};
@@ -194,29 +220,37 @@ Maybe<std::string> Artifact::ToArtifactName(const StringPiece& format, IDiagnost
   return result;
 }
 
-Maybe<std::string> Artifact::Name(const StringPiece& base_name, const StringPiece& ext,
-                                  IDiagnostics* diag) const {
+Maybe<std::string> Artifact::Name(const StringPiece& apk_name, IDiagnostics* diag) const {
   if (!name) {
     return {};
   }
 
-  std::string result = name.value();
+  return ToBaseName(name.value(), apk_name, diag);
+}
 
-  // Base name is optional.
-  if (result.find("${basename}") != std::string::npos) {
-    if (!ReplacePlaceholder("${basename}", {base_name}, &result, diag)) {
-      return {};
+bool PostProcessingConfiguration::AllArtifactNames(const StringPiece& apk_name,
+                                                   std::vector<std::string>* artifact_names,
+                                                   IDiagnostics* diag) const {
+  for (const auto& artifact : artifacts) {
+    Maybe<std::string> name;
+    if (artifact.name) {
+      name = artifact.Name(apk_name, diag);
+    } else {
+      if (!artifact_format) {
+        diag->Error(DiagMessage() << "No global artifact template and an artifact name is missing");
+        return false;
+      }
+      name = artifact.ToArtifactName(artifact_format.value(), apk_name, diag);
     }
-  }
 
-  // Extension is optional.
-  if (result.find("${ext}") != std::string::npos) {
-    if (!ReplacePlaceholder("${ext}", {ext}, &result, diag)) {
-      return {};
+    if (!name) {
+      return false;
     }
+
+    artifact_names->push_back(std::move(name.value()));
   }
 
-  return result;
+  return true;
 }
 
 }  // namespace configuration
diff --git a/tools/aapt2/configuration/ConfigurationParser.h b/tools/aapt2/configuration/ConfigurationParser.h
index 6259ce8e28ea..a58685e3b8a6 100644
--- a/tools/aapt2/configuration/ConfigurationParser.h
+++ b/tools/aapt2/configuration/ConfigurationParser.h
@@ -51,13 +51,11 @@ struct Artifact {
   Maybe<std::string> gl_texture_group;
 
   /** Convert an artifact name template into a name string based on configuration contents. */
-  Maybe<std::string> ToArtifactName(const android::StringPiece& format, IDiagnostics* diag,
-                                    const android::StringPiece& base_name = "",
-                                    const android::StringPiece& ext = "apk") const;
+  Maybe<std::string> ToArtifactName(const android::StringPiece& format,
+                                    const android::StringPiece& apk_name, IDiagnostics* diag) const;
 
   /** Convert an artifact name template into a name string based on configuration contents. */
-  Maybe<std::string> Name(const android::StringPiece& base_name, const android::StringPiece& ext,
-                          IDiagnostics* diag) const;
+  Maybe<std::string> Name(const android::StringPiece& apk_name, IDiagnostics* diag) const;
 };
 
 /** Enumeration of currently supported ABIs. */
@@ -139,6 +137,10 @@ struct PostProcessingConfiguration {
   Group<AndroidSdk> android_sdk_groups;
   Group<DeviceFeature> device_feature_groups;
   Group<GlTexture> gl_texture_groups;
+
+  /** Helper method that generates a list of artifact names and returns true on success. */
+  bool AllArtifactNames(const android::StringPiece& apk_name,
+                        std::vector<std::string>* artifact_names, IDiagnostics* diag) const;
 };
 
 }  // namespace configuration
diff --git a/tools/aapt2/configuration/ConfigurationParser_test.cpp b/tools/aapt2/configuration/ConfigurationParser_test.cpp
index ece70a9e42ba..d3bfd330fe9d 100644
--- a/tools/aapt2/configuration/ConfigurationParser_test.cpp
+++ b/tools/aapt2/configuration/ConfigurationParser_test.cpp
@@ -414,16 +414,36 @@ TEST(ArtifactTest, Simple) {
   Artifact x86;
   x86.abi_group = {"x86"};
 
-  auto x86_result = x86.ToArtifactName("something.${abi}.apk", &diag);
+  auto x86_result = x86.ToArtifactName("something.${abi}.apk", "", &diag);
   ASSERT_TRUE(x86_result);
   EXPECT_EQ(x86_result.value(), "something.x86.apk");
 
   Artifact arm;
   arm.abi_group = {"armeabi-v7a"};
 
-  auto arm_result = arm.ToArtifactName("app.${abi}.apk", &diag);
-  ASSERT_TRUE(arm_result);
-  EXPECT_EQ(arm_result.value(), "app.armeabi-v7a.apk");
+  {
+    auto arm_result = arm.ToArtifactName("app.${abi}.apk", "", &diag);
+    ASSERT_TRUE(arm_result);
+    EXPECT_EQ(arm_result.value(), "app.armeabi-v7a.apk");
+  }
+
+  {
+    auto arm_result = arm.ToArtifactName("app.${abi}.apk", "different_name.apk", &diag);
+    ASSERT_TRUE(arm_result);
+    EXPECT_EQ(arm_result.value(), "app.armeabi-v7a.apk");
+  }
+
+  {
+    auto arm_result = arm.ToArtifactName("${basename}.${abi}.apk", "app.apk", &diag);
+    ASSERT_TRUE(arm_result);
+    EXPECT_EQ(arm_result.value(), "app.armeabi-v7a.apk");
+  }
+
+  {
+    auto arm_result = arm.ToArtifactName("app.${abi}.${ext}", "app.apk", &diag);
+    ASSERT_TRUE(arm_result);
+    EXPECT_EQ(arm_result.value(), "app.armeabi-v7a.apk");
+  }
 }
 
 TEST(ArtifactTest, Complex) {
@@ -436,10 +456,40 @@ TEST(ArtifactTest, Complex) {
   artifact.locale_group = {"en-AU"};
   artifact.android_sdk_group = {"26"};
 
-  auto result = artifact.ToArtifactName(
-      "app.${density}_${locale}_${feature}_${gl}.sdk${sdk}.${abi}.apk", &diag);
-  ASSERT_TRUE(result);
-  EXPECT_EQ(result.value(), "app.ldpi_en-AU_df1_glx1.sdk26.mips64.apk");
+  {
+    auto result = artifact.ToArtifactName(
+        "app.${density}_${locale}_${feature}_${gl}.sdk${sdk}.${abi}.apk", "", &diag);
+    ASSERT_TRUE(result);
+    EXPECT_EQ(result.value(), "app.ldpi_en-AU_df1_glx1.sdk26.mips64.apk");
+  }
+
+  {
+    auto result = artifact.ToArtifactName(
+        "app.${density}_${locale}_${feature}_${gl}.sdk${sdk}.${abi}.apk", "app.apk", &diag);
+    ASSERT_TRUE(result);
+    EXPECT_EQ(result.value(), "app.ldpi_en-AU_df1_glx1.sdk26.mips64.apk");
+  }
+
+  {
+    auto result = artifact.ToArtifactName(
+        "${basename}.${density}_${locale}_${feature}_${gl}.sdk${sdk}.${abi}.apk", "app.apk", &diag);
+    ASSERT_TRUE(result);
+    EXPECT_EQ(result.value(), "app.ldpi_en-AU_df1_glx1.sdk26.mips64.apk");
+  }
+
+  {
+    auto result = artifact.ToArtifactName(
+        "app.${density}_${locale}_${feature}_${gl}.sdk${sdk}.${abi}.${ext}", "app.apk", &diag);
+    ASSERT_TRUE(result);
+    EXPECT_EQ(result.value(), "app.ldpi_en-AU_df1_glx1.sdk26.mips64.apk");
+  }
+
+  {
+    auto result = artifact.ToArtifactName(
+        "${basename}.${density}_${locale}_${feature}_${gl}.sdk${sdk}.${abi}", "app.apk", &diag);
+    ASSERT_TRUE(result);
+    EXPECT_EQ(result.value(), "app.ldpi_en-AU_df1_glx1.sdk26.mips64.apk");
+  }
 }
 
 TEST(ArtifactTest, Missing) {
@@ -447,16 +497,20 @@ TEST(ArtifactTest, Missing) {
   Artifact x86;
   x86.abi_group = {"x86"};
 
-  EXPECT_FALSE(x86.ToArtifactName("something.${density}.apk", &diag));
-  EXPECT_FALSE(x86.ToArtifactName("something.apk", &diag));
+  EXPECT_FALSE(x86.ToArtifactName("something.${density}.apk", "", &diag));
+  EXPECT_FALSE(x86.ToArtifactName("something.apk", "", &diag));
+  EXPECT_FALSE(x86.ToArtifactName("something.${density}.apk", "something.apk", &diag));
+  EXPECT_FALSE(x86.ToArtifactName("something.apk", "something.apk", &diag));
 }
 
 TEST(ArtifactTest, Empty) {
   StdErrDiagnostics diag;
   Artifact artifact;
 
-  EXPECT_FALSE(artifact.ToArtifactName("something.${density}.apk", &diag));
-  EXPECT_TRUE(artifact.ToArtifactName("something.apk", &diag));
+  EXPECT_FALSE(artifact.ToArtifactName("something.${density}.apk", "", &diag));
+  EXPECT_TRUE(artifact.ToArtifactName("something.apk", "", &diag));
+  EXPECT_FALSE(artifact.ToArtifactName("something.${density}.apk", "something.apk", &diag));
+  EXPECT_TRUE(artifact.ToArtifactName("something.apk", "something.apk", &diag));
 }
 
 TEST(ArtifactTest, Repeated) {
@@ -464,8 +518,9 @@ TEST(ArtifactTest, Repeated) {
   Artifact artifact;
   artifact.screen_density_group = {"mdpi"};
 
-  ASSERT_TRUE(artifact.ToArtifactName("something.${density}.apk", &diag));
-  EXPECT_FALSE(artifact.ToArtifactName("something.${density}.${density}.apk", &diag));
+  ASSERT_TRUE(artifact.ToArtifactName("something.${density}.apk", "", &diag));
+  EXPECT_FALSE(artifact.ToArtifactName("something.${density}.${density}.apk", "", &diag));
+  ASSERT_TRUE(artifact.ToArtifactName("something.${density}.apk", "something.apk", &diag));
 }
 
 TEST(ArtifactTest, Nesting) {
@@ -473,9 +528,9 @@ TEST(ArtifactTest, Nesting) {
   Artifact x86;
   x86.abi_group = {"x86"};
 
-  EXPECT_FALSE(x86.ToArtifactName("something.${abi${density}}.apk", &diag));
+  EXPECT_FALSE(x86.ToArtifactName("something.${abi${density}}.apk", "", &diag));
 
-  const Maybe<std::string>& name = x86.ToArtifactName("something.${abi${abi}}.apk", &diag);
+  const Maybe<std::string>& name = x86.ToArtifactName("something.${abi${abi}}.apk", "", &diag);
   ASSERT_TRUE(name);
   EXPECT_EQ(name.value(), "something.${abix86}.apk");
 }
@@ -486,12 +541,12 @@ TEST(ArtifactTest, Recursive) {
   artifact.device_feature_group = {"${gl}"};
   artifact.gl_texture_group = {"glx1"};
 
-  EXPECT_FALSE(artifact.ToArtifactName("app.${feature}.${gl}.apk", &diag));
+  EXPECT_FALSE(artifact.ToArtifactName("app.${feature}.${gl}.apk", "", &diag));
 
   artifact.device_feature_group = {"df1"};
   artifact.gl_texture_group = {"${feature}"};
   {
-    const auto& result = artifact.ToArtifactName("app.${feature}.${gl}.apk", &diag);
+    const auto& result = artifact.ToArtifactName("app.${feature}.${gl}.apk", "", &diag);
     ASSERT_TRUE(result);
     EXPECT_EQ(result.value(), "app.df1.${feature}.apk");
   }
@@ -501,7 +556,7 @@ TEST(ArtifactTest, Recursive) {
   artifact.device_feature_group = {"${gl}"};
   artifact.gl_texture_group = {"glx1"};
   {
-    const auto& result = artifact.ToArtifactName("app.${feature}.apk", &diag);
+    const auto& result = artifact.ToArtifactName("app.${feature}.apk", "", &diag);
     ASSERT_TRUE(result);
     EXPECT_EQ(result.value(), "app.glx1.apk");
   }
diff --git a/tools/aapt2/optimize/MultiApkGenerator.cpp b/tools/aapt2/optimize/MultiApkGenerator.cpp
index 83162640a5d9..e7a4f8578529 100644
--- a/tools/aapt2/optimize/MultiApkGenerator.cpp
+++ b/tools/aapt2/optimize/MultiApkGenerator.cpp
@@ -44,11 +44,11 @@ bool MultiApkGenerator::FromBaseApk(const std::string& out_dir,
                                     const PostProcessingConfiguration& config,
                                     const TableFlattenerOptions& table_flattener_options) {
   // TODO(safarmer): Handle APK version codes for the generated APKs.
-  // TODO(safarmer): Handle explicit outputs/generating an output file list for other tools.
+  IDiagnostics* diag = context_->GetDiagnostics();
 
-  const std::string& apk_path = file::GetFilename(apk_->GetSource().path).to_string();
-  const StringPiece ext = file::GetExtension(apk_path);
-  const std::string base_name = apk_path.substr(0, apk_path.rfind(ext.to_string()));
+  const std::string& apk_name = file::GetFilename(apk_->GetSource().path).to_string();
+  const StringPiece ext = file::GetExtension(apk_name);
+  const std::string base_name = apk_name.substr(0, apk_name.rfind(ext.to_string()));
 
   // For now, just write out the stripped APK since ABI splitting doesn't modify anything else.
   for (const Artifact& artifact : config.artifacts) {
@@ -57,20 +57,17 @@ bool MultiApkGenerator::FromBaseApk(const std::string& out_dir,
     AxisConfigFilter axis_filter;
 
     if (!artifact.name && !config.artifact_format) {
-      context_->GetDiagnostics()->Error(
+      diag->Error(
           DiagMessage() << "Artifact does not have a name and no global name template defined");
       return false;
     }
 
     Maybe<std::string> artifact_name =
-        (artifact.name)
-            ? artifact.Name(base_name, ext.substr(1), context_->GetDiagnostics())
-            : artifact.ToArtifactName(config.artifact_format.value(), context_->GetDiagnostics(),
-                                      base_name, ext.substr(1));
+        (artifact.name) ? artifact.Name(apk_name, diag)
+                        : artifact.ToArtifactName(config.artifact_format.value(), apk_name, diag);
 
     if (!artifact_name) {
-      context_->GetDiagnostics()->Error(DiagMessage()
-                                        << "Could not determine split APK artifact name");
+      diag->Error(DiagMessage() << "Could not determine split APK artifact name");
       return false;
     }
 
@@ -80,8 +77,7 @@ bool MultiApkGenerator::FromBaseApk(const std::string& out_dir,
       auto group = config.abi_groups.find(group_name);
       // TODO: Remove validation when configuration parser ensures referential integrity.
       if (group == config.abi_groups.end()) {
-        context_->GetDiagnostics()->Error(DiagMessage() << "could not find referenced ABI group '"
-                                                        << group_name << "'");
+        diag->Error(DiagMessage() << "could not find referenced ABI group '" << group_name << "'");
         return false;
       }
       filters.AddFilter(AbiFilter::FromAbiList(group->second));
@@ -93,8 +89,7 @@ bool MultiApkGenerator::FromBaseApk(const std::string& out_dir,
       auto group = config.screen_density_groups.find(group_name);
       // TODO: Remove validation when configuration parser ensures referential integrity.
       if (group == config.screen_density_groups.end()) {
-        context_->GetDiagnostics()->Error(DiagMessage() << "could not find referenced group '"
-                                                        << group_name << "'");
+        diag->Error(DiagMessage() << "could not find referenced group '" << group_name << "'");
         return false;
       }
 
@@ -109,8 +104,7 @@ bool MultiApkGenerator::FromBaseApk(const std::string& out_dir,
       auto group = config.locale_groups.find(group_name);
       // TODO: Remove validation when configuration parser ensures referential integrity.
       if (group == config.locale_groups.end()) {
-        context_->GetDiagnostics()->Error(DiagMessage() << "could not find referenced group '"
-                                                        << group_name << "'");
+        diag->Error(DiagMessage() << "could not find referenced group '" << group_name << "'");
         return false;
       }
 
@@ -132,11 +126,10 @@ bool MultiApkGenerator::FromBaseApk(const std::string& out_dir,
     }
     file::AppendPath(&out, artifact_name.value());
 
-    std::unique_ptr<IArchiveWriter> writer =
-        CreateZipFileArchiveWriter(context_->GetDiagnostics(), out);
+    std::unique_ptr<IArchiveWriter> writer = CreateZipFileArchiveWriter(diag, out);
 
     if (context_->IsVerbose()) {
-      context_->GetDiagnostics()->Note(DiagMessage() << "Writing output: " << out);
+      diag->Note(DiagMessage() << "Writing output: " << out);
     }
 
     if (!apk_->WriteToArchive(context_, table.get(), table_flattener_options, &filters,