Merge "De-duplicate entries written with AAPT2 convert"

4 files changed, 54 insertions, 5 deletions

diff --git a/tools/aapt2/Android.bp b/tools/aapt2/Android.bp
index 7783e108f674..8f752871355f 100644
--- a/tools/aapt2/Android.bp
+++ b/tools/aapt2/Android.bp
@@ -182,7 +182,8 @@ cc_test_host {
     defaults: ["aapt2_defaults"],
     data: [
          "integration-tests/CompileTest/**/*",
-         "integration-tests/CommandTests/**/*"
+         "integration-tests/CommandTests/**/*",
+         "integration-tests/ConvertTest/**/*"
     ],
 }
 
diff --git a/tools/aapt2/cmd/Convert.cpp b/tools/aapt2/cmd/Convert.cpp
index 85f90806752f..7a74ba925ba0 100644
--- a/tools/aapt2/cmd/Convert.cpp
+++ b/tools/aapt2/cmd/Convert.cpp
@@ -284,6 +284,8 @@ int Convert(IAaptContext* context, LoadedApk* apk, IArchiveWriter* output_writer
     // The table might be modified by below code.
     auto converted_table = apk->GetResourceTable();
 
+    std::unordered_set<std::string> files_written;
+
     // Resources
     for (const auto& package : converted_table->packages) {
       for (const auto& type : package->types) {
@@ -297,10 +299,14 @@ int Convert(IAaptContext* context, LoadedApk* apk, IArchiveWriter* output_writer
                 return 1;
               }
 
-              if (!serializer->SerializeFile(file, output_writer)) {
-                context->GetDiagnostics()->Error(DiagMessage(apk->GetSource())
-                                                 << "failed to serialize file " << *file->path);
-                return 1;
+              // Only serialize if we haven't seen this file before
+              if (files_written.insert(*file->path).second) {
+                if (!serializer->SerializeFile(file, output_writer)) {
+                  context->GetDiagnostics()->Error(DiagMessage(apk->GetSource())
+                                                       << "failed to serialize file "
+                                                       << *file->path);
+                  return 1;
+                }
               }
             } // file
           } // config_value
diff --git a/tools/aapt2/cmd/Convert_test.cpp b/tools/aapt2/cmd/Convert_test.cpp
index 8da5bb8d5dd6..3c0fe370c516 100644
--- a/tools/aapt2/cmd/Convert_test.cpp
+++ b/tools/aapt2/cmd/Convert_test.cpp
@@ -18,6 +18,7 @@
 
 #include "LoadedApk.h"
 #include "test/Test.h"
+#include "ziparchive/zip_archive.h"
 
 using testing::Eq;
 using testing::Ne;
@@ -103,4 +104,45 @@ TEST_F(ConvertTest, KeepRawXmlStrings) {
   EXPECT_THAT(util::GetString(tree.getStrings(), static_cast<size_t>(raw_index)), Eq("007"));
 }
 
+TEST_F(ConvertTest, DuplicateEntriesWrittenOnce) {
+  StdErrDiagnostics diag;
+  const std::string apk_path =
+      file::BuildPath({android::base::GetExecutableDirectory(),
+                       "integration-tests", "ConvertTest", "duplicate_entries.apk"});
+
+  const std::string out_convert_apk = GetTestPath("out_convert.apk");
+  std::vector<android::StringPiece> convert_args = {
+      "-o", out_convert_apk,
+      "--output-format", "proto",
+      apk_path
+  };
+  ASSERT_THAT(ConvertCommand().Execute(convert_args, &std::cerr), Eq(0));
+
+  ZipArchiveHandle handle;
+  ASSERT_THAT(OpenArchive(out_convert_apk.c_str(), &handle), Eq(0));
+
+  void* cookie = nullptr;
+
+  ZipString prefix("res/theme/10");
+  int32_t result = StartIteration(handle, &cookie, &prefix, nullptr);
+
+  // If this is -5, that means we've found a duplicate entry and this test has failed
+  EXPECT_THAT(result, Eq(0));
+
+  // But if read succeeds, verify only one res/theme/10 entry
+  int count = 0;
+
+  // Can't pass nullptrs into Next()
+  ZipString zip_name;
+  ZipEntry zip_data;
+
+  while ((result = Next(cookie, &zip_data, &zip_name)) == 0) {
+    count++;
+  }
+
+  EndIteration(cookie);
+
+  EXPECT_THAT(count, Eq(1));
+}
+
 }  // namespace aapt
\ No newline at end of file
diff --git a/tools/aapt2/integration-tests/ConvertTest/duplicate_entries.apk b/tools/aapt2/integration-tests/ConvertTest/duplicate_entries.apk
new file mode 100644
index 000000000000..c558a334b369
--- /dev/null
+++ b/tools/aapt2/integration-tests/ConvertTest/duplicate_entries.apk