diff options
author | 2025-01-09 12:18:04 -0800 | |
---|---|---|
committer | 2025-01-09 12:18:04 -0800 | |
commit | c9c3150896e1b637df6c8ecc485cb0e3384448c7 (patch) | |
tree | 0e80776c8a6cc2d9e99837f2373acaf9d0f2effb | |
parent | ca618d9f16633b1cbf29eac795956d201e1bc6e3 (diff) |
Always enable sparse encoding if minSdk >= 32
Sparse encoding has been enabled in the framework since T QPR2 with no major issues. It should be safe to enable by default for any app with minSdk >= 32 (S_V2) which is when the last fix for sparse encoding was made.
This CL makes the `--enable-sparse-encoding` flag a no-op. As such, we are also bumping the version to 2.20.
Bug: 302179337
Change-Id: Ia886a0da9d61175d00c24e30f4e263f7e5df511a
Test: build + boot on Pixel 6 Pro
Flag: EXEMPT command line tool
-rw-r--r-- | tools/aapt2/cmd/Command.cpp | 24 | ||||
-rw-r--r-- | tools/aapt2/cmd/Command_test.cpp | 18 | ||||
-rw-r--r-- | tools/aapt2/cmd/Convert.cpp | 3 | ||||
-rw-r--r-- | tools/aapt2/cmd/Convert.h | 9 | ||||
-rw-r--r-- | tools/aapt2/cmd/Link.cpp | 3 | ||||
-rw-r--r-- | tools/aapt2/cmd/Link.h | 9 | ||||
-rw-r--r-- | tools/aapt2/cmd/Optimize.cpp | 3 | ||||
-rw-r--r-- | tools/aapt2/cmd/Optimize.h | 11 | ||||
-rw-r--r-- | tools/aapt2/format/binary/TableFlattener.cpp | 2 | ||||
-rw-r--r-- | tools/aapt2/format/binary/TableFlattener.h | 5 | ||||
-rw-r--r-- | tools/aapt2/format/binary/TableFlattener_test.cpp | 12 | ||||
-rw-r--r-- | tools/aapt2/integration-tests/DumpTest/components_full_proto.txt | 2 | ||||
-rw-r--r-- | tools/aapt2/readme.md | 6 | ||||
-rw-r--r-- | tools/aapt2/util/Util.cpp | 2 |
14 files changed, 64 insertions, 45 deletions
diff --git a/tools/aapt2/cmd/Command.cpp b/tools/aapt2/cmd/Command.cpp index 20315561cceb..f00a6cad6b46 100644 --- a/tools/aapt2/cmd/Command.cpp +++ b/tools/aapt2/cmd/Command.cpp @@ -54,7 +54,9 @@ std::string GetSafePath(StringPiece arg) { void Command::AddRequiredFlag(StringPiece name, StringPiece description, std::string* value, uint32_t flags) { auto func = [value, flags](StringPiece arg, std::ostream*) -> bool { - *value = (flags & Command::kPath) ? GetSafePath(arg) : std::string(arg); + if (value) { + *value = (flags & Command::kPath) ? GetSafePath(arg) : std::string(arg); + } return true; }; @@ -65,7 +67,9 @@ void Command::AddRequiredFlag(StringPiece name, StringPiece description, std::st void Command::AddRequiredFlagList(StringPiece name, StringPiece description, std::vector<std::string>* value, uint32_t flags) { auto func = [value, flags](StringPiece arg, std::ostream*) -> bool { - value->push_back((flags & Command::kPath) ? GetSafePath(arg) : std::string(arg)); + if (value) { + value->push_back((flags & Command::kPath) ? GetSafePath(arg) : std::string(arg)); + } return true; }; @@ -76,7 +80,9 @@ void Command::AddRequiredFlagList(StringPiece name, StringPiece description, void Command::AddOptionalFlag(StringPiece name, StringPiece description, std::optional<std::string>* value, uint32_t flags) { auto func = [value, flags](StringPiece arg, std::ostream*) -> bool { - *value = (flags & Command::kPath) ? GetSafePath(arg) : std::string(arg); + if (value) { + *value = (flags & Command::kPath) ? GetSafePath(arg) : std::string(arg); + } return true; }; @@ -87,7 +93,9 @@ void Command::AddOptionalFlag(StringPiece name, StringPiece description, void Command::AddOptionalFlagList(StringPiece name, StringPiece description, std::vector<std::string>* value, uint32_t flags) { auto func = [value, flags](StringPiece arg, std::ostream*) -> bool { - value->push_back((flags & Command::kPath) ? GetSafePath(arg) : std::string(arg)); + if (value) { + value->push_back((flags & Command::kPath) ? GetSafePath(arg) : std::string(arg)); + } return true; }; @@ -98,7 +106,9 @@ void Command::AddOptionalFlagList(StringPiece name, StringPiece description, void Command::AddOptionalFlagList(StringPiece name, StringPiece description, std::unordered_set<std::string>* value) { auto func = [value](StringPiece arg, std::ostream* out_error) -> bool { - value->emplace(arg); + if (value) { + value->emplace(arg); + } return true; }; @@ -108,7 +118,9 @@ void Command::AddOptionalFlagList(StringPiece name, StringPiece description, void Command::AddOptionalSwitch(StringPiece name, StringPiece description, bool* value) { auto func = [value](StringPiece arg, std::ostream* out_error) -> bool { - *value = true; + if (value) { + *value = true; + } return true; }; diff --git a/tools/aapt2/cmd/Command_test.cpp b/tools/aapt2/cmd/Command_test.cpp index 2a3cb2a0c65d..ad167c979662 100644 --- a/tools/aapt2/cmd/Command_test.cpp +++ b/tools/aapt2/cmd/Command_test.cpp @@ -159,4 +159,22 @@ TEST(CommandTest, ShortOptions) { ASSERT_NE(0, command.Execute({"-w"s, "2"s}, &std::cerr)); } +TEST(CommandTest, OptionsWithNullptrToAcceptValues) { + TestCommand command; + command.AddRequiredFlag("--rflag", "", nullptr); + command.AddRequiredFlagList("--rlflag", "", nullptr); + command.AddOptionalFlag("--oflag", "", nullptr); + command.AddOptionalFlagList("--olflag", "", (std::vector<std::string>*)nullptr); + command.AddOptionalFlagList("--olflag2", "", (std::unordered_set<std::string>*)nullptr); + command.AddOptionalSwitch("--switch", "", nullptr); + + ASSERT_EQ(0, command.Execute({ + "--rflag"s, "1"s, + "--rlflag"s, "1"s, + "--oflag"s, "1"s, + "--olflag"s, "1"s, + "--olflag2"s, "1"s, + "--switch"s}, &std::cerr)); +} + } // namespace aapt
\ No newline at end of file diff --git a/tools/aapt2/cmd/Convert.cpp b/tools/aapt2/cmd/Convert.cpp index 6c3eae11eab9..060bc5fa2242 100644 --- a/tools/aapt2/cmd/Convert.cpp +++ b/tools/aapt2/cmd/Convert.cpp @@ -425,9 +425,6 @@ int ConvertCommand::Action(const std::vector<std::string>& args) { << output_format_.value()); return 1; } - if (enable_sparse_encoding_) { - table_flattener_options_.sparse_entries = SparseEntriesMode::Enabled; - } if (force_sparse_encoding_) { table_flattener_options_.sparse_entries = SparseEntriesMode::Forced; } diff --git a/tools/aapt2/cmd/Convert.h b/tools/aapt2/cmd/Convert.h index 9452e588953e..98c8f5ff89c0 100644 --- a/tools/aapt2/cmd/Convert.h +++ b/tools/aapt2/cmd/Convert.h @@ -36,11 +36,9 @@ class ConvertCommand : public Command { kOutputFormatProto, kOutputFormatBinary, kOutputFormatBinary), &output_format_); AddOptionalSwitch( "--enable-sparse-encoding", - "Enables encoding sparse entries using a binary search tree.\n" - "This decreases APK size at the cost of resource retrieval performance.\n" - "Only applies sparse encoding to Android O+ resources or all resources if minSdk of " - "the APK is O+", - &enable_sparse_encoding_); + "[DEPRECATED] This flag is a no-op as of aapt2 v2.20. Sparse encoding is always\n" + "enabled if minSdk of the APK is >= 32.", + nullptr); AddOptionalSwitch("--force-sparse-encoding", "Enables encoding sparse entries using a binary search tree.\n" "This decreases APK size at the cost of resource retrieval performance.\n" @@ -87,7 +85,6 @@ class ConvertCommand : public Command { std::string output_path_; std::optional<std::string> output_format_; bool verbose_ = false; - bool enable_sparse_encoding_ = false; bool force_sparse_encoding_ = false; bool enable_compact_entries_ = false; std::optional<std::string> resources_config_path_; diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp index 232b4024abd2..eb71189ffc46 100644 --- a/tools/aapt2/cmd/Link.cpp +++ b/tools/aapt2/cmd/Link.cpp @@ -2504,9 +2504,6 @@ int LinkCommand::Action(const std::vector<std::string>& args) { << "the --merge-only flag can be only used when building a static library"); return 1; } - if (options_.use_sparse_encoding) { - options_.table_flattener_options.sparse_entries = SparseEntriesMode::Enabled; - } // The default build type. context.SetPackageType(PackageType::kApp); diff --git a/tools/aapt2/cmd/Link.h b/tools/aapt2/cmd/Link.h index 2f17853718ec..b5bd905c02be 100644 --- a/tools/aapt2/cmd/Link.h +++ b/tools/aapt2/cmd/Link.h @@ -75,7 +75,6 @@ struct LinkOptions { bool no_resource_removal = false; bool no_xml_namespaces = false; bool do_not_compress_anything = false; - bool use_sparse_encoding = false; std::unordered_set<std::string> extensions_to_not_compress; std::optional<std::regex> regex_to_not_compress; FeatureFlagValues feature_flag_values; @@ -163,9 +162,11 @@ class LinkCommand : public Command { AddOptionalSwitch("--no-resource-removal", "Disables automatic removal of resources without\n" "defaults. Use this only when building runtime resource overlay packages.", &options_.no_resource_removal); - AddOptionalSwitch("--enable-sparse-encoding", - "This decreases APK size at the cost of resource retrieval performance.", - &options_.use_sparse_encoding); + AddOptionalSwitch( + "--enable-sparse-encoding", + "[DEPRECATED] This flag is a no-op as of aapt2 v2.20. Sparse encoding is always\n" + "enabled if minSdk of the APK is >= 32.", + nullptr); AddOptionalSwitch("--enable-compact-entries", "This decreases APK size by using compact resource entries for simple data types.", &options_.table_flattener_options.use_compact_entries); diff --git a/tools/aapt2/cmd/Optimize.cpp b/tools/aapt2/cmd/Optimize.cpp index 762441ee1872..f218307af578 100644 --- a/tools/aapt2/cmd/Optimize.cpp +++ b/tools/aapt2/cmd/Optimize.cpp @@ -406,9 +406,6 @@ int OptimizeCommand::Action(const std::vector<std::string>& args) { return 1; } - if (options_.enable_sparse_encoding) { - options_.table_flattener_options.sparse_entries = SparseEntriesMode::Enabled; - } if (options_.force_sparse_encoding) { options_.table_flattener_options.sparse_entries = SparseEntriesMode::Forced; } diff --git a/tools/aapt2/cmd/Optimize.h b/tools/aapt2/cmd/Optimize.h index 012b0f230ca2..e3af584cbbd9 100644 --- a/tools/aapt2/cmd/Optimize.h +++ b/tools/aapt2/cmd/Optimize.h @@ -61,9 +61,6 @@ struct OptimizeOptions { // TODO(b/246489170): keep the old option and format until transform to the new one std::optional<std::string> shortened_paths_map_path; - // Whether sparse encoding should be used for O+ resources. - bool enable_sparse_encoding = false; - // Whether sparse encoding should be used for all resources. bool force_sparse_encoding = false; @@ -106,11 +103,9 @@ class OptimizeCommand : public Command { &kept_artifacts_); AddOptionalSwitch( "--enable-sparse-encoding", - "Enables encoding sparse entries using a binary search tree.\n" - "This decreases APK size at the cost of resource retrieval performance.\n" - "Only applies sparse encoding to Android O+ resources or all resources if minSdk of " - "the APK is O+", - &options_.enable_sparse_encoding); + "[DEPRECATED] This flag is a no-op as of aapt2 v2.20. Sparse encoding is always\n" + "enabled if minSdk of the APK is >= 32.", + nullptr); AddOptionalSwitch("--force-sparse-encoding", "Enables encoding sparse entries using a binary search tree.\n" "This decreases APK size at the cost of resource retrieval performance.\n" diff --git a/tools/aapt2/format/binary/TableFlattener.cpp b/tools/aapt2/format/binary/TableFlattener.cpp index 1a82021bce71..b8ac7925d44e 100644 --- a/tools/aapt2/format/binary/TableFlattener.cpp +++ b/tools/aapt2/format/binary/TableFlattener.cpp @@ -201,7 +201,7 @@ class PackageFlattener { (context_->GetMinSdkVersion() == 0 && config.sdkVersion == 0)) { // Sparse encode if forced or sdk version is not set in context and config. } else { - // Otherwise, only sparse encode if the entries will be read on platforms S_V2+. + // Otherwise, only sparse encode if the entries will be read on platforms S_V2+ (32). sparse_encode = sparse_encode && (context_->GetMinSdkVersion() >= SDK_S_V2); } diff --git a/tools/aapt2/format/binary/TableFlattener.h b/tools/aapt2/format/binary/TableFlattener.h index 0633bc81cb25..f1c4c3512ed3 100644 --- a/tools/aapt2/format/binary/TableFlattener.h +++ b/tools/aapt2/format/binary/TableFlattener.h @@ -37,8 +37,7 @@ constexpr const size_t kSparseEncodingThreshold = 60; enum class SparseEntriesMode { // Disables sparse encoding for entries. Disabled, - // Enables sparse encoding for all entries for APKs with O+ minSdk. For APKs with minSdk less - // than O only applies sparse encoding for resource configuration available on O+. + // Enables sparse encoding for all entries for APKs with minSdk >= 32 (S_V2). Enabled, // Enables sparse encoding for all entries regardless of minSdk. Forced, @@ -47,7 +46,7 @@ enum class SparseEntriesMode { struct TableFlattenerOptions { // When enabled, types for configurations with a sparse set of entries are encoded // as a sparse map of entry ID and offset to actual data. - SparseEntriesMode sparse_entries = SparseEntriesMode::Disabled; + SparseEntriesMode sparse_entries = SparseEntriesMode::Enabled; // When true, use compact entries for simple data bool use_compact_entries = false; diff --git a/tools/aapt2/format/binary/TableFlattener_test.cpp b/tools/aapt2/format/binary/TableFlattener_test.cpp index 0f1168514c4a..e3d589eb078b 100644 --- a/tools/aapt2/format/binary/TableFlattener_test.cpp +++ b/tools/aapt2/format/binary/TableFlattener_test.cpp @@ -337,13 +337,13 @@ TEST_F(TableFlattenerTest, FlattenSparseEntryWithMinSdkSV2) { auto table_in = BuildTableWithSparseEntries(context.get(), sparse_config, 0.25f); TableFlattenerOptions options; - options.sparse_entries = SparseEntriesMode::Enabled; + options.sparse_entries = SparseEntriesMode::Disabled; std::string no_sparse_contents; - ASSERT_TRUE(Flatten(context.get(), {}, table_in.get(), &no_sparse_contents)); + ASSERT_TRUE(Flatten(context.get(), options, table_in.get(), &no_sparse_contents)); std::string sparse_contents; - ASSERT_TRUE(Flatten(context.get(), options, table_in.get(), &sparse_contents)); + ASSERT_TRUE(Flatten(context.get(), {}, table_in.get(), &sparse_contents)); EXPECT_GT(no_sparse_contents.size(), sparse_contents.size()); @@ -421,13 +421,13 @@ TEST_F(TableFlattenerTest, FlattenSparseEntryWithSdkVersionNotSet) { auto table_in = BuildTableWithSparseEntries(context.get(), sparse_config, 0.25f); TableFlattenerOptions options; - options.sparse_entries = SparseEntriesMode::Enabled; + options.sparse_entries = SparseEntriesMode::Disabled; std::string no_sparse_contents; - ASSERT_TRUE(Flatten(context.get(), {}, table_in.get(), &no_sparse_contents)); + ASSERT_TRUE(Flatten(context.get(), options, table_in.get(), &no_sparse_contents)); std::string sparse_contents; - ASSERT_TRUE(Flatten(context.get(), options, table_in.get(), &sparse_contents)); + ASSERT_TRUE(Flatten(context.get(), {}, table_in.get(), &sparse_contents)); EXPECT_GT(no_sparse_contents.size(), sparse_contents.size()); diff --git a/tools/aapt2/integration-tests/DumpTest/components_full_proto.txt b/tools/aapt2/integration-tests/DumpTest/components_full_proto.txt index 6da6fc6f12c3..d0807f2ecd34 100644 --- a/tools/aapt2/integration-tests/DumpTest/components_full_proto.txt +++ b/tools/aapt2/integration-tests/DumpTest/components_full_proto.txt @@ -877,7 +877,7 @@ resource_table { } tool_fingerprint { tool: "Android Asset Packaging Tool (aapt)" - version: "2.19-SOONG BUILD NUMBER PLACEHOLDER" + version: "2.20-SOONG BUILD NUMBER PLACEHOLDER" } } xml_files { diff --git a/tools/aapt2/readme.md b/tools/aapt2/readme.md index 8368f9d16af8..664d8412a3be 100644 --- a/tools/aapt2/readme.md +++ b/tools/aapt2/readme.md @@ -1,5 +1,11 @@ # Android Asset Packaging Tool 2.0 (AAPT2) release notes +## Version 2.20 +- Too many features, bug fixes, and improvements to list since the last minor version update in + 2017. This README will be updated more frequently in the future. +- Sparse encoding is now always enabled by default if the minSdkVersion is >= 32 (S_V2). The + `--enable-sparse-encoding` flag still exists, but is a no-op. + ## Version 2.19 - Added navigation resource type. - Fixed issue with resource deduplication. (bug 64397629) diff --git a/tools/aapt2/util/Util.cpp b/tools/aapt2/util/Util.cpp index 3d83caf29bba..6a4dfa629394 100644 --- a/tools/aapt2/util/Util.cpp +++ b/tools/aapt2/util/Util.cpp @@ -227,7 +227,7 @@ std::string GetToolFingerprint() { static const char* const sMajorVersion = "2"; // Update minor version whenever a feature or flag is added. - static const char* const sMinorVersion = "19"; + static const char* const sMinorVersion = "20"; // The build id of aapt2 binary. static const std::string sBuildId = [] { |