commit a9e316051b85e9c33e45ed815be94e2e9e2704e4
author Ryan Mitchell <rtmitchell@google.com> Thu Jun 28 16:41:38 2018 -0700
committer Ryan Mitchell <rtmitchell@google.com> Tue Jul 31 16:11:49 2018 -0700
tree eb026cae6f0e3ee8e6a93360df238a5069429128
parent 0b015de77e40f9dc55d7add73089e4251fb43f82

AAPT2: Do not overlay comments

Always take the original comment of a resource. Resources passed in
through regular file arguments and resources added using the -R and
--auto-add-overlay flags will keep the same comments when overlayed by
another resource.

This will allow R.java files to remain consistent across devices between
product overlays.

Bug: 70641212
Test: aapt2_tests
Change-Id: I4ed1037a8d42f7e77d85e5c2a8ee78051053831a

tools/aapt2/link/TableMerger.cpp

diff --git a/tools/aapt2/link/TableMerger.cpp b/tools/aapt2/link/TableMerger.cpp
index 91a55b3..afb8ae0 100644
--- a/tools/aapt2/link/TableMerger.cpp
+++ b/tools/aapt2/link/TableMerger.cpp
@@ -281,8 +281,17 @@
           dst_config_value->value = std::move(new_file_ref);
 
         } else {
+          Maybe<std::string> original_comment = (dst_config_value->value)
+              ? dst_config_value->value->GetComment() : Maybe<std::string>();
+
           dst_config_value->value = std::unique_ptr<Value>(
               src_config_value->value->Clone(&master_table_->string_pool));
+
+          // Keep the comment from the original resource and ignore all comments from overlaying
+          // resources
+          if (overlay && original_comment) {
+            dst_config_value->value->SetComment(original_comment.value());
+          }
         }
       }
     }

tools/aapt2/link/TableMerger_test.cpp

diff --git a/tools/aapt2/link/TableMerger_test.cpp b/tools/aapt2/link/TableMerger_test.cpp
index cf504c4..79a734b 100644
--- a/tools/aapt2/link/TableMerger_test.cpp
+++ b/tools/aapt2/link/TableMerger_test.cpp
@@ -178,6 +178,49 @@
               Pointee(Field(&BinaryPrimitive::value, Field(&android::Res_value::data, Eq(0u)))));
 }
 
+TEST_F(TableMergerTest, DoNotOverrideResourceComment) {
+  std::unique_ptr<Value> foo_original = ResourceUtils::TryParseBool("true");
+  foo_original->SetComment(android::StringPiece("Original foo comment"));
+  std::unique_ptr<Value> bar_original = ResourceUtils::TryParseBool("true");
+
+  std::unique_ptr<Value> foo_overlay =  ResourceUtils::TryParseBool("false");
+  foo_overlay->SetComment(android::StringPiece("Overlay foo comment"));
+  std::unique_ptr<Value> bar_overlay =  ResourceUtils::TryParseBool("false");
+  bar_overlay->SetComment(android::StringPiece("Overlay bar comment"));
+  std::unique_ptr<Value> baz_overlay =  ResourceUtils::TryParseBool("false");
+  baz_overlay->SetComment(android::StringPiece("Overlay baz comment"));
+
+  std::unique_ptr<ResourceTable> base =
+      test::ResourceTableBuilder()
+          .SetPackageId("", 0x00)
+          .AddValue("bool/foo", std::move(foo_original))
+          .AddValue("bool/bar", std::move(bar_original))
+          .Build();
+
+  std::unique_ptr<ResourceTable> overlay =
+      test::ResourceTableBuilder()
+          .SetPackageId("", 0x00)
+          .AddValue("bool/foo", std::move(foo_overlay))
+          .AddValue("bool/bar", std::move(bar_overlay))
+          .AddValue("bool/baz", std::move(baz_overlay))
+          .Build();
+
+  ResourceTable final_table;
+  TableMergerOptions options;
+  options.auto_add_overlay = true;
+  TableMerger merger(context_.get(), &final_table, options);
+
+  ASSERT_TRUE(merger.Merge({}, base.get(), false /*overlay*/));
+  ASSERT_TRUE(merger.Merge({}, overlay.get(), true /*overlay*/));
+
+  BinaryPrimitive* foo = test::GetValue<BinaryPrimitive>(&final_table, "com.app.a:bool/foo");
+  EXPECT_THAT(foo, Pointee(Property(&BinaryPrimitive::GetComment, StrEq("Original foo comment"))));
+  BinaryPrimitive* bar = test::GetValue<BinaryPrimitive>(&final_table, "com.app.a:bool/bar");
+  EXPECT_THAT(bar, Pointee(Property(&BinaryPrimitive::GetComment, StrEq(""))));
+  BinaryPrimitive* baz = test::GetValue<BinaryPrimitive>(&final_table, "com.app.a:bool/baz");
+  EXPECT_THAT(baz, Pointee(Property(&BinaryPrimitive::GetComment, StrEq("Overlay baz comment"))));
+}
+
 TEST_F(TableMergerTest, OverrideSameResourceIdsWithOverlay) {
   std::unique_ptr<ResourceTable> base =
       test::ResourceTableBuilder()