From 054e43317ea0998137453d4d81ad71873a11e1ae Mon Sep 17 00:00:00 2001 From: Iurii Makhno Date: Wed, 12 Oct 2022 16:03:03 +0000 Subject: [PATCH] Expose flags for collapse resource name to 'convert' command. To achieve this ParseResourceConfig is extracted to Utils.cpp and tests are moved from Optimize_test.cpp to Util_test.cpp. Bug: b/249793372 Test: Util_test, Convert_test Change-Id: I5a0458e3834d5ea62c96013abc14527285e895e0 --- tools/aapt2/cmd/Convert.cpp | 28 +++++++++++ tools/aapt2/cmd/Convert.h | 20 ++++++++ tools/aapt2/cmd/Convert_test.cpp | 77 +++++++++++++++++++++++++++++++ tools/aapt2/cmd/Optimize.cpp | 41 +--------------- tools/aapt2/cmd/Optimize.h | 8 ++++ tools/aapt2/cmd/Optimize_test.cpp | 68 --------------------------- tools/aapt2/cmd/Util.cpp | 37 +++++++++++++++ tools/aapt2/cmd/Util.h | 7 +++ tools/aapt2/cmd/Util_test.cpp | 58 +++++++++++++++++++++++ 9 files changed, 237 insertions(+), 107 deletions(-) delete mode 100644 tools/aapt2/cmd/Optimize_test.cpp diff --git a/tools/aapt2/cmd/Convert.cpp b/tools/aapt2/cmd/Convert.cpp index aeedf8b5805e5..52e113e0dbdc3 100644 --- a/tools/aapt2/cmd/Convert.cpp +++ b/tools/aapt2/cmd/Convert.cpp @@ -21,7 +21,9 @@ #include "Diagnostics.h" #include "LoadedApk.h" #include "ValueVisitor.h" +#include "android-base/file.h" #include "android-base/macros.h" +#include "android-base/stringprintf.h" #include "androidfw/StringPiece.h" #include "cmd/Util.h" #include "format/binary/TableFlattener.h" @@ -353,6 +355,27 @@ int Convert(IAaptContext* context, LoadedApk* apk, IArchiveWriter* output_writer return 0; } +bool ExtractResourceConfig(const std::string& path, IAaptContext* context, + TableFlattenerOptions& out_options) { + std::string content; + if (!android::base::ReadFileToString(path, &content, true /*follow_symlinks*/)) { + context->GetDiagnostics()->Error(android::DiagMessage(path) << "failed reading config file"); + return false; + } + std::unordered_set resources_exclude_list; + bool result = ParseResourceConfig(content, context, resources_exclude_list, + out_options.name_collapse_exemptions); + if (!result) { + return false; + } + if (!resources_exclude_list.empty()) { + context->GetDiagnostics()->Error(android::DiagMessage(path) + << "Unsupported '#remove' directive in resource config."); + return false; + } + return true; +} + const char* ConvertCommand::kOutputFormatProto = "proto"; const char* ConvertCommand::kOutputFormatBinary = "binary"; @@ -401,6 +424,11 @@ int ConvertCommand::Action(const std::vector& args) { if (force_sparse_encoding_) { table_flattener_options_.sparse_entries = SparseEntriesMode::Forced; } + if (resources_config_path_) { + if (!ExtractResourceConfig(*resources_config_path_, &context, table_flattener_options_)) { + return 1; + } + } return Convert(&context, apk.get(), writer.get(), format, table_flattener_options_, xml_flattener_options_); diff --git a/tools/aapt2/cmd/Convert.h b/tools/aapt2/cmd/Convert.h index 6c096496a1ea5..15fe11fd91cc0 100644 --- a/tools/aapt2/cmd/Convert.h +++ b/tools/aapt2/cmd/Convert.h @@ -50,6 +50,25 @@ class ConvertCommand : public Command { android::base::StringPrintf("Preserve raw attribute values in xml files when using the" " '%s' output format", kOutputFormatBinary), &xml_flattener_options_.keep_raw_values); + AddOptionalFlag("--resources-config-path", + "Path to the resources.cfg file containing the list of resources and \n" + "directives to each resource. \n" + "Format: type/resource_name#[directive][,directive]", + &resources_config_path_); + AddOptionalSwitch( + "--collapse-resource-names", + "Collapses resource names to a single value in the key string pool. Resources can \n" + "be exempted using the \"no_collapse\" directive in a file specified by " + "--resources-config-path.", + &table_flattener_options_.collapse_key_stringpool); + AddOptionalSwitch( + "--deduplicate-entry-values", + "Whether to deduplicate pairs of resource entry and value for simple resources.\n" + "This is recommended to be used together with '--collapse-resource-names' flag or for\n" + "APKs where resource names are manually collapsed. For such APKs this flag allows to\n" + "store the same resource value only once in resource table which decreases APK size.\n" + "Has no effect on APKs where resource names are kept.", + &table_flattener_options_.deduplicate_entry_values); AddOptionalSwitch("-v", "Enables verbose logging", &verbose_); } @@ -66,6 +85,7 @@ class ConvertCommand : public Command { bool verbose_ = false; bool enable_sparse_encoding_ = false; bool force_sparse_encoding_ = false; + std::optional resources_config_path_; }; int Convert(IAaptContext* context, LoadedApk* input, IArchiveWriter* output_writer, diff --git a/tools/aapt2/cmd/Convert_test.cpp b/tools/aapt2/cmd/Convert_test.cpp index 27df8c1b324e1..2c9388b7d711b 100644 --- a/tools/aapt2/cmd/Convert_test.cpp +++ b/tools/aapt2/cmd/Convert_test.cpp @@ -17,13 +17,18 @@ #include "Convert.h" #include "LoadedApk.h" +#include "test/Common.h" #include "test/Test.h" #include "ziparchive/zip_archive.h" +using testing::AnyOfArray; using testing::Eq; using testing::Ne; +using testing::Not; +using testing::SizeIs; namespace aapt { +using namespace aapt::test; using ConvertTest = CommandTestFixture; @@ -145,4 +150,76 @@ TEST_F(ConvertTest, DuplicateEntriesWrittenOnce) { EXPECT_THAT(count, Eq(1)); } +TEST_F(ConvertTest, ConvertWithResourceNameCollapsing) { + StdErrDiagnostics diag; + const std::string compiled_files_dir = GetTestPath("compiled"); + ASSERT_TRUE(CompileFile(GetTestPath("res/values/values.xml"), + R"( + string + string + another string + + true + true + true + + 10 + 10 + )", + compiled_files_dir, &diag)); + std::string resource_config_path = GetTestPath("resource-config"); + WriteFile(resource_config_path, "integer/int1#no_collapse\ninteger/int2#no_collapse"); + + const std::string proto_apk = GetTestPath("proto.apk"); + std::vector link_args = { + "--proto-format", "--manifest", GetDefaultManifest(kDefaultPackageName), "-o", proto_apk, + }; + ASSERT_TRUE(Link(link_args, compiled_files_dir, &diag)); + + const std::string binary_apk = GetTestPath("binary.apk"); + std::vector convert_args = {"-o", + binary_apk, + "--output-format", + "binary", + "--collapse-resource-names", + "--deduplicate-entry-values", + "--resources-config-path", + resource_config_path, + proto_apk}; + ASSERT_THAT(ConvertCommand().Execute(convert_args, &std::cerr), Eq(0)); + + std::unique_ptr apk = LoadedApk::LoadApkFromPath(binary_apk, &diag); + for (const auto& package : apk->GetResourceTable()->packages) { + for (const auto& type : package->types) { + switch (type->named_type.type) { + case ResourceType::kBool: + EXPECT_THAT(type->entries, SizeIs(3)); + for (const auto& entry : type->entries) { + auto value = ValueCast(entry->FindValue({})->value.get())->value; + EXPECT_THAT(value.data, Eq(0xffffffffu)); + } + break; + case ResourceType::kString: + EXPECT_THAT(type->entries, SizeIs(3)); + for (const auto& entry : type->entries) { + auto value = ValueCast(entry->FindValue({})->value.get())->value; + EXPECT_THAT(entry->name, Not(AnyOfArray({"first", "second", "third"}))); + EXPECT_THAT(*value, AnyOfArray({"string", "another string"})); + } + break; + case ResourceType::kInteger: + EXPECT_THAT(type->entries, SizeIs(2)); + for (const auto& entry : type->entries) { + auto value = ValueCast(entry->FindValue({})->value.get())->value; + EXPECT_THAT(entry->name, AnyOfArray({"int1", "int2"})); + EXPECT_THAT(value.data, Eq(10)); + } + break; + default: + break; + } + } + } +} + } // namespace aapt diff --git a/tools/aapt2/cmd/Optimize.cpp b/tools/aapt2/cmd/Optimize.cpp index 9feaf524eaf18..042926c1943a0 100644 --- a/tools/aapt2/cmd/Optimize.cpp +++ b/tools/aapt2/cmd/Optimize.cpp @@ -305,51 +305,14 @@ class Optimizer { OptimizeContext* context_; }; -bool ParseConfig(const std::string& content, IAaptContext* context, OptimizeOptions* options) { - size_t line_no = 0; - for (StringPiece line : util::Tokenize(content, '\n')) { - line_no++; - line = util::TrimWhitespace(line); - if (line.empty()) { - continue; - } - - auto split_line = util::Split(line, '#'); - if (split_line.size() < 2) { - context->GetDiagnostics()->Error(android::DiagMessage(line) << "No # found in line"); - return false; - } - StringPiece resource_string = split_line[0]; - StringPiece directives = split_line[1]; - ResourceNameRef resource_name; - if (!ResourceUtils::ParseResourceName(resource_string, &resource_name)) { - context->GetDiagnostics()->Error(android::DiagMessage(line) << "Malformed resource name"); - return false; - } - if (!resource_name.package.empty()) { - context->GetDiagnostics()->Error(android::DiagMessage(line) - << "Package set for resource. Only use type/name"); - return false; - } - for (StringPiece directive : util::Tokenize(directives, ',')) { - if (directive == "remove") { - options->resources_exclude_list.insert(resource_name.ToResourceName()); - } else if (directive == "no_collapse" || directive == "no_obfuscate") { - options->table_flattener_options.name_collapse_exemptions.insert( - resource_name.ToResourceName()); - } - } - } - return true; -} - bool ExtractConfig(const std::string& path, IAaptContext* context, OptimizeOptions* options) { std::string content; if (!android::base::ReadFileToString(path, &content, true /*follow_symlinks*/)) { context->GetDiagnostics()->Error(android::DiagMessage(path) << "failed reading config file"); return false; } - return ParseConfig(content, context, options); + return ParseResourceConfig(content, context, options->resources_exclude_list, + options->table_flattener_options.name_collapse_exemptions); } bool ExtractAppDataFromManifest(OptimizeContext* context, const LoadedApk* apk, diff --git a/tools/aapt2/cmd/Optimize.h b/tools/aapt2/cmd/Optimize.h index 790bb74b25660..794a87b0faa58 100644 --- a/tools/aapt2/cmd/Optimize.h +++ b/tools/aapt2/cmd/Optimize.h @@ -123,6 +123,14 @@ class OptimizeCommand : public Command { AddOptionalFlag("--resource-path-shortening-map", "Path to output the map of old resource paths to shortened paths.", &options_.shortened_paths_map_path); + AddOptionalSwitch( + "--deduplicate-entry-values", + "Whether to deduplicate pairs of resource entry and value for simple resources.\n" + "This is recommended to be used together with '--collapse-resource-names' flag or for\n" + "APKs where resource names are manually collapsed. For such APKs this flag allows to\n" + "store the same resource value only once in resource table which decreases APK size.\n" + "Has no effect on APKs where resource names are kept.", + &options_.table_flattener_options.deduplicate_entry_values); AddOptionalSwitch("-v", "Enables verbose logging", &verbose_); } diff --git a/tools/aapt2/cmd/Optimize_test.cpp b/tools/aapt2/cmd/Optimize_test.cpp deleted file mode 100644 index d180c87de81e0..0000000000000 --- a/tools/aapt2/cmd/Optimize_test.cpp +++ /dev/null @@ -1,68 +0,0 @@ -/* - * Copyright (C) 2019 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.h" - -#include "AppInfo.h" -#include "LoadedApk.h" -#include "Resource.h" -#include "androidfw/IDiagnostics.h" -#include "test/Test.h" - -using testing::Contains; -using testing::Eq; - -namespace aapt { - -bool ParseConfig(const std::string&, IAaptContext*, OptimizeOptions*); - -using OptimizeTest = CommandTestFixture; - -TEST_F(OptimizeTest, ParseConfigWithNoCollapseExemptions) { - const std::string& content = R"( -string/foo#no_collapse -dimen/bar#no_collapse -)"; - aapt::test::Context context; - OptimizeOptions options; - ParseConfig(content, &context, &options); - - const std::set& name_collapse_exemptions = - options.table_flattener_options.name_collapse_exemptions; - - ASSERT_THAT(name_collapse_exemptions.size(), Eq(2)); - EXPECT_THAT(name_collapse_exemptions, Contains(ResourceName({}, ResourceType::kString, "foo"))); - EXPECT_THAT(name_collapse_exemptions, Contains(ResourceName({}, ResourceType::kDimen, "bar"))); -} - -TEST_F(OptimizeTest, ParseConfigWithNoObfuscateExemptions) { - const std::string& content = R"( -string/foo#no_obfuscate -dimen/bar#no_obfuscate -)"; - aapt::test::Context context; - OptimizeOptions options; - ParseConfig(content, &context, &options); - - const std::set& name_collapse_exemptions = - options.table_flattener_options.name_collapse_exemptions; - - ASSERT_THAT(name_collapse_exemptions.size(), Eq(2)); - EXPECT_THAT(name_collapse_exemptions, Contains(ResourceName({}, ResourceType::kString, "foo"))); - EXPECT_THAT(name_collapse_exemptions, Contains(ResourceName({}, ResourceType::kDimen, "bar"))); -} - -} // namespace aapt diff --git a/tools/aapt2/cmd/Util.cpp b/tools/aapt2/cmd/Util.cpp index c3a6ed1007306..56e2f5243e96e 100644 --- a/tools/aapt2/cmd/Util.cpp +++ b/tools/aapt2/cmd/Util.cpp @@ -447,4 +447,41 @@ std::regex GetRegularExpression(const std::string &input) { return case_insensitive; } +bool ParseResourceConfig(const std::string& content, IAaptContext* context, + std::unordered_set& out_resource_exclude_list, + std::set& out_name_collapse_exemptions) { + for (StringPiece line : util::Tokenize(content, '\n')) { + line = util::TrimWhitespace(line); + if (line.empty()) { + continue; + } + + auto split_line = util::Split(line, '#'); + if (split_line.size() < 2) { + context->GetDiagnostics()->Error(android::DiagMessage(line) << "No # found in line"); + return false; + } + StringPiece resource_string = split_line[0]; + StringPiece directives = split_line[1]; + ResourceNameRef resource_name; + if (!ResourceUtils::ParseResourceName(resource_string, &resource_name)) { + context->GetDiagnostics()->Error(android::DiagMessage(line) << "Malformed resource name"); + return false; + } + if (!resource_name.package.empty()) { + context->GetDiagnostics()->Error(android::DiagMessage(line) + << "Package set for resource. Only use type/name"); + return false; + } + for (StringPiece directive : util::Tokenize(directives, ',')) { + if (directive == "remove") { + out_resource_exclude_list.insert(resource_name.ToResourceName()); + } else if (directive == "no_collapse" || directive == "no_obfuscate") { + out_name_collapse_exemptions.insert(resource_name.ToResourceName()); + } + } + } + return true; +} + } // namespace aapt diff --git a/tools/aapt2/cmd/Util.h b/tools/aapt2/cmd/Util.h index 7af27f57c39c1..3d4ca245ee284 100644 --- a/tools/aapt2/cmd/Util.h +++ b/tools/aapt2/cmd/Util.h @@ -18,12 +18,15 @@ #define AAPT_SPLIT_UTIL_H #include +#include +#include #include "AppInfo.h" #include "SdkConstants.h" #include "androidfw/IDiagnostics.h" #include "androidfw/StringPiece.h" #include "filter/ConfigFilter.h" +#include "process/IResourceTableConsumer.h" #include "split/TableSplitter.h" #include "xml/XmlDom.h" @@ -76,6 +79,10 @@ void SetLongVersionCode(xml::Element* manifest, uint64_t version_code); // Returns a case insensitive regular expression based on the input. std::regex GetRegularExpression(const std::string &input); +bool ParseResourceConfig(const std::string& content, IAaptContext* context, + std::unordered_set& out_resource_exclude_list, + std::set& out_name_collapse_exemptions); + } // namespace aapt #endif /* AAPT_SPLIT_UTIL_H */ diff --git a/tools/aapt2/cmd/Util_test.cpp b/tools/aapt2/cmd/Util_test.cpp index 91accfe0511fa..28a6de80e7ced 100644 --- a/tools/aapt2/cmd/Util_test.cpp +++ b/tools/aapt2/cmd/Util_test.cpp @@ -25,6 +25,7 @@ #include "util/Files.h" using ::android::ConfigDescription; +using testing::UnorderedElementsAre; namespace aapt { @@ -411,4 +412,61 @@ TEST (UtilTest, RegularExpressionNonEnglish) { EXPECT_FALSE(std::regex_search("file.koncowka", expression)); } +TEST(UtilTest, ParseConfigWithDirectives) { + const std::string& content = R"( +bool/remove_me#remove +bool/keep_name#no_collapse +string/foo#no_obfuscate +dimen/bar#no_obfuscate +)"; + aapt::test::Context context; + std::unordered_set resource_exclusion; + std::set name_collapse_exemptions; + + EXPECT_TRUE(ParseResourceConfig(content, &context, resource_exclusion, name_collapse_exemptions)); + + EXPECT_THAT(name_collapse_exemptions, + UnorderedElementsAre(ResourceName({}, ResourceType::kString, "foo"), + ResourceName({}, ResourceType::kDimen, "bar"), + ResourceName({}, ResourceType::kBool, "keep_name"))); + EXPECT_THAT(resource_exclusion, + UnorderedElementsAre(ResourceName({}, ResourceType::kBool, "remove_me"))); +} + +TEST(UtilTest, ParseConfigResourceWithPackage) { + const std::string& content = R"( +package:bool/remove_me#remove +)"; + aapt::test::Context context; + std::unordered_set resource_exclusion; + std::set name_collapse_exemptions; + + EXPECT_FALSE( + ParseResourceConfig(content, &context, resource_exclusion, name_collapse_exemptions)); +} + +TEST(UtilTest, ParseConfigInvalidName) { + const std::string& content = R"( +package:bool/1231#remove +)"; + aapt::test::Context context; + std::unordered_set resource_exclusion; + std::set name_collapse_exemptions; + + EXPECT_FALSE( + ParseResourceConfig(content, &context, resource_exclusion, name_collapse_exemptions)); +} + +TEST(UtilTest, ParseConfigNoHash) { + const std::string& content = R"( +package:bool/my_bool +)"; + aapt::test::Context context; + std::unordered_set resource_exclusion; + std::set name_collapse_exemptions; + + EXPECT_FALSE( + ParseResourceConfig(content, &context, resource_exclusion, name_collapse_exemptions)); +} + } // namespace aapt