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