AAPT2: Disambiguate merging of resources

Merging local app resources is slightly different than merging
resources from a static library.

Local app resources may not have a package name set, but we do take interest in the
ID set for the package (should be 0x0 or match the ID of the app we're building).

Static library resources have an explicit package name defined for them, so we
only merge resources from that package.

Change-Id: I95e559ae94cc1df6972e77a347b1b37a93674c4d

5 files changed, 64 insertions, 24 deletions

diff --git a/tools/aapt2/compile/Compile.cpp b/tools/aapt2/compile/Compile.cpp
index 5d90ab9f6bad..39088bcee140 100644
--- a/tools/aapt2/compile/Compile.cpp
+++ b/tools/aapt2/compile/Compile.cpp
@@ -148,10 +148,14 @@ static bool compileTable(IAaptContext* context, const CompileOptions& options,
         fin.close();
     }
 
-    ResourceTablePackage* pkg = table.createPackage(context->getCompilationPackage());
-    if (!pkg->id) {
-        // If no package ID was set while parsing (public identifiers), auto assign an ID.
-        pkg->id = context->getPackageId();
+    // Ensure we have the compilation package at least.
+    table.createPackage(context->getCompilationPackage());
+
+    for (auto& pkg : table.packages) {
+        if (!pkg->id) {
+            // If no package ID was set while parsing (public identifiers), auto assign an ID.
+            pkg->id = context->getPackageId();
+        }
     }
 
     // Assign IDs to prepare the table for flattening.
diff --git a/tools/aapt2/link/Link.cpp b/tools/aapt2/link/Link.cpp
index 0236e988162a..9ce3734f1914 100644
--- a/tools/aapt2/link/Link.cpp
+++ b/tools/aapt2/link/Link.cpp
@@ -49,6 +49,7 @@ struct LinkOptions {
     std::string manifestPath;
     std::vector<std::string> includePaths;
     Maybe<std::string> generateJavaClassPath;
+    std::vector<std::string> extraJavaPackages;
     Maybe<std::string> generateProguardRulesPath;
     bool noAutoVersion = false;
     bool staticLib = false;
@@ -695,6 +696,9 @@ struct LinkCommand {
                 options.useFinal = false;
             }
 
+            StringPiece16 actualPackage = mContext.getCompilationPackage();
+            StringPiece16 outputPackage = mContext.getCompilationPackage();
+
             if (mOptions.privateSymbols) {
                 // If we defined a private symbols package, we only emit Public symbols
                 // to the original package, and private and public symbols to the private package.
@@ -706,16 +710,16 @@ struct LinkCommand {
                 }
 
                 options.types = JavaClassGeneratorOptions::SymbolTypes::kPublicPrivate;
-                if (!writeJavaFile(&mergedTable, mContext.getCompilationPackage(),
-                                   mOptions.privateSymbols.value(), options)) {
-                    return 1;
-                }
+                outputPackage = mOptions.privateSymbols.value();
+            }
 
-            } else {
-                // Emit Everything.
+            if (!writeJavaFile(&mergedTable, actualPackage, outputPackage, options)) {
+                return 1;
+            }
 
-                if (!writeJavaFile(&mergedTable, mContext.getCompilationPackage(),
-                                   mContext.getCompilationPackage(), options)) {
+            for (std::string& extraPackage : mOptions.extraJavaPackages) {
+                if (!writeJavaFile(&mergedTable, actualPackage, util::utf8ToUtf16(extraPackage),
+                                   options)) {
                     return 1;
                 }
             }
@@ -770,6 +774,8 @@ int link(const std::vector<StringPiece>& args) {
                           "private symbols.\n"
                           "If not specified, public and private symbols will use the application's "
                           "package name", &privateSymbolsPackage)
+            .optionalFlagList("--extra-packages", "Generate the same R.java but with different "
+                              "package names", &options.extraJavaPackages)
             .optionalSwitch("-v", "Enables verbose logging", &options.verbose);
 
     if (!flags.parse("aapt2 link", args, &std::cerr)) {
diff --git a/tools/aapt2/link/TableMerger.cpp b/tools/aapt2/link/TableMerger.cpp
index 0bcb5baebf8d..1eea410cf2a6 100644
--- a/tools/aapt2/link/TableMerger.cpp
+++ b/tools/aapt2/link/TableMerger.cpp
@@ -34,6 +34,9 @@ TableMerger::TableMerger(IAaptContext* context, ResourceTable* outTable) :
     assert(mMasterPackage && "package name or ID already taken");
 }
 
+/**
+ * This will merge packages with the same package name (or no package name).
+ */
 bool TableMerger::merge(const Source& src, ResourceTable* table) {
     const uint8_t desiredPackageId = mContext->getPackageId();
 
@@ -46,18 +49,37 @@ bool TableMerger::merge(const Source& src, ResourceTable* table) {
             continue;
         }
 
-        bool manglePackage = false;
-        if (!package->name.empty() && mContext->getCompilationPackage() != package->name) {
-            manglePackage = true;
-            mMergedPackages.insert(package->name);
+        if (package->name.empty() || mContext->getCompilationPackage() == package->name) {
+            // Merge here. Once the entries are merged and mangled, any references to
+            // them are still valid. This is because un-mangled references are
+            // mangled, then looked up at resolution time.
+            // Also, when linking, we convert references with no package name to use
+            // the compilation package name.
+            if (!doMerge(src, table, package.get(), false)) {
+                error = true;
+            }
+        }
+    }
+    return !error;
+}
+
+/**
+ * This will merge and mangle resources from a static library.
+ */
+bool TableMerger::mergeAndMangle(const Source& src, const StringPiece16& packageName,
+                                 ResourceTable* table) {
+    bool error = false;
+    for (auto& package : table->packages) {
+        // Warn of packages with an unrelated ID.
+        if (packageName != package->name) {
+            mContext->getDiagnostics()->warn(DiagMessage(src)
+                                             << "ignoring package " << package->name);
+            continue;
         }
 
-        // Merge here. Once the entries are merged and mangled, any references to
-        // them are still valid. This is because un-mangled references are
-        // mangled, then looked up at resolution time.
-        // Also, when linking, we convert references with no package name to use
-        // the compilation package name.
-        if (!doMerge(src, table, package.get(), manglePackage)) {
+        bool mangle = packageName != mContext->getCompilationPackage();
+        mMergedPackages.insert(package->name);
+        if (!doMerge(src, table, package.get(), mangle)) {
             error = true;
         }
     }
diff --git a/tools/aapt2/link/TableMerger.h b/tools/aapt2/link/TableMerger.h
index 157c16ee7c98..c903f1bf218e 100644
--- a/tools/aapt2/link/TableMerger.h
+++ b/tools/aapt2/link/TableMerger.h
@@ -60,8 +60,16 @@ public:
         return mMergedPackages;
     }
 
+    /**
+     * Merges resources from the same or empty package. This is for local sources.
+     */
     bool merge(const Source& src, ResourceTable* table);
 
+    /**
+     * Merges resources from the given package, mangling the name. This is for static libraries.
+     */
+    bool mergeAndMangle(const Source& src, const StringPiece16& package, ResourceTable* table);
+
 private:
     IAaptContext* mContext;
     ResourceTable* mMasterTable;
diff --git a/tools/aapt2/link/TableMerger_test.cpp b/tools/aapt2/link/TableMerger_test.cpp
index fa7ce8627cf5..0af4314a762e 100644
--- a/tools/aapt2/link/TableMerger_test.cpp
+++ b/tools/aapt2/link/TableMerger_test.cpp
@@ -60,7 +60,7 @@ TEST_F(TableMergerTest, SimpleMerge) {
     TableMerger merger(mContext.get(), &finalTable);
 
     ASSERT_TRUE(merger.merge({}, tableA.get()));
-    ASSERT_TRUE(merger.merge({}, tableB.get()));
+    ASSERT_TRUE(merger.mergeAndMangle({}, u"com.app.b", tableB.get()));
 
     EXPECT_TRUE(merger.getMergedPackages().count(u"com.app.b") != 0);
 
@@ -90,7 +90,7 @@ TEST_F(TableMergerTest, MergeFileReferences) {
     TableMerger merger(mContext.get(), &finalTable);
 
     ASSERT_TRUE(merger.merge({}, tableA.get()));
-    ASSERT_TRUE(merger.merge({}, tableB.get()));
+    ASSERT_TRUE(merger.mergeAndMangle({}, u"com.app.b", tableB.get()));
 
     FileReference* f = test::getValue<FileReference>(&finalTable, u"@com.app.a:xml/file");
     ASSERT_NE(f, nullptr);