diff options
author | 2019-06-12 12:51:57 -0700 | |
---|---|---|
committer | 2019-06-12 16:58:45 -0700 | |
commit | 121c6e8aa037efcbd047bdd5f53e6686a07fa002 (patch) | |
tree | 158c687dcb14b6ee62f549366cbc9ce8e7bbf37a | |
parent | dea0ef9e61155165726b8c63a662f0bdc6039658 (diff) |
[aapt2] Add "link" option to override styles instead of overlaying.
For normal app development, the desired linking semantics are:
* styleables - take union of all definitions
* all other resources - take last non-weak definition
This differs from the semantics needed in other scenarios, where
merging/overlaying styles is desired.
Bug: 134525082
Change-Id: Iac0c43ca2ecf1f3fddc9c3367f8914c12c9258e1
Tested: aapt2_tests
-rw-r--r-- | tools/aapt2/cmd/Link.cpp | 2 | ||||
-rw-r--r-- | tools/aapt2/cmd/Link.h | 5 | ||||
-rw-r--r-- | tools/aapt2/cmd/Link_test.cpp | 84 | ||||
-rw-r--r-- | tools/aapt2/link/TableMerger.cpp | 27 | ||||
-rw-r--r-- | tools/aapt2/link/TableMerger.h | 2 | ||||
-rw-r--r-- | tools/aapt2/link/TableMerger_test.cpp | 47 |
6 files changed, 155 insertions, 12 deletions
diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp index 4b977225fd01..04d12f829e3c 100644 --- a/tools/aapt2/cmd/Link.cpp +++ b/tools/aapt2/cmd/Link.cpp @@ -1691,6 +1691,8 @@ class Linker { TableMergerOptions table_merger_options; table_merger_options.auto_add_overlay = options_.auto_add_overlay; + table_merger_options.override_styles_instead_of_overlaying = + options_.override_styles_instead_of_overlaying; table_merger_options.strict_visibility = options_.strict_visibility; table_merger_ = util::make_unique<TableMerger>(context_, &final_table_, table_merger_options); diff --git a/tools/aapt2/cmd/Link.h b/tools/aapt2/cmd/Link.h index 5b0653ed53bd..37765f6b5d08 100644 --- a/tools/aapt2/cmd/Link.h +++ b/tools/aapt2/cmd/Link.h @@ -42,6 +42,7 @@ struct LinkOptions { std::vector<std::string> assets_dirs; bool output_to_directory = false; bool auto_add_overlay = false; + bool override_styles_instead_of_overlaying = false; OutputFormat output_format = OutputFormat::kApk; // Java/Proguard options. @@ -242,6 +243,10 @@ class LinkCommand : public Command { "Allows the addition of new resources in overlays without\n" "<add-resource> tags.", &options_.auto_add_overlay); + AddOptionalSwitch("--override-styles-instead-of-overlaying", + "Causes styles defined in -R resources to replace previous definitions\n" + "instead of merging into them\n", + &options_.override_styles_instead_of_overlaying); AddOptionalFlag("--rename-manifest-package", "Renames the package in AndroidManifest.xml.", &options_.manifest_fixer_options.rename_manifest_package); AddOptionalFlag("--rename-instrumentation-target-package", diff --git a/tools/aapt2/cmd/Link_test.cpp b/tools/aapt2/cmd/Link_test.cpp index 32ed1dd81b3f..bf8f043f6b68 100644 --- a/tools/aapt2/cmd/Link_test.cpp +++ b/tools/aapt2/cmd/Link_test.cpp @@ -171,4 +171,86 @@ TEST_F(LinkTest, NoCompressResources) { EXPECT_FALSE(file->WasCompressed()); } -} // namespace aapt
\ No newline at end of file +TEST_F(LinkTest, OverlayStyles) { + StdErrDiagnostics diag; + const std::string compiled_files_dir = GetTestPath("compiled"); + const std::string override_files_dir = GetTestPath("compiled-override"); + ASSERT_TRUE(CompileFile(GetTestPath("res/values/values.xml"), + R"(<resources> + <style name="MyStyle"> + <item name="android:textColor">#123</item> + </style> + </resources>)", + compiled_files_dir, &diag)); + ASSERT_TRUE(CompileFile(GetTestPath("res/values/values-override.xml"), + R"(<resources> + <style name="MyStyle"> + <item name="android:background">#456</item> + </style> + </resources>)", + override_files_dir, &diag)); + + + const std::string out_apk = GetTestPath("out.apk"); + std::vector<std::string> link_args = { + "--manifest", GetDefaultManifest(kDefaultPackageName), + "-o", out_apk, + }; + const auto override_files = file::FindFiles(override_files_dir, &diag); + for (const auto &override_file : override_files.value()) { + link_args.push_back("-R"); + link_args.push_back(file::BuildPath({override_files_dir, override_file})); + } + ASSERT_TRUE(Link(link_args, compiled_files_dir, &diag)); + + std::unique_ptr<LoadedApk> apk = LoadedApk::LoadApkFromPath(out_apk, &diag); + const Style* actual_style = test::GetValue<Style>( + apk->GetResourceTable(), std::string(kDefaultPackageName) + ":style/MyStyle"); + ASSERT_NE(actual_style, nullptr); + ASSERT_EQ(actual_style->entries.size(), 2); + EXPECT_EQ(actual_style->entries[0].key.id, 0x01010098); // android:textColor + EXPECT_EQ(actual_style->entries[1].key.id, 0x010100d4); // android:background +} + +TEST_F(LinkTest, OverrideStylesInsteadOfOverlaying) { + StdErrDiagnostics diag; + const std::string compiled_files_dir = GetTestPath("compiled"); + const std::string override_files_dir = GetTestPath("compiled-override"); + ASSERT_TRUE(CompileFile(GetTestPath("res/values/values.xml"), + R"(<resources> + <style name="MyStyle"> + <item name="android:textColor">#123</item> + </style> + </resources>)", + compiled_files_dir, &diag)); + ASSERT_TRUE(CompileFile(GetTestPath("res/values/values-override.xml"), + R"(<resources> + <style name="MyStyle"> + <item name="android:background">#456</item> + </style> + </resources>)", + override_files_dir, &diag)); + + + const std::string out_apk = GetTestPath("out.apk"); + std::vector<std::string> link_args = { + "--manifest", GetDefaultManifest(kDefaultPackageName), + "--override-styles-instead-of-overlaying", + "-o", out_apk, + }; + const auto override_files = file::FindFiles(override_files_dir, &diag); + for (const auto &override_file : override_files.value()) { + link_args.push_back("-R"); + link_args.push_back(file::BuildPath({override_files_dir, override_file})); + } + ASSERT_TRUE(Link(link_args, compiled_files_dir, &diag)); + + std::unique_ptr<LoadedApk> apk = LoadedApk::LoadApkFromPath(out_apk, &diag); + const Style* actual_style = test::GetValue<Style>( + apk->GetResourceTable(), std::string(kDefaultPackageName) + ":style/MyStyle"); + ASSERT_NE(actual_style, nullptr); + ASSERT_EQ(actual_style->entries.size(), 1); + EXPECT_EQ(actual_style->entries[0].key.id, 0x010100d4); // android:background +} + +} // namespace aapt diff --git a/tools/aapt2/link/TableMerger.cpp b/tools/aapt2/link/TableMerger.cpp index c0802e60103a..c25e4503a208 100644 --- a/tools/aapt2/link/TableMerger.cpp +++ b/tools/aapt2/link/TableMerger.cpp @@ -172,19 +172,22 @@ static bool MergeEntry(IAaptContext* context, const Source& src, // // Styleables and Styles don't simply overlay each other, their definitions merge and accumulate. // If both values are Styleables/Styles, we just merge them into the existing value. -static ResourceTable::CollisionResult ResolveMergeCollision(Value* existing, Value* incoming, - StringPool* pool) { +static ResourceTable::CollisionResult ResolveMergeCollision( + bool override_styles_instead_of_overlaying, Value* existing, Value* incoming, + StringPool* pool) { if (Styleable* existing_styleable = ValueCast<Styleable>(existing)) { if (Styleable* incoming_styleable = ValueCast<Styleable>(incoming)) { // Styleables get merged. existing_styleable->MergeWith(incoming_styleable); return ResourceTable::CollisionResult::kKeepOriginal; } - } else if (Style* existing_style = ValueCast<Style>(existing)) { - if (Style* incoming_style = ValueCast<Style>(incoming)) { - // Styles get merged. - existing_style->MergeWith(incoming_style, pool); - return ResourceTable::CollisionResult::kKeepOriginal; + } else if (!override_styles_instead_of_overlaying) { + if (Style* existing_style = ValueCast<Style>(existing)) { + if (Style* incoming_style = ValueCast<Style>(incoming)) { + // Styles get merged. + existing_style->MergeWith(incoming_style, pool); + return ResourceTable::CollisionResult::kKeepOriginal; + } } } // Delegate to the default handler. @@ -194,6 +197,7 @@ static ResourceTable::CollisionResult ResolveMergeCollision(Value* existing, Val static ResourceTable::CollisionResult MergeConfigValue(IAaptContext* context, const ResourceNameRef& res_name, bool overlay, + bool override_styles_instead_of_overlaying, ResourceConfigValue* dst_config_value, ResourceConfigValue* src_config_value, StringPool* pool) { @@ -204,7 +208,8 @@ static ResourceTable::CollisionResult MergeConfigValue(IAaptContext* context, CollisionResult collision_result; if (overlay) { - collision_result = ResolveMergeCollision(dst_value, src_value, pool); + collision_result = + ResolveMergeCollision(override_styles_instead_of_overlaying, dst_value, src_value, pool); } else { collision_result = ResourceTable::ResolveValueCollision(dst_value, src_value); } @@ -272,9 +277,9 @@ bool TableMerger::DoMerge(const Source& src, ResourceTablePackage* src_package, ResourceConfigValue* dst_config_value = dst_entry->FindValue( src_config_value->config, src_config_value->product); if (dst_config_value) { - CollisionResult collision_result = - MergeConfigValue(context_, res_name, overlay, dst_config_value, - src_config_value.get(), &master_table_->string_pool); + CollisionResult collision_result = MergeConfigValue( + context_, res_name, overlay, options_.override_styles_instead_of_overlaying, + dst_config_value, src_config_value.get(), &master_table_->string_pool); if (collision_result == CollisionResult::kConflict) { error = true; continue; diff --git a/tools/aapt2/link/TableMerger.h b/tools/aapt2/link/TableMerger.h index 51305cfcdd25..a35a134a887d 100644 --- a/tools/aapt2/link/TableMerger.h +++ b/tools/aapt2/link/TableMerger.h @@ -37,6 +37,8 @@ struct TableMergerOptions { bool auto_add_overlay = false; // If true, resource overlays with conflicting visibility are not allowed. bool strict_visibility = false; + // If true, styles specified via "aapt2 link -R" completely replace any previously-seen resources. + bool override_styles_instead_of_overlaying = false; }; // TableMerger takes resource tables and merges all packages within the tables that have the same diff --git a/tools/aapt2/link/TableMerger_test.cpp b/tools/aapt2/link/TableMerger_test.cpp index 9dd31e682937..0be4ccf9ae85 100644 --- a/tools/aapt2/link/TableMerger_test.cpp +++ b/tools/aapt2/link/TableMerger_test.cpp @@ -436,6 +436,53 @@ TEST_F(TableMergerTest, OverlaidStyleablesAndStylesShouldBeMerged) { Eq(make_value(Reference(test::ParseNameOrDie("com.app.a:style/OverlayParent"))))); } +TEST_F(TableMergerTest, OverrideStyleInsteadOfOverlaying) { + std::unique_ptr<ResourceTable> table_a = + test::ResourceTableBuilder() + .SetPackageId("com.app.a", 0x7f) + .AddValue( + "com.app.a:styleable/MyWidget", + test::StyleableBuilder().AddItem("com.app.a:attr/foo", ResourceId(0x1234)).Build()) + .AddValue("com.app.a:style/Theme", + test::StyleBuilder() + .AddItem("com.app.a:attr/foo", ResourceUtils::MakeBool(false)) + .Build()) + .Build(); + std::unique_ptr<ResourceTable> table_b = + test::ResourceTableBuilder() + .SetPackageId("com.app.a", 0x7f) + .AddValue( + "com.app.a:styleable/MyWidget", + test::StyleableBuilder().AddItem("com.app.a:attr/bar", ResourceId(0x5678)).Build()) + .AddValue( + "com.app.a:style/Theme", + test::StyleBuilder().AddItem("com.app.a:attr/bat", util::make_unique<Id>()).Build()) + .Build(); + + ResourceTable final_table; + TableMergerOptions options; + options.auto_add_overlay = true; + options.override_styles_instead_of_overlaying = true; + TableMerger merger(context_.get(), &final_table, options); + ASSERT_TRUE(merger.Merge({}, table_a.get(), false /*overlay*/)); + ASSERT_TRUE(merger.Merge({}, table_b.get(), true /*overlay*/)); + + // Styleables are always overlaid + std::unique_ptr<Styleable> expected_styleable = test::StyleableBuilder() + // The merged Styleable has its entries ordered by name. + .AddItem("com.app.a:attr/bar", ResourceId(0x5678)) + .AddItem("com.app.a:attr/foo", ResourceId(0x1234)) + .Build(); + const Styleable* actual_styleable = + test::GetValue<Styleable>(&final_table, "com.app.a:styleable/MyWidget"); + ASSERT_NE(actual_styleable, nullptr); + EXPECT_TRUE(actual_styleable->Equals(expected_styleable.get())); + // Style should be overridden + const Style* actual_style = test::GetValue<Style>(&final_table, "com.app.a:style/Theme"); + ASSERT_NE(actual_style, nullptr); + EXPECT_TRUE(actual_style->Equals(test::GetValue<Style>(table_b.get(), "com.app.a:style/Theme"))); +} + TEST_F(TableMergerTest, SetOverlayable) { auto overlayable = std::make_shared<Overlayable>("CustomizableResources", "overlay://customization"); |