From 0a5b201156f1dd01a7da7b7674798ed57cfafc5a Mon Sep 17 00:00:00 2001 From: Shane Farmer Date: Thu, 22 Jun 2017 12:24:12 -0700 Subject: [PATCH] AAPT2: Add a APK filtering. Allow resource files to be removed from the final artifact based on the density and locale configuration in the config file. The APK is split along the density, locale and ABI axis. Each split is generated from the original APK without modifying the original. The new resource table is written back to the file system with unneeded assets etc removed. Test: Unit tests Test: Manually run optimize command against an APK and inspect results Test: Installed split searchlite APK (after resigning) and ran on N6 Change-Id: If73597dcfd88c02d2616518585d0e25a5c6a84d1 --- tools/aapt2/Android.bp | 1 + tools/aapt2/LoadedApk.cpp | 11 +- tools/aapt2/LoadedApk.h | 9 +- tools/aapt2/ResourceTable.cpp | 30 ++++ tools/aapt2/ResourceTable.h | 2 + tools/aapt2/cmd/Optimize.cpp | 68 +++----- .../configuration/ConfigurationParser.cpp | 96 +++++++++--- .../aapt2/configuration/ConfigurationParser.h | 12 +- .../ConfigurationParser_test.cpp | 86 +++++----- tools/aapt2/filter/ConfigFilter.h | 10 +- tools/aapt2/filter/Filter_test.cpp | 8 +- tools/aapt2/optimize/MultiApkGenerator.cpp | 147 ++++++++++++++++++ tools/aapt2/optimize/MultiApkGenerator.h | 53 +++++++ .../aapt2/optimize/MultiApkGenerator_test.cpp | 108 +++++++++++++ tools/aapt2/split/TableSplitter.cpp | 70 +++------ tools/aapt2/test/Builders.cpp | 58 +++++++ tools/aapt2/test/Builders.h | 33 ++++ tools/aapt2/test/Common.h | 2 +- 18 files changed, 618 insertions(+), 186 deletions(-) create mode 100644 tools/aapt2/optimize/MultiApkGenerator.cpp create mode 100644 tools/aapt2/optimize/MultiApkGenerator.h create mode 100644 tools/aapt2/optimize/MultiApkGenerator_test.cpp diff --git a/tools/aapt2/Android.bp b/tools/aapt2/Android.bp index 53794e641b0f3..15fb016038e0d 100644 --- a/tools/aapt2/Android.bp +++ b/tools/aapt2/Android.bp @@ -106,6 +106,7 @@ cc_library_host_static { "link/XmlCompatVersioner.cpp", "link/XmlNamespaceRemover.cpp", "link/XmlReferenceLinker.cpp", + "optimize/MultiApkGenerator.cpp", "optimize/ResourceDeduper.cpp", "optimize/VersionCollapser.cpp", "process/SymbolTable.cpp", diff --git a/tools/aapt2/LoadedApk.cpp b/tools/aapt2/LoadedApk.cpp index abc0e4c1b89d2..b80780e33dfa0 100644 --- a/tools/aapt2/LoadedApk.cpp +++ b/tools/aapt2/LoadedApk.cpp @@ -58,14 +58,15 @@ std::unique_ptr LoadedApk::LoadApkFromPath(IAaptContext* context, bool LoadedApk::WriteToArchive(IAaptContext* context, const TableFlattenerOptions& options, IArchiveWriter* writer) { FilterChain empty; - return WriteToArchive(context, options, &empty, writer); + return WriteToArchive(context, table_.get(), options, &empty, writer); } -bool LoadedApk::WriteToArchive(IAaptContext* context, const TableFlattenerOptions& options, - FilterChain* filters, IArchiveWriter* writer) { +bool LoadedApk::WriteToArchive(IAaptContext* context, ResourceTable* split_table, + const TableFlattenerOptions& options, FilterChain* filters, + IArchiveWriter* writer) { std::set referenced_resources; // List the files being referenced in the resource table. - for (auto& pkg : table_->packages) { + for (auto& pkg : split_table->packages) { for (auto& type : pkg->types) { for (auto& entry : type->entries) { for (auto& config_value : entry->values) { @@ -108,7 +109,7 @@ bool LoadedApk::WriteToArchive(IAaptContext* context, const TableFlattenerOption // TODO(adamlesinski): How to determine if there were sparse entries (and if to encode // with sparse entries) b/35389232. TableFlattener flattener(options, &buffer); - if (!flattener.Consume(context, table_.get())) { + if (!flattener.Consume(context, split_table)) { return false; } diff --git a/tools/aapt2/LoadedApk.h b/tools/aapt2/LoadedApk.h index 8aa9674aa2ed9..dacd0c2130a90 100644 --- a/tools/aapt2/LoadedApk.h +++ b/tools/aapt2/LoadedApk.h @@ -47,16 +47,17 @@ class LoadedApk { * Writes the APK on disk at the given path, while also removing the resource * files that are not referenced in the resource table. */ - bool WriteToArchive(IAaptContext* context, const TableFlattenerOptions& options, - IArchiveWriter* writer); + virtual bool WriteToArchive(IAaptContext* context, const TableFlattenerOptions& options, + IArchiveWriter* writer); /** * Writes the APK on disk at the given path, while also removing the resource * files that are not referenced in the resource table. The provided filter * chain is applied to each entry in the APK file. */ - bool WriteToArchive(IAaptContext* context, const TableFlattenerOptions& options, - FilterChain* filters, IArchiveWriter* writer); + virtual bool WriteToArchive(IAaptContext* context, ResourceTable* split_table, + const TableFlattenerOptions& options, FilterChain* filters, + IArchiveWriter* writer); static std::unique_ptr LoadApkFromPath(IAaptContext* context, const android::StringPiece& path); diff --git a/tools/aapt2/ResourceTable.cpp b/tools/aapt2/ResourceTable.cpp index ab59560d33a3d..0304e21698df1 100644 --- a/tools/aapt2/ResourceTable.cpp +++ b/tools/aapt2/ResourceTable.cpp @@ -546,4 +546,34 @@ Maybe ResourceTable::FindResource(const ResourceNam return SearchResult{package, type, entry}; } +std::unique_ptr ResourceTable::Clone() const { + std::unique_ptr new_table = util::make_unique(); + for (const auto& pkg : packages) { + ResourceTablePackage* new_pkg = new_table->CreatePackage(pkg->name, pkg->id); + for (const auto& type : pkg->types) { + ResourceTableType* new_type = new_pkg->FindOrCreateType(type->type); + if (!new_type->id) { + new_type->id = type->id; + new_type->symbol_status = type->symbol_status; + } + + for (const auto& entry : type->entries) { + ResourceEntry* new_entry = new_type->FindOrCreateEntry(entry->name); + if (!new_entry->id) { + new_entry->id = entry->id; + new_entry->symbol_status = entry->symbol_status; + } + + for (const auto& config_value : entry->values) { + ResourceConfigValue* new_value = + new_entry->FindOrCreateValue(config_value->config, config_value->product); + Value* value = config_value->value->Clone(&new_table->string_pool); + new_value->value = std::unique_ptr(value); + } + } + } + } + return new_table; +} + } // namespace aapt diff --git a/tools/aapt2/ResourceTable.h b/tools/aapt2/ResourceTable.h index 4295d06747740..d5db67e77f511 100644 --- a/tools/aapt2/ResourceTable.h +++ b/tools/aapt2/ResourceTable.h @@ -251,6 +251,8 @@ class ResourceTable { ResourceTablePackage* CreatePackage(const android::StringPiece& name, Maybe id = {}); + std::unique_ptr Clone() const; + /** * The string pool used by this resource table. Values that reference strings * must use diff --git a/tools/aapt2/cmd/Optimize.cpp b/tools/aapt2/cmd/Optimize.cpp index 9d71775889d4b..84b79274b9a6e 100644 --- a/tools/aapt2/cmd/Optimize.cpp +++ b/tools/aapt2/cmd/Optimize.cpp @@ -18,6 +18,7 @@ #include #include "android-base/stringprintf.h" +#include "androidfw/ResourceTypes.h" #include "androidfw/StringPiece.h" #include "Diagnostics.h" @@ -33,15 +34,19 @@ #include "flatten/XmlFlattener.h" #include "io/BigBufferInputStream.h" #include "io/Util.h" +#include "optimize/MultiApkGenerator.h" #include "optimize/ResourceDeduper.h" #include "optimize/VersionCollapser.h" #include "split/TableSplitter.h" #include "util/Files.h" +#include "util/Util.h" using ::aapt::configuration::Abi; using ::aapt::configuration::Artifact; using ::aapt::configuration::PostProcessingConfiguration; +using ::android::ResTable_config; using ::android::StringPiece; +using ::android::base::StringAppendF; using ::android::base::StringPrintf; namespace aapt { @@ -188,42 +193,10 @@ class OptimizeCommand { } if (options_.configuration && options_.output_dir) { - PostProcessingConfiguration& config = options_.configuration.value(); - - // For now, just write out the stripped APK since ABI splitting doesn't modify anything else. - for (const Artifact& artifact : config.artifacts) { - if (artifact.abi_group) { - const std::string& group = artifact.abi_group.value(); - - auto abi_group = config.abi_groups.find(group); - // TODO: Remove validation when configuration parser ensures referential integrity. - if (abi_group == config.abi_groups.end()) { - context_->GetDiagnostics()->Note( - DiagMessage() << "could not find referenced ABI group '" << group << "'"); - return 1; - } - FilterChain filters; - filters.AddFilter(AbiFilter::FromAbiList(abi_group->second)); - - const std::string& path = apk->GetSource().path; - const StringPiece ext = file::GetExtension(path); - const std::string name = path.substr(0, path.rfind(ext.to_string())); - - // Name is hard coded for now since only one split dimension is supported. - // TODO: Incorporate name generation into the configuration objects. - const std::string file_name = - StringPrintf("%s.%s%s", name.c_str(), group.c_str(), ext.data()); - std::string out = options_.output_dir.value(); - file::AppendPath(&out, file_name); - - std::unique_ptr writer = - CreateZipFileArchiveWriter(context_->GetDiagnostics(), out); - - if (!apk->WriteToArchive(context_, options_.table_flattener_options, &filters, - writer.get())) { - return 1; - } - } + MultiApkGenerator generator{apk.get(), context_}; + if (!generator.FromBaseApk(options_.output_dir.value(), options_.configuration.value(), + options_.table_flattener_options)) { + return 1; } } @@ -260,7 +233,7 @@ class OptimizeCommand { for (auto& entry : type->entries) { for (auto& config_value : entry->values) { - FileReference* file_ref = ValueCast(config_value->value.get()); + auto* file_ref = ValueCast(config_value->value.get()); if (file_ref == nullptr) { continue; } @@ -297,11 +270,8 @@ class OptimizeCommand { } io::BigBufferInputStream table_buffer_in(&table_buffer); - if (!io::CopyInputStreamToArchive(context_, &table_buffer_in, "resources.arsc", - ArchiveEntry::kAlign, writer)) { - return false; - } - return true; + return io::CopyInputStreamToArchive(context_, &table_buffer_in, "resources.arsc", + ArchiveEntry::kAlign, writer); } OptimizeOptions options_; @@ -349,6 +319,7 @@ int Optimize(const std::vector& args) { OptimizeOptions options; Maybe config_path; Maybe target_densities; + Maybe target_abis; std::vector configs; std::vector split_args; bool verbose = false; @@ -363,6 +334,12 @@ int Optimize(const std::vector& args) { "All the resources that would be unused on devices of the given densities will be \n" "removed from the APK.", &target_densities) + .OptionalFlag( + "--target-abis", + "Comma separated list of the CPU ABIs that the APK will be optimized for.\n" + "All the native libraries that would be unused on devices of the given ABIs will \n" + "be removed from the APK.", + &target_abis) .OptionalFlagList("-c", "Comma separated list of configurations to include. The default\n" "is all configurations.", @@ -388,7 +365,8 @@ int Optimize(const std::vector& args) { return 1; } - std::unique_ptr apk = LoadedApk::LoadApkFromPath(&context, flags.GetArgs()[0]); + const std::string& apk_path = flags.GetArgs()[0]; + std::unique_ptr apk = LoadedApk::LoadApkFromPath(&context, apk_path); if (!apk) { return 1; } @@ -418,8 +396,8 @@ int Optimize(const std::vector& args) { // Parse the split parameters. for (const std::string& split_arg : split_args) { - options.split_paths.push_back({}); - options.split_constraints.push_back({}); + options.split_paths.emplace_back(); + options.split_constraints.emplace_back(); if (!ParseSplitParameter(split_arg, context.GetDiagnostics(), &options.split_paths.back(), &options.split_constraints.back())) { return 1; diff --git a/tools/aapt2/configuration/ConfigurationParser.cpp b/tools/aapt2/configuration/ConfigurationParser.cpp index b0ed792215df3..a9d6da0258cbf 100644 --- a/tools/aapt2/configuration/ConfigurationParser.cpp +++ b/tools/aapt2/configuration/ConfigurationParser.cpp @@ -59,6 +59,7 @@ using ::aapt::xml::XmlActionExecutor; using ::aapt::xml::XmlActionExecutorPolicy; using ::aapt::xml::XmlNodeAction; using ::android::base::ReadFileToString; +using ::android::StringPiece; const std::unordered_map kStringToAbiMap = { {"armeabi", Abi::kArmeV6}, {"armeabi-v7a", Abi::kArmV7a}, {"arm64-v8a", Abi::kArm64V8a}, @@ -117,9 +118,9 @@ const std::string& AbiToString(Abi abi) { * success, or false if the either the placeholder is not found in the name, or the value is not * present and the placeholder was. */ -static bool ReplacePlaceholder(const std::string& placeholder, const Maybe& value, +static bool ReplacePlaceholder(const StringPiece& placeholder, const Maybe& value, std::string* name, IDiagnostics* diag) { - size_t offset = name->find(placeholder); + size_t offset = name->find(placeholder.data()); bool found = (offset != std::string::npos); // Make sure the placeholder was present if the desired value is present. @@ -139,43 +140,83 @@ static bool ReplacePlaceholder(const std::string& placeholder, const Maybereplace(offset, placeholder.length(), value.value()); + name->replace(offset, placeholder.length(), value.value().data()); // Make sure there was only one instance of the placeholder. - if (name->find(placeholder) != std::string::npos) { + if (name->find(placeholder.data()) != std::string::npos) { diag->Error(DiagMessage() << "Placeholder present multiple times: " << placeholder); return false; } return true; } -Maybe Artifact::ToArtifactName(const std::string& format, IDiagnostics* diag) const { - std::string result = format; +Maybe Artifact::ToArtifactName(const StringPiece& format, IDiagnostics* diag, + const StringPiece& base_name, + const StringPiece& ext) const { + std::string result = format.to_string(); - if (!ReplacePlaceholder("{abi}", abi_group, &result, diag)) { + Maybe maybe_base_name = + base_name.empty() ? Maybe{} : Maybe{base_name}; + if (!ReplacePlaceholder("${basename}", maybe_base_name, &result, diag)) { return {}; } - if (!ReplacePlaceholder("{density}", screen_density_group, &result, diag)) { + // Extension is optional. + if (result.find("${ext}") != std::string::npos) { + if (!ReplacePlaceholder("${ext}", {ext}, &result, diag)) { + return {}; + } + } + + if (!ReplacePlaceholder("${abi}", abi_group, &result, diag)) { return {}; } - if (!ReplacePlaceholder("{locale}", locale_group, &result, diag)) { + if (!ReplacePlaceholder("${density}", screen_density_group, &result, diag)) { return {}; } - if (!ReplacePlaceholder("{sdk}", android_sdk_group, &result, diag)) { + if (!ReplacePlaceholder("${locale}", locale_group, &result, diag)) { return {}; } - if (!ReplacePlaceholder("{feature}", device_feature_group, &result, diag)) { + if (!ReplacePlaceholder("${sdk}", android_sdk_group, &result, diag)) { return {}; } - if (!ReplacePlaceholder("{gl}", gl_texture_group, &result, diag)) { + if (!ReplacePlaceholder("${feature}", device_feature_group, &result, diag)) { return {}; } + if (!ReplacePlaceholder("${gl}", gl_texture_group, &result, diag)) { + return {}; + } + + return result; +} + +Maybe Artifact::Name(const StringPiece& base_name, const StringPiece& ext, + IDiagnostics* diag) const { + if (!name) { + return {}; + } + + std::string result = name.value(); + + // Base name is optional. + if (result.find("${basename}") != std::string::npos) { + if (!ReplacePlaceholder("${basename}", {base_name}, &result, diag)) { + return {}; + } + } + + // Extension is optional. + if (result.find("${ext}") != std::string::npos) { + if (!ReplacePlaceholder("${ext}", {ext}, &result, diag)) { + return {}; + } + } + return result; } @@ -346,7 +387,10 @@ ConfigurationParser::ActionHandler ConfigurationParser::screen_density_group_han if ((t = NodeCast(node.get())) != nullptr) { ConfigDescription config_descriptor; const android::StringPiece& text = TrimWhitespace(t->text); - if (ConfigDescription::Parse(text, &config_descriptor)) { + bool parsed = ConfigDescription::Parse(text, &config_descriptor); + if (parsed && + (config_descriptor.CopyWithoutSdkVersion().diff(ConfigDescription::DefaultConfig()) == + android::ResTable_config::CONFIG_DENSITY)) { // Copy the density with the minimum SDK version stripped out. group.push_back(config_descriptor.CopyWithoutSdkVersion()); } else { @@ -379,17 +423,25 @@ ConfigurationParser::ActionHandler ConfigurationParser::locale_group_handler_ = << child->name); valid = false; } else { - Locale entry; - for (const auto& attr : child->attributes) { - if (attr.name == "lang") { - entry.lang = {attr.value}; - } else if (attr.name == "region") { - entry.region = {attr.value}; - } else { - diag->Warn(DiagMessage() << "Unknown attribute: " << attr.name << " = " << attr.value); + for (auto& node : child->children) { + xml::Text* t; + if ((t = NodeCast(node.get())) != nullptr) { + ConfigDescription config_descriptor; + const android::StringPiece& text = TrimWhitespace(t->text); + bool parsed = ConfigDescription::Parse(text, &config_descriptor); + if (parsed && + (config_descriptor.CopyWithoutSdkVersion().diff(ConfigDescription::DefaultConfig()) == + android::ResTable_config::CONFIG_LOCALE)) { + // Copy the locale with the minimum SDK version stripped out. + group.push_back(config_descriptor.CopyWithoutSdkVersion()); + } else { + diag->Error(DiagMessage() + << "Could not parse config descriptor for screen-density: " << text); + valid = false; + } + break; } } - group.push_back(entry); } } diff --git a/tools/aapt2/configuration/ConfigurationParser.h b/tools/aapt2/configuration/ConfigurationParser.h index 28c355e396433..6259ce8e28ea1 100644 --- a/tools/aapt2/configuration/ConfigurationParser.h +++ b/tools/aapt2/configuration/ConfigurationParser.h @@ -36,7 +36,7 @@ using Group = std::unordered_map>; /** Output artifact configuration options. */ struct Artifact { /** Name to use for output of processing foo.apk -> foo..apk. */ - std::string name; + Maybe name; /** If present, uses the ABI group with this name. */ Maybe abi_group; /** If present, uses the screen density group with this name. */ @@ -51,7 +51,13 @@ struct Artifact { Maybe gl_texture_group; /** Convert an artifact name template into a name string based on configuration contents. */ - Maybe ToArtifactName(const std::string& format, IDiagnostics* diag) const; + Maybe ToArtifactName(const android::StringPiece& format, IDiagnostics* diag, + const android::StringPiece& base_name = "", + const android::StringPiece& ext = "apk") const; + + /** Convert an artifact name template into a name string based on configuration contents. */ + Maybe Name(const android::StringPiece& base_name, const android::StringPiece& ext, + IDiagnostics* diag) const; }; /** Enumeration of currently supported ABIs. */ @@ -129,7 +135,7 @@ struct PostProcessingConfiguration { Group abi_groups; Group screen_density_groups; - Group locale_groups; + Group locale_groups; Group android_sdk_groups; Group device_feature_groups; Group gl_texture_groups; diff --git a/tools/aapt2/configuration/ConfigurationParser_test.cpp b/tools/aapt2/configuration/ConfigurationParser_test.cpp index ab3b7ec2003b2..5bd083120c4d4 100644 --- a/tools/aapt2/configuration/ConfigurationParser_test.cpp +++ b/tools/aapt2/configuration/ConfigurationParser_test.cpp @@ -67,18 +67,15 @@ constexpr const char* kValidConfig = R"( xxxhdpi - - - - + en + es + fr + de - - - - - - + en + es-rMX + fr-rCA - - - - + en + es + fr + de )xml"; auto doc = test::BuildXmlDom(xml); @@ -313,16 +309,12 @@ TEST_F(ConfigurationParserTest, LocaleGroupAction) { ASSERT_EQ(1ul, config.locale_groups.size()); ASSERT_EQ(1u, config.locale_groups.count("europe")); - auto& out = config.locale_groups["europe"]; + const auto& out = config.locale_groups["europe"]; - Locale en; - en.lang = std::string("en"); - Locale es; - es.lang = std::string("es"); - Locale fr; - fr.lang = std::string("fr"); - Locale de; - de.lang = std::string("de"); + ConfigDescription en = test::ParseConfigOrDie("en"); + ConfigDescription es = test::ParseConfigOrDie("es"); + ConfigDescription fr = test::ParseConfigOrDie("fr"); + ConfigDescription de = test::ParseConfigOrDie("de"); ASSERT_THAT(out, ElementsAre(en, es, fr, de)); } @@ -425,14 +417,14 @@ 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); + auto arm_result = arm.ToArtifactName("app.${abi}.apk", &diag); ASSERT_TRUE(arm_result); EXPECT_EQ(arm_result.value(), "app.armeabi-v7a.apk"); } @@ -447,8 +439,8 @@ 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); + 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"); } @@ -458,7 +450,7 @@ TEST(ArtifactTest, Missing) { Artifact x86; x86.abi_group = {"x86"}; - EXPECT_FALSE(x86.ToArtifactName("something.{density}.apk", &diag)); + EXPECT_FALSE(x86.ToArtifactName("something.${density}.apk", &diag)); EXPECT_FALSE(x86.ToArtifactName("something.apk", &diag)); } @@ -466,7 +458,7 @@ TEST(ArtifactTest, Empty) { StdErrDiagnostics diag; Artifact artifact; - EXPECT_FALSE(artifact.ToArtifactName("something.{density}.apk", &diag)); + EXPECT_FALSE(artifact.ToArtifactName("something.${density}.apk", &diag)); EXPECT_TRUE(artifact.ToArtifactName("something.apk", &diag)); } @@ -475,8 +467,8 @@ TEST(ArtifactTest, Repeated) { Artifact artifact; artifact.screen_density_group = {"mdpi"}; - EXPECT_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)); } TEST(ArtifactTest, Nesting) { @@ -484,36 +476,36 @@ 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& name = x86.ToArtifactName("something.{abi{abi}}.apk", &diag); - EXPECT_TRUE(name); - EXPECT_EQ(name.value(), "something.{abix86}.apk"); + const Maybe& name = x86.ToArtifactName("something.${abi${abi}}.apk", &diag); + ASSERT_TRUE(name); + EXPECT_EQ(name.value(), "something.${abix86}.apk"); } TEST(ArtifactTest, Recursive) { StdErrDiagnostics diag; Artifact artifact; - artifact.device_feature_group = {"{gl}"}; + 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}"}; + artifact.gl_texture_group = {"${feature}"}; { - const auto& result = artifact.ToArtifactName("app.{feature}.{gl}.apk", &diag); - EXPECT_TRUE(result); - EXPECT_EQ(result.value(), "app.df1.{feature}.apk"); + const auto& result = artifact.ToArtifactName("app.${feature}.${gl}.apk", &diag); + ASSERT_TRUE(result); + EXPECT_EQ(result.value(), "app.df1.${feature}.apk"); } // This is an invalid case, but should be the only possible case due to the ordering of // replacement. - artifact.device_feature_group = {"{gl}"}; + artifact.device_feature_group = {"${gl}"}; artifact.gl_texture_group = {"glx1"}; { - const auto& result = artifact.ToArtifactName("app.{feature}.apk", &diag); - EXPECT_TRUE(result); + const auto& result = artifact.ToArtifactName("app.${feature}.apk", &diag); + ASSERT_TRUE(result); EXPECT_EQ(result.value(), "app.glx1.apk"); } } diff --git a/tools/aapt2/filter/ConfigFilter.h b/tools/aapt2/filter/ConfigFilter.h index 3f1341684912a..ebb81519dcebd 100644 --- a/tools/aapt2/filter/ConfigFilter.h +++ b/tools/aapt2/filter/ConfigFilter.h @@ -38,13 +38,9 @@ class IConfigFilter { }; /** - * Implements config axis matching. An axis is one component of a configuration, - * like screen - * density or locale. If an axis is specified in the filter, and the axis is - * specified in - * the configuration to match, they must be compatible. Otherwise the - * configuration to match is - * accepted. + * Implements config axis matching. An axis is one component of a configuration, like screen density + * or locale. If an axis is specified in the filter, and the axis is specified in the configuration + * to match, they must be compatible. Otherwise the configuration to match is accepted. * * Used when handling "-c" options. */ diff --git a/tools/aapt2/filter/Filter_test.cpp b/tools/aapt2/filter/Filter_test.cpp index fb75a4b4d7c19..db2e69fc90d38 100644 --- a/tools/aapt2/filter/Filter_test.cpp +++ b/tools/aapt2/filter/Filter_test.cpp @@ -25,22 +25,16 @@ namespace aapt { namespace { -TEST(FilterChainTest, EmptyChain) { +TEST(FilterTest, FilterChain) { FilterChain chain; ASSERT_TRUE(chain.Keep("some/random/path")); -} -TEST(FilterChainTest, SingleFilter) { - FilterChain chain; chain.AddFilter(util::make_unique("keep/")); ASSERT_FALSE(chain.Keep("removed/path")); ASSERT_TRUE(chain.Keep("keep/path/1")); ASSERT_TRUE(chain.Keep("keep/path/2")); -} -TEST(FilterChainTest, MultipleFilters) { - FilterChain chain; chain.AddFilter(util::make_unique("keep/")); chain.AddFilter(util::make_unique("keep/really/")); diff --git a/tools/aapt2/optimize/MultiApkGenerator.cpp b/tools/aapt2/optimize/MultiApkGenerator.cpp new file mode 100644 index 0000000000000..f413ee9602649 --- /dev/null +++ b/tools/aapt2/optimize/MultiApkGenerator.cpp @@ -0,0 +1,147 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "MultiApkGenerator.h" + +#include +#include + +#include "androidfw/StringPiece.h" + +#include "LoadedApk.h" +#include "configuration/ConfigurationParser.h" +#include "filter/AbiFilter.h" +#include "filter/Filter.h" +#include "flatten/Archive.h" +#include "process/IResourceTableConsumer.h" +#include "split/TableSplitter.h" +#include "util/Files.h" + +namespace aapt { + +using ::aapt::configuration::Artifact; +using ::aapt::configuration::PostProcessingConfiguration; +using ::android::StringPiece; + +MultiApkGenerator::MultiApkGenerator(LoadedApk* apk, IAaptContext* context) + : apk_(apk), context_(context) { +} + +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. + + const std::string& apk_path = apk_->GetSource().path; + const StringPiece ext = file::GetExtension(apk_path); + const std::string base_name = apk_path.substr(0, apk_path.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) { + FilterChain filters; + TableSplitterOptions splits; + AxisConfigFilter axis_filter; + + if (!artifact.name && !config.artifact_format) { + context_->GetDiagnostics()->Error( + DiagMessage() << "Artifact does not have a name and no global name template defined"); + return false; + } + + Maybe 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)); + + if (!artifact_name) { + context_->GetDiagnostics()->Error(DiagMessage() + << "Could not determine split APK artifact name"); + return false; + } + + if (artifact.abi_group) { + const std::string& group_name = artifact.abi_group.value(); + + 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 << "'"); + return false; + } + filters.AddFilter(AbiFilter::FromAbiList(group->second)); + } + + if (artifact.screen_density_group) { + const std::string& group_name = artifact.screen_density_group.value(); + + 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 << "'"); + return false; + } + + const std::vector& densities = group->second; + std::for_each(densities.begin(), densities.end(), [&](const ConfigDescription& c) { + splits.preferred_densities.push_back(c.density); + }); + } + + if (artifact.locale_group) { + const std::string& group_name = artifact.locale_group.value(); + 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 << "'"); + return false; + } + + const std::vector& locales = group->second; + std::for_each(locales.begin(), locales.end(), + [&](const ConfigDescription& c) { axis_filter.AddConfig(c); }); + splits.config_filter = &axis_filter; + } + + std::unique_ptr table = apk_->GetResourceTable()->Clone(); + + TableSplitter splitter{{}, splits}; + splitter.SplitTable(table.get()); + + std::string out = out_dir; + file::AppendPath(&out, artifact_name.value()); + + std::unique_ptr writer = + CreateZipFileArchiveWriter(context_->GetDiagnostics(), out); + + if (context_->IsVerbose()) { + context_->GetDiagnostics()->Note(DiagMessage() << "Writing output: " << out); + } + + if (!apk_->WriteToArchive(context_, table.get(), table_flattener_options, &filters, + writer.get())) { + return false; + } + } + + return true; +} + +} // namespace aapt diff --git a/tools/aapt2/optimize/MultiApkGenerator.h b/tools/aapt2/optimize/MultiApkGenerator.h new file mode 100644 index 0000000000000..f325d83053f49 --- /dev/null +++ b/tools/aapt2/optimize/MultiApkGenerator.h @@ -0,0 +1,53 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef AAPT2_APKSPLITTER_H +#define AAPT2_APKSPLITTER_H + +#include "Diagnostics.h" +#include "LoadedApk.h" +#include "configuration/ConfigurationParser.h" + +namespace aapt { + +/** + * Generates a set of APKs that are a subset of the original base APKs. Each of the new APKs contain + * only the resources and assets for an artifact in the configuration file. + */ +class MultiApkGenerator { + public: + MultiApkGenerator(LoadedApk* apk, IAaptContext* context); + + /** + * Writes a set of APKs to the provided output directory. Each APK is a subset fo the base APK and + * represents an artifact in the post processing configuration. + */ + bool FromBaseApk(const std::string& out_dir, + const configuration::PostProcessingConfiguration& config, + const TableFlattenerOptions& table_flattener_options); + + private: + IDiagnostics* GetDiagnostics() { + return context_->GetDiagnostics(); + } + + LoadedApk* apk_; + IAaptContext* context_; +}; + +} // namespace aapt + +#endif // AAPT2_APKSPLITTER_H diff --git a/tools/aapt2/optimize/MultiApkGenerator_test.cpp b/tools/aapt2/optimize/MultiApkGenerator_test.cpp new file mode 100644 index 0000000000000..6c928d94e8791 --- /dev/null +++ b/tools/aapt2/optimize/MultiApkGenerator_test.cpp @@ -0,0 +1,108 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "optimize/MultiApkGenerator.h" + +#include + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +#include "LoadedApk.h" +#include "ResourceTable.h" +#include "configuration/ConfigurationParser.h" +#include "filter/Filter.h" +#include "flatten/Archive.h" +#include "flatten/TableFlattener.h" +#include "process/IResourceTableConsumer.h" +#include "test/Context.h" +#include "test/Test.h" + +namespace aapt { +namespace { + +using ::aapt::configuration::Abi; +using ::aapt::configuration::Artifact; +using ::aapt::configuration::PostProcessingConfiguration; + +using ::testing::Eq; +using ::testing::Return; +using ::testing::_; + +class MockApk : public LoadedApk { + public: + MockApk(std::unique_ptr table) : LoadedApk({"test.apk"}, {}, std::move(table)){}; + MOCK_METHOD5(WriteToArchive, bool(IAaptContext*, ResourceTable*, const TableFlattenerOptions&, + FilterChain*, IArchiveWriter*)); +}; + +TEST(MultiApkGeneratorTest, FromBaseApk) { + std::unique_ptr table = + test::ResourceTableBuilder() + .AddFileReference("android:drawable/icon", "res/drawable-mdpi/icon.png", + test::ParseConfigOrDie("mdpi")) + .AddFileReference("android:drawable/icon", "res/drawable-hdpi/icon.png", + test::ParseConfigOrDie("hdpi")) + .AddFileReference("android:drawable/icon", "res/drawable-xhdpi/icon.png", + test::ParseConfigOrDie("xhdpi")) + .AddFileReference("android:drawable/icon", "res/drawable-xxhdpi/icon.png", + test::ParseConfigOrDie("xxhdpi")) + .AddSimple("android:string/one") + .Build(); + + MockApk apk{std::move(table)}; + + EXPECT_CALL(apk, WriteToArchive(_, _, _, _, _)).Times(0); + + test::Context ctx; + PostProcessingConfiguration empty_config; + TableFlattenerOptions table_flattener_options; + + MultiApkGenerator generator{&apk, &ctx}; + EXPECT_TRUE(generator.FromBaseApk("out", empty_config, table_flattener_options)); + + Artifact x64 = test::ArtifactBuilder() + .SetName("${basename}.x64.apk") + .SetAbiGroup("x64") + .SetLocaleGroup("en") + .SetDensityGroup("xhdpi") + .Build(); + + Artifact intel = test::ArtifactBuilder() + .SetName("${basename}.intel.apk") + .SetAbiGroup("intel") + .SetLocaleGroup("europe") + .SetDensityGroup("large") + .Build(); + + auto config = test::PostProcessingConfigurationBuilder() + .SetLocaleGroup("en", {"en"}) + .SetLocaleGroup("europe", {"en", "fr", "de", "es"}) + .SetAbiGroup("x64", {Abi::kX86_64}) + .SetAbiGroup("intel", {Abi::kX86_64, Abi::kX86}) + .SetDensityGroup("xhdpi", {"xhdpi"}) + .SetDensityGroup("large", {"xhdpi", "xxhdpi", "xxxhdpi"}) + .AddArtifact(x64) + .AddArtifact(intel) + .Build(); + + // Called once for each artifact. + EXPECT_CALL(apk, WriteToArchive(Eq(&ctx), _, _, _, _)).Times(2).WillRepeatedly(Return(true)); + EXPECT_TRUE(generator.FromBaseApk("out", config, table_flattener_options)); +} + +} // namespace +} // namespace aapt diff --git a/tools/aapt2/split/TableSplitter.cpp b/tools/aapt2/split/TableSplitter.cpp index 27e13d81ff495..9d49ca6c0aa96 100644 --- a/tools/aapt2/split/TableSplitter.cpp +++ b/tools/aapt2/split/TableSplitter.cpp @@ -32,8 +32,7 @@ namespace aapt { using ConfigClaimedMap = std::unordered_map; -using ConfigDensityGroups = - std::map>; +using ConfigDensityGroups = std::map>; static ConfigDescription CopyWithoutDensity(const ConfigDescription& config) { ConfigDescription without_density = config; @@ -51,8 +50,7 @@ class SplitValueSelector { if (config.density == 0) { density_independent_configs_.insert(config); } else { - density_dependent_config_to_density_map_[CopyWithoutDensity(config)] = - config.density; + density_dependent_config_to_density_map_[CopyWithoutDensity(config)] = config.density; } } } @@ -94,9 +92,7 @@ class SplitValueSelector { ResourceConfigValue* best_value = nullptr; for (ResourceConfigValue* this_value : related_values) { - if (!best_value || - this_value->config.isBetterThan(best_value->config, - &target_density)) { + if (!best_value || this_value->config.isBetterThan(best_value->config, &target_density)) { best_value = this_value; } } @@ -120,9 +116,8 @@ class SplitValueSelector { }; /** - * Marking non-preferred densities as claimed will make sure the base doesn't - * include them, - * leaving only the preferred density behind. + * Marking non-preferred densities as claimed will make sure the base doesn't include them, leaving + * only the preferred density behind. */ static void MarkNonPreferredDensitiesAsClaimed( const std::vector& preferred_densities, const ConfigDensityGroups& density_groups, @@ -161,8 +156,7 @@ bool TableSplitter::VerifySplitConstraints(IAaptContext* context) { for (size_t i = 0; i < split_constraints_.size(); i++) { for (size_t j = i + 1; j < split_constraints_.size(); j++) { for (const ConfigDescription& config : split_constraints_[i].configs) { - if (split_constraints_[j].configs.find(config) != - split_constraints_[j].configs.end()) { + if (split_constraints_[j].configs.find(config) != split_constraints_[j].configs.end()) { context->GetDiagnostics()->Error(DiagMessage() << "config '" << config << "' appears in multiple splits, " @@ -193,28 +187,22 @@ void TableSplitter::SplitTable(ResourceTable* original_table) { for (auto& entry : type->entries) { if (options_.config_filter) { // First eliminate any resource that we definitely don't want. - for (std::unique_ptr& config_value : - entry->values) { + for (std::unique_ptr& config_value : entry->values) { if (!options_.config_filter->Match(config_value->config)) { - // null out the entry. We will clean up and remove nulls at the - // end for performance reasons. + // null out the entry. We will clean up and remove nulls at the end for performance + // reasons. config_value.reset(); } } } - // Organize the values into two separate buckets. Those that are - // density-dependent - // and those that are density-independent. - // One density technically matches all density, it's just that some - // densities - // match better. So we need to be aware of the full set of densities to - // make this - // decision. + // Organize the values into two separate buckets. Those that are density-dependent and those + // that are density-independent. One density technically matches all density, it's just that + // some densities match better. So we need to be aware of the full set of densities to make + // this decision. ConfigDensityGroups density_groups; ConfigClaimedMap config_claimed_map; - for (const std::unique_ptr& config_value : - entry->values) { + for (const std::unique_ptr& config_value : entry->values) { if (config_value) { config_claimed_map[config_value.get()] = false; @@ -226,9 +214,8 @@ void TableSplitter::SplitTable(ResourceTable* original_table) { } } - // First we check all the splits. If it doesn't match one of the splits, - // we - // leave it in the base. + // First we check all the splits. If it doesn't match one of the splits, we leave it in the + // base. for (size_t idx = 0; idx < split_count; idx++) { const SplitConstraints& split_constraint = split_constraints_[idx]; ResourceTable* split_table = splits_[idx].get(); @@ -240,20 +227,16 @@ void TableSplitter::SplitTable(ResourceTable* original_table) { // No need to do any work if we selected nothing. if (!selected_values.empty()) { - // Create the same resource structure in the split. We do this - // lazily because we might not have actual values for each - // type/entry. - ResourceTablePackage* split_pkg = - split_table->FindPackage(pkg->name); - ResourceTableType* split_type = - split_pkg->FindOrCreateType(type->type); + // Create the same resource structure in the split. We do this lazily because we might + // not have actual values for each type/entry. + ResourceTablePackage* split_pkg = split_table->FindPackage(pkg->name); + ResourceTableType* split_type = split_pkg->FindOrCreateType(type->type); if (!split_type->id) { split_type->id = type->id; split_type->symbol_status = type->symbol_status; } - ResourceEntry* split_entry = - split_type->FindOrCreateEntry(entry->name); + ResourceEntry* split_entry = split_type->FindOrCreateEntry(entry->name); if (!split_entry->id) { split_entry->id = entry->id; split_entry->symbol_status = entry->symbol_status; @@ -262,8 +245,7 @@ void TableSplitter::SplitTable(ResourceTable* original_table) { // Copy the selected values into the new Split Entry. for (ResourceConfigValue* config_value : selected_values) { ResourceConfigValue* new_config_value = - split_entry->FindOrCreateValue(config_value->config, - config_value->product); + split_entry->FindOrCreateValue(config_value->config, config_value->product); new_config_value->value = std::unique_ptr( config_value->value->Clone(&split_table->string_pool)); } @@ -276,11 +258,9 @@ void TableSplitter::SplitTable(ResourceTable* original_table) { &config_claimed_map); } - // All splits are handled, now check to see what wasn't claimed and - // remove - // whatever exists in other splits. - for (std::unique_ptr& config_value : - entry->values) { + // All splits are handled, now check to see what wasn't claimed and remove whatever exists + // in other splits. + for (std::unique_ptr& config_value : entry->values) { if (config_value && config_claimed_map[config_value.get()]) { // Claimed, remove from base. config_value.reset(); diff --git a/tools/aapt2/test/Builders.cpp b/tools/aapt2/test/Builders.cpp index b5795451e7499..80e6adf134e6f 100644 --- a/tools/aapt2/test/Builders.cpp +++ b/tools/aapt2/test/Builders.cpp @@ -212,5 +212,63 @@ std::unique_ptr BuildXmlDomForPackageName(IAaptContext* contex return doc; } +PostProcessingConfigurationBuilder& PostProcessingConfigurationBuilder::SetAbiGroup( + const std::string& name, const std::vector& abis) { + config_.abi_groups[name] = abis; + return *this; +} + +PostProcessingConfigurationBuilder& PostProcessingConfigurationBuilder::SetLocaleGroup( + const std::string& name, const std::vector& locales) { + auto& group = config_.locale_groups[name]; + for (const auto& locale : locales) { + group.push_back(ParseConfigOrDie(locale)); + } + return *this; +} + +PostProcessingConfigurationBuilder& PostProcessingConfigurationBuilder::SetDensityGroup( + const std::string& name, const std::vector& densities) { + auto& group = config_.screen_density_groups[name]; + for (const auto& density : densities) { + group.push_back(ParseConfigOrDie(density)); + } + return *this; +} + +PostProcessingConfigurationBuilder& PostProcessingConfigurationBuilder::AddArtifact( + const configuration::Artifact& artifact) { + config_.artifacts.push_back(artifact); + return *this; +} + +configuration::PostProcessingConfiguration PostProcessingConfigurationBuilder::Build() { + return config_; +} + +ArtifactBuilder& ArtifactBuilder::SetName(const std::string& name) { + artifact_.name = {name}; + return *this; +} + +ArtifactBuilder& ArtifactBuilder::SetAbiGroup(const std::string& name) { + artifact_.abi_group = {name}; + return *this; +} + +ArtifactBuilder& ArtifactBuilder::SetDensityGroup(const std::string& name) { + artifact_.screen_density_group = {name}; + return *this; +} + +ArtifactBuilder& ArtifactBuilder::SetLocaleGroup(const std::string& name) { + artifact_.locale_group = {name}; + return *this; +} + +configuration::Artifact ArtifactBuilder::Build() { + return artifact_; +} + } // namespace test } // namespace aapt diff --git a/tools/aapt2/test/Builders.h b/tools/aapt2/test/Builders.h index d9f3912fb4c66..e8cefc1853b67 100644 --- a/tools/aapt2/test/Builders.h +++ b/tools/aapt2/test/Builders.h @@ -24,7 +24,9 @@ #include "Resource.h" #include "ResourceTable.h" #include "ResourceValues.h" +#include "configuration/ConfigurationParser.h" #include "process/IResourceTableConsumer.h" +#include "test/Common.h" #include "util/Maybe.h" #include "xml/XmlDom.h" @@ -149,6 +151,37 @@ std::unique_ptr BuildXmlDom(const android::StringPiece& str); std::unique_ptr BuildXmlDomForPackageName(IAaptContext* context, const android::StringPiece& str); +class PostProcessingConfigurationBuilder { + public: + PostProcessingConfigurationBuilder() = default; + + PostProcessingConfigurationBuilder& SetAbiGroup(const std::string& name, + const std::vector& abis); + PostProcessingConfigurationBuilder& SetLocaleGroup(const std::string& name, + const std::vector& locales); + PostProcessingConfigurationBuilder& SetDensityGroup(const std::string& name, + const std::vector& densities); + PostProcessingConfigurationBuilder& AddArtifact(const configuration::Artifact& artifact); + configuration::PostProcessingConfiguration Build(); + + private: + configuration::PostProcessingConfiguration config_; +}; + +class ArtifactBuilder { + public: + ArtifactBuilder() = default; + + ArtifactBuilder& SetName(const std::string& name); + ArtifactBuilder& SetAbiGroup(const std::string& name); + ArtifactBuilder& SetDensityGroup(const std::string& name); + ArtifactBuilder& SetLocaleGroup(const std::string& name); + configuration::Artifact Build(); + + private: + configuration::Artifact artifact_; +}; + } // namespace test } // namespace aapt diff --git a/tools/aapt2/test/Common.h b/tools/aapt2/test/Common.h index d7b46caf8c941..d53c92f6a7b9d 100644 --- a/tools/aapt2/test/Common.h +++ b/tools/aapt2/test/Common.h @@ -41,7 +41,7 @@ IDiagnostics* GetDiagnostics(); inline ResourceName ParseNameOrDie(const android::StringPiece& str) { ResourceNameRef ref; - CHECK(ResourceUtils::ParseResourceName(str, &ref)) << "invalid resource name"; + CHECK(ResourceUtils::ParseResourceName(str, &ref)) << "invalid resource name: " << str; return ref.ToResourceName(); }