From c744ae8aca97edfb2422598ea620e8219449fa9b Mon Sep 17 00:00:00 2001 From: Adam Lesinski Date: Wed, 17 May 2017 19:28:38 -0700 Subject: [PATCH] AAPT2: Implement attribute compat versioning This change defines some hardcoded rules to degrade attributes in newer SDKs to specific older attributes. An attribute with a degrade rule will generate a new XML for the API in which the attribute resulting from the degradation was introduced. Since API 22 (Lollipop MR1), attributes are correctly ignored and do not need to be versioned. In XML files defined for APIs 22+, the original and degraded attributes coexist in the same XML file. One such example is paddingHorizontal, introduced in API 26. paddingHorizontal degrades to paddingLeft and paddingRight, which were both introduced in API 1. Bug: 35763493 Test: make aapt2_tests Change-Id: I4aa8755a9ee2c0cc5afdc55c3d30093fd3a47f3d --- tools/aapt2/Android.bp | 1 + tools/aapt2/Debug.h | 1 + tools/aapt2/Main.cpp | 2 +- tools/aapt2/Resource.h | 11 +- tools/aapt2/ResourceValues.cpp | 6 + tools/aapt2/ResourceValues.h | 2 + tools/aapt2/SdkConstants.cpp | 28 +- tools/aapt2/SdkConstants.h | 12 +- tools/aapt2/SdkConstants_test.cpp | 13 +- tools/aapt2/cmd/Link.cpp | 303 +++++++++-------- tools/aapt2/cmd/Util.cpp | 2 +- tools/aapt2/flatten/XmlFlattener.cpp | 11 +- tools/aapt2/flatten/XmlFlattener.h | 5 - tools/aapt2/flatten/XmlFlattener_test.cpp | 34 +- .../AutoVersionTest/Android.mk | 23 ++ .../AutoVersionTest/AndroidManifest.xml | 21 ++ .../res/layout-v21/layout3.xml | 19 ++ .../AutoVersionTest/res/layout/layout.xml | 35 ++ .../AutoVersionTest/res/layout/layout2.xml | 47 +++ .../AutoVersionTest/res/layout/layout3.xml | 19 ++ .../AutoVersionTest/res/values-v21/styles.xml | 28 ++ .../AutoVersionTest/res/values/styles.xml | 84 +++++ tools/aapt2/link/AutoVersioner.cpp | 48 ++- tools/aapt2/link/Linkers.h | 41 +-- tools/aapt2/link/ReferenceLinker.cpp | 2 +- tools/aapt2/link/XmlCompatVersioner.cpp | 179 ++++++++++ tools/aapt2/link/XmlCompatVersioner.h | 100 ++++++ tools/aapt2/link/XmlCompatVersioner_test.cpp | 320 ++++++++++++++++++ tools/aapt2/link/XmlReferenceLinker.cpp | 25 +- tools/aapt2/link/XmlReferenceLinker_test.cpp | 10 - tools/aapt2/process/SymbolTable.cpp | 6 +- tools/aapt2/process/SymbolTable.h | 14 +- tools/aapt2/readme.md | 6 + tools/aapt2/util/Util.h | 6 + tools/aapt2/xml/XmlDom.cpp | 28 +- tools/aapt2/xml/XmlDom.h | 22 +- 36 files changed, 1177 insertions(+), 337 deletions(-) create mode 100644 tools/aapt2/integration-tests/AutoVersionTest/Android.mk create mode 100644 tools/aapt2/integration-tests/AutoVersionTest/AndroidManifest.xml create mode 100644 tools/aapt2/integration-tests/AutoVersionTest/res/layout-v21/layout3.xml create mode 100644 tools/aapt2/integration-tests/AutoVersionTest/res/layout/layout.xml create mode 100644 tools/aapt2/integration-tests/AutoVersionTest/res/layout/layout2.xml create mode 100644 tools/aapt2/integration-tests/AutoVersionTest/res/layout/layout3.xml create mode 100644 tools/aapt2/integration-tests/AutoVersionTest/res/values-v21/styles.xml create mode 100644 tools/aapt2/integration-tests/AutoVersionTest/res/values/styles.xml create mode 100644 tools/aapt2/link/XmlCompatVersioner.cpp create mode 100644 tools/aapt2/link/XmlCompatVersioner.h create mode 100644 tools/aapt2/link/XmlCompatVersioner_test.cpp diff --git a/tools/aapt2/Android.bp b/tools/aapt2/Android.bp index b460258248af7..bf189492b0824 100644 --- a/tools/aapt2/Android.bp +++ b/tools/aapt2/Android.bp @@ -99,6 +99,7 @@ cc_library_host_static { "link/PrivateAttributeMover.cpp", "link/ReferenceLinker.cpp", "link/TableMerger.cpp", + "link/XmlCompatVersioner.cpp", "link/XmlNamespaceRemover.cpp", "link/XmlReferenceLinker.cpp", "optimize/ResourceDeduper.cpp", diff --git a/tools/aapt2/Debug.h b/tools/aapt2/Debug.h index 56e2e95df6cbe..e2456c7f98b24 100644 --- a/tools/aapt2/Debug.h +++ b/tools/aapt2/Debug.h @@ -37,6 +37,7 @@ struct Debug { const ResourceName& target_style); static void DumpHex(const void* data, size_t len); static void DumpXml(xml::XmlResource* doc); + static std::string ToString(xml::XmlResource* doc); }; } // namespace aapt diff --git a/tools/aapt2/Main.cpp b/tools/aapt2/Main.cpp index e45d1420a902b..1d2e3a4f2df0d 100644 --- a/tools/aapt2/Main.cpp +++ b/tools/aapt2/Main.cpp @@ -27,7 +27,7 @@ namespace aapt { static const char* sMajorVersion = "2"; // Update minor version whenever a feature or flag is added. -static const char* sMinorVersion = "15"; +static const char* sMinorVersion = "16"; int PrintVersion() { std::cerr << "Android Asset Packaging Tool (aapt) " << sMajorVersion << "." diff --git a/tools/aapt2/Resource.h b/tools/aapt2/Resource.h index 493f238b033da..0a74c1a0f77df 100644 --- a/tools/aapt2/Resource.h +++ b/tools/aapt2/Resource.h @@ -388,13 +388,20 @@ template <> struct hash { size_t operator()(const aapt::ResourceName& name) const { android::hash_t h = 0; - h = android::JenkinsHashMix(h, hash()(name.package)); + h = android::JenkinsHashMix(h, static_cast(hash()(name.package))); h = android::JenkinsHashMix(h, static_cast(name.type)); - h = android::JenkinsHashMix(h, hash()(name.entry)); + h = android::JenkinsHashMix(h, static_cast(hash()(name.entry))); return static_cast(h); } }; +template <> +struct hash { + size_t operator()(const aapt::ResourceId& id) const { + return id.id; + } +}; + } // namespace std #endif // AAPT_RESOURCE_H diff --git a/tools/aapt2/ResourceValues.cpp b/tools/aapt2/ResourceValues.cpp index 0cb8c67705f9d..34bd2b4361ca4 100644 --- a/tools/aapt2/ResourceValues.cpp +++ b/tools/aapt2/ResourceValues.cpp @@ -333,6 +333,12 @@ void BinaryPrimitive::Print(std::ostream* out) const { } } +Attribute::Attribute() + : type_mask(0u), + min_int(std::numeric_limits::min()), + max_int(std::numeric_limits::max()) { +} + Attribute::Attribute(bool w, uint32_t t) : type_mask(t), min_int(std::numeric_limits::min()), diff --git a/tools/aapt2/ResourceValues.h b/tools/aapt2/ResourceValues.h index c71c738892dad..06f949f9555c7 100644 --- a/tools/aapt2/ResourceValues.h +++ b/tools/aapt2/ResourceValues.h @@ -18,6 +18,7 @@ #define AAPT_RESOURCE_VALUES_H #include +#include #include #include @@ -251,6 +252,7 @@ struct Attribute : public BaseValue { int32_t max_int; std::vector symbols; + Attribute(); explicit Attribute(bool w, uint32_t t = 0u); bool Equals(const Value* value) const override; diff --git a/tools/aapt2/SdkConstants.cpp b/tools/aapt2/SdkConstants.cpp index e8067143dc753..041cb4fa96cda 100644 --- a/tools/aapt2/SdkConstants.cpp +++ b/tools/aapt2/SdkConstants.cpp @@ -26,9 +26,9 @@ using android::StringPiece; namespace aapt { static const char* sDevelopmentSdkCodeName = "O"; -static int sDevelopmentSdkLevel = 26; +static ApiVersion sDevelopmentSdkLevel = 26; -static const std::vector> sAttrIdMap = { +static const std::vector> sAttrIdMap = { {0x021c, 1}, {0x021d, 2}, {0x0269, SDK_CUPCAKE}, @@ -48,26 +48,29 @@ static const std::vector> sAttrIdMap = { {0x03f1, SDK_KITKAT}, {0x03f6, SDK_KITKAT_WATCH}, {0x04ce, SDK_LOLLIPOP}, + {0x04d8, SDK_LOLLIPOP_MR1}, + {0x04f1, SDK_MARSHMALLOW}, + {0x0527, SDK_NOUGAT}, + {0x0530, SDK_NOUGAT_MR1}, + {0x0568, SDK_O}, }; -static bool less_entry_id(const std::pair& p, - uint16_t entryId) { +static bool less_entry_id(const std::pair& p, uint16_t entryId) { return p.first < entryId; } -size_t FindAttributeSdkLevel(const ResourceId& id) { - if (id.package_id() != 0x01 && id.type_id() != 0x01) { +ApiVersion FindAttributeSdkLevel(const ResourceId& id) { + if (id.package_id() != 0x01 || id.type_id() != 0x01) { return 0; } - auto iter = std::lower_bound(sAttrIdMap.begin(), sAttrIdMap.end(), - id.entry_id(), less_entry_id); + auto iter = std::lower_bound(sAttrIdMap.begin(), sAttrIdMap.end(), id.entry_id(), less_entry_id); if (iter == sAttrIdMap.end()) { return SDK_LOLLIPOP_MR1; } return iter->second; } -static const std::unordered_map sAttrMap = { +static const std::unordered_map sAttrMap = { {"marqueeRepeatLimit", 2}, {"windowNoDisplay", 3}, {"backgroundDimEnabled", 3}, @@ -729,7 +732,7 @@ static const std::unordered_map sAttrMap = { {"windowActivityTransitions", 21}, {"colorEdgeEffect", 21}}; -size_t FindAttributeSdkLevel(const ResourceName& name) { +ApiVersion FindAttributeSdkLevel(const ResourceName& name) { if (name.package != "android" && name.type != ResourceType::kAttr) { return 0; } @@ -741,9 +744,8 @@ size_t FindAttributeSdkLevel(const ResourceName& name) { return SDK_LOLLIPOP_MR1; } -std::pair GetDevelopmentSdkCodeNameAndVersion() { - return std::make_pair(StringPiece(sDevelopmentSdkCodeName), - sDevelopmentSdkLevel); +std::pair GetDevelopmentSdkCodeNameAndVersion() { + return std::make_pair(StringPiece(sDevelopmentSdkCodeName), sDevelopmentSdkLevel); } } // namespace aapt diff --git a/tools/aapt2/SdkConstants.h b/tools/aapt2/SdkConstants.h index c2ee2524c5e8b..e3745e8ce0dab 100644 --- a/tools/aapt2/SdkConstants.h +++ b/tools/aapt2/SdkConstants.h @@ -25,7 +25,9 @@ namespace aapt { -enum : int { +using ApiVersion = int; + +enum : ApiVersion { SDK_CUPCAKE = 3, SDK_DONUT = 4, SDK_ECLAIR = 5, @@ -49,12 +51,12 @@ enum : int { SDK_MARSHMALLOW = 23, SDK_NOUGAT = 24, SDK_NOUGAT_MR1 = 25, - SDK_O = 26, // STOPSHIP Replace with real version + SDK_O = 26, }; -size_t FindAttributeSdkLevel(const ResourceId& id); -size_t FindAttributeSdkLevel(const ResourceName& name); -std::pair GetDevelopmentSdkCodeNameAndVersion(); +ApiVersion FindAttributeSdkLevel(const ResourceId& id); +ApiVersion FindAttributeSdkLevel(const ResourceName& name); +std::pair GetDevelopmentSdkCodeNameAndVersion(); } // namespace aapt diff --git a/tools/aapt2/SdkConstants_test.cpp b/tools/aapt2/SdkConstants_test.cpp index 716d922fb5a06..61f4d71b7fb2c 100644 --- a/tools/aapt2/SdkConstants_test.cpp +++ b/tools/aapt2/SdkConstants_test.cpp @@ -21,18 +21,11 @@ namespace aapt { TEST(SdkConstantsTest, FirstAttributeIsSdk1) { - EXPECT_EQ(1u, FindAttributeSdkLevel(ResourceId(0x01010000))); + EXPECT_EQ(1, FindAttributeSdkLevel(ResourceId(0x01010000))); } -TEST(SdkConstantsTest, AllAttributesAfterLollipopAreLollipopMR1) { - EXPECT_EQ(SDK_LOLLIPOP, FindAttributeSdkLevel(ResourceId(0x010103f7))); - EXPECT_EQ(SDK_LOLLIPOP, FindAttributeSdkLevel(ResourceId(0x010104ce))); - - EXPECT_EQ(SDK_LOLLIPOP_MR1, FindAttributeSdkLevel(ResourceId(0x010104cf))); - EXPECT_EQ(SDK_LOLLIPOP_MR1, FindAttributeSdkLevel(ResourceId(0x010104d8))); - - EXPECT_EQ(SDK_LOLLIPOP_MR1, FindAttributeSdkLevel(ResourceId(0x010104d9))); - EXPECT_EQ(SDK_LOLLIPOP_MR1, FindAttributeSdkLevel(ResourceId(0x0101ffff))); +TEST(SdkConstantsTest, NonFrameworkAttributeIsSdk0) { + EXPECT_EQ(0, FindAttributeSdkLevel(ResourceId(0x7f010345))); } } // namespace aapt diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp index 8accfa84c0a6c..24a687cd4d864 100644 --- a/tools/aapt2/cmd/Link.cpp +++ b/tools/aapt2/cmd/Link.cpp @@ -50,6 +50,7 @@ #include "link/ManifestFixer.h" #include "link/ReferenceLinker.h" #include "link/TableMerger.h" +#include "link/XmlCompatVersioner.h" #include "optimize/ResourceDeduper.h" #include "optimize/VersionCollapser.h" #include "process/IResourceTableConsumer.h" @@ -247,25 +248,20 @@ class FeatureSplitSymbolTableDelegate : public DefaultSymbolTableDelegate { IAaptContext* context_; }; -static bool FlattenXml(xml::XmlResource* xml_res, const StringPiece& path, - Maybe max_sdk_level, bool keep_raw_values, IArchiveWriter* writer, - IAaptContext* context) { +static bool FlattenXml(IAaptContext* context, xml::XmlResource* xml_res, const StringPiece& path, + bool keep_raw_values, IArchiveWriter* writer) { BigBuffer buffer(1024); XmlFlattenerOptions options = {}; options.keep_raw_values = keep_raw_values; - options.max_sdk_level = max_sdk_level; XmlFlattener flattener(&buffer, options); if (!flattener.Consume(context, xml_res)) { return false; } if (context->IsVerbose()) { - DiagMessage msg; - msg << "writing " << path << " to archive"; - if (max_sdk_level) { - msg << " maxSdkLevel=" << max_sdk_level.value() << " keepRawValues=" << keep_raw_values; - } - context->GetDiagnostics()->Note(msg); + context->GetDiagnostics()->Note(DiagMessage(path) << "writing to archive (keep_raw_values=" + << (keep_raw_values ? "true" : "false") + << ")"); } io::BigBufferInputStream input_stream(&buffer); @@ -311,12 +307,33 @@ struct ResourceFileFlattenerOptions { std::unordered_set extensions_to_not_compress; }; +// A sampling of public framework resource IDs. +struct R { + struct attr { + enum : uint32_t { + paddingLeft = 0x010100d6u, + paddingRight = 0x010100d8u, + paddingHorizontal = 0x0101053du, + + paddingTop = 0x010100d7u, + paddingBottom = 0x010100d9u, + paddingVertical = 0x0101053eu, + + layout_marginLeft = 0x010100f7u, + layout_marginRight = 0x010100f9u, + layout_marginHorizontal = 0x0101053bu, + + layout_marginTop = 0x010100f8u, + layout_marginBottom = 0x010100fau, + layout_marginVertical = 0x0101053cu, + }; + }; +}; + class ResourceFileFlattener { public: ResourceFileFlattener(const ResourceFileFlattenerOptions& options, IAaptContext* context, - proguard::KeepSet* keep_set) - : options_(options), context_(context), keep_set_(keep_set) { - } + proguard::KeepSet* keep_set); bool Flatten(ResourceTable* table, IArchiveWriter* archive_writer); @@ -325,7 +342,7 @@ class ResourceFileFlattener { ConfigDescription config; // The entry this file came from. - const ResourceEntry* entry; + ResourceEntry* entry; // The file to copy as-is. io::IFile* file_to_copy; @@ -335,19 +352,72 @@ class ResourceFileFlattener { // The destination to write this file to. std::string dst_path; - bool skip_version = false; }; uint32_t GetCompressionFlags(const StringPiece& str); - bool LinkAndVersionXmlFile(ResourceTable* table, FileOperation* file_op, - std::queue* out_file_op_queue); + std::vector> LinkAndVersionXmlFile(ResourceTable* table, + FileOperation* file_op); ResourceFileFlattenerOptions options_; IAaptContext* context_; proguard::KeepSet* keep_set_; + XmlCompatVersioner::Rules rules_; }; +ResourceFileFlattener::ResourceFileFlattener(const ResourceFileFlattenerOptions& options, + IAaptContext* context, proguard::KeepSet* keep_set) + : options_(options), context_(context), keep_set_(keep_set) { + SymbolTable* symm = context_->GetExternalSymbols(); + + // Build up the rules for degrading newer attributes to older ones. + // NOTE(adamlesinski): These rules are hardcoded right now, but they should be + // generated from the attribute definitions themselves (b/62028956). + if (const SymbolTable::Symbol* s = symm->FindById(R::attr::paddingHorizontal)) { + std::vector replacements{ + {"paddingLeft", R::attr::paddingLeft, + Attribute(false, android::ResTable_map::TYPE_DIMENSION)}, + {"paddingRight", R::attr::paddingRight, + Attribute(false, android::ResTable_map::TYPE_DIMENSION)}, + }; + rules_[R::attr::paddingHorizontal] = + util::make_unique(std::move(replacements)); + } + + if (const SymbolTable::Symbol* s = symm->FindById(R::attr::paddingVertical)) { + std::vector replacements{ + {"paddingTop", R::attr::paddingTop, + Attribute(false, android::ResTable_map::TYPE_DIMENSION)}, + {"paddingBottom", R::attr::paddingBottom, + Attribute(false, android::ResTable_map::TYPE_DIMENSION)}, + }; + rules_[R::attr::paddingVertical] = + util::make_unique(std::move(replacements)); + } + + if (const SymbolTable::Symbol* s = symm->FindById(R::attr::layout_marginHorizontal)) { + std::vector replacements{ + {"layout_marginLeft", R::attr::layout_marginLeft, + Attribute(false, android::ResTable_map::TYPE_DIMENSION)}, + {"layout_marginRight", R::attr::layout_marginRight, + Attribute(false, android::ResTable_map::TYPE_DIMENSION)}, + }; + rules_[R::attr::layout_marginHorizontal] = + util::make_unique(std::move(replacements)); + } + + if (const SymbolTable::Symbol* s = symm->FindById(R::attr::layout_marginVertical)) { + std::vector replacements{ + {"layout_marginTop", R::attr::layout_marginTop, + Attribute(false, android::ResTable_map::TYPE_DIMENSION)}, + {"layout_marginBottom", R::attr::layout_marginBottom, + Attribute(false, android::ResTable_map::TYPE_DIMENSION)}, + }; + rules_[R::attr::layout_marginVertical] = + util::make_unique(std::move(replacements)); + } +} + uint32_t ResourceFileFlattener::GetCompressionFlags(const StringPiece& str) { if (options_.do_not_compress_anything) { return 0; @@ -369,8 +439,19 @@ static bool IsTransitionElement(const std::string& name) { name == "transitionManager"; } -bool ResourceFileFlattener::LinkAndVersionXmlFile(ResourceTable* table, FileOperation* file_op, - std::queue* out_file_op_queue) { +static bool IsVectorElement(const std::string& name) { + return name == "vector" || name == "animated-vector"; +} + +template +std::vector make_singleton_vec(T&& val) { + std::vector vec; + vec.emplace_back(std::forward(val)); + return vec; +} + +std::vector> ResourceFileFlattener::LinkAndVersionXmlFile( + ResourceTable* table, FileOperation* file_op) { xml::XmlResource* doc = file_op->xml_to_flatten.get(); const Source& src = doc->file.source; @@ -380,107 +461,60 @@ bool ResourceFileFlattener::LinkAndVersionXmlFile(ResourceTable* table, FileOper XmlReferenceLinker xml_linker; if (!xml_linker.Consume(context_, doc)) { - return false; + return {}; } if (options_.update_proguard_spec && !proguard::CollectProguardRules(src, doc, keep_set_)) { - return false; + return {}; } if (options_.no_xml_namespaces) { XmlNamespaceRemover namespace_remover; if (!namespace_remover.Consume(context_, doc)) { - return false; + return {}; } } - if (!options_.no_auto_version) { - if (options_.no_version_vectors) { - // Skip this if it is a vector or animated-vector. - xml::Element* el = xml::FindRootElement(doc); - if (el && el->namespace_uri.empty()) { - if (el->name == "vector" || el->name == "animated-vector") { - // We are NOT going to version this file. - file_op->skip_version = true; - return true; - } - } - } - if (options_.no_version_transitions) { - // Skip this if it is a transition resource. - xml::Element* el = xml::FindRootElement(doc); - if (el && el->namespace_uri.empty()) { - if (IsTransitionElement(el->name)) { - // We are NOT going to version this file. - file_op->skip_version = true; - return true; - } - } - } + if (options_.no_auto_version) { + return make_singleton_vec(std::move(file_op->xml_to_flatten)); + } - const ConfigDescription& config = file_op->config; - - // Find the first SDK level used that is higher than this defined config and - // not superseded by a lower or equal SDK level resource. - const int min_sdk_version = context_->GetMinSdkVersion(); - for (int sdk_level : xml_linker.sdk_levels()) { - if (sdk_level > min_sdk_version && sdk_level > config.sdkVersion) { - if (!ShouldGenerateVersionedResource(file_op->entry, config, sdk_level)) { - // If we shouldn't generate a versioned resource, stop checking. - break; - } - - ResourceFile versioned_file_desc = doc->file; - versioned_file_desc.config.sdkVersion = (uint16_t)sdk_level; - - FileOperation new_file_op; - new_file_op.xml_to_flatten = util::make_unique( - versioned_file_desc, StringPool{}, doc->root->Clone()); - new_file_op.config = versioned_file_desc.config; - new_file_op.entry = file_op->entry; - new_file_op.dst_path = - ResourceUtils::BuildResourceFileName(versioned_file_desc, context_->GetNameMangler()); - - if (context_->IsVerbose()) { - context_->GetDiagnostics()->Note(DiagMessage(versioned_file_desc.source) - << "auto-versioning resource from config '" << config - << "' -> '" << versioned_file_desc.config << "'"); - } - - bool added = table->AddFileReferenceAllowMangled( - versioned_file_desc.name, versioned_file_desc.config, versioned_file_desc.source, - new_file_op.dst_path, nullptr, context_->GetDiagnostics()); - if (!added) { - return false; - } - - out_file_op_queue->push(std::move(new_file_op)); - break; + if (options_.no_version_vectors || options_.no_version_transitions) { + // Skip this if it is a vector or animated-vector. + xml::Element* el = xml::FindRootElement(doc); + if (el && el->namespace_uri.empty()) { + if ((options_.no_version_vectors && IsVectorElement(el->name)) || + (options_.no_version_transitions && IsTransitionElement(el->name))) { + return make_singleton_vec(std::move(file_op->xml_to_flatten)); } } } - return true; + + const ConfigDescription& config = file_op->config; + ResourceEntry* entry = file_op->entry; + + XmlCompatVersioner xml_compat_versioner(&rules_); + const util::Range api_range{config.sdkVersion, + FindNextApiVersionForConfig(entry, config)}; + return xml_compat_versioner.Process(context_, doc, api_range); } -/** - * Do not insert or remove any resources while executing in this function. It - * will - * corrupt the iteration order. - */ bool ResourceFileFlattener::Flatten(ResourceTable* table, IArchiveWriter* archive_writer) { bool error = false; std::map, FileOperation> config_sorted_files; for (auto& pkg : table->packages) { for (auto& type : pkg->types) { - // Sort by config and name, so that we get better locality in the zip - // file. + // Sort by config and name, so that we get better locality in the zip file. config_sorted_files.clear(); std::queue file_operations; // Populate the queue with all files in the ResourceTable. for (auto& entry : type->entries) { for (auto& config_value : entry->values) { + // WARNING! Do not insert or remove any resources while executing in this scope. It will + // corrupt the iteration order. + FileReference* file_ref = ValueCast(config_value->value.get()); if (!file_ref) { continue; @@ -497,6 +531,7 @@ bool ResourceFileFlattener::Flatten(ResourceTable* table, IArchiveWriter* archiv file_op.entry = entry.get(); file_op.dst_path = *file_ref->path; file_op.config = config_value->config; + file_op.file_to_copy = file; const StringPiece src_path = file->GetSource().path; if (type->type != ResourceType::kRaw && @@ -518,68 +553,51 @@ bool ResourceFileFlattener::Flatten(ResourceTable* table, IArchiveWriter* archiv file_op.xml_to_flatten->file.config = config_value->config; file_op.xml_to_flatten->file.source = file_ref->GetSource(); file_op.xml_to_flatten->file.name = ResourceName(pkg->name, type->type, entry->name); - - // Enqueue the XML files to be processed. - file_operations.push(std::move(file_op)); - } else { - file_op.file_to_copy = file; - - // NOTE(adamlesinski): Explicitly construct a StringPiece here, or - // else we end up copying the string in the std::make_pair() method, - // then creating a StringPiece from the copy, which would cause us - // to end up referencing garbage in the map. - const StringPiece entry_name(entry->name); - config_sorted_files[std::make_pair(config_value->config, entry_name)] = - std::move(file_op); } + + // NOTE(adamlesinski): Explicitly construct a StringPiece here, or + // else we end up copying the string in the std::make_pair() method, + // then creating a StringPiece from the copy, which would cause us + // to end up referencing garbage in the map. + const StringPiece entry_name(entry->name); + config_sorted_files[std::make_pair(config_value->config, entry_name)] = + std::move(file_op); } } - // Now process the XML queue - for (; !file_operations.empty(); file_operations.pop()) { - FileOperation& file_op = file_operations.front(); - - if (!LinkAndVersionXmlFile(table, &file_op, &file_operations)) { - error = true; - continue; - } - - // NOTE(adamlesinski): Explicitly construct a StringPiece here, or else - // we end up copying the string in the std::make_pair() method, then - // creating a StringPiece from the copy, which would cause us to end up - // referencing garbage in the map. - const StringPiece entry_name(file_op.entry->name); - config_sorted_files[std::make_pair(file_op.config, entry_name)] = std::move(file_op); - } - - if (error) { - return false; - } - // Now flatten the sorted values. for (auto& map_entry : config_sorted_files) { const ConfigDescription& config = map_entry.first.first; - const FileOperation& file_op = map_entry.second; + FileOperation& file_op = map_entry.second; if (file_op.xml_to_flatten) { - Maybe max_sdk_level; - if (!options_.no_auto_version && !file_op.skip_version) { - max_sdk_level = std::max(std::max(config.sdkVersion, 1u), - context_->GetMinSdkVersion()); - } + std::vector> versioned_docs = + LinkAndVersionXmlFile(table, &file_op); + for (std::unique_ptr& doc : versioned_docs) { + std::string dst_path = file_op.dst_path; + if (doc->file.config != file_op.config) { + // Only add the new versioned configurations. + if (context_->IsVerbose()) { + context_->GetDiagnostics()->Note(DiagMessage(doc->file.source) + << "auto-versioning resource from config '" + << config << "' -> '" << doc->file.config << "'"); + } - bool result = FlattenXml(file_op.xml_to_flatten.get(), file_op.dst_path, max_sdk_level, - options_.keep_raw_values, archive_writer, context_); - if (!result) { - error = true; + dst_path = + ResourceUtils::BuildResourceFileName(doc->file, context_->GetNameMangler()); + bool result = table->AddFileReferenceAllowMangled(doc->file.name, doc->file.config, + doc->file.source, dst_path, nullptr, + context_->GetDiagnostics()); + if (!result) { + return false; + } + } + error |= !FlattenXml(context_, doc.get(), dst_path, options_.keep_raw_values, + archive_writer); } } else { - bool result = - io::CopyFileToArchive(context_, file_op.file_to_copy, file_op.dst_path, - GetCompressionFlags(file_op.dst_path), archive_writer); - if (!result) { - error = true; - } + error |= !io::CopyFileToArchive(context_, file_op.file_to_copy, file_op.dst_path, + GetCompressionFlags(file_op.dst_path), archive_writer); } } } @@ -1358,8 +1376,7 @@ class LinkCommand { bool WriteApk(IArchiveWriter* writer, proguard::KeepSet* keep_set, xml::XmlResource* manifest, ResourceTable* table) { const bool keep_raw_values = context_->GetPackageType() == PackageType::kStaticLib; - bool result = - FlattenXml(manifest, "AndroidManifest.xml", {}, keep_raw_values, writer, context_); + bool result = FlattenXml(context_, manifest, "AndroidManifest.xml", keep_raw_values, writer); if (!result) { return false; } diff --git a/tools/aapt2/cmd/Util.cpp b/tools/aapt2/cmd/Util.cpp index 14d426061e561..8741b7b678ecf 100644 --- a/tools/aapt2/cmd/Util.cpp +++ b/tools/aapt2/cmd/Util.cpp @@ -131,7 +131,7 @@ std::vector AdjustSplitConstraintsForMinSdk( } static xml::AaptAttribute CreateAttributeWithId(const ResourceId& id) { - return xml::AaptAttribute{id, Attribute(true)}; + return xml::AaptAttribute(Attribute(), id); } std::unique_ptr GenerateSplitManifest(const AppInfo& app_info, diff --git a/tools/aapt2/flatten/XmlFlattener.cpp b/tools/aapt2/flatten/XmlFlattener.cpp index e98d2d758df47..0711749d03789 100644 --- a/tools/aapt2/flatten/XmlFlattener.cpp +++ b/tools/aapt2/flatten/XmlFlattener.cpp @@ -194,16 +194,9 @@ class XmlFlattenerVisitor : public xml::Visitor { // Filter the attributes. for (xml::Attribute& attr : node->attributes) { - if (options_.max_sdk_level && attr.compiled_attribute && attr.compiled_attribute.value().id) { - size_t sdk_level = FindAttributeSdkLevel(attr.compiled_attribute.value().id.value()); - if (sdk_level > options_.max_sdk_level.value()) { - continue; - } + if (attr.namespace_uri != xml::kSchemaTools) { + filtered_attrs_.push_back(&attr); } - if (attr.namespace_uri == xml::kSchemaTools) { - continue; - } - filtered_attrs_.push_back(&attr); } if (filtered_attrs_.empty()) { diff --git a/tools/aapt2/flatten/XmlFlattener.h b/tools/aapt2/flatten/XmlFlattener.h index f5129fd40e996..87557f29f3bea 100644 --- a/tools/aapt2/flatten/XmlFlattener.h +++ b/tools/aapt2/flatten/XmlFlattener.h @@ -30,11 +30,6 @@ struct XmlFlattenerOptions { * Keep attribute raw string values along with typed values. */ bool keep_raw_values = false; - - /** - * If set, the max SDK level of attribute to flatten. All others are ignored. - */ - Maybe max_sdk_level; }; class XmlFlattener : public IXmlResourceConsumer { diff --git a/tools/aapt2/flatten/XmlFlattener_test.cpp b/tools/aapt2/flatten/XmlFlattener_test.cpp index cfa89bb4b09f9..f1e903f2151ec 100644 --- a/tools/aapt2/flatten/XmlFlattener_test.cpp +++ b/tools/aapt2/flatten/XmlFlattener_test.cpp @@ -149,31 +149,6 @@ TEST_F(XmlFlattenerTest, FlattenXmlWithNoCompiledAttributes) { ASSERT_EQ(android::ResXMLTree::END_DOCUMENT, tree.next()); } -TEST_F(XmlFlattenerTest, FlattenCompiledXmlAndStripSdk21) { - std::unique_ptr doc = test::BuildXmlDom(R"EOF( - )EOF"); - - XmlReferenceLinker linker; - ASSERT_TRUE(linker.Consume(context_.get(), doc.get())); - ASSERT_TRUE(linker.sdk_levels().count(17) == 1); - ASSERT_TRUE(linker.sdk_levels().count(21) == 1); - - android::ResXMLTree tree; - XmlFlattenerOptions options; - options.max_sdk_level = 17; - ASSERT_TRUE(Flatten(doc.get(), &tree, options)); - - while (tree.next() != android::ResXMLTree::START_TAG) { - ASSERT_NE(tree.getEventType(), android::ResXMLTree::BAD_DOCUMENT); - ASSERT_NE(tree.getEventType(), android::ResXMLTree::END_DOCUMENT); - } - - ASSERT_EQ(1u, tree.getAttributeCount()); - EXPECT_EQ(uint32_t(0x010103b3), tree.getAttributeNameResID(0)); -} - TEST_F(XmlFlattenerTest, FlattenCompiledXmlAndStripOnlyTools) { std::unique_ptr doc = test::BuildXmlDom(R"EOF( doc = - test::BuildXmlDom(""); + std::unique_ptr doc = test::BuildXmlDom(""); android::ResXMLTree tree; ASSERT_TRUE(Flatten(doc.get(), &tree)); @@ -251,8 +224,7 @@ TEST_F(XmlFlattenerTest, EmptyStringValueInAttributeIsNotNull) { } const StringPiece16 kPackage = u"package"; - ssize_t idx = - tree.indexOfAttribute(nullptr, 0, kPackage.data(), kPackage.size()); + ssize_t idx = tree.indexOfAttribute(nullptr, 0, kPackage.data(), kPackage.size()); ASSERT_GE(idx, 0); size_t len; diff --git a/tools/aapt2/integration-tests/AutoVersionTest/Android.mk b/tools/aapt2/integration-tests/AutoVersionTest/Android.mk new file mode 100644 index 0000000000000..012728f1c18f6 --- /dev/null +++ b/tools/aapt2/integration-tests/AutoVersionTest/Android.mk @@ -0,0 +1,23 @@ +# +# 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. +# + +LOCAL_PATH := $(call my-dir) + +include $(CLEAR_VARS) +LOCAL_USE_AAPT2 := true +LOCAL_PACKAGE_NAME := AaptAutoVersionTest +LOCAL_MODULE_TAGS := tests +include $(BUILD_PACKAGE) diff --git a/tools/aapt2/integration-tests/AutoVersionTest/AndroidManifest.xml b/tools/aapt2/integration-tests/AutoVersionTest/AndroidManifest.xml new file mode 100644 index 0000000000000..e66d709943a48 --- /dev/null +++ b/tools/aapt2/integration-tests/AutoVersionTest/AndroidManifest.xml @@ -0,0 +1,21 @@ + + + + + + + diff --git a/tools/aapt2/integration-tests/AutoVersionTest/res/layout-v21/layout3.xml b/tools/aapt2/integration-tests/AutoVersionTest/res/layout-v21/layout3.xml new file mode 100644 index 0000000000000..bfc5445b1e1b9 --- /dev/null +++ b/tools/aapt2/integration-tests/AutoVersionTest/res/layout-v21/layout3.xml @@ -0,0 +1,19 @@ + + + + + diff --git a/tools/aapt2/integration-tests/AutoVersionTest/res/layout/layout.xml b/tools/aapt2/integration-tests/AutoVersionTest/res/layout/layout.xml new file mode 100644 index 0000000000000..091a8ce2d7eda --- /dev/null +++ b/tools/aapt2/integration-tests/AutoVersionTest/res/layout/layout.xml @@ -0,0 +1,35 @@ + + + + + diff --git a/tools/aapt2/integration-tests/AutoVersionTest/res/layout/layout2.xml b/tools/aapt2/integration-tests/AutoVersionTest/res/layout/layout2.xml new file mode 100644 index 0000000000000..339337a2c9530 --- /dev/null +++ b/tools/aapt2/integration-tests/AutoVersionTest/res/layout/layout2.xml @@ -0,0 +1,47 @@ + + + + + + + + + + + + + + diff --git a/tools/aapt2/integration-tests/AutoVersionTest/res/layout/layout3.xml b/tools/aapt2/integration-tests/AutoVersionTest/res/layout/layout3.xml new file mode 100644 index 0000000000000..8025ce10aed03 --- /dev/null +++ b/tools/aapt2/integration-tests/AutoVersionTest/res/layout/layout3.xml @@ -0,0 +1,19 @@ + + + + + diff --git a/tools/aapt2/integration-tests/AutoVersionTest/res/values-v21/styles.xml b/tools/aapt2/integration-tests/AutoVersionTest/res/values-v21/styles.xml new file mode 100644 index 0000000000000..ff13faab01a3d --- /dev/null +++ b/tools/aapt2/integration-tests/AutoVersionTest/res/values-v21/styles.xml @@ -0,0 +1,28 @@ + + + + + + diff --git a/tools/aapt2/integration-tests/AutoVersionTest/res/values/styles.xml b/tools/aapt2/integration-tests/AutoVersionTest/res/values/styles.xml new file mode 100644 index 0000000000000..c4f09846ed4b8 --- /dev/null +++ b/tools/aapt2/integration-tests/AutoVersionTest/res/values/styles.xml @@ -0,0 +1,84 @@ + + + + + + + + diff --git a/tools/aapt2/link/AutoVersioner.cpp b/tools/aapt2/link/AutoVersioner.cpp index 77471ea5d0dad..f80c6e9b34d32 100644 --- a/tools/aapt2/link/AutoVersioner.cpp +++ b/tools/aapt2/link/AutoVersioner.cpp @@ -27,13 +27,15 @@ namespace aapt { -bool ShouldGenerateVersionedResource(const ResourceEntry* entry, - const ConfigDescription& config, - const int sdk_version_to_generate) { - // We assume the caller is trying to generate a version greater than the - // current configuration. +bool ShouldGenerateVersionedResource(const ResourceEntry* entry, const ConfigDescription& config, + const ApiVersion sdk_version_to_generate) { + // We assume the caller is trying to generate a version greater than the current configuration. CHECK(sdk_version_to_generate > config.sdkVersion); + return sdk_version_to_generate < FindNextApiVersionForConfig(entry, config); +} +ApiVersion FindNextApiVersionForConfig(const ResourceEntry* entry, + const ConfigDescription& config) { const auto end_iter = entry->values.end(); auto iter = entry->values.begin(); for (; iter != end_iter; ++iter) { @@ -46,26 +48,23 @@ bool ShouldGenerateVersionedResource(const ResourceEntry* entry, CHECK(iter != entry->values.end()); ++iter; - // The next configuration either only varies in sdkVersion, or it is - // completely different - // and therefore incompatible. If it is incompatible, we must generate the - // versioned resource. + // The next configuration either only varies in sdkVersion, or it is completely different + // and therefore incompatible. If it is incompatible, we must generate the versioned resource. - // NOTE: The ordering of configurations takes sdkVersion as higher precedence - // than other + // NOTE: The ordering of configurations takes sdkVersion as higher precedence than other // qualifiers, so we need to iterate through the entire list to be sure there // are no higher sdk level versions of this resource. ConfigDescription temp_config(config); for (; iter != end_iter; ++iter) { temp_config.sdkVersion = (*iter)->config.sdkVersion; if (temp_config == (*iter)->config) { - // The two configs are the same, check the sdk version. - return sdk_version_to_generate < (*iter)->config.sdkVersion; + // The two configs are the same, return the sdkVersion. + return (*iter)->config.sdkVersion; } } - // No match was found, so we should generate the versioned resource. - return true; + // Didn't find another config with a different sdk version, so return the highest possible value. + return std::numeric_limits::max(); } bool AutoVersioner::Consume(IAaptContext* context, ResourceTable* table) { @@ -86,7 +85,7 @@ bool AutoVersioner::Consume(IAaptContext* context, ResourceTable* table) { } if (Style* style = ValueCast