diff options
Diffstat (limited to 'tools')
113 files changed, 2646 insertions, 988 deletions
diff --git a/tools/aapt/SdkConstants.h b/tools/aapt/SdkConstants.h index bf56ec024699..b982d0d2ea63 100644 --- a/tools/aapt/SdkConstants.h +++ b/tools/aapt/SdkConstants.h @@ -43,6 +43,7 @@ enum { SDK_NOUGAT_MR1 = 25, SDK_O = 26, SDK_O_MR1 = 27, + SDK_P = 10000, // STOPSHIP Replace with the real version. }; #endif // H_AAPT_SDK_CONSTANTS diff --git a/tools/aapt2/Android.bp b/tools/aapt2/Android.bp index c6cfd3032762..43918da75ad4 100644 --- a/tools/aapt2/Android.bp +++ b/tools/aapt2/Android.bp @@ -102,6 +102,7 @@ cc_library_host_static { "link/XmlCompatVersioner.cpp", "link/XmlNamespaceRemover.cpp", "link/XmlReferenceLinker.cpp", + "optimize/MultiApkGenerator.cpp", "optimize/ResourceDeduper.cpp", "optimize/VersionCollapser.cpp", "process/SymbolTable.cpp", diff --git a/tools/aapt2/Debug.cpp b/tools/aapt2/Debug.cpp index 49ed7780f950..dba9cb5c2803 100644 --- a/tools/aapt2/Debug.cpp +++ b/tools/aapt2/Debug.cpp @@ -291,7 +291,13 @@ class XmlPrinter : public xml::Visitor { std::cerr << "(" << attr.compiled_attribute.value().id.value_or_default(ResourceId(0x0)) << ")"; } - std::cerr << "=" << attr.value << "\n"; + std::cerr << "="; + if (attr.compiled_value != nullptr) { + std::cerr << *attr.compiled_value; + } else { + std::cerr << attr.value; + } + std::cerr << "\n"; } prefix_ += " "; diff --git a/tools/aapt2/LoadedApk.cpp b/tools/aapt2/LoadedApk.cpp index 7e5efa15f61b..b80780e33dfa 100644 --- a/tools/aapt2/LoadedApk.cpp +++ b/tools/aapt2/LoadedApk.cpp @@ -58,14 +58,15 @@ std::unique_ptr<LoadedApk> LoadedApk::LoadApkFromPath(IAaptContext* context, bool LoadedApk::WriteToArchive(IAaptContext* context, const TableFlattenerOptions& options, IArchiveWriter* writer) { FilterChain empty; - return WriteToArchive(context, options, &empty, writer); + return WriteToArchive(context, table_.get(), options, &empty, writer); } -bool LoadedApk::WriteToArchive(IAaptContext* context, const TableFlattenerOptions& options, - FilterChain* filters, IArchiveWriter* writer) { +bool LoadedApk::WriteToArchive(IAaptContext* context, ResourceTable* split_table, + const TableFlattenerOptions& options, FilterChain* filters, + IArchiveWriter* writer) { std::set<std::string> referenced_resources; // List the files being referenced in the resource table. - for (auto& pkg : table_->packages) { + for (auto& pkg : split_table->packages) { for (auto& type : pkg->types) { for (auto& entry : type->entries) { for (auto& config_value : entry->values) { @@ -84,7 +85,7 @@ bool LoadedApk::WriteToArchive(IAaptContext* context, const TableFlattenerOption std::string path = file->GetSource().path; // The name of the path has the format "<zip-file-name>@<path-to-file>". - path = path.substr(path.find("@") + 1); + path = path.substr(path.find('@') + 1); // Skip resources that are not referenced if requested. if (path.find("res/") == 0 && referenced_resources.find(path) == referenced_resources.end()) { @@ -108,7 +109,7 @@ bool LoadedApk::WriteToArchive(IAaptContext* context, const TableFlattenerOption // TODO(adamlesinski): How to determine if there were sparse entries (and if to encode // with sparse entries) b/35389232. TableFlattener flattener(options, &buffer); - if (!flattener.Consume(context, table_.get())) { + if (!flattener.Consume(context, split_table)) { return false; } diff --git a/tools/aapt2/LoadedApk.h b/tools/aapt2/LoadedApk.h index 8aa9674aa2ed..dacd0c2130a9 100644 --- a/tools/aapt2/LoadedApk.h +++ b/tools/aapt2/LoadedApk.h @@ -47,16 +47,17 @@ class LoadedApk { * Writes the APK on disk at the given path, while also removing the resource * files that are not referenced in the resource table. */ - bool WriteToArchive(IAaptContext* context, const TableFlattenerOptions& options, - IArchiveWriter* writer); + virtual bool WriteToArchive(IAaptContext* context, const TableFlattenerOptions& options, + IArchiveWriter* writer); /** * Writes the APK on disk at the given path, while also removing the resource * files that are not referenced in the resource table. The provided filter * chain is applied to each entry in the APK file. */ - bool WriteToArchive(IAaptContext* context, const TableFlattenerOptions& options, - FilterChain* filters, IArchiveWriter* writer); + virtual bool WriteToArchive(IAaptContext* context, ResourceTable* split_table, + const TableFlattenerOptions& options, FilterChain* filters, + IArchiveWriter* writer); static std::unique_ptr<LoadedApk> LoadApkFromPath(IAaptContext* context, const android::StringPiece& path); diff --git a/tools/aapt2/NameMangler.h b/tools/aapt2/NameMangler.h index 1305a4cf0710..f1aad29a5c58 100644 --- a/tools/aapt2/NameMangler.h +++ b/tools/aapt2/NameMangler.h @@ -69,8 +69,7 @@ class NameMangler { * The mangled name should contain symbols that are illegal to define in XML, * so that there will never be name mangling collisions. */ - static std::string MangleEntry(const std::string& package, - const std::string& name) { + static std::string MangleEntry(const std::string& package, const std::string& name) { return package + "$" + name; } @@ -86,8 +85,8 @@ class NameMangler { } out_package->assign(out_name->data(), pivot); - out_name->assign(out_name->data() + pivot + 1, - out_name->size() - (pivot + 1)); + std::string new_name = out_name->substr(pivot + 1); + *out_name = std::move(new_name); return true; } diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp index 1c3ac2ad4f17..4cc60a8dbb07 100644 --- a/tools/aapt2/ResourceParser.cpp +++ b/tools/aapt2/ResourceParser.cpp @@ -392,6 +392,7 @@ bool ResourceParser::ParseResource(xml::XmlPullParser* parser, {"declare-styleable", std::mem_fn(&ResourceParser::ParseDeclareStyleable)}, {"integer-array", std::mem_fn(&ResourceParser::ParseIntegerArray)}, {"java-symbol", std::mem_fn(&ResourceParser::ParseSymbol)}, + {"overlayable", std::mem_fn(&ResourceParser::ParseOverlayable)}, {"plurals", std::mem_fn(&ResourceParser::ParsePlural)}, {"public", std::mem_fn(&ResourceParser::ParsePublic)}, {"public-group", std::mem_fn(&ResourceParser::ParsePublicGroup)}, @@ -498,7 +499,7 @@ bool ResourceParser::ParseResource(xml::XmlPullParser* parser, const auto bag_iter = elToBagMap.find(resource_type); if (bag_iter != elToBagMap.end()) { // Ensure we have a name (unless this is a <public-group>). - if (resource_type != "public-group") { + if (resource_type != "public-group" && resource_type != "overlayable") { if (!maybe_name) { diag_->Error(DiagMessage(out_resource->source) << "<" << parser->element_name() << "> missing 'name' attribute"); @@ -605,7 +606,7 @@ std::unique_ptr<Item> ResourceParser::ParseXml(xml::XmlPullParser* parser, if (processed_item) { // Fix up the reference. if (Reference* ref = ValueCast<Reference>(processed_item.get())) { - TransformReferenceFromNamespace(parser, "", ref); + ResolvePackage(parser, ref); } return processed_item; } @@ -690,6 +691,11 @@ bool ResourceParser::ParseString(xml::XmlPullParser* parser, bool ResourceParser::ParsePublic(xml::XmlPullParser* parser, ParsedResource* out_resource) { + if (out_resource->config != ConfigDescription::DefaultConfig()) { + diag_->Warn(DiagMessage(out_resource->source) + << "ignoring configuration '" << out_resource->config << "' for <public> tag"); + } + Maybe<StringPiece> maybe_type = xml::FindNonEmptyAttribute(parser, "type"); if (!maybe_type) { diag_->Error(DiagMessage(out_resource->source) @@ -726,8 +732,13 @@ bool ResourceParser::ParsePublic(xml::XmlPullParser* parser, return true; } -bool ResourceParser::ParsePublicGroup(xml::XmlPullParser* parser, - ParsedResource* out_resource) { +bool ResourceParser::ParsePublicGroup(xml::XmlPullParser* parser, ParsedResource* out_resource) { + if (out_resource->config != ConfigDescription::DefaultConfig()) { + diag_->Warn(DiagMessage(out_resource->source) + << "ignoring configuration '" << out_resource->config + << "' for <public-group> tag"); + } + Maybe<StringPiece> maybe_type = xml::FindNonEmptyAttribute(parser, "type"); if (!maybe_type) { diag_->Error(DiagMessage(out_resource->source) @@ -842,13 +853,83 @@ bool ResourceParser::ParseSymbolImpl(xml::XmlPullParser* parser, return true; } -bool ResourceParser::ParseSymbol(xml::XmlPullParser* parser, - ParsedResource* out_resource) { - if (ParseSymbolImpl(parser, out_resource)) { - out_resource->symbol_state = SymbolState::kPrivate; - return true; +bool ResourceParser::ParseSymbol(xml::XmlPullParser* parser, ParsedResource* out_resource) { + if (out_resource->config != ConfigDescription::DefaultConfig()) { + diag_->Warn(DiagMessage(out_resource->source) + << "ignoring configuration '" << out_resource->config << "' for <" + << parser->element_name() << "> tag"); } - return false; + + if (!ParseSymbolImpl(parser, out_resource)) { + return false; + } + + out_resource->symbol_state = SymbolState::kPrivate; + return true; +} + +bool ResourceParser::ParseOverlayable(xml::XmlPullParser* parser, ParsedResource* out_resource) { + if (out_resource->config != ConfigDescription::DefaultConfig()) { + diag_->Warn(DiagMessage(out_resource->source) + << "ignoring configuration '" << out_resource->config << "' for <overlayable> tag"); + } + + if (Maybe<StringPiece> maybe_policy = xml::FindNonEmptyAttribute(parser, "policy")) { + const StringPiece& policy = maybe_policy.value(); + if (policy != "system") { + diag_->Error(DiagMessage(out_resource->source) + << "<overlayable> has invalid policy '" << policy << "'"); + return false; + } + } + + bool error = false; + const size_t depth = parser->depth(); + while (xml::XmlPullParser::NextChildNode(parser, depth)) { + if (parser->event() != xml::XmlPullParser::Event::kStartElement) { + // Skip text/comments. + continue; + } + + const Source item_source = source_.WithLine(parser->line_number()); + const std::string& element_namespace = parser->element_namespace(); + const std::string& element_name = parser->element_name(); + if (element_namespace.empty() && element_name == "item") { + Maybe<StringPiece> maybe_name = xml::FindNonEmptyAttribute(parser, "name"); + if (!maybe_name) { + diag_->Error(DiagMessage(item_source) + << "<item> within an <overlayable> tag must have a 'name' attribute"); + error = true; + continue; + } + + Maybe<StringPiece> maybe_type = xml::FindNonEmptyAttribute(parser, "type"); + if (!maybe_type) { + diag_->Error(DiagMessage(item_source) + << "<item> within an <overlayable> tag must have a 'type' attribute"); + error = true; + continue; + } + + const ResourceType* type = ParseResourceType(maybe_type.value()); + if (type == nullptr) { + diag_->Error(DiagMessage(out_resource->source) + << "invalid resource type '" << maybe_type.value() + << "' in <item> within an <overlayable>"); + error = true; + continue; + } + + // TODO(b/64980941): Mark the symbol as overlayable and allow marking which entity can overlay + // the resource (system/app). + + xml::XmlPullParser::SkipCurrentElement(parser); + } else if (!ShouldIgnoreElement(element_namespace, element_name)) { + diag_->Error(DiagMessage(item_source) << ":" << element_name << ">"); + error = true; + } + } + return !error; } bool ResourceParser::ParseAddResource(xml::XmlPullParser* parser, @@ -1074,15 +1155,13 @@ bool ResourceParser::ParseStyleItem(xml::XmlPullParser* parser, Style* style) { return false; } - Maybe<Reference> maybe_key = - ResourceUtils::ParseXmlAttributeName(maybe_name.value()); + Maybe<Reference> maybe_key = ResourceUtils::ParseXmlAttributeName(maybe_name.value()); if (!maybe_key) { - diag_->Error(DiagMessage(source) << "invalid attribute name '" - << maybe_name.value() << "'"); + diag_->Error(DiagMessage(source) << "invalid attribute name '" << maybe_name.value() << "'"); return false; } - TransformReferenceFromNamespace(parser, "", &maybe_key.value()); + ResolvePackage(parser, &maybe_key.value()); maybe_key.value().SetSource(source); std::unique_ptr<Item> value = ParseXml(parser, 0, kAllowRawString); @@ -1091,8 +1170,7 @@ bool ResourceParser::ParseStyleItem(xml::XmlPullParser* parser, Style* style) { return false; } - style->entries.push_back( - Style::Entry{std::move(maybe_key.value()), std::move(value)}); + style->entries.push_back(Style::Entry{std::move(maybe_key.value()), std::move(value)}); return true; } @@ -1104,21 +1182,18 @@ bool ResourceParser::ParseStyle(const ResourceType type, xml::XmlPullParser* par Maybe<StringPiece> maybe_parent = xml::FindAttribute(parser, "parent"); if (maybe_parent) { - // If the parent is empty, we don't have a parent, but we also don't infer - // either. + // If the parent is empty, we don't have a parent, but we also don't infer either. if (!maybe_parent.value().empty()) { std::string err_str; - style->parent = ResourceUtils::ParseStyleParentReference( - maybe_parent.value(), &err_str); + style->parent = ResourceUtils::ParseStyleParentReference(maybe_parent.value(), &err_str); if (!style->parent) { diag_->Error(DiagMessage(out_resource->source) << err_str); return false; } - // Transform the namespace prefix to the actual package name, and mark the - // reference as + // Transform the namespace prefix to the actual package name, and mark the reference as // private if appropriate. - TransformReferenceFromNamespace(parser, "", &style->parent.value()); + ResolvePackage(parser, &style->parent.value()); } } else { @@ -1127,8 +1202,7 @@ bool ResourceParser::ParseStyle(const ResourceType type, xml::XmlPullParser* par size_t pos = style_name.find_last_of(u'.'); if (pos != std::string::npos) { style->parent_inferred = true; - style->parent = Reference( - ResourceName({}, ResourceType::kStyle, style_name.substr(0, pos))); + style->parent = Reference(ResourceName({}, ResourceType::kStyle, style_name.substr(0, pos))); } } @@ -1373,7 +1447,7 @@ bool ResourceParser::ParseDeclareStyleable(xml::XmlPullParser* parser, } Reference& child_ref = maybe_ref.value(); - xml::TransformReferenceFromNamespace(parser, "", &child_ref); + xml::ResolvePackage(parser, &child_ref); // Create the ParsedResource that will add the attribute to the table. ParsedResource child_resource; diff --git a/tools/aapt2/ResourceParser.h b/tools/aapt2/ResourceParser.h index 5631dc2ad29c..fb9dbd0cd0fd 100644 --- a/tools/aapt2/ResourceParser.h +++ b/tools/aapt2/ResourceParser.h @@ -91,6 +91,7 @@ class ResourceParser { bool ParsePublicGroup(xml::XmlPullParser* parser, ParsedResource* out_resource); bool ParseSymbolImpl(xml::XmlPullParser* parser, ParsedResource* out_resource); bool ParseSymbol(xml::XmlPullParser* parser, ParsedResource* out_resource); + bool ParseOverlayable(xml::XmlPullParser* parser, ParsedResource* out_resource); bool ParseAddResource(xml::XmlPullParser* parser, ParsedResource* out_resource); bool ParseAttr(xml::XmlPullParser* parser, ParsedResource* out_resource); bool ParseAttrImpl(xml::XmlPullParser* parser, ParsedResource* out_resource, bool weak); diff --git a/tools/aapt2/ResourceParser_test.cpp b/tools/aapt2/ResourceParser_test.cpp index 144ebd22e105..f08b03e7b8af 100644 --- a/tools/aapt2/ResourceParser_test.cpp +++ b/tools/aapt2/ResourceParser_test.cpp @@ -790,4 +790,49 @@ TEST_F(ResourceParserTest, ParsePlatformIndependentNewline) { ASSERT_TRUE(TestParse(R"(<string name="foo">%1$s %n %2$s</string>)")); } +TEST_F(ResourceParserTest, ParseOverlayableTagWithSystemPolicy) { + std::string input = R"( + <overlayable policy="illegal_policy"> + <item type="string" name="foo" /> + </overlayable>)"; + EXPECT_FALSE(TestParse(input)); + + input = R"( + <overlayable policy="system"> + <item name="foo" /> + </overlayable>)"; + EXPECT_FALSE(TestParse(input)); + + input = R"( + <overlayable policy="system"> + <item type="attr" /> + </overlayable>)"; + EXPECT_FALSE(TestParse(input)); + + input = R"( + <overlayable policy="system"> + <item type="bad_type" name="foo" /> + </overlayable>)"; + EXPECT_FALSE(TestParse(input)); + + input = R"(<overlayable policy="system" />)"; + EXPECT_TRUE(TestParse(input)); + + input = R"(<overlayable />)"; + EXPECT_TRUE(TestParse(input)); + + input = R"( + <overlayable policy="system"> + <item type="string" name="foo" /> + <item type="dimen" name="foo" /> + </overlayable>)"; + ASSERT_TRUE(TestParse(input)); + + input = R"( + <overlayable> + <item type="string" name="bar" /> + </overlayable>)"; + ASSERT_TRUE(TestParse(input)); +} + } // namespace aapt diff --git a/tools/aapt2/ResourceTable.cpp b/tools/aapt2/ResourceTable.cpp index ab59560d33a3..0304e21698df 100644 --- a/tools/aapt2/ResourceTable.cpp +++ b/tools/aapt2/ResourceTable.cpp @@ -546,4 +546,34 @@ Maybe<ResourceTable::SearchResult> ResourceTable::FindResource(const ResourceNam return SearchResult{package, type, entry}; } +std::unique_ptr<ResourceTable> ResourceTable::Clone() const { + std::unique_ptr<ResourceTable> new_table = util::make_unique<ResourceTable>(); + for (const auto& pkg : packages) { + ResourceTablePackage* new_pkg = new_table->CreatePackage(pkg->name, pkg->id); + for (const auto& type : pkg->types) { + ResourceTableType* new_type = new_pkg->FindOrCreateType(type->type); + if (!new_type->id) { + new_type->id = type->id; + new_type->symbol_status = type->symbol_status; + } + + for (const auto& entry : type->entries) { + ResourceEntry* new_entry = new_type->FindOrCreateEntry(entry->name); + if (!new_entry->id) { + new_entry->id = entry->id; + new_entry->symbol_status = entry->symbol_status; + } + + for (const auto& config_value : entry->values) { + ResourceConfigValue* new_value = + new_entry->FindOrCreateValue(config_value->config, config_value->product); + Value* value = config_value->value->Clone(&new_table->string_pool); + new_value->value = std::unique_ptr<Value>(value); + } + } + } + } + return new_table; +} + } // namespace aapt diff --git a/tools/aapt2/ResourceTable.h b/tools/aapt2/ResourceTable.h index 4295d0674774..d5db67e77f51 100644 --- a/tools/aapt2/ResourceTable.h +++ b/tools/aapt2/ResourceTable.h @@ -251,6 +251,8 @@ class ResourceTable { ResourceTablePackage* CreatePackage(const android::StringPiece& name, Maybe<uint8_t> id = {}); + std::unique_ptr<ResourceTable> Clone() const; + /** * The string pool used by this resource table. Values that reference strings * must use diff --git a/tools/aapt2/SdkConstants.h b/tools/aapt2/SdkConstants.h index 5c32ed4fd849..13584c0bf840 100644 --- a/tools/aapt2/SdkConstants.h +++ b/tools/aapt2/SdkConstants.h @@ -53,6 +53,7 @@ enum : ApiVersion { SDK_NOUGAT_MR1 = 25, SDK_O = 26, SDK_O_MR1 = 27, + SDK_P = 10000, // STOPSHIP Replace with the real version. }; ApiVersion FindAttributeSdkLevel(const ResourceId& id); diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp index d9e7ac6c1e08..3a2faa9f0368 100644 --- a/tools/aapt2/cmd/Link.cpp +++ b/tools/aapt2/cmd/Link.cpp @@ -457,7 +457,8 @@ std::vector<std::unique_ptr<xml::XmlResource>> ResourceFileFlattener::LinkAndVer const Source& src = doc->file.source; if (context_->IsVerbose()) { - context_->GetDiagnostics()->Note(DiagMessage() << "linking " << src.path); + context_->GetDiagnostics()->Note(DiagMessage() + << "linking " << src.path << " (" << doc->file.name << ")"); } XmlReferenceLinker xml_linker; @@ -505,6 +506,8 @@ bool ResourceFileFlattener::Flatten(ResourceTable* table, IArchiveWriter* archiv std::map<std::pair<ConfigDescription, StringPiece>, FileOperation> config_sorted_files; for (auto& pkg : table->packages) { + CHECK(!pkg->name.empty()) << "Packages must have names when being linked"; + for (auto& type : pkg->types) { // Sort by config and name, so that we get better locality in the zip file. config_sorted_files.clear(); @@ -701,7 +704,7 @@ class LinkCommand { util::make_unique<AssetManagerSymbolSource>(); for (const std::string& path : options_.include_paths) { if (context_->IsVerbose()) { - context_->GetDiagnostics()->Note(DiagMessage(path) << "loading include path"); + context_->GetDiagnostics()->Note(DiagMessage() << "including " << path); } // First try to load the file as a static lib. @@ -819,11 +822,9 @@ class LinkCommand { return app_info; } - /** - * Precondition: ResourceTable doesn't have any IDs assigned yet, nor is it linked. - * Postcondition: ResourceTable has only one package left. All others are - * stripped, or there is an error and false is returned. - */ + // Precondition: ResourceTable doesn't have any IDs assigned yet, nor is it linked. + // Postcondition: ResourceTable has only one package left. All others are + // stripped, or there is an error and false is returned. bool VerifyNoExternalPackages() { auto is_ext_package_func = [&](const std::unique_ptr<ResourceTablePackage>& pkg) -> bool { return context_->GetCompilationPackage() != pkg->name || !pkg->id || @@ -920,7 +921,7 @@ class LinkCommand { bool WriteJavaFile(ResourceTable* table, const StringPiece& package_name_to_generate, const StringPiece& out_package, const JavaClassGeneratorOptions& java_options, - const Maybe<std::string> out_text_symbols_path = {}) { + const Maybe<std::string>& out_text_symbols_path = {}) { if (!options_.generate_java_class_path) { return true; } @@ -965,7 +966,91 @@ class LinkCommand { context_->GetDiagnostics()->Error(DiagMessage() << "failed writing to '" << out_path << "': " << android::base::SystemErrorCodeToString(errno)); + return false; + } + return true; + } + + bool GenerateJavaClasses() { + // The set of packages whose R class to call in the main classes onResourcesLoaded callback. + std::vector<std::string> packages_to_callback; + + JavaClassGeneratorOptions template_options; + template_options.types = JavaClassGeneratorOptions::SymbolTypes::kAll; + template_options.javadoc_annotations = options_.javadoc_annotations; + + if (context_->GetPackageType() == PackageType::kStaticLib || options_.generate_non_final_ids) { + template_options.use_final = false; + } + + if (context_->GetPackageType() == PackageType::kSharedLib) { + template_options.use_final = false; + template_options.rewrite_callback_options = OnResourcesLoadedCallbackOptions{}; + } + + const StringPiece actual_package = context_->GetCompilationPackage(); + StringPiece output_package = context_->GetCompilationPackage(); + if (options_.custom_java_package) { + // Override the output java package to the custom one. + output_package = options_.custom_java_package.value(); + } + + // Generate the private symbols if required. + if (options_.private_symbols) { + packages_to_callback.push_back(options_.private_symbols.value()); + + // If we defined a private symbols package, we only emit Public symbols + // to the original package, and private and public symbols to the private package. + JavaClassGeneratorOptions options = template_options; + options.types = JavaClassGeneratorOptions::SymbolTypes::kPublicPrivate; + if (!WriteJavaFile(&final_table_, actual_package, options_.private_symbols.value(), + options)) { + return false; + } + } + + // Generate copies of the original package R class but with different package names. + // This is to support non-namespaced builds. + for (const std::string& extra_package : options_.extra_java_packages) { + packages_to_callback.push_back(extra_package); + + JavaClassGeneratorOptions options = template_options; + options.types = JavaClassGeneratorOptions::SymbolTypes::kAll; + if (!WriteJavaFile(&final_table_, actual_package, extra_package, options)) { + return false; + } + } + + // Generate R classes for each package that was merged (static library). + // Use the actual package's resources only. + for (const std::string& package : table_merger_->merged_packages()) { + packages_to_callback.push_back(package); + + JavaClassGeneratorOptions options = template_options; + options.types = JavaClassGeneratorOptions::SymbolTypes::kAll; + if (!WriteJavaFile(&final_table_, package, package, options)) { + return false; + } } + + // Generate the main public R class. + JavaClassGeneratorOptions options = template_options; + + // Only generate public symbols if we have a private package. + if (options_.private_symbols) { + options.types = JavaClassGeneratorOptions::SymbolTypes::kPublic; + } + + if (options.rewrite_callback_options) { + options.rewrite_callback_options.value().packages_to_callback = + std::move(packages_to_callback); + } + + if (!WriteJavaFile(&final_table_, actual_package, output_package, options, + options_.generate_text_symbols_path)) { + return false; + } + return true; } @@ -1097,15 +1182,17 @@ class LinkCommand { bool result; if (options_.no_static_lib_packages) { - // Merge all resources as if they were in the compilation package. This is - // the old behavior of aapt. + // Merge all resources as if they were in the compilation package. This is the old behavior + // of aapt. - // Add the package to the set of --extra-packages so we emit an R.java for - // each library package. + // Add the package to the set of --extra-packages so we emit an R.java for each library + // package. if (!pkg->name.empty()) { options_.extra_java_packages.insert(pkg->name); } + // Clear the package name, so as to make the resources look like they are coming from the + // local package. pkg->name = ""; if (override) { result = table_merger_->MergeOverlay(Source(input), table.get(), collection.get()); @@ -1673,8 +1760,7 @@ class LinkCommand { bool error = false; { - // AndroidManifest.xml has no resource name, but the CallSite is built - // from the name + // AndroidManifest.xml has no resource name, but the CallSite is built from the name // (aka, which package the AndroidManifest.xml is coming from). // So we give it a package name so it can see local resources. manifest_xml->file.name.package = context_->GetCompilationPackage(); @@ -1727,72 +1813,7 @@ class LinkCommand { } if (options_.generate_java_class_path) { - // The set of packages whose R class to call in the main classes - // onResourcesLoaded callback. - std::vector<std::string> packages_to_callback; - - JavaClassGeneratorOptions template_options; - template_options.types = JavaClassGeneratorOptions::SymbolTypes::kAll; - template_options.javadoc_annotations = options_.javadoc_annotations; - - if (context_->GetPackageType() == PackageType::kStaticLib || - options_.generate_non_final_ids) { - template_options.use_final = false; - } - - if (context_->GetPackageType() == PackageType::kSharedLib) { - template_options.use_final = false; - template_options.rewrite_callback_options = OnResourcesLoadedCallbackOptions{}; - } - - const StringPiece actual_package = context_->GetCompilationPackage(); - StringPiece output_package = context_->GetCompilationPackage(); - if (options_.custom_java_package) { - // Override the output java package to the custom one. - output_package = options_.custom_java_package.value(); - } - - // Generate the private symbols if required. - if (options_.private_symbols) { - packages_to_callback.push_back(options_.private_symbols.value()); - - // If we defined a private symbols package, we only emit Public symbols - // to the original package, and private and public symbols to the - // private package. - JavaClassGeneratorOptions options = template_options; - options.types = JavaClassGeneratorOptions::SymbolTypes::kPublicPrivate; - if (!WriteJavaFile(&final_table_, actual_package, options_.private_symbols.value(), - options)) { - return 1; - } - } - - // Generate all the symbols for all extra packages. - for (const std::string& extra_package : options_.extra_java_packages) { - packages_to_callback.push_back(extra_package); - - JavaClassGeneratorOptions options = template_options; - options.types = JavaClassGeneratorOptions::SymbolTypes::kAll; - if (!WriteJavaFile(&final_table_, actual_package, extra_package, options)) { - return 1; - } - } - - // Generate the main public R class. - JavaClassGeneratorOptions options = template_options; - - // Only generate public symbols if we have a private package. - if (options_.private_symbols) { - options.types = JavaClassGeneratorOptions::SymbolTypes::kPublic; - } - - if (options.rewrite_callback_options) { - options.rewrite_callback_options.value().packages_to_callback = - std::move(packages_to_callback); - } - - if (!WriteJavaFile(&final_table_, actual_package, output_package, options, - options_.generate_text_symbols_path)) { + if (!GenerateJavaClasses()) { return 1; } } diff --git a/tools/aapt2/cmd/Optimize.cpp b/tools/aapt2/cmd/Optimize.cpp index 9d71775889d4..33a1d3a704f1 100644 --- a/tools/aapt2/cmd/Optimize.cpp +++ b/tools/aapt2/cmd/Optimize.cpp @@ -18,6 +18,7 @@ #include <vector> #include "android-base/stringprintf.h" +#include "androidfw/ResourceTypes.h" #include "androidfw/StringPiece.h" #include "Diagnostics.h" @@ -33,15 +34,19 @@ #include "flatten/XmlFlattener.h" #include "io/BigBufferInputStream.h" #include "io/Util.h" +#include "optimize/MultiApkGenerator.h" #include "optimize/ResourceDeduper.h" #include "optimize/VersionCollapser.h" #include "split/TableSplitter.h" #include "util/Files.h" +#include "util/Util.h" using ::aapt::configuration::Abi; using ::aapt::configuration::Artifact; using ::aapt::configuration::PostProcessingConfiguration; +using ::android::ResTable_config; using ::android::StringPiece; +using ::android::base::StringAppendF; using ::android::base::StringPrintf; namespace aapt { @@ -188,42 +193,12 @@ class OptimizeCommand { } if (options_.configuration && options_.output_dir) { - PostProcessingConfiguration& config = options_.configuration.value(); - - // For now, just write out the stripped APK since ABI splitting doesn't modify anything else. - for (const Artifact& artifact : config.artifacts) { - if (artifact.abi_group) { - const std::string& group = artifact.abi_group.value(); - - auto abi_group = config.abi_groups.find(group); - // TODO: Remove validation when configuration parser ensures referential integrity. - if (abi_group == config.abi_groups.end()) { - context_->GetDiagnostics()->Note( - DiagMessage() << "could not find referenced ABI group '" << group << "'"); - return 1; - } - FilterChain filters; - filters.AddFilter(AbiFilter::FromAbiList(abi_group->second)); - - const std::string& path = apk->GetSource().path; - const StringPiece ext = file::GetExtension(path); - const std::string name = path.substr(0, path.rfind(ext.to_string())); - - // Name is hard coded for now since only one split dimension is supported. - // TODO: Incorporate name generation into the configuration objects. - const std::string file_name = - StringPrintf("%s.%s%s", name.c_str(), group.c_str(), ext.data()); - std::string out = options_.output_dir.value(); - file::AppendPath(&out, file_name); - - std::unique_ptr<IArchiveWriter> writer = - CreateZipFileArchiveWriter(context_->GetDiagnostics(), out); - - if (!apk->WriteToArchive(context_, options_.table_flattener_options, &filters, - writer.get())) { - return 1; - } - } + MultiApkGenerator generator{apk.get(), context_}; + MultiApkGeneratorOptions generator_options = {options_.output_dir.value(), + options_.configuration.value(), + options_.table_flattener_options}; + if (!generator.FromBaseApk(generator_options)) { + return 1; } } @@ -260,7 +235,7 @@ class OptimizeCommand { for (auto& entry : type->entries) { for (auto& config_value : entry->values) { - FileReference* file_ref = ValueCast<FileReference>(config_value->value.get()); + auto* file_ref = ValueCast<FileReference>(config_value->value.get()); if (file_ref == nullptr) { continue; } @@ -297,11 +272,8 @@ class OptimizeCommand { } io::BigBufferInputStream table_buffer_in(&table_buffer); - if (!io::CopyInputStreamToArchive(context_, &table_buffer_in, "resources.arsc", - ArchiveEntry::kAlign, writer)) { - return false; - } - return true; + return io::CopyInputStreamToArchive(context_, &table_buffer_in, "resources.arsc", + ArchiveEntry::kAlign, writer); } OptimizeOptions options_; @@ -349,20 +321,29 @@ int Optimize(const std::vector<StringPiece>& args) { OptimizeOptions options; Maybe<std::string> config_path; Maybe<std::string> target_densities; + Maybe<std::string> target_abis; std::vector<std::string> configs; std::vector<std::string> split_args; bool verbose = false; + bool print_only = false; Flags flags = Flags() .OptionalFlag("-o", "Path to the output APK.", &options.output_path) .OptionalFlag("-d", "Path to the output directory (for splits).", &options.output_dir) .OptionalFlag("-x", "Path to XML configuration file.", &config_path) + .OptionalSwitch("-p", "Print the multi APK artifacts and exit.", &print_only) .OptionalFlag( "--target-densities", "Comma separated list of the screen densities that the APK will be optimized for.\n" "All the resources that would be unused on devices of the given densities will be \n" "removed from the APK.", &target_densities) + .OptionalFlag( + "--target-abis", + "Comma separated list of the CPU ABIs that the APK will be optimized for.\n" + "All the native libraries that would be unused on devices of the given ABIs will \n" + "be removed from the APK.", + &target_abis) .OptionalFlagList("-c", "Comma separated list of configurations to include. The default\n" "is all configurations.", @@ -388,18 +369,19 @@ int Optimize(const std::vector<StringPiece>& args) { return 1; } - std::unique_ptr<LoadedApk> apk = LoadedApk::LoadApkFromPath(&context, flags.GetArgs()[0]); + const std::string& apk_path = flags.GetArgs()[0]; + std::unique_ptr<LoadedApk> apk = LoadedApk::LoadApkFromPath(&context, apk_path); if (!apk) { return 1; } context.SetVerbose(verbose); + IDiagnostics* diag = context.GetDiagnostics(); if (target_densities) { // Parse the target screen densities. for (const StringPiece& config_str : util::Tokenize(target_densities.value(), ',')) { - Maybe<uint16_t> target_density = - ParseTargetDensityParameter(config_str, context.GetDiagnostics()); + Maybe<uint16_t> target_density = ParseTargetDensityParameter(config_str, diag); if (!target_density) { return 1; } @@ -409,7 +391,7 @@ int Optimize(const std::vector<StringPiece>& args) { std::unique_ptr<IConfigFilter> filter; if (!configs.empty()) { - filter = ParseConfigFilterParameters(configs, context.GetDiagnostics()); + filter = ParseConfigFilterParameters(configs, diag); if (filter == nullptr) { return 1; } @@ -418,28 +400,47 @@ int Optimize(const std::vector<StringPiece>& args) { // Parse the split parameters. for (const std::string& split_arg : split_args) { - options.split_paths.push_back({}); - options.split_constraints.push_back({}); - if (!ParseSplitParameter(split_arg, context.GetDiagnostics(), &options.split_paths.back(), + options.split_paths.emplace_back(); + options.split_constraints.emplace_back(); + if (!ParseSplitParameter(split_arg, diag, &options.split_paths.back(), &options.split_constraints.back())) { return 1; } } if (config_path) { - if (!options.output_dir) { - context.GetDiagnostics()->Error( - DiagMessage() << "Output directory is required when using a configuration file"); - return 1; - } std::string& path = config_path.value(); Maybe<ConfigurationParser> for_path = ConfigurationParser::ForPath(path); if (for_path) { - options.configuration = for_path.value().WithDiagnostics(context.GetDiagnostics()).Parse(); + options.configuration = for_path.value().WithDiagnostics(diag).Parse(); } else { - context.GetDiagnostics()->Error(DiagMessage() << "Could not parse config file " << path); + diag->Error(DiagMessage() << "Could not parse config file " << path); return 1; } + + if (print_only) { + std::vector<std::string> names; + const PostProcessingConfiguration& config = options.configuration.value(); + if (!config.AllArtifactNames(file::GetFilename(apk_path), &names, diag)) { + diag->Error(DiagMessage() << "Failed to generate output artifact list"); + return 1; + } + + for (const auto& name : names) { + std::cout << name << std::endl; + } + return 0; + } + + // Since we know that we are going to process the APK (not just print targets), make sure we + // have somewhere to write them to. + if (!options.output_dir) { + diag->Error(DiagMessage() << "Output directory is required when using a configuration file"); + return 1; + } + } else if (print_only) { + diag->Error(DiagMessage() << "Asked to print artifacts without providing a configurations"); + return 1; } if (!ExtractAppDataFromManifest(&context, apk.get(), &options)) { diff --git a/tools/aapt2/compile/InlineXmlFormatParser.cpp b/tools/aapt2/compile/InlineXmlFormatParser.cpp index 857cdd5365a0..a17926067a0b 100644 --- a/tools/aapt2/compile/InlineXmlFormatParser.cpp +++ b/tools/aapt2/compile/InlineXmlFormatParser.cpp @@ -69,10 +69,10 @@ class Visitor : public xml::PackageAwareVisitor { // Use an empty string for the compilation package because we don't want to default to // the local package if the user specified name="style" or something. This should just // be the default namespace. - Maybe<xml::ExtractedPackage> maybe_pkg = TransformPackageAlias(name.package, {}); + Maybe<xml::ExtractedPackage> maybe_pkg = TransformPackageAlias(name.package); if (!maybe_pkg) { - context_->GetDiagnostics()->Error(DiagMessage(src) << "invalid namespace prefix '" - << name.package << "'"); + context_->GetDiagnostics()->Error(DiagMessage(src) + << "invalid namespace prefix '" << name.package << "'"); error_ = true; return; } diff --git a/tools/aapt2/configuration/ConfigurationParser.cpp b/tools/aapt2/configuration/ConfigurationParser.cpp index bdccf8bcae3a..9d6d3286f0ef 100644 --- a/tools/aapt2/configuration/ConfigurationParser.cpp +++ b/tools/aapt2/configuration/ConfigurationParser.cpp @@ -30,6 +30,7 @@ #include "io/File.h" #include "io/FileSystem.h" #include "io/StringInputStream.h" +#include "util/Files.h" #include "util/Maybe.h" #include "util/Util.h" #include "xml/XmlActionExecutor.h" @@ -58,6 +59,7 @@ using ::aapt::xml::XmlActionExecutor; using ::aapt::xml::XmlActionExecutorPolicy; using ::aapt::xml::XmlNodeAction; using ::android::base::ReadFileToString; +using ::android::StringPiece; const std::unordered_map<std::string, Abi> kStringToAbiMap = { {"armeabi", Abi::kArmeV6}, {"armeabi-v7a", Abi::kArmV7a}, {"arm64-v8a", Abi::kArm64V8a}, @@ -116,56 +118,141 @@ const std::string& AbiToString(Abi abi) { * success, or false if the either the placeholder is not found in the name, or the value is not * present and the placeholder was. */ -static bool ReplacePlaceholder(const std::string& placeholder, const Maybe<std::string>& value, +static bool ReplacePlaceholder(const StringPiece& placeholder, const Maybe<StringPiece>& value, std::string* name, IDiagnostics* diag) { - size_t offset = name->find(placeholder); - if (value) { - if (offset == std::string::npos) { + size_t offset = name->find(placeholder.data()); + bool found = (offset != std::string::npos); + + // Make sure the placeholder was present if the desired value is present. + if (!found) { + if (value) { diag->Error(DiagMessage() << "Missing placeholder for artifact: " << placeholder); return false; } - name->replace(offset, placeholder.length(), value.value()); return true; } + DCHECK(found) << "Missing return path for placeholder not found"; + // Make sure the placeholder was not present if the desired value was not present. - bool result = (offset == std::string::npos); - if (!result) { + if (!value) { diag->Error(DiagMessage() << "Placeholder present but no value for artifact: " << placeholder); + return false; + } + + name->replace(offset, placeholder.length(), value.value().data()); + + // Make sure there was only one instance of the placeholder. + if (name->find(placeholder.data()) != std::string::npos) { + diag->Error(DiagMessage() << "Placeholder present multiple times: " << placeholder); + return false; + } + return true; +} + +/** + * Returns the common artifact base name from a template string. + */ +Maybe<std::string> ToBaseName(std::string result, const StringPiece& apk_name, IDiagnostics* diag) { + const StringPiece ext = file::GetExtension(apk_name); + size_t end_index = apk_name.to_string().rfind(ext.to_string()); + const std::string base_name = + (end_index != std::string::npos) ? std::string{apk_name.begin(), end_index} : ""; + + // Base name is optional. + if (result.find("${basename}") != std::string::npos) { + Maybe<StringPiece> maybe_base_name = + base_name.empty() ? Maybe<StringPiece>{} : Maybe<StringPiece>{base_name}; + if (!ReplacePlaceholder("${basename}", maybe_base_name, &result, diag)) { + return {}; + } } + + // Extension is optional. + if (result.find("${ext}") != std::string::npos) { + // Make sure we disregard the '.' in the extension when replacing the placeholder. + if (!ReplacePlaceholder("${ext}", {ext.substr(1)}, &result, diag)) { + return {}; + } + } else { + // If no extension is specified, and the name template does not end in the current extension, + // add the existing extension. + if (!util::EndsWith(result, ext)) { + result.append(ext.to_string()); + } + } + return result; } -Maybe<std::string> Artifact::ToArtifactName(const std::string& format, IDiagnostics* diag) const { - std::string result = format; +Maybe<std::string> Artifact::ToArtifactName(const StringPiece& format, const StringPiece& apk_name, + IDiagnostics* diag) const { + Maybe<std::string> base = ToBaseName(format.to_string(), apk_name, diag); + if (!base) { + return {}; + } + std::string result = std::move(base.value()); - if (!ReplacePlaceholder("{abi}", abi_group, &result, diag)) { + if (!ReplacePlaceholder("${abi}", abi_group, &result, diag)) { return {}; } - if (!ReplacePlaceholder("{density}", screen_density_group, &result, diag)) { + if (!ReplacePlaceholder("${density}", screen_density_group, &result, diag)) { return {}; } - if (!ReplacePlaceholder("{locale}", locale_group, &result, diag)) { + if (!ReplacePlaceholder("${locale}", locale_group, &result, diag)) { return {}; } - if (!ReplacePlaceholder("{sdk}", android_sdk_group, &result, diag)) { + if (!ReplacePlaceholder("${sdk}", android_sdk_group, &result, diag)) { return {}; } - if (!ReplacePlaceholder("{feature}", device_feature_group, &result, diag)) { + if (!ReplacePlaceholder("${feature}", device_feature_group, &result, diag)) { return {}; } - if (!ReplacePlaceholder("{gl}", gl_texture_group, &result, diag)) { + if (!ReplacePlaceholder("${gl}", gl_texture_group, &result, diag)) { return {}; } return result; } +Maybe<std::string> Artifact::Name(const StringPiece& apk_name, IDiagnostics* diag) const { + if (!name) { + return {}; + } + + return ToBaseName(name.value(), apk_name, diag); +} + +bool PostProcessingConfiguration::AllArtifactNames(const StringPiece& apk_name, + std::vector<std::string>* artifact_names, + IDiagnostics* diag) const { + for (const auto& artifact : artifacts) { + Maybe<std::string> name; + if (artifact.name) { + name = artifact.Name(apk_name, diag); + } else { + if (!artifact_format) { + diag->Error(DiagMessage() << "No global artifact template and an artifact name is missing"); + return false; + } + name = artifact.ToArtifactName(artifact_format.value(), apk_name, diag); + } + + if (!name) { + return false; + } + + artifact_names->push_back(std::move(name.value())); + } + + return true; +} + } // namespace configuration /** Returns a ConfigurationParser for the file located at the provided path. */ @@ -333,7 +420,10 @@ ConfigurationParser::ActionHandler ConfigurationParser::screen_density_group_han if ((t = NodeCast<xml::Text>(node.get())) != nullptr) { ConfigDescription config_descriptor; const android::StringPiece& text = TrimWhitespace(t->text); - if (ConfigDescription::Parse(text, &config_descriptor)) { + bool parsed = ConfigDescription::Parse(text, &config_descriptor); + if (parsed && + (config_descriptor.CopyWithoutSdkVersion().diff(ConfigDescription::DefaultConfig()) == + android::ResTable_config::CONFIG_DENSITY)) { // Copy the density with the minimum SDK version stripped out. group.push_back(config_descriptor.CopyWithoutSdkVersion()); } else { @@ -366,17 +456,25 @@ ConfigurationParser::ActionHandler ConfigurationParser::locale_group_handler_ = << child->name); valid = false; } else { - Locale entry; - for (const auto& attr : child->attributes) { - if (attr.name == "lang") { - entry.lang = {attr.value}; - } else if (attr.name == "region") { - entry.region = {attr.value}; - } else { - diag->Warn(DiagMessage() << "Unknown attribute: " << attr.name << " = " << attr.value); + for (auto& node : child->children) { + xml::Text* t; + if ((t = NodeCast<xml::Text>(node.get())) != nullptr) { + ConfigDescription config_descriptor; + const android::StringPiece& text = TrimWhitespace(t->text); + bool parsed = ConfigDescription::Parse(text, &config_descriptor); + if (parsed && + (config_descriptor.CopyWithoutSdkVersion().diff(ConfigDescription::DefaultConfig()) == + android::ResTable_config::CONFIG_LOCALE)) { + // Copy the locale with the minimum SDK version stripped out. + group.push_back(config_descriptor.CopyWithoutSdkVersion()); + } else { + diag->Error(DiagMessage() + << "Could not parse config descriptor for screen-density: " << text); + valid = false; + } + break; } } - group.push_back(entry); } } @@ -390,8 +488,8 @@ ConfigurationParser::ActionHandler ConfigurationParser::android_sdk_group_handle return false; } - auto& group = config->android_sdk_groups[label]; bool valid = true; + bool found = false; for (auto* child : root_element->GetChildElements()) { if (child->name != "android-sdk") { @@ -422,7 +520,11 @@ ConfigurationParser::ActionHandler ConfigurationParser::android_sdk_group_handle } } - group.push_back(entry); + config->android_sdk_groups[label] = entry; + if (found) { + valid = false; + } + found = true; } } diff --git a/tools/aapt2/configuration/ConfigurationParser.h b/tools/aapt2/configuration/ConfigurationParser.h index 28c355e39643..9bc9081b8ae4 100644 --- a/tools/aapt2/configuration/ConfigurationParser.h +++ b/tools/aapt2/configuration/ConfigurationParser.h @@ -33,10 +33,14 @@ namespace configuration { template<class T> using Group = std::unordered_map<std::string, std::vector<T>>; +/** A mapping of group label to a single configuration item. */ +template <class T> +using Entry = std::unordered_map<std::string, T>; + /** Output artifact configuration options. */ struct Artifact { /** Name to use for output of processing foo.apk -> foo.<name>.apk. */ - std::string name; + Maybe<std::string> name; /** If present, uses the ABI group with this name. */ Maybe<std::string> abi_group; /** If present, uses the screen density group with this name. */ @@ -51,7 +55,11 @@ struct Artifact { Maybe<std::string> gl_texture_group; /** Convert an artifact name template into a name string based on configuration contents. */ - Maybe<std::string> ToArtifactName(const std::string& format, IDiagnostics* diag) const; + Maybe<std::string> ToArtifactName(const android::StringPiece& format, + const android::StringPiece& apk_name, IDiagnostics* diag) const; + + /** Convert an artifact name template into a name string based on configuration contents. */ + Maybe<std::string> Name(const android::StringPiece& apk_name, IDiagnostics* diag) const; }; /** Enumeration of currently supported ABIs. */ @@ -100,6 +108,12 @@ struct AndroidSdk { Maybe<std::string> max_sdk_version; Maybe<AndroidManifest> manifest; + static AndroidSdk ForMinSdk(std::string min_sdk) { + AndroidSdk sdk; + sdk.min_sdk_version = {std::move(min_sdk)}; + return sdk; + } + inline friend bool operator==(const AndroidSdk& lhs, const AndroidSdk& rhs) { return lhs.min_sdk_version == rhs.min_sdk_version && lhs.target_sdk_version == rhs.target_sdk_version && @@ -129,10 +143,14 @@ struct PostProcessingConfiguration { Group<Abi> abi_groups; Group<ConfigDescription> screen_density_groups; - Group<Locale> locale_groups; - Group<AndroidSdk> android_sdk_groups; + Group<ConfigDescription> locale_groups; + Entry<AndroidSdk> android_sdk_groups; Group<DeviceFeature> device_feature_groups; Group<GlTexture> gl_texture_groups; + + /** Helper method that generates a list of artifact names and returns true on success. */ + bool AllArtifactNames(const android::StringPiece& apk_name, + std::vector<std::string>* artifact_names, IDiagnostics* diag) const; }; } // namespace configuration diff --git a/tools/aapt2/configuration/ConfigurationParser_test.cpp b/tools/aapt2/configuration/ConfigurationParser_test.cpp index f89773720cc5..7ffb3d515079 100644 --- a/tools/aapt2/configuration/ConfigurationParser_test.cpp +++ b/tools/aapt2/configuration/ConfigurationParser_test.cpp @@ -64,24 +64,21 @@ constexpr const char* kValidConfig = R"(<?xml version="1.0" encoding="utf-8" ?> <screen-density>xxxhdpi</screen-density> </screen-density-group> <locale-group label="europe"> - <locale lang="en"/> - <locale lang="es"/> - <locale lang="fr"/> - <locale lang="de"/> + <locale>en</locale> + <locale>es</locale> + <locale>fr</locale> + <locale>de</locale> </locale-group> <locale-group label="north-america"> - <locale lang="en"/> - <locale lang="es" region="MX"/> - <locale lang="fr" region="CA"/> + <locale>en</locale> + <locale>es-rMX</locale> + <locale>fr-rCA</locale> </locale-group> - <locale-group label="all"> - <locale/> - </locale-group> - <android-sdk-group label="19"> + <android-sdk-group label="v19"> <android-sdk - minSdkVersion="19" - targetSdkVersion="24" - maxSdkVersion="25"> + minSdkVersion="v19" + targetSdkVersion="v24" + maxSdkVersion="v25"> <manifest> <!--- manifest additions here XSLT? TODO --> </manifest> @@ -105,7 +102,7 @@ constexpr const char* kValidConfig = R"(<?xml version="1.0" encoding="utf-8" ?> abi-group="arm" screen-density-group="large" locale-group="europe" - android-sdk-group="19" + android-sdk-group="v19" gl-texture-group="dxt1" device-feature-group="low-latency"/> <artifact @@ -113,7 +110,7 @@ constexpr const char* kValidConfig = R"(<?xml version="1.0" encoding="utf-8" ?> abi-group="other" screen-density-group="alldpi" locale-group="north-america" - android-sdk-group="19" + android-sdk-group="v19" gl-texture-group="dxt1" device-feature-group="low-latency"/> </artifacts> @@ -153,13 +150,13 @@ TEST_F(ConfigurationParserTest, ValidateFile) { EXPECT_EQ(3ul, value.screen_density_groups["large"].size()); EXPECT_EQ(6ul, value.screen_density_groups["alldpi"].size()); - EXPECT_EQ(3ul, value.locale_groups.size()); + EXPECT_EQ(2ul, value.locale_groups.size()); EXPECT_EQ(4ul, value.locale_groups["europe"].size()); EXPECT_EQ(3ul, value.locale_groups["north-america"].size()); - EXPECT_EQ(1ul, value.locale_groups["all"].size()); EXPECT_EQ(1ul, value.android_sdk_groups.size()); - EXPECT_EQ(1ul, value.android_sdk_groups["19"].size()); + EXPECT_TRUE(value.android_sdk_groups["v19"].min_sdk_version); + EXPECT_EQ("v19", value.android_sdk_groups["v19"].min_sdk_version.value()); EXPECT_EQ(1ul, value.gl_texture_groups.size()); EXPECT_EQ(1ul, value.gl_texture_groups["dxt1"].size()); @@ -182,7 +179,7 @@ TEST_F(ConfigurationParserTest, ArtifactAction) { abi-group="arm" screen-density-group="large" locale-group="europe" - android-sdk-group="19" + android-sdk-group="v19" gl-texture-group="dxt1" device-feature-group="low-latency"/>)xml"; @@ -195,11 +192,11 @@ TEST_F(ConfigurationParserTest, ArtifactAction) { EXPECT_EQ(1ul, config.artifacts.size()); auto& artifact = config.artifacts.front(); - EXPECT_EQ("", artifact.name); // TODO: make this fail. + EXPECT_FALSE(artifact.name); // TODO: make this fail. EXPECT_EQ("arm", artifact.abi_group.value()); EXPECT_EQ("large", artifact.screen_density_group.value()); EXPECT_EQ("europe", artifact.locale_group.value()); - EXPECT_EQ("19", artifact.android_sdk_group.value()); + EXPECT_EQ("v19", artifact.android_sdk_group.value()); EXPECT_EQ("dxt1", artifact.gl_texture_group.value()); EXPECT_EQ("low-latency", artifact.device_feature_group.value()); @@ -209,7 +206,7 @@ TEST_F(ConfigurationParserTest, ArtifactAction) { abi-group="other" screen-density-group="large" locale-group="europe" - android-sdk-group="19" + android-sdk-group="v19" gl-texture-group="dxt1" device-feature-group="low-latency"/>)xml"; doc = test::BuildXmlDom(second); @@ -295,10 +292,10 @@ TEST_F(ConfigurationParserTest, ScreenDensityGroupAction) { TEST_F(ConfigurationParserTest, LocaleGroupAction) { static constexpr const char* xml = R"xml( <locale-group label="europe"> - <locale lang="en"/> - <locale lang="es"/> - <locale lang="fr"/> - <locale lang="de"/> + <locale>en</locale> + <locale>es</locale> + <locale>fr</locale> + <locale>de</locale> </locale-group>)xml"; auto doc = test::BuildXmlDom(xml); @@ -310,27 +307,23 @@ TEST_F(ConfigurationParserTest, LocaleGroupAction) { ASSERT_EQ(1ul, config.locale_groups.size()); ASSERT_EQ(1u, config.locale_groups.count("europe")); - auto& out = config.locale_groups["europe"]; + const auto& out = config.locale_groups["europe"]; - Locale en; - en.lang = std::string("en"); - Locale es; - es.lang = std::string("es"); - Locale fr; - fr.lang = std::string("fr"); - Locale de; - de.lang = std::string("de"); + ConfigDescription en = test::ParseConfigOrDie("en"); + ConfigDescription es = test::ParseConfigOrDie("es"); + ConfigDescription fr = test::ParseConfigOrDie("fr"); + ConfigDescription de = test::ParseConfigOrDie("de"); ASSERT_THAT(out, ElementsAre(en, es, fr, de)); } TEST_F(ConfigurationParserTest, AndroidSdkGroupAction) { static constexpr const char* xml = R"xml( - <android-sdk-group label="19"> + <android-sdk-group label="v19"> <android-sdk - minSdkVersion="19" - targetSdkVersion="24" - maxSdkVersion="25"> + minSdkVersion="v19" + targetSdkVersion="v24" + maxSdkVersion="v25"> <manifest> <!--- manifest additions here XSLT? TODO --> </manifest> @@ -344,18 +337,17 @@ TEST_F(ConfigurationParserTest, AndroidSdkGroupAction) { ASSERT_TRUE(ok); ASSERT_EQ(1ul, config.android_sdk_groups.size()); - ASSERT_EQ(1u, config.android_sdk_groups.count("19")); + ASSERT_EQ(1u, config.android_sdk_groups.count("v19")); - auto& out = config.android_sdk_groups["19"]; + auto& out = config.android_sdk_groups["v19"]; AndroidSdk sdk; - sdk.min_sdk_version = std::string("19"); - sdk.target_sdk_version = std::string("24"); - sdk.max_sdk_version = std::string("25"); + sdk.min_sdk_version = std::string("v19"); + sdk.target_sdk_version = std::string("v24"); + sdk.max_sdk_version = std::string("v25"); sdk.manifest = AndroidManifest(); - ASSERT_EQ(1ul, out.size()); - ASSERT_EQ(sdk, out[0]); + ASSERT_EQ(sdk, out); } TEST_F(ConfigurationParserTest, GlTextureGroupAction) { @@ -415,21 +407,43 @@ TEST_F(ConfigurationParserTest, DeviceFeatureGroupAction) { ASSERT_THAT(out, ElementsAre(low_latency, pro)); } +// Artifact name parser test cases. + TEST(ArtifactTest, Simple) { StdErrDiagnostics diag; Artifact x86; x86.abi_group = {"x86"}; - auto x86_result = x86.ToArtifactName("something.{abi}.apk", &diag); + auto x86_result = x86.ToArtifactName("something.${abi}.apk", "", &diag); ASSERT_TRUE(x86_result); EXPECT_EQ(x86_result.value(), "something.x86.apk"); Artifact arm; arm.abi_group = {"armeabi-v7a"}; - auto arm_result = arm.ToArtifactName("app.{abi}.apk", &diag); - ASSERT_TRUE(arm_result); - EXPECT_EQ(arm_result.value(), "app.armeabi-v7a.apk"); + { + auto arm_result = arm.ToArtifactName("app.${abi}.apk", "", &diag); + ASSERT_TRUE(arm_result); + EXPECT_EQ(arm_result.value(), "app.armeabi-v7a.apk"); + } + + { + auto arm_result = arm.ToArtifactName("app.${abi}.apk", "different_name.apk", &diag); + ASSERT_TRUE(arm_result); + EXPECT_EQ(arm_result.value(), "app.armeabi-v7a.apk"); + } + + { + auto arm_result = arm.ToArtifactName("${basename}.${abi}.apk", "app.apk", &diag); + ASSERT_TRUE(arm_result); + EXPECT_EQ(arm_result.value(), "app.armeabi-v7a.apk"); + } + + { + auto arm_result = arm.ToArtifactName("app.${abi}.${ext}", "app.apk", &diag); + ASSERT_TRUE(arm_result); + EXPECT_EQ(arm_result.value(), "app.armeabi-v7a.apk"); + } } TEST(ArtifactTest, Complex) { @@ -440,12 +454,42 @@ TEST(ArtifactTest, Complex) { artifact.device_feature_group = {"df1"}; artifact.gl_texture_group = {"glx1"}; artifact.locale_group = {"en-AU"}; - artifact.android_sdk_group = {"26"}; - - auto result = - artifact.ToArtifactName("app.{density}_{locale}_{feature}_{gl}.sdk{sdk}.{abi}.apk", &diag); - ASSERT_TRUE(result); - EXPECT_EQ(result.value(), "app.ldpi_en-AU_df1_glx1.sdk26.mips64.apk"); + artifact.android_sdk_group = {"v26"}; + + { + auto result = artifact.ToArtifactName( + "app.${density}_${locale}_${feature}_${gl}.${sdk}.${abi}.apk", "", &diag); + ASSERT_TRUE(result); + EXPECT_EQ(result.value(), "app.ldpi_en-AU_df1_glx1.v26.mips64.apk"); + } + + { + auto result = artifact.ToArtifactName( + "app.${density}_${locale}_${feature}_${gl}.${sdk}.${abi}.apk", "app.apk", &diag); + ASSERT_TRUE(result); + EXPECT_EQ(result.value(), "app.ldpi_en-AU_df1_glx1.v26.mips64.apk"); + } + + { + auto result = artifact.ToArtifactName( + "${basename}.${density}_${locale}_${feature}_${gl}.${sdk}.${abi}.apk", "app.apk", &diag); + ASSERT_TRUE(result); + EXPECT_EQ(result.value(), "app.ldpi_en-AU_df1_glx1.v26.mips64.apk"); + } + + { + auto result = artifact.ToArtifactName( + "app.${density}_${locale}_${feature}_${gl}.${sdk}.${abi}.${ext}", "app.apk", &diag); + ASSERT_TRUE(result); + EXPECT_EQ(result.value(), "app.ldpi_en-AU_df1_glx1.v26.mips64.apk"); + } + + { + auto result = artifact.ToArtifactName( + "${basename}.${density}_${locale}_${feature}_${gl}.${sdk}.${abi}", "app.apk", &diag); + ASSERT_TRUE(result); + EXPECT_EQ(result.value(), "app.ldpi_en-AU_df1_glx1.v26.mips64.apk"); + } } TEST(ArtifactTest, Missing) { @@ -453,16 +497,69 @@ TEST(ArtifactTest, Missing) { Artifact x86; x86.abi_group = {"x86"}; - EXPECT_FALSE(x86.ToArtifactName("something.{density}.apk", &diag)); - EXPECT_FALSE(x86.ToArtifactName("something.apk", &diag)); + EXPECT_FALSE(x86.ToArtifactName("something.${density}.apk", "", &diag)); + EXPECT_FALSE(x86.ToArtifactName("something.apk", "", &diag)); + EXPECT_FALSE(x86.ToArtifactName("something.${density}.apk", "something.apk", &diag)); + EXPECT_FALSE(x86.ToArtifactName("something.apk", "something.apk", &diag)); } TEST(ArtifactTest, Empty) { StdErrDiagnostics diag; Artifact artifact; - EXPECT_FALSE(artifact.ToArtifactName("something.{density}.apk", &diag)); - EXPECT_TRUE(artifact.ToArtifactName("something.apk", &diag)); + EXPECT_FALSE(artifact.ToArtifactName("something.${density}.apk", "", &diag)); + EXPECT_TRUE(artifact.ToArtifactName("something.apk", "", &diag)); + EXPECT_FALSE(artifact.ToArtifactName("something.${density}.apk", "something.apk", &diag)); + EXPECT_TRUE(artifact.ToArtifactName("something.apk", "something.apk", &diag)); +} + +TEST(ArtifactTest, Repeated) { + StdErrDiagnostics diag; + Artifact artifact; + artifact.screen_density_group = {"mdpi"}; + + ASSERT_TRUE(artifact.ToArtifactName("something.${density}.apk", "", &diag)); + EXPECT_FALSE(artifact.ToArtifactName("something.${density}.${density}.apk", "", &diag)); + ASSERT_TRUE(artifact.ToArtifactName("something.${density}.apk", "something.apk", &diag)); +} + +TEST(ArtifactTest, Nesting) { + StdErrDiagnostics diag; + Artifact x86; + x86.abi_group = {"x86"}; + + EXPECT_FALSE(x86.ToArtifactName("something.${abi${density}}.apk", "", &diag)); + + const Maybe<std::string>& name = x86.ToArtifactName("something.${abi${abi}}.apk", "", &diag); + ASSERT_TRUE(name); + EXPECT_EQ(name.value(), "something.${abix86}.apk"); +} + +TEST(ArtifactTest, Recursive) { + StdErrDiagnostics diag; + Artifact artifact; + artifact.device_feature_group = {"${gl}"}; + artifact.gl_texture_group = {"glx1"}; + + EXPECT_FALSE(artifact.ToArtifactName("app.${feature}.${gl}.apk", "", &diag)); + + artifact.device_feature_group = {"df1"}; + artifact.gl_texture_group = {"${feature}"}; + { + const auto& result = artifact.ToArtifactName("app.${feature}.${gl}.apk", "", &diag); + ASSERT_TRUE(result); + EXPECT_EQ(result.value(), "app.df1.${feature}.apk"); + } + + // This is an invalid case, but should be the only possible case due to the ordering of + // replacement. + artifact.device_feature_group = {"${gl}"}; + artifact.gl_texture_group = {"glx1"}; + { + const auto& result = artifact.ToArtifactName("app.${feature}.apk", "", &diag); + ASSERT_TRUE(result); + EXPECT_EQ(result.value(), "app.glx1.apk"); + } } } // namespace diff --git a/tools/aapt2/filter/ConfigFilter.h b/tools/aapt2/filter/ConfigFilter.h index 3f1341684912..ebb81519dceb 100644 --- a/tools/aapt2/filter/ConfigFilter.h +++ b/tools/aapt2/filter/ConfigFilter.h @@ -38,13 +38,9 @@ class IConfigFilter { }; /** - * Implements config axis matching. An axis is one component of a configuration, - * like screen - * density or locale. If an axis is specified in the filter, and the axis is - * specified in - * the configuration to match, they must be compatible. Otherwise the - * configuration to match is - * accepted. + * Implements config axis matching. An axis is one component of a configuration, like screen density + * or locale. If an axis is specified in the filter, and the axis is specified in the configuration + * to match, they must be compatible. Otherwise the configuration to match is accepted. * * Used when handling "-c" options. */ diff --git a/tools/aapt2/filter/Filter_test.cpp b/tools/aapt2/filter/Filter_test.cpp index fb75a4b4d7c1..db2e69fc90d3 100644 --- a/tools/aapt2/filter/Filter_test.cpp +++ b/tools/aapt2/filter/Filter_test.cpp @@ -25,22 +25,16 @@ namespace aapt { namespace { -TEST(FilterChainTest, EmptyChain) { +TEST(FilterTest, FilterChain) { FilterChain chain; ASSERT_TRUE(chain.Keep("some/random/path")); -} -TEST(FilterChainTest, SingleFilter) { - FilterChain chain; chain.AddFilter(util::make_unique<PrefixFilter>("keep/")); ASSERT_FALSE(chain.Keep("removed/path")); ASSERT_TRUE(chain.Keep("keep/path/1")); ASSERT_TRUE(chain.Keep("keep/path/2")); -} -TEST(FilterChainTest, MultipleFilters) { - FilterChain chain; chain.AddFilter(util::make_unique<PrefixFilter>("keep/")); chain.AddFilter(util::make_unique<PrefixFilter>("keep/really/")); diff --git a/tools/aapt2/integration-tests/NamespaceTest/Android.mk b/tools/aapt2/integration-tests/NamespaceTest/Android.mk new file mode 100644 index 000000000000..6361f9b8ae7d --- /dev/null +++ b/tools/aapt2/integration-tests/NamespaceTest/Android.mk @@ -0,0 +1,2 @@ +LOCAL_PATH := $(call my-dir) +include $(call all-makefiles-under,$(LOCAL_PATH)) diff --git a/tools/aapt2/integration-tests/NamespaceTest/App/Android.mk b/tools/aapt2/integration-tests/NamespaceTest/App/Android.mk new file mode 100644 index 000000000000..6ed07b0c5c73 --- /dev/null +++ b/tools/aapt2/integration-tests/NamespaceTest/App/Android.mk @@ -0,0 +1,29 @@ +# +# 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_AAPT_NAMESPACES := true +LOCAL_PACKAGE_NAME := AaptTestNamespace_App +LOCAL_MODULE_TAGS := tests +LOCAL_SRC_FILES := $(call all-java-files-under,src) +LOCAL_STATIC_ANDROID_LIBRARIES := \ + AaptTestNamespace_LibOne \ + AaptTestNamespace_LibTwo +LOCAL_AAPT_FLAGS := -v +include $(BUILD_PACKAGE) diff --git a/tools/aapt2/integration-tests/NamespaceTest/App/AndroidManifest.xml b/tools/aapt2/integration-tests/NamespaceTest/App/AndroidManifest.xml new file mode 100644 index 000000000000..6398a83ea1d2 --- /dev/null +++ b/tools/aapt2/integration-tests/NamespaceTest/App/AndroidManifest.xml @@ -0,0 +1,30 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- 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. +--> + +<manifest xmlns:android="http://schemas.android.com/apk/res/android" + package="com.android.aapt.namespace.app"> + + <uses-sdk android:minSdkVersion="21" android:targetSdkVersion="26"/> + + <application android:theme="@style/AppTheme" android:label="@string/app_name"> + <activity android:name=".MainActivity"> + <intent-filter> + <action android:name="android.intent.action.MAIN" /> + <category android:name="android.intent.category.LAUNCHER" /> + </intent-filter> + </activity> + </application> +</manifest> diff --git a/tools/aapt2/integration-tests/NamespaceTest/App/res/layout/activity_main.xml b/tools/aapt2/integration-tests/NamespaceTest/App/res/layout/activity_main.xml new file mode 100644 index 000000000000..19bfd942a5bd --- /dev/null +++ b/tools/aapt2/integration-tests/NamespaceTest/App/res/layout/activity_main.xml @@ -0,0 +1,29 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- 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. +--> + +<RelativeLayout xmlns:android="http://schemas.android.com/apk/res/android" + xmlns:libone="http://schemas.android.com/apk/res/com.android.aapt.namespace.libone" + xmlns:libtwo="http://schemas.android.com/apk/res/com.android.aapt.namespace.libtwo" + android:layout_width="match_parent" + android:layout_height="match_parent"> + <com.android.aapt.namespace.libtwo.TextView + android:id="@+id/textview" + android:layout_width="wrap_content" + android:layout_height="wrap_content" + android:layout_centerInParent="@bool/always_true" + android:text="@libone:string/textview_text" + libtwo:textview_attr="?libone:theme_attr" /> +</RelativeLayout>
\ No newline at end of file diff --git a/tools/aapt2/integration-tests/NamespaceTest/App/res/values/values.xml b/tools/aapt2/integration-tests/NamespaceTest/App/res/values/values.xml new file mode 100644 index 000000000000..1b80d9542881 --- /dev/null +++ b/tools/aapt2/integration-tests/NamespaceTest/App/res/values/values.xml @@ -0,0 +1,29 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- 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. +--> + +<resources> + <string name="app_name">Namespace App</string> + <bool name="always_true">true</bool> + + <style name="AppTheme" parent="com.android.aapt.namespace.libone:style/Theme"> + <item name="android:colorPrimary">#3F51B5</item> + <item name="android:colorPrimaryDark">#303F9F</item> + <item name="android:colorAccent">#FF4081</item> + <item name="com.android.aapt.namespace.libone:theme_attr"> + @com.android.aapt.namespace.libtwo:string/public_string + </item> + </style> +</resources>
\ No newline at end of file diff --git a/tools/aapt2/integration-tests/NamespaceTest/App/src/com/android/aapt/namespace/app/MainActivity.java b/tools/aapt2/integration-tests/NamespaceTest/App/src/com/android/aapt/namespace/app/MainActivity.java new file mode 100644 index 000000000000..fcb4c3c12f81 --- /dev/null +++ b/tools/aapt2/integration-tests/NamespaceTest/App/src/com/android/aapt/namespace/app/MainActivity.java @@ -0,0 +1,34 @@ +/* + * 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. + */ +package com.android.aapt.namespace.app; + +import android.app.Activity; +import android.os.Bundle; +import android.widget.Toast; + +public class MainActivity extends Activity { + @Override + protected void onCreate(Bundle savedInstanceState) { + super.onCreate(savedInstanceState); + setContentView(R.layout.activity_main); + + com.android.aapt.namespace.libtwo.TextView tv = findViewById(R.id.textview); + + + + Toast.makeText(this, tv.getTextViewAttr(), Toast.LENGTH_LONG).show(); + } +} diff --git a/tools/aapt2/integration-tests/NamespaceTest/LibOne/Android.mk b/tools/aapt2/integration-tests/NamespaceTest/LibOne/Android.mk new file mode 100644 index 000000000000..b1cac68dae7a --- /dev/null +++ b/tools/aapt2/integration-tests/NamespaceTest/LibOne/Android.mk @@ -0,0 +1,28 @@ +# +# 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_AAPT_NAMESPACES := true +LOCAL_MODULE := AaptTestNamespace_LibOne +LOCAL_MODULE_TAGS := tests +LOCAL_RESOURCE_DIR := $(LOCAL_PATH)/res + +# We need this to retain the R.java generated for this library. +LOCAL_JAR_EXCLUDE_FILES := none +include $(BUILD_STATIC_JAVA_LIBRARY) diff --git a/tools/aapt2/integration-tests/NamespaceTest/LibOne/AndroidManifest.xml b/tools/aapt2/integration-tests/NamespaceTest/LibOne/AndroidManifest.xml new file mode 100644 index 000000000000..70b4b226624b --- /dev/null +++ b/tools/aapt2/integration-tests/NamespaceTest/LibOne/AndroidManifest.xml @@ -0,0 +1,21 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- 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. +--> + +<manifest xmlns:android="http://schemas.android.com/apk/res/android" + package="com.android.aapt.namespace.libone"> + + <uses-sdk android:minSdkVersion="21" /> +</manifest> diff --git a/tools/aapt2/integration-tests/NamespaceTest/LibOne/res/values/values.xml b/tools/aapt2/integration-tests/NamespaceTest/LibOne/res/values/values.xml new file mode 100644 index 000000000000..d2dcea0c0b1e --- /dev/null +++ b/tools/aapt2/integration-tests/NamespaceTest/LibOne/res/values/values.xml @@ -0,0 +1,28 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- 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. +--> + +<resources> + <public type="string" name="textview_text" /> + <string name="textview_text">LibOne\'s textview_text string!</string> + + <public type="attr" name="theme_attr" /> + <attr name="theme_attr" format="string" /> + + <public type="style" name="Theme" /> + <style name="Theme" parent="android:Theme.Material.Light.DarkActionBar"> + <item name="theme_attr">[Please override with your own value]</item> + </style> +</resources>
\ No newline at end of file diff --git a/tools/aapt2/integration-tests/NamespaceTest/LibTwo/Android.mk b/tools/aapt2/integration-tests/NamespaceTest/LibTwo/Android.mk new file mode 100644 index 000000000000..dc16d1bbb420 --- /dev/null +++ b/tools/aapt2/integration-tests/NamespaceTest/LibTwo/Android.mk @@ -0,0 +1,29 @@ +# +# 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_AAPT_NAMESPACES := true +LOCAL_MODULE := AaptTestNamespace_LibTwo +LOCAL_MODULE_TAGS := tests +LOCAL_SRC_FILES := $(call all-java-files-under,src) +LOCAL_RESOURCE_DIR := $(LOCAL_PATH)/res + +# We need this to retain the R.java generated for this library. +LOCAL_JAR_EXCLUDE_FILES := none +include $(BUILD_STATIC_JAVA_LIBRARY) diff --git a/tools/aapt2/integration-tests/NamespaceTest/LibTwo/AndroidManifest.xml b/tools/aapt2/integration-tests/NamespaceTest/LibTwo/AndroidManifest.xml new file mode 100644 index 000000000000..32944a9b8f0b --- /dev/null +++ b/tools/aapt2/integration-tests/NamespaceTest/LibTwo/AndroidManifest.xml @@ -0,0 +1,21 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- 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. +--> + +<manifest xmlns:android="http://schemas.android.com/apk/res/android" + package="com.android.aapt.namespace.libtwo"> + + <uses-sdk android:minSdkVersion="21" /> +</manifest> diff --git a/tools/aapt2/integration-tests/NamespaceTest/LibTwo/res/values/values.xml b/tools/aapt2/integration-tests/NamespaceTest/LibTwo/res/values/values.xml new file mode 100644 index 000000000000..0c5f5d8d55e0 --- /dev/null +++ b/tools/aapt2/integration-tests/NamespaceTest/LibTwo/res/values/values.xml @@ -0,0 +1,27 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- 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. +--> + +<resources> + <public type="string" name="public_string" /> + <string name="public_string">LibTwo\'s public string!</string> + + <public type="attr" name="textview_attr" /> + <attr name="textview_attr" format="string" /> + + <declare-styleable name="TextView"> + <attr name="textview_attr" /> + </declare-styleable> +</resources>
\ No newline at end of file diff --git a/tools/aapt2/integration-tests/NamespaceTest/LibTwo/src/com/android/aapt/namespace/libtwo/TextView.java b/tools/aapt2/integration-tests/NamespaceTest/LibTwo/src/com/android/aapt/namespace/libtwo/TextView.java new file mode 100644 index 000000000000..0f8024e58797 --- /dev/null +++ b/tools/aapt2/integration-tests/NamespaceTest/LibTwo/src/com/android/aapt/namespace/libtwo/TextView.java @@ -0,0 +1,53 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.android.aapt.namespace.libtwo; + +import android.content.Context; +import android.content.res.TypedArray; +import android.util.AttributeSet; + +public class TextView extends android.widget.TextView { + + private String mTextViewAttr; + + public TextView(Context context) { + this(context, null); + } + + public TextView(Context context, AttributeSet attrs) { + this(context, attrs, 0); + } + + public TextView(Context context, AttributeSet attrs, int defStyleAttr) { + this(context, attrs, defStyleAttr, 0); + } + + public TextView(Context context, AttributeSet attrs, int defStyleAttr, int defStyleRes) { + super(context, attrs, defStyleAttr, defStyleRes); + + final TypedArray ta = context.getTheme().obtainStyledAttributes(attrs, R.styleable.TextView, + 0, 0); + try { + mTextViewAttr = ta.getString(R.styleable.TextView_textview_attr); + } finally { + ta.recycle(); + } + } + + public String getTextViewAttr() { + return mTextViewAttr; + } +} diff --git a/tools/aapt2/integration-tests/StaticLibTest/Android.mk b/tools/aapt2/integration-tests/StaticLibTest/Android.mk new file mode 100644 index 000000000000..6361f9b8ae7d --- /dev/null +++ b/tools/aapt2/integration-tests/StaticLibTest/Android.mk @@ -0,0 +1,2 @@ +LOCAL_PATH := $(call my-dir) +include $(call all-makefiles-under,$(LOCAL_PATH)) diff --git a/tools/aapt2/integration-tests/AppOne/Android.mk b/tools/aapt2/integration-tests/StaticLibTest/App/Android.mk index 38bd5b5e3275..4d0c01d565a5 100644 --- a/tools/aapt2/integration-tests/AppOne/Android.mk +++ b/tools/aapt2/integration-tests/StaticLibTest/App/Android.mk @@ -18,12 +18,12 @@ LOCAL_PATH := $(call my-dir) include $(CLEAR_VARS) LOCAL_USE_AAPT2 := true -LOCAL_PACKAGE_NAME := AaptTestAppOne +LOCAL_PACKAGE_NAME := AaptTestStaticLib_App LOCAL_MODULE_TAGS := tests LOCAL_SRC_FILES := $(call all-java-files-under,src) LOCAL_ASSET_DIR := $(LOCAL_PATH)/assets $(LOCAL_PATH)/assets2 LOCAL_STATIC_ANDROID_LIBRARIES := \ - AaptTestStaticLibOne \ - AaptTestStaticLibTwo + AaptTestStaticLib_LibOne \ + AaptTestStaticLib_LibTwo LOCAL_AAPT_FLAGS := --no-version-vectors --no-version-transitions include $(BUILD_PACKAGE) diff --git a/tools/aapt2/integration-tests/AppOne/AndroidManifest.xml b/tools/aapt2/integration-tests/StaticLibTest/App/AndroidManifest.xml index a5f202dd22fc..a5f202dd22fc 100644 --- a/tools/aapt2/integration-tests/AppOne/AndroidManifest.xml +++ b/tools/aapt2/integration-tests/StaticLibTest/App/AndroidManifest.xml diff --git a/tools/aapt2/integration-tests/AppOne/assets/subdir/subsubdir/test.txt b/tools/aapt2/integration-tests/StaticLibTest/App/assets/subdir/subsubdir/test.txt index 125194943ec8..125194943ec8 100644 --- a/tools/aapt2/integration-tests/AppOne/assets/subdir/subsubdir/test.txt +++ b/tools/aapt2/integration-tests/StaticLibTest/App/assets/subdir/subsubdir/test.txt diff --git a/tools/aapt2/integration-tests/AppOne/assets/test.txt b/tools/aapt2/integration-tests/StaticLibTest/App/assets/test.txt index 88266de2b4d4..88266de2b4d4 100644 --- a/tools/aapt2/integration-tests/AppOne/assets/test.txt +++ b/tools/aapt2/integration-tests/StaticLibTest/App/assets/test.txt diff --git a/tools/aapt2/integration-tests/AppOne/assets2/new.txt b/tools/aapt2/integration-tests/StaticLibTest/App/assets2/new.txt index f4963a95503a..f4963a95503a 100644 --- a/tools/aapt2/integration-tests/AppOne/assets2/new.txt +++ b/tools/aapt2/integration-tests/StaticLibTest/App/assets2/new.txt diff --git a/tools/aapt2/integration-tests/AppOne/assets2/test.txt b/tools/aapt2/integration-tests/StaticLibTest/App/assets2/test.txt index 5d8b36c0d52d..5d8b36c0d52d 100644 --- a/tools/aapt2/integration-tests/AppOne/assets2/test.txt +++ b/tools/aapt2/integration-tests/StaticLibTest/App/assets2/test.txt diff --git a/tools/aapt2/integration-tests/AppOne/res/drawable/cheap_transparency.png b/tools/aapt2/integration-tests/StaticLibTest/App/res/drawable/cheap_transparency.png Binary files differindex 0522a9979db9..0522a9979db9 100644 --- a/tools/aapt2/integration-tests/AppOne/res/drawable/cheap_transparency.png +++ b/tools/aapt2/integration-tests/StaticLibTest/App/res/drawable/cheap_transparency.png diff --git a/tools/aapt2/integration-tests/AppOne/res/drawable/complex.9.png b/tools/aapt2/integration-tests/StaticLibTest/App/res/drawable/complex.9.png Binary files differindex baf9fff13ab5..baf9fff13ab5 100644 --- a/tools/aapt2/integration-tests/AppOne/res/drawable/complex.9.png +++ b/tools/aapt2/integration-tests/StaticLibTest/App/res/drawable/complex.9.png diff --git a/tools/aapt2/integration-tests/AppOne/res/drawable/icon.png b/tools/aapt2/integration-tests/StaticLibTest/App/res/drawable/icon.png Binary files differindex 4bff9b900643..4bff9b900643 100644 --- a/tools/aapt2/integration-tests/AppOne/res/drawable/icon.png +++ b/tools/aapt2/integration-tests/StaticLibTest/App/res/drawable/icon.png diff --git a/tools/aapt2/integration-tests/AppOne/res/drawable/image.xml b/tools/aapt2/integration-tests/StaticLibTest/App/res/drawable/image.xml index 6132a75d85d0..6132a75d85d0 100644 --- a/tools/aapt2/integration-tests/AppOne/res/drawable/image.xml +++ b/tools/aapt2/integration-tests/StaticLibTest/App/res/drawable/image.xml diff --git a/tools/aapt2/integration-tests/AppOne/res/drawable/outline_8x8.9.png b/tools/aapt2/integration-tests/StaticLibTest/App/res/drawable/outline_8x8.9.png Binary files differindex 7b331e16fcbd..7b331e16fcbd 100644 --- a/tools/aapt2/integration-tests/AppOne/res/drawable/outline_8x8.9.png +++ b/tools/aapt2/integration-tests/StaticLibTest/App/res/drawable/outline_8x8.9.png diff --git a/tools/aapt2/integration-tests/AppOne/res/drawable/round_rect_off_center_outline_32x16.9.png b/tools/aapt2/integration-tests/StaticLibTest/App/res/drawable/round_rect_off_center_outline_32x16.9.png Binary files differindex 0ec6c70a2b9f..0ec6c70a2b9f 100644 --- a/tools/aapt2/integration-tests/AppOne/res/drawable/round_rect_off_center_outline_32x16.9.png +++ b/tools/aapt2/integration-tests/StaticLibTest/App/res/drawable/round_rect_off_center_outline_32x16.9.png diff --git a/tools/aapt2/integration-tests/AppOne/res/drawable/round_rect_outline_32x16.9.png b/tools/aapt2/integration-tests/StaticLibTest/App/res/drawable/round_rect_outline_32x16.9.png Binary files differindex e05708a089a3..e05708a089a3 100644 --- a/tools/aapt2/integration-tests/AppOne/res/drawable/round_rect_outline_32x16.9.png +++ b/tools/aapt2/integration-tests/StaticLibTest/App/res/drawable/round_rect_outline_32x16.9.png diff --git a/tools/aapt2/integration-tests/AppOne/res/drawable/test.9.png b/tools/aapt2/integration-tests/StaticLibTest/App/res/drawable/test.9.png Binary files differindex 33daa117ea9d..33daa117ea9d 100644 --- a/tools/aapt2/integration-tests/AppOne/res/drawable/test.9.png +++ b/tools/aapt2/integration-tests/StaticLibTest/App/res/drawable/test.9.png diff --git a/tools/aapt2/integration-tests/AppOne/res/drawable/transparent_3x3.9.png b/tools/aapt2/integration-tests/StaticLibTest/App/res/drawable/transparent_3x3.9.png Binary files differindex a11377a0d670..a11377a0d670 100644 --- a/tools/aapt2/integration-tests/AppOne/res/drawable/transparent_3x3.9.png +++ b/tools/aapt2/integration-tests/StaticLibTest/App/res/drawable/transparent_3x3.9.png diff --git a/tools/aapt2/integration-tests/AppOne/res/drawable/transparent_optical_bounds_3x3.9.png b/tools/aapt2/integration-tests/StaticLibTest/App/res/drawable/transparent_optical_bounds_3x3.9.png Binary files differindex 6803e4243484..6803e4243484 100644 --- a/tools/aapt2/integration-tests/AppOne/res/drawable/transparent_optical_bounds_3x3.9.png +++ b/tools/aapt2/integration-tests/StaticLibTest/App/res/drawable/transparent_optical_bounds_3x3.9.png diff --git a/tools/aapt2/integration-tests/AppOne/res/drawable/white_3x3.9.png b/tools/aapt2/integration-tests/StaticLibTest/App/res/drawable/white_3x3.9.png Binary files differindex 1a3731bbc8b8..1a3731bbc8b8 100644 --- a/tools/aapt2/integration-tests/AppOne/res/drawable/white_3x3.9.png +++ b/tools/aapt2/integration-tests/StaticLibTest/App/res/drawable/white_3x3.9.png diff --git a/tools/aapt2/integration-tests/AppOne/res/drawable/white_optical_bounds_3x3.9.png b/tools/aapt2/integration-tests/StaticLibTest/App/res/drawable/white_optical_bounds_3x3.9.png Binary files differindex 489ace292e1f..489ace292e1f 100644 --- a/tools/aapt2/integration-tests/AppOne/res/drawable/white_optical_bounds_3x3.9.png +++ b/tools/aapt2/integration-tests/StaticLibTest/App/res/drawable/white_optical_bounds_3x3.9.png diff --git a/tools/aapt2/integration-tests/AppOne/res/font/myfont-italic.ttf b/tools/aapt2/integration-tests/StaticLibTest/App/res/font/myfont-italic.ttf index e69de29bb2d1..e69de29bb2d1 100644 --- a/tools/aapt2/integration-tests/AppOne/res/font/myfont-italic.ttf +++ b/tools/aapt2/integration-tests/StaticLibTest/App/res/font/myfont-italic.ttf diff --git a/tools/aapt2/integration-tests/AppOne/res/font/myfont-normal.ttf b/tools/aapt2/integration-tests/StaticLibTest/App/res/font/myfont-normal.ttf index e69de29bb2d1..e69de29bb2d1 100644 --- a/tools/aapt2/integration-tests/AppOne/res/font/myfont-normal.ttf +++ b/tools/aapt2/integration-tests/StaticLibTest/App/res/font/myfont-normal.ttf diff --git a/tools/aapt2/integration-tests/AppOne/res/font/myfont.xml b/tools/aapt2/integration-tests/StaticLibTest/App/res/font/myfont.xml index 1fb67914894e..1fb67914894e 100644 --- a/tools/aapt2/integration-tests/AppOne/res/font/myfont.xml +++ b/tools/aapt2/integration-tests/StaticLibTest/App/res/font/myfont.xml diff --git a/tools/aapt2/integration-tests/AppOne/res/layout-v21/main.xml b/tools/aapt2/integration-tests/StaticLibTest/App/res/layout-v21/main.xml index 9f5a4a85cbcf..724bfe4a9536 100644 --- a/tools/aapt2/integration-tests/AppOne/res/layout-v21/main.xml +++ b/tools/aapt2/integration-tests/StaticLibTest/App/res/layout-v21/main.xml @@ -15,7 +15,6 @@ --> <LinearLayout xmlns:android="http://schemas.android.com/apk/res/android" - xmlns:support="http://schemas.android.com/apk/res/android.appcompat" android:id="@+id/view" android:layout_width="match_parent" android:layout_height="wrap_content"> diff --git a/tools/aapt2/integration-tests/AppOne/res/layout/main.xml b/tools/aapt2/integration-tests/StaticLibTest/App/res/layout/main.xml index ab1a251a7d56..aaa884bf7084 100644 --- a/tools/aapt2/integration-tests/AppOne/res/layout/main.xml +++ b/tools/aapt2/integration-tests/StaticLibTest/App/res/layout/main.xml @@ -15,7 +15,6 @@ --> <LinearLayout xmlns:android="http://schemas.android.com/apk/res/android" - xmlns:support="http://schemas.android.com/apk/res/android.appcompat" android:id="@+id/view" android:layout_width="match_parent" android:layout_height="wrap_content"> diff --git a/tools/aapt2/integration-tests/AppOne/res/layout/special.xml b/tools/aapt2/integration-tests/StaticLibTest/App/res/layout/special.xml index 28c85ca92019..28c85ca92019 100644 --- a/tools/aapt2/integration-tests/AppOne/res/layout/special.xml +++ b/tools/aapt2/integration-tests/StaticLibTest/App/res/layout/special.xml diff --git a/tools/aapt2/integration-tests/AppOne/res/navigation/home.xml b/tools/aapt2/integration-tests/StaticLibTest/App/res/navigation/home.xml index ade271d60ab6..ade271d60ab6 100644 --- a/tools/aapt2/integration-tests/AppOne/res/navigation/home.xml +++ b/tools/aapt2/integration-tests/StaticLibTest/App/res/navigation/home.xml diff --git a/tools/aapt2/integration-tests/AppOne/res/raw/test.txt b/tools/aapt2/integration-tests/StaticLibTest/App/res/raw/test.txt index b14df6442ea5..b14df6442ea5 100644 --- a/tools/aapt2/integration-tests/AppOne/res/raw/test.txt +++ b/tools/aapt2/integration-tests/StaticLibTest/App/res/raw/test.txt diff --git a/tools/aapt2/integration-tests/AppOne/res/transition/transition_set.xml b/tools/aapt2/integration-tests/StaticLibTest/App/res/transition/transition_set.xml index e10e6c2f53da..e10e6c2f53da 100644 --- a/tools/aapt2/integration-tests/AppOne/res/transition/transition_set.xml +++ b/tools/aapt2/integration-tests/StaticLibTest/App/res/transition/transition_set.xml diff --git a/tools/aapt2/integration-tests/AppOne/res/values-v4/styles.xml b/tools/aapt2/integration-tests/StaticLibTest/App/res/values-v4/styles.xml index d8c11e210eda..d8c11e210eda 100644 --- a/tools/aapt2/integration-tests/AppOne/res/values-v4/styles.xml +++ b/tools/aapt2/integration-tests/StaticLibTest/App/res/values-v4/styles.xml diff --git a/tools/aapt2/integration-tests/AppOne/res/values/colors.xml b/tools/aapt2/integration-tests/StaticLibTest/App/res/values/colors.xml index 4df5077051d2..4df5077051d2 100644 --- a/tools/aapt2/integration-tests/AppOne/res/values/colors.xml +++ b/tools/aapt2/integration-tests/StaticLibTest/App/res/values/colors.xml diff --git a/tools/aapt2/integration-tests/AppOne/res/values/styles.xml b/tools/aapt2/integration-tests/StaticLibTest/App/res/values/styles.xml index 19d96c0809db..a088e5d0e1a2 100644 --- a/tools/aapt2/integration-tests/AppOne/res/values/styles.xml +++ b/tools/aapt2/integration-tests/StaticLibTest/App/res/values/styles.xml @@ -14,7 +14,7 @@ limitations under the License. --> -<resources xmlns:lib="http://schemas.android.com/apk/res/android.appcompat"> +<resources> <style name="App"> <item name="android:background">@color/primary</item> <item name="android:colorPrimary">@color/primary</item> diff --git a/tools/aapt2/integration-tests/AppOne/res/values/test.xml b/tools/aapt2/integration-tests/StaticLibTest/App/res/values/test.xml index 2c9e8b877565..2c9e8b877565 100644 --- a/tools/aapt2/integration-tests/AppOne/res/values/test.xml +++ b/tools/aapt2/integration-tests/StaticLibTest/App/res/values/test.xml diff --git a/tools/aapt2/integration-tests/AppOne/src/com/android/aapt/app/one/AppOne.java b/tools/aapt2/integration-tests/StaticLibTest/App/src/com/android/aapt/app/one/AppOne.java index 472b35a781fe..472b35a781fe 100644 --- a/tools/aapt2/integration-tests/AppOne/src/com/android/aapt/app/one/AppOne.java +++ b/tools/aapt2/integration-tests/StaticLibTest/App/src/com/android/aapt/app/one/AppOne.java diff --git a/tools/aapt2/integration-tests/StaticLibOne/Android.mk b/tools/aapt2/integration-tests/StaticLibTest/LibOne/Android.mk index 0b7129aa0d38..0c828b80b3b3 100644 --- a/tools/aapt2/integration-tests/StaticLibOne/Android.mk +++ b/tools/aapt2/integration-tests/StaticLibTest/LibOne/Android.mk @@ -18,11 +18,11 @@ LOCAL_PATH := $(call my-dir) include $(CLEAR_VARS) LOCAL_USE_AAPT2 := true -LOCAL_MODULE := AaptTestStaticLibOne +LOCAL_MODULE := AaptTestStaticLib_LibOne LOCAL_MODULE_TAGS := tests LOCAL_SRC_FILES := $(call all-java-files-under,src) LOCAL_RESOURCE_DIR := $(LOCAL_PATH)/res -# We need this to compile the Java sources of AaptTestStaticLibTwo using javac. +# We need this to compile the Java sources of AaptTestStaticLib_LibTwo using javac. LOCAL_JAR_EXCLUDE_FILES := none include $(BUILD_STATIC_JAVA_LIBRARY) diff --git a/tools/aapt2/integration-tests/StaticLibOne/AndroidManifest.xml b/tools/aapt2/integration-tests/StaticLibTest/LibOne/AndroidManifest.xml index 705047e71300..705047e71300 100644 --- a/tools/aapt2/integration-tests/StaticLibOne/AndroidManifest.xml +++ b/tools/aapt2/integration-tests/StaticLibTest/LibOne/AndroidManifest.xml diff --git a/tools/aapt2/integration-tests/StaticLibOne/res/layout/layout.xml b/tools/aapt2/integration-tests/StaticLibTest/LibOne/res/layout/layout.xml index 683c91cd9cf5..683c91cd9cf5 100644 --- a/tools/aapt2/integration-tests/StaticLibOne/res/layout/layout.xml +++ b/tools/aapt2/integration-tests/StaticLibTest/LibOne/res/layout/layout.xml diff --git a/tools/aapt2/integration-tests/StaticLibOne/res/values/values.xml b/tools/aapt2/integration-tests/StaticLibTest/LibOne/res/values/values.xml index b4dc90b3e640..b4dc90b3e640 100644 --- a/tools/aapt2/integration-tests/StaticLibOne/res/values/values.xml +++ b/tools/aapt2/integration-tests/StaticLibTest/LibOne/res/values/values.xml diff --git a/tools/aapt2/integration-tests/StaticLibOne/src/com/android/aapt/staticlib/one/StaticLibOne.java b/tools/aapt2/integration-tests/StaticLibTest/LibOne/src/com/android/aapt/staticlib/one/StaticLibOne.java index cf48f67056cf..cf48f67056cf 100644 --- a/tools/aapt2/integration-tests/StaticLibOne/src/com/android/aapt/staticlib/one/StaticLibOne.java +++ b/tools/aapt2/integration-tests/StaticLibTest/LibOne/src/com/android/aapt/staticlib/one/StaticLibOne.java diff --git a/tools/aapt2/integration-tests/StaticLibTwo/Android.mk b/tools/aapt2/integration-tests/StaticLibTest/LibTwo/Android.mk index 8b6eb41b08cd..538d5251b203 100644 --- a/tools/aapt2/integration-tests/StaticLibTwo/Android.mk +++ b/tools/aapt2/integration-tests/StaticLibTest/LibTwo/Android.mk @@ -18,10 +18,10 @@ LOCAL_PATH := $(call my-dir) include $(CLEAR_VARS) LOCAL_USE_AAPT2 := true -LOCAL_MODULE := AaptTestStaticLibTwo +LOCAL_MODULE := AaptTestStaticLib_LibTwo LOCAL_MODULE_TAGS := tests LOCAL_SRC_FILES := $(call all-java-files-under,src) LOCAL_RESOURCE_DIR := $(LOCAL_PATH)/res -LOCAL_SHARED_ANDROID_LIBRARIES := AaptTestStaticLibOne +LOCAL_SHARED_ANDROID_LIBRARIES := AaptTestStaticLib_LibOne include $(BUILD_STATIC_JAVA_LIBRARY) diff --git a/tools/aapt2/integration-tests/StaticLibTwo/AndroidManifest.xml b/tools/aapt2/integration-tests/StaticLibTest/LibTwo/AndroidManifest.xml index 28f069932452..28f069932452 100644 --- a/tools/aapt2/integration-tests/StaticLibTwo/AndroidManifest.xml +++ b/tools/aapt2/integration-tests/StaticLibTest/LibTwo/AndroidManifest.xml diff --git a/tools/aapt2/integration-tests/StaticLibTwo/res/drawable/vector.xml b/tools/aapt2/integration-tests/StaticLibTest/LibTwo/res/drawable/vector.xml index dd5979f7e838..dd5979f7e838 100644 --- a/tools/aapt2/integration-tests/StaticLibTwo/res/drawable/vector.xml +++ b/tools/aapt2/integration-tests/StaticLibTest/LibTwo/res/drawable/vector.xml diff --git a/tools/aapt2/integration-tests/StaticLibTwo/res/layout/layout_two.xml b/tools/aapt2/integration-tests/StaticLibTest/LibTwo/res/layout/layout_two.xml index ba9830708eb0..ba9830708eb0 100644 --- a/tools/aapt2/integration-tests/StaticLibTwo/res/layout/layout_two.xml +++ b/tools/aapt2/integration-tests/StaticLibTest/LibTwo/res/layout/layout_two.xml diff --git a/tools/aapt2/integration-tests/StaticLibTwo/res/values/values.xml b/tools/aapt2/integration-tests/StaticLibTest/LibTwo/res/values/values.xml index 97bb2a53d9f6..97bb2a53d9f6 100644 --- a/tools/aapt2/integration-tests/StaticLibTwo/res/values/values.xml +++ b/tools/aapt2/integration-tests/StaticLibTest/LibTwo/res/values/values.xml diff --git a/tools/aapt2/integration-tests/StaticLibTwo/src/com/android/aapt/staticlib/two/StaticLibTwo.java b/tools/aapt2/integration-tests/StaticLibTest/LibTwo/src/com/android/aapt/staticlib/two/StaticLibTwo.java index 7110dcdd017a..7110dcdd017a 100644 --- a/tools/aapt2/integration-tests/StaticLibTwo/src/com/android/aapt/staticlib/two/StaticLibTwo.java +++ b/tools/aapt2/integration-tests/StaticLibTest/LibTwo/src/com/android/aapt/staticlib/two/StaticLibTwo.java diff --git a/tools/aapt2/java/AnnotationProcessor.cpp b/tools/aapt2/java/AnnotationProcessor.cpp index 1f83fa098d74..c93461a66899 100644 --- a/tools/aapt2/java/AnnotationProcessor.cpp +++ b/tools/aapt2/java/AnnotationProcessor.cpp @@ -17,6 +17,7 @@ #include "java/AnnotationProcessor.h" #include <algorithm> +#include <array> #include "text/Unicode.h" #include "text/Utf8Iterator.h" @@ -41,30 +42,54 @@ StringPiece AnnotationProcessor::ExtractFirstSentence(const StringPiece& comment return comment; } -void AnnotationProcessor::AppendCommentLine(std::string& comment) { +struct AnnotationRule { + enum : uint32_t { + kDeprecated = 0x01, + kSystemApi = 0x02, + kTestApi = 0x04, + }; + + StringPiece doc_str; + uint32_t bit_mask; + StringPiece annotation; +}; + +static std::array<AnnotationRule, 2> sAnnotationRules = {{ + {"@SystemApi", AnnotationRule::kSystemApi, "@android.annotation.SystemApi"}, + {"@TestApi", AnnotationRule::kTestApi, "@android.annotation.TestApi"}, +}}; + +void AnnotationProcessor::AppendCommentLine(std::string comment) { static const std::string sDeprecated = "@deprecated"; - static const std::string sSystemApi = "@SystemApi"; + // Treat deprecated specially, since we don't remove it from the source comment. if (comment.find(sDeprecated) != std::string::npos) { - annotation_bit_mask_ |= kDeprecated; + annotation_bit_mask_ |= AnnotationRule::kDeprecated; } - std::string::size_type idx = comment.find(sSystemApi); - if (idx != std::string::npos) { - annotation_bit_mask_ |= kSystemApi; - comment.erase(comment.begin() + idx, - comment.begin() + idx + sSystemApi.size()); + for (const AnnotationRule& rule : sAnnotationRules) { + std::string::size_type idx = comment.find(rule.doc_str.data()); + if (idx != std::string::npos) { + annotation_bit_mask_ |= rule.bit_mask; + comment.erase(comment.begin() + idx, comment.begin() + idx + rule.doc_str.size()); + } } - if (util::TrimWhitespace(comment).empty()) { + // Check if after removal of annotations the line is empty. + const StringPiece trimmed = util::TrimWhitespace(comment); + if (trimmed.empty()) { return; } + // If there was trimming to do, copy the string. + if (trimmed.size() != comment.size()) { + comment = trimmed.to_string(); + } + if (!has_comments_) { has_comments_ = true; comment_ << "/**"; } - comment_ << "\n * " << std::move(comment); } @@ -73,16 +98,18 @@ void AnnotationProcessor::AppendComment(const StringPiece& comment) { for (StringPiece line : util::Tokenize(comment, '\n')) { line = util::TrimWhitespace(line); if (!line.empty()) { - std::string lineCopy = line.to_string(); - AppendCommentLine(lineCopy); + AppendCommentLine(line.to_string()); } } } -void AnnotationProcessor::AppendNewLine() { comment_ << "\n *"; } +void AnnotationProcessor::AppendNewLine() { + if (has_comments_) { + comment_ << "\n *"; + } +} -void AnnotationProcessor::WriteToStream(std::ostream* out, - const StringPiece& prefix) const { +void AnnotationProcessor::WriteToStream(const StringPiece& prefix, std::ostream* out) const { if (has_comments_) { std::string result = comment_.str(); for (StringPiece line : util::Tokenize(result, '\n')) { @@ -92,12 +119,14 @@ void AnnotationProcessor::WriteToStream(std::ostream* out, << "\n"; } - if (annotation_bit_mask_ & kDeprecated) { + if (annotation_bit_mask_ & AnnotationRule::kDeprecated) { *out << prefix << "@Deprecated\n"; } - if (annotation_bit_mask_ & kSystemApi) { - *out << prefix << "@android.annotation.SystemApi\n"; + for (const AnnotationRule& rule : sAnnotationRules) { + if (annotation_bit_mask_ & rule.bit_mask) { + *out << prefix << rule.annotation << "\n"; + } } } diff --git a/tools/aapt2/java/AnnotationProcessor.h b/tools/aapt2/java/AnnotationProcessor.h index a06eda0f9c5c..a7bf73f50de5 100644 --- a/tools/aapt2/java/AnnotationProcessor.h +++ b/tools/aapt2/java/AnnotationProcessor.h @@ -24,64 +24,53 @@ namespace aapt { -/** - * Builds a JavaDoc comment from a set of XML comments. - * This will also look for instances of @SystemApi and convert them to - * actual Java annotations. - * - * Example: - * - * Input XML: - * - * <!-- This is meant to be hidden because - * It is system api. Also it is @deprecated - * @SystemApi - * --> - * - * Output JavaDoc: - * - * /\* - * * This is meant to be hidden because - * * It is system api. Also it is @deprecated - * *\/ - * - * Output Annotations: - * - * @Deprecated - * @android.annotation.SystemApi - * - */ +// Builds a JavaDoc comment from a set of XML comments. +// This will also look for instances of @SystemApi and convert them to +// actual Java annotations. +// +// Example: +// +// Input XML: +// +// <!-- This is meant to be hidden because +// It is system api. Also it is @deprecated +// @SystemApi +// --> +// +// Output JavaDoc: +// +// /** +// * This is meant to be hidden because +// * It is system api. Also it is @deprecated +// */ +// +// Output Annotations: +// +// @Deprecated +// @android.annotation.SystemApi class AnnotationProcessor { public: + // Extracts the first sentence of a comment. The algorithm selects the substring starting from + // the beginning of the string, and ending at the first '.' character that is followed by a + // whitespace character. If these requirements are not met, the whole string is returned. static android::StringPiece ExtractFirstSentence(const android::StringPiece& comment); - /** - * Adds more comments. Since resources can have various values with different - * configurations, - * we need to collect all the comments. - */ + // Adds more comments. Resources can have value definitions for various configurations, and + // each of the definitions may have comments that need to be processed. void AppendComment(const android::StringPiece& comment); void AppendNewLine(); - /** - * Writes the comments and annotations to the stream, with the given prefix - * before each line. - */ - void WriteToStream(std::ostream* out, const android::StringPiece& prefix) const; + // Writes the comments and annotations to the stream, with the given prefix before each line. + void WriteToStream(const android::StringPiece& prefix, std::ostream* out) const; private: - enum : uint32_t { - kDeprecated = 0x01, - kSystemApi = 0x02, - }; - std::stringstream comment_; std::stringstream mAnnotations; bool has_comments_ = false; uint32_t annotation_bit_mask_ = 0; - void AppendCommentLine(std::string& line); + void AppendCommentLine(std::string line); }; } // namespace aapt diff --git a/tools/aapt2/java/AnnotationProcessor_test.cpp b/tools/aapt2/java/AnnotationProcessor_test.cpp index 9ccac8888426..856f4ccbd7f0 100644 --- a/tools/aapt2/java/AnnotationProcessor_test.cpp +++ b/tools/aapt2/java/AnnotationProcessor_test.cpp @@ -34,7 +34,7 @@ TEST(AnnotationProcessorTest, EmitsDeprecated) { processor.AppendComment(comment); std::stringstream result; - processor.WriteToStream(&result, ""); + processor.WriteToStream("", &result); std::string annotations = result.str(); EXPECT_THAT(annotations, HasSubstr("@Deprecated")); @@ -45,7 +45,7 @@ TEST(AnnotationProcessorTest, EmitsSystemApiAnnotationAndRemovesFromComment) { processor.AppendComment("@SystemApi This is a system API"); std::stringstream result; - processor.WriteToStream(&result, ""); + processor.WriteToStream("", &result); std::string annotations = result.str(); EXPECT_THAT(annotations, HasSubstr("@android.annotation.SystemApi")); @@ -53,6 +53,19 @@ TEST(AnnotationProcessorTest, EmitsSystemApiAnnotationAndRemovesFromComment) { EXPECT_THAT(annotations, HasSubstr("This is a system API")); } +TEST(AnnotationProcessorTest, EmitsTestApiAnnotationAndRemovesFromComment) { + AnnotationProcessor processor; + processor.AppendComment("@TestApi This is a test API"); + + std::stringstream result; + processor.WriteToStream("", &result); + std::string annotations = result.str(); + + EXPECT_THAT(annotations, HasSubstr("@android.annotation.TestApi")); + EXPECT_THAT(annotations, Not(HasSubstr("@TestApi"))); + EXPECT_THAT(annotations, HasSubstr("This is a test API")); +} + TEST(AnnotationProcessor, ExtractsFirstSentence) { EXPECT_THAT(AnnotationProcessor::ExtractFirstSentence("This is the only sentence"), Eq("This is the only sentence")); diff --git a/tools/aapt2/java/ClassDefinition.cpp b/tools/aapt2/java/ClassDefinition.cpp index d7508d257db4..6ad0dd68cb6a 100644 --- a/tools/aapt2/java/ClassDefinition.cpp +++ b/tools/aapt2/java/ClassDefinition.cpp @@ -18,12 +18,12 @@ #include "androidfw/StringPiece.h" -using android::StringPiece; +using ::android::StringPiece; namespace aapt { void ClassMember::WriteToStream(const StringPiece& prefix, bool final, std::ostream* out) const { - processor_.WriteToStream(out, prefix); + processor_.WriteToStream(prefix, out); } void MethodDefinition::AppendStatement(const StringPiece& statement) { @@ -92,9 +92,8 @@ constexpr static const char* sWarningHeader = " * should not be modified by hand.\n" " */\n\n"; -bool ClassDefinition::WriteJavaFile(const ClassDefinition* def, - const StringPiece& package, bool final, - std::ostream* out) { +bool ClassDefinition::WriteJavaFile(const ClassDefinition* def, const StringPiece& package, + bool final, std::ostream* out) { *out << sWarningHeader << "package " << package << ";\n\n"; def->WriteToStream("", final, out); return bool(*out); diff --git a/tools/aapt2/java/JavaClassGenerator.cpp b/tools/aapt2/java/JavaClassGenerator.cpp index 44fa0f19a0e5..8da9106aa8d7 100644 --- a/tools/aapt2/java/JavaClassGenerator.cpp +++ b/tools/aapt2/java/JavaClassGenerator.cpp @@ -480,7 +480,7 @@ Maybe<std::string> JavaClassGenerator::UnmangleResource(const StringPiece& packa if (NameMangler::Unmangle(&unmangled_name, &unmangled_package)) { // The entry name was mangled, and we successfully unmangled it. // Check that we want to emit this symbol. - if (package_name != unmangled_package) { + if (package_name_to_generate != unmangled_package) { // Skip the entry if it doesn't belong to the package we're writing. return {}; } @@ -579,8 +579,7 @@ bool JavaClassGenerator::Generate(const StringPiece& package_name_to_generate, continue; } - // Stay consistent with AAPT and generate an empty type class if the R class - // is public. + // Stay consistent with AAPT and generate an empty type class if the R class is public. const bool force_creation_if_empty = (options_.types == JavaClassGeneratorOptions::SymbolTypes::kPublic); diff --git a/tools/aapt2/java/JavaClassGenerator_test.cpp b/tools/aapt2/java/JavaClassGenerator_test.cpp index 271279ff5e92..4f449f0db41a 100644 --- a/tools/aapt2/java/JavaClassGenerator_test.cpp +++ b/tools/aapt2/java/JavaClassGenerator_test.cpp @@ -22,7 +22,9 @@ #include "test/Test.h" #include "util/Util.h" -using android::StringPiece; +using ::android::StringPiece; +using ::testing::HasSubstr; +using ::testing::Not; namespace aapt { @@ -52,17 +54,15 @@ TEST(JavaClassGeneratorTest, TransformInvalidJavaIdentifierCharacter) { .AddSimple("android:id/hey-man", ResourceId(0x01020000)) .AddValue("android:attr/cool.attr", ResourceId(0x01010000), test::AttributeBuilder(false).Build()) - .AddValue( - "android:styleable/hey.dude", ResourceId(0x01030000), - test::StyleableBuilder() - .AddItem("android:attr/cool.attr", ResourceId(0x01010000)) - .Build()) + .AddValue("android:styleable/hey.dude", ResourceId(0x01030000), + test::StyleableBuilder() + .AddItem("android:attr/cool.attr", ResourceId(0x01010000)) + .Build()) .Build(); std::unique_ptr<IAaptContext> context = test::ContextBuilder() - .AddSymbolSource( - util::make_unique<ResourceTableSymbolSource>(table.get())) + .AddSymbolSource(util::make_unique<ResourceTableSymbolSource>(table.get())) .SetNameManglerPolicy(NameManglerPolicy{"android"}) .Build(); JavaClassGenerator generator(context.get(), table.get(), {}); @@ -72,14 +72,9 @@ TEST(JavaClassGeneratorTest, TransformInvalidJavaIdentifierCharacter) { std::string output = out.str(); - EXPECT_NE(std::string::npos, - output.find("public static final int hey_man=0x01020000;")); - - EXPECT_NE(std::string::npos, - output.find("public static final int[] hey_dude={")); - - EXPECT_NE(std::string::npos, - output.find("public static final int hey_dude_cool_attr=0;")); + EXPECT_THAT(output, HasSubstr("public static final int hey_man=0x01020000;")); + EXPECT_THAT(output, HasSubstr("public static final int[] hey_dude={")); + EXPECT_THAT(output, HasSubstr("public static final int hey_dude_cool_attr=0;")); } TEST(JavaClassGeneratorTest, CorrectPackageNameIsUsed) { @@ -92,8 +87,7 @@ TEST(JavaClassGeneratorTest, CorrectPackageNameIsUsed) { std::unique_ptr<IAaptContext> context = test::ContextBuilder() - .AddSymbolSource( - util::make_unique<ResourceTableSymbolSource>(table.get())) + .AddSymbolSource(util::make_unique<ResourceTableSymbolSource>(table.get())) .SetNameManglerPolicy(NameManglerPolicy{"android"}) .Build(); JavaClassGenerator generator(context.get(), table.get(), {}); @@ -101,11 +95,10 @@ TEST(JavaClassGeneratorTest, CorrectPackageNameIsUsed) { ASSERT_TRUE(generator.Generate("android", "com.android.internal", &out)); std::string output = out.str(); - EXPECT_NE(std::string::npos, output.find("package com.android.internal;")); - EXPECT_NE(std::string::npos, - output.find("public static final int one=0x01020000;")); - EXPECT_EQ(std::string::npos, output.find("two")); - EXPECT_EQ(std::string::npos, output.find("com_foo$two")); + EXPECT_THAT(output, HasSubstr("package com.android.internal;")); + EXPECT_THAT(output, HasSubstr("public static final int one=0x01020000;")); + EXPECT_THAT(output, Not(HasSubstr("two"))); + EXPECT_THAT(output, Not(HasSubstr("com_foo$two"))); } TEST(JavaClassGeneratorTest, AttrPrivateIsWrittenAsAttr) { @@ -118,8 +111,7 @@ TEST(JavaClassGeneratorTest, AttrPrivateIsWrittenAsAttr) { std::unique_ptr<IAaptContext> context = test::ContextBuilder() - .AddSymbolSource( - util::make_unique<ResourceTableSymbolSource>(table.get())) + .AddSymbolSource(util::make_unique<ResourceTableSymbolSource>(table.get())) .SetNameManglerPolicy(NameManglerPolicy{"android"}) .Build(); JavaClassGenerator generator(context.get(), table.get(), {}); @@ -127,9 +119,8 @@ TEST(JavaClassGeneratorTest, AttrPrivateIsWrittenAsAttr) { ASSERT_TRUE(generator.Generate("android", &out)); std::string output = out.str(); - EXPECT_NE(std::string::npos, output.find("public static final class attr")); - EXPECT_EQ(std::string::npos, - output.find("public static final class ^attr-private")); + EXPECT_THAT(output, HasSubstr("public static final class attr")); + EXPECT_THAT(output, Not(HasSubstr("public static final class ^attr-private"))); } TEST(JavaClassGeneratorTest, OnlyWritePublicResources) { @@ -140,16 +131,13 @@ TEST(JavaClassGeneratorTest, OnlyWritePublicResources) { .AddSimple("android:id/one", ResourceId(0x01020000)) .AddSimple("android:id/two", ResourceId(0x01020001)) .AddSimple("android:id/three", ResourceId(0x01020002)) - .SetSymbolState("android:id/one", ResourceId(0x01020000), - SymbolState::kPublic) - .SetSymbolState("android:id/two", ResourceId(0x01020001), - SymbolState::kPrivate) + .SetSymbolState("android:id/one", ResourceId(0x01020000), SymbolState::kPublic) + .SetSymbolState("android:id/two", ResourceId(0x01020001), SymbolState::kPrivate) .Build(); std::unique_ptr<IAaptContext> context = test::ContextBuilder() - .AddSymbolSource( - util::make_unique<ResourceTableSymbolSource>(table.get())) + .AddSymbolSource(util::make_unique<ResourceTableSymbolSource>(table.get())) .SetNameManglerPolicy(NameManglerPolicy{"android"}) .Build(); @@ -160,10 +148,9 @@ TEST(JavaClassGeneratorTest, OnlyWritePublicResources) { std::stringstream out; ASSERT_TRUE(generator.Generate("android", &out)); std::string output = out.str(); - EXPECT_NE(std::string::npos, - output.find("public static final int one=0x01020000;")); - EXPECT_EQ(std::string::npos, output.find("two")); - EXPECT_EQ(std::string::npos, output.find("three")); + EXPECT_THAT(output, HasSubstr("public static final int one=0x01020000;")); + EXPECT_THAT(output, Not(HasSubstr("two"))); + EXPECT_THAT(output, Not(HasSubstr("three"))); } options.types = JavaClassGeneratorOptions::SymbolTypes::kPublicPrivate; @@ -172,11 +159,9 @@ TEST(JavaClassGeneratorTest, OnlyWritePublicResources) { std::stringstream out; ASSERT_TRUE(generator.Generate("android", &out)); std::string output = out.str(); - EXPECT_NE(std::string::npos, - output.find("public static final int one=0x01020000;")); - EXPECT_NE(std::string::npos, - output.find("public static final int two=0x01020001;")); - EXPECT_EQ(std::string::npos, output.find("three")); + EXPECT_THAT(output, HasSubstr("public static final int one=0x01020000;")); + EXPECT_THAT(output, HasSubstr("public static final int two=0x01020001;")); + EXPECT_THAT(output, Not(HasSubstr("three"))); } options.types = JavaClassGeneratorOptions::SymbolTypes::kAll; @@ -185,12 +170,9 @@ TEST(JavaClassGeneratorTest, OnlyWritePublicResources) { std::stringstream out; ASSERT_TRUE(generator.Generate("android", &out)); std::string output = out.str(); - EXPECT_NE(std::string::npos, - output.find("public static final int one=0x01020000;")); - EXPECT_NE(std::string::npos, - output.find("public static final int two=0x01020001;")); - EXPECT_NE(std::string::npos, - output.find("public static final int three=0x01020002;")); + EXPECT_THAT(output, HasSubstr("public static final int one=0x01020000;")); + EXPECT_THAT(output, HasSubstr("public static final int two=0x01020001;")); + EXPECT_THAT(output, HasSubstr("public static final int three=0x01020002;")); } } @@ -246,8 +228,7 @@ TEST(JavaClassGeneratorTest, EmitOtherPackagesAttributesInStyleable) { std::unique_ptr<IAaptContext> context = test::ContextBuilder() - .AddSymbolSource( - util::make_unique<ResourceTableSymbolSource>(table.get())) + .AddSymbolSource(util::make_unique<ResourceTableSymbolSource>(table.get())) .SetNameManglerPolicy(NameManglerPolicy{"android"}) .Build(); JavaClassGenerator generator(context.get(), table.get(), {}); @@ -256,8 +237,8 @@ TEST(JavaClassGeneratorTest, EmitOtherPackagesAttributesInStyleable) { EXPECT_TRUE(generator.Generate("android", &out)); std::string output = out.str(); - EXPECT_NE(std::string::npos, output.find("int foo_bar=")); - EXPECT_NE(std::string::npos, output.find("int foo_com_lib_bar=")); + EXPECT_THAT(output, HasSubstr("int foo_bar=")); + EXPECT_THAT(output, HasSubstr("int foo_com_lib_bar=")); } TEST(JavaClassGeneratorTest, CommentsForSimpleResourcesArePresent) { @@ -271,24 +252,22 @@ TEST(JavaClassGeneratorTest, CommentsForSimpleResourcesArePresent) { std::unique_ptr<IAaptContext> context = test::ContextBuilder() - .AddSymbolSource( - util::make_unique<ResourceTableSymbolSource>(table.get())) + .AddSymbolSource(util::make_unique<ResourceTableSymbolSource>(table.get())) .SetNameManglerPolicy(NameManglerPolicy{"android"}) .Build(); JavaClassGenerator generator(context.get(), table.get(), {}); std::stringstream out; ASSERT_TRUE(generator.Generate("android", &out)); - std::string actual = out.str(); + std::string output = out.str(); - const char* expectedText = + const char* expected_text = R"EOF(/** * This is a comment * @deprecated */ @Deprecated public static final int foo=0x01010000;)EOF"; - - EXPECT_NE(std::string::npos, actual.find(expectedText)); + EXPECT_THAT(output, HasSubstr(expected_text)); } TEST(JavaClassGeneratorTest, CommentsForEnumAndFlagAttributesArePresent) {} @@ -298,8 +277,7 @@ TEST(JavaClassGeneratorTest, CommentsForStyleablesAndNestedAttributesArePresent) attr.SetComment(StringPiece("This is an attribute")); Styleable styleable; - styleable.entries.push_back( - Reference(test::ParseNameOrDie("android:attr/one"))); + styleable.entries.push_back(Reference(test::ParseNameOrDie("android:attr/one"))); styleable.SetComment(StringPiece("This is a styleable")); std::unique_ptr<ResourceTable> table = @@ -312,8 +290,7 @@ TEST(JavaClassGeneratorTest, CommentsForStyleablesAndNestedAttributesArePresent) std::unique_ptr<IAaptContext> context = test::ContextBuilder() - .AddSymbolSource( - util::make_unique<ResourceTableSymbolSource>(table.get())) + .AddSymbolSource(util::make_unique<ResourceTableSymbolSource>(table.get())) .SetNameManglerPolicy(NameManglerPolicy{"android"}) .Build(); JavaClassGeneratorOptions options; @@ -321,12 +298,12 @@ TEST(JavaClassGeneratorTest, CommentsForStyleablesAndNestedAttributesArePresent) JavaClassGenerator generator(context.get(), table.get(), options); std::stringstream out; ASSERT_TRUE(generator.Generate("android", &out)); - std::string actual = out.str(); + std::string output = out.str(); - EXPECT_NE(std::string::npos, actual.find("attr name android:one")); - EXPECT_NE(std::string::npos, actual.find("attr description")); - EXPECT_NE(std::string::npos, actual.find(attr.GetComment().data())); - EXPECT_NE(std::string::npos, actual.find(styleable.GetComment().data())); + EXPECT_THAT(output, HasSubstr("attr name android:one")); + EXPECT_THAT(output, HasSubstr("attr description")); + EXPECT_THAT(output, HasSubstr(attr.GetComment())); + EXPECT_THAT(output, HasSubstr(styleable.GetComment())); } TEST(JavaClassGeneratorTest, CommentsForRemovedAttributesAreNotPresentInClass) { @@ -341,8 +318,7 @@ TEST(JavaClassGeneratorTest, CommentsForRemovedAttributesAreNotPresentInClass) { std::unique_ptr<IAaptContext> context = test::ContextBuilder() - .AddSymbolSource( - util::make_unique<ResourceTableSymbolSource>(table.get())) + .AddSymbolSource(util::make_unique<ResourceTableSymbolSource>(table.get())) .SetNameManglerPolicy(NameManglerPolicy{"android"}) .Build(); JavaClassGeneratorOptions options; @@ -350,17 +326,17 @@ TEST(JavaClassGeneratorTest, CommentsForRemovedAttributesAreNotPresentInClass) { JavaClassGenerator generator(context.get(), table.get(), options); std::stringstream out; ASSERT_TRUE(generator.Generate("android", &out)); - std::string actual = out.str(); + std::string output = out.str(); - EXPECT_EQ(std::string::npos, actual.find("@attr name android:one")); - EXPECT_EQ(std::string::npos, actual.find("@attr description")); + EXPECT_THAT(output, Not(HasSubstr("@attr name android:one"))); + EXPECT_THAT(output, Not(HasSubstr("@attr description"))); // We should find @removed only in the attribute javadoc and not anywhere else - // (i.e. the class - // javadoc). - const size_t pos = actual.find("removed"); - EXPECT_NE(std::string::npos, pos); - EXPECT_EQ(std::string::npos, actual.find("removed", pos + 1)); + // (i.e. the class javadoc). + const std::string kRemoved("removed"); + ASSERT_THAT(output, HasSubstr(kRemoved)); + std::string after_first_match = output.substr(output.find(kRemoved) + kRemoved.size()); + EXPECT_THAT(after_first_match, Not(HasSubstr(kRemoved))); } TEST(JavaClassGeneratorTest, GenerateOnResourcesLoadedCallbackForSharedLibrary) { @@ -381,19 +357,17 @@ TEST(JavaClassGeneratorTest, GenerateOnResourcesLoadedCallbackForSharedLibrary) JavaClassGeneratorOptions options; options.use_final = false; - options.rewrite_callback_options = OnResourcesLoadedCallbackOptions{ - {"com.foo", "com.boo"}, - }; + options.rewrite_callback_options = OnResourcesLoadedCallbackOptions{{"com.foo", "com.boo"}}; JavaClassGenerator generator(context.get(), table.get(), options); std::stringstream out; ASSERT_TRUE(generator.Generate("android", &out)); - std::string actual = out.str(); + std::string output = out.str(); - EXPECT_NE(std::string::npos, actual.find("void onResourcesLoaded")); - EXPECT_NE(std::string::npos, actual.find("com.foo.R.onResourcesLoaded")); - EXPECT_NE(std::string::npos, actual.find("com.boo.R.onResourcesLoaded")); + EXPECT_THAT(output, HasSubstr("void onResourcesLoaded")); + EXPECT_THAT(output, HasSubstr("com.foo.R.onResourcesLoaded")); + EXPECT_THAT(output, HasSubstr("com.boo.R.onResourcesLoaded")); } } // namespace aapt diff --git a/tools/aapt2/java/ManifestClassGenerator.cpp b/tools/aapt2/java/ManifestClassGenerator.cpp index cad4c6c7c94f..c4b36176aa71 100644 --- a/tools/aapt2/java/ManifestClassGenerator.cpp +++ b/tools/aapt2/java/ManifestClassGenerator.cpp @@ -21,24 +21,21 @@ #include "Source.h" #include "java/AnnotationProcessor.h" #include "java/ClassDefinition.h" +#include "text/Unicode.h" #include "util/Maybe.h" #include "xml/XmlDom.h" -using android::StringPiece; +using ::android::StringPiece; +using ::aapt::text::IsJavaIdentifier; namespace aapt { -static Maybe<StringPiece> ExtractJavaIdentifier(IDiagnostics* diag, - const Source& source, - const StringPiece& value) { - const StringPiece sep = "."; - auto iter = std::find_end(value.begin(), value.end(), sep.begin(), sep.end()); - - StringPiece result; - if (iter != value.end()) { - result.assign(iter + sep.size(), value.end() - (iter + sep.size())); - } else { - result = value; +static Maybe<StringPiece> ExtractJavaIdentifier(IDiagnostics* diag, const Source& source, + const std::string& value) { + StringPiece result = value; + size_t pos = value.rfind('.'); + if (pos != std::string::npos) { + result = result.substr(pos + 1); } if (result.empty()) { @@ -46,33 +43,23 @@ static Maybe<StringPiece> ExtractJavaIdentifier(IDiagnostics* diag, return {}; } - iter = util::FindNonAlphaNumericAndNotInSet(result, "_"); - if (iter != result.end()) { - diag->Error(DiagMessage(source) << "invalid character '" - << StringPiece(iter, 1) << "' in '" - << result << "'"); + if (!IsJavaIdentifier(result)) { + diag->Error(DiagMessage(source) << "invalid Java identifier '" << result << "'"); return {}; } - - if (*result.begin() >= '0' && *result.begin() <= '9') { - diag->Error(DiagMessage(source) << "symbol can not start with a digit"); - return {}; - } - return result; } -static bool WriteSymbol(const Source& source, IDiagnostics* diag, - xml::Element* el, ClassDefinition* class_def) { +static bool WriteSymbol(const Source& source, IDiagnostics* diag, xml::Element* el, + ClassDefinition* class_def) { xml::Attribute* attr = el->FindAttribute(xml::kSchemaAndroid, "name"); if (!attr) { - diag->Error(DiagMessage(source) << "<" << el->name - << "> must define 'android:name'"); + diag->Error(DiagMessage(source) << "<" << el->name << "> must define 'android:name'"); return false; } - Maybe<StringPiece> result = ExtractJavaIdentifier( - diag, source.WithLine(el->line_number), attr->value); + Maybe<StringPiece> result = + ExtractJavaIdentifier(diag, source.WithLine(el->line_number), attr->value); if (!result) { return false; } @@ -88,8 +75,7 @@ static bool WriteSymbol(const Source& source, IDiagnostics* diag, return true; } -std::unique_ptr<ClassDefinition> GenerateManifestClass(IDiagnostics* diag, - xml::XmlResource* res) { +std::unique_ptr<ClassDefinition> GenerateManifestClass(IDiagnostics* diag, xml::XmlResource* res) { xml::Element* el = xml::FindRootElement(res->root.get()); if (!el) { diag->Error(DiagMessage(res->file.source) << "no root tag defined"); @@ -97,8 +83,7 @@ std::unique_ptr<ClassDefinition> GenerateManifestClass(IDiagnostics* diag, } if (el->name != "manifest" && !el->namespace_uri.empty()) { - diag->Error(DiagMessage(res->file.source) - << "no <manifest> root tag defined"); + diag->Error(DiagMessage(res->file.source) << "no <manifest> root tag defined"); return {}; } @@ -112,11 +97,9 @@ std::unique_ptr<ClassDefinition> GenerateManifestClass(IDiagnostics* diag, for (xml::Element* child_el : children) { if (child_el->namespace_uri.empty()) { if (child_el->name == "permission") { - error |= !WriteSymbol(res->file.source, diag, child_el, - permission_class.get()); + error |= !WriteSymbol(res->file.source, diag, child_el, permission_class.get()); } else if (child_el->name == "permission-group") { - error |= !WriteSymbol(res->file.source, diag, child_el, - permission_group_class.get()); + error |= !WriteSymbol(res->file.source, diag, child_el, permission_group_class.get()); } } } diff --git a/tools/aapt2/java/ManifestClassGenerator_test.cpp b/tools/aapt2/java/ManifestClassGenerator_test.cpp index 44b6a1ffd5ae..ada563409d19 100644 --- a/tools/aapt2/java/ManifestClassGenerator_test.cpp +++ b/tools/aapt2/java/ManifestClassGenerator_test.cpp @@ -84,6 +84,8 @@ TEST(ManifestClassGeneratorTest, CommentsAndAnnotationsArePresent) { @hide @SystemApi --> <permission android:name="android.permission.SECRET" /> + <!-- @TestApi This is a test only permission. --> + <permission android:name="android.permission.TEST_ONLY" /> </manifest>)"); std::string actual; @@ -110,6 +112,13 @@ TEST(ManifestClassGeneratorTest, CommentsAndAnnotationsArePresent) { @android.annotation.SystemApi public static final String SECRET="android.permission.SECRET";)"; EXPECT_THAT(actual, HasSubstr(expected_secret)); + + const char* expected_test = R"( /** + * This is a test only permission. + */ + @android.annotation.TestApi + public static final String TEST_ONLY="android.permission.TEST_ONLY";)"; + EXPECT_THAT(actual, HasSubstr(expected_test)); } // This is bad but part of public API behaviour so we need to preserve it. diff --git a/tools/aapt2/link/Linkers.h b/tools/aapt2/link/Linkers.h index 5527f9092c87..3c9c4767b3d1 100644 --- a/tools/aapt2/link/Linkers.h +++ b/tools/aapt2/link/Linkers.h @@ -21,6 +21,7 @@ #include <unordered_set> #include "android-base/macros.h" +#include "androidfw/StringPiece.h" #include "Resource.h" #include "SdkConstants.h" @@ -33,18 +34,15 @@ class ResourceTable; class ResourceEntry; struct ConfigDescription; -/** - * Defines the location in which a value exists. This determines visibility of - * other package's private symbols. - */ +// Defines the context in which a resource value is defined. Most resources are defined with the +// implicit package name of their compilation context. Understanding the package name of a resource +// allows to determine visibility of other symbols which may or may not have their packages defined. struct CallSite { - ResourceNameRef resource; + std::string package; }; -/** - * Determines whether a versioned resource should be created. If a versioned - * resource already exists, it takes precedence. - */ +// Determines whether a versioned resource should be created. If a versioned resource already +// exists, it takes precedence. bool ShouldGenerateVersionedResource(const ResourceEntry* entry, const ConfigDescription& config, const ApiVersion sdk_version_to_generate); @@ -62,39 +60,26 @@ class AutoVersioner : public IResourceTableConsumer { DISALLOW_COPY_AND_ASSIGN(AutoVersioner); }; -/** - * If any attribute resource values are defined as public, this consumer will - * move all private - * attribute resource values to a private ^private-attr type, avoiding backwards - * compatibility - * issues with new apps running on old platforms. - * - * The Android platform ignores resource attributes it doesn't recognize, so an - * app developer can - * use new attributes in their layout XML files without worrying about - * versioning. This assumption - * actually breaks on older platforms. OEMs may add private attributes that are - * used internally. - * AAPT originally assigned all private attributes IDs immediately proceeding - * the public attributes' - * IDs. - * - * This means that on a newer Android platform, an ID previously assigned to a - * private attribute - * may end up assigned to a public attribute. - * - * App developers assume using the newer attribute is safe on older platforms - * because it will - * be ignored. Instead, the platform thinks the new attribute is an older, - * private attribute and - * will interpret it as such. This leads to unintended styling and exceptions - * thrown due to - * unexpected types. - * - * By moving the private attributes to a completely different type, this ID - * conflict will never - * occur. - */ +// If any attribute resource values are defined as public, this consumer will move all private +// attribute resource values to a private ^private-attr type, avoiding backwards compatibility +// issues with new apps running on old platforms. +// +// The Android platform ignores resource attributes it doesn't recognize, so an app developer can +// use new attributes in their layout XML files without worrying about versioning. This assumption +// actually breaks on older platforms. OEMs may add private attributes that are used internally. +// AAPT originally assigned all private attributes IDs immediately proceeding the public attributes' +// IDs. +// +// This means that on a newer Android platform, an ID previously assigned to a private attribute +// may end up assigned to a public attribute. +// +// App developers assume using the newer attribute is safe on older platforms because it will +// be ignored. Instead, the platform thinks the new attribute is an older, private attribute and +// will interpret it as such. This leads to unintended styling and exceptions thrown due to +// unexpected types. +// +// By moving the private attributes to a completely different type, this ID conflict will never +// occur. class PrivateAttributeMover : public IResourceTableConsumer { public: PrivateAttributeMover() = default; @@ -126,14 +111,10 @@ class ProductFilter : public IResourceTableConsumer { std::unordered_set<std::string> products_; }; -/** - * Removes namespace nodes and URI information from the XmlResource. - * - * Once an XmlResource is processed by this consumer, it is no longer able to - * have its attributes - * parsed. As such, this XmlResource must have already been processed by - * XmlReferenceLinker. - */ +// Removes namespace nodes and URI information from the XmlResource. +// +// Once an XmlResource is processed by this consumer, it is no longer able to have its attributes +// parsed. As such, this XmlResource must have already been processed by XmlReferenceLinker. class XmlNamespaceRemover : public IXmlResourceConsumer { public: explicit XmlNamespaceRemover(bool keep_uris = false) : keep_uris_(keep_uris){}; @@ -146,11 +127,8 @@ class XmlNamespaceRemover : public IXmlResourceConsumer { bool keep_uris_; }; -/** - * Resolves attributes in the XmlResource and compiles string values to resource - * values. - * Once an XmlResource is processed by this linker, it is ready to be flattened. - */ +// Resolves attributes in the XmlResource and compiles string values to resource values. +// Once an XmlResource is processed by this linker, it is ready to be flattened. class XmlReferenceLinker : public IXmlResourceConsumer { public: XmlReferenceLinker() = default; diff --git a/tools/aapt2/link/ReferenceLinker.cpp b/tools/aapt2/link/ReferenceLinker.cpp index 414e56eb5dcc..71e828b039e1 100644 --- a/tools/aapt2/link/ReferenceLinker.cpp +++ b/tools/aapt2/link/ReferenceLinker.cpp @@ -30,23 +30,18 @@ #include "util/Util.h" #include "xml/XmlUtil.h" -using android::StringPiece; +using ::android::StringPiece; namespace aapt { namespace { -/** - * The ReferenceLinkerVisitor will follow all references and make sure they - * point - * to resources that actually exist, either in the local resource table, or as - * external - * symbols. Once the target resource has been found, the ID of the resource will - * be assigned - * to the reference object. - * - * NOTE: All of the entries in the ResourceTable must be assigned IDs. - */ +// The ReferenceLinkerVisitor will follow all references and make sure they point +// to resources that actually exist, either in the local resource table, or as external +// symbols. Once the target resource has been found, the ID of the resource will be assigned +// to the reference object. +// +// NOTE: All of the entries in the ResourceTable must be assigned IDs. class ReferenceLinkerVisitor : public ValueVisitor { public: using ValueVisitor::Visit; @@ -65,14 +60,9 @@ class ReferenceLinkerVisitor : public ValueVisitor { } } - /** - * We visit the Style specially because during this phase, values of - * attributes are - * all RawString values. Now that we are expected to resolve all symbols, we - * can - * lookup the attributes to find out which types are allowed for the - * attributes' values. - */ + // We visit the Style specially because during this phase, values of attributes are + // all RawString values. Now that we are expected to resolve all symbols, we can + // lookup the attributes to find out which types are allowed for the attributes' values. void Visit(Style* style) override { if (style->parent) { Visit(&style->parent.value()); @@ -81,28 +71,21 @@ class ReferenceLinkerVisitor : public ValueVisitor { for (Style::Entry& entry : style->entries) { std::string err_str; - // Transform the attribute reference so that it is using the fully - // qualified package - // name. This will also mark the reference as being able to see private - // resources if - // there was a '*' in the reference or if the package came from the - // private namespace. + // Transform the attribute reference so that it is using the fully qualified package + // name. This will also mark the reference as being able to see private resources if + // there was a '*' in the reference or if the package came from the private namespace. Reference transformed_reference = entry.key; - TransformReferenceFromNamespace(package_decls_, - context_->GetCompilationPackage(), - &transformed_reference); + ResolvePackage(package_decls_, &transformed_reference); - // Find the attribute in the symbol table and check if it is visible from - // this callsite. + // Find the attribute in the symbol table and check if it is visible from this callsite. const SymbolTable::Symbol* symbol = ReferenceLinker::ResolveAttributeCheckVisibility( transformed_reference, callsite_, symbols_, &err_str); if (symbol) { - // Assign our style key the correct ID. - // The ID may not exist. + // Assign our style key the correct ID. The ID may not exist. entry.key.id = symbol->id; - // Try to convert the value to a more specific, typed value based on the - // attribute it is set to. + // Try to convert the value to a more specific, typed value based on the attribute it is + // set to. entry.value = ParseValueWithAttribute(std::move(entry.value), symbol->attribute.get()); // Link/resolve the final value (mostly if it's a reference). @@ -115,8 +98,8 @@ class ReferenceLinkerVisitor : public ValueVisitor { // The actual type of this item is incompatible with the attribute. DiagMessage msg(entry.key.GetSource()); - // Call the matches method again, this time with a DiagMessage so we - // fill in the actual error message. + // Call the matches method again, this time with a DiagMessage so we fill in the actual + // error message. symbol->attribute->Matches(*entry.value, &msg); context_->GetDiagnostics()->Error(msg); error_ = true; @@ -125,7 +108,7 @@ class ReferenceLinkerVisitor : public ValueVisitor { } else { DiagMessage msg(entry.key.GetSource()); msg << "style attribute '"; - ReferenceLinker::WriteResourceName(&msg, entry.key, transformed_reference); + ReferenceLinker::WriteResourceName(entry.key, callsite_, package_decls_, &msg); msg << "' " << err_str; context_->GetDiagnostics()->Error(msg); error_ = true; @@ -133,17 +116,15 @@ class ReferenceLinkerVisitor : public ValueVisitor { } } - bool HasError() { return error_; } + bool HasError() { + return error_; + } private: DISALLOW_COPY_AND_ASSIGN(ReferenceLinkerVisitor); - /** - * Transform a RawString value into a more specific, appropriate value, based - * on the - * Attribute. If a non RawString value is passed in, this is an identity - * transform. - */ + // Transform a RawString value into a more specific, appropriate value, based on the + // Attribute. If a non RawString value is passed in, this is an identity transform. std::unique_ptr<Item> ParseValueWithAttribute(std::unique_ptr<Item> value, const Attribute* attr) { if (RawString* raw_string = ValueCast<RawString>(value.get())) { @@ -178,11 +159,9 @@ class EmptyDeclStack : public xml::IPackageDeclStack { public: EmptyDeclStack() = default; - Maybe<xml::ExtractedPackage> TransformPackageAlias( - const StringPiece& alias, - const StringPiece& local_package) const override { + Maybe<xml::ExtractedPackage> TransformPackageAlias(const StringPiece& alias) const override { if (alias.empty()) { - return xml::ExtractedPackage{local_package.to_string(), true /* private */}; + return xml::ExtractedPackage{{}, true /*private*/}; } return {}; } @@ -191,32 +170,44 @@ class EmptyDeclStack : public xml::IPackageDeclStack { DISALLOW_COPY_AND_ASSIGN(EmptyDeclStack); }; -} // namespace +// The symbol is visible if it is public, or if the reference to it is requesting private access +// or if the callsite comes from the same package. +bool IsSymbolVisible(const SymbolTable::Symbol& symbol, const Reference& ref, + const CallSite& callsite) { + if (symbol.is_public || ref.private_reference) { + return true; + } -/** - * The symbol is visible if it is public, or if the reference to it is - * requesting private access - * or if the callsite comes from the same package. - */ -bool ReferenceLinker::IsSymbolVisible(const SymbolTable::Symbol& symbol, - const Reference& ref, - const CallSite& callsite) { - if (!symbol.is_public && !ref.private_reference) { - if (ref.name) { - return callsite.resource.package == ref.name.value().package; - } else if (ref.id && symbol.id) { - return ref.id.value().package_id() == symbol.id.value().package_id(); - } else { - return false; + if (ref.name) { + const ResourceName& name = ref.name.value(); + if (name.package.empty()) { + // If the symbol was found, and the package is empty, that means it was found in the local + // scope, which is always visible (private local). + return true; } + + // The symbol is visible if the reference is local to the same package it is defined in. + return callsite.package == name.package; } - return true; + + if (ref.id && symbol.id) { + return ref.id.value().package_id() == symbol.id.value().package_id(); + } + return false; } +} // namespace + const SymbolTable::Symbol* ReferenceLinker::ResolveSymbol(const Reference& reference, + const CallSite& callsite, SymbolTable* symbols) { if (reference.name) { - return symbols->FindByName(reference.name.value()); + const ResourceName& name = reference.name.value(); + if (name.package.empty()) { + // Use the callsite's package name if no package name was defined. + return symbols->FindByName(ResourceName(callsite.package, name.type, name.entry)); + } + return symbols->FindByName(name); } else if (reference.id) { return symbols->FindById(reference.id.value()); } else { @@ -228,7 +219,7 @@ const SymbolTable::Symbol* ReferenceLinker::ResolveSymbolCheckVisibility(const R const CallSite& callsite, SymbolTable* symbols, std::string* out_error) { - const SymbolTable::Symbol* symbol = ResolveSymbol(reference, symbols); + const SymbolTable::Symbol* symbol = ResolveSymbol(reference, callsite, symbols); if (!symbol) { if (out_error) *out_error = "not found"; return nullptr; @@ -274,24 +265,62 @@ Maybe<xml::AaptAttribute> ReferenceLinker::CompileXmlAttribute(const Reference& return xml::AaptAttribute(*symbol->attribute, symbol->id); } -void ReferenceLinker::WriteResourceName(DiagMessage* out_msg, - const Reference& orig, - const Reference& transformed) { +void ReferenceLinker::WriteResourceName(const Reference& ref, const CallSite& callsite, + const xml::IPackageDeclStack* decls, DiagMessage* out_msg) { CHECK(out_msg != nullptr); + if (!ref.name) { + *out_msg << ref.id.value(); + return; + } - if (orig.name) { - *out_msg << orig.name.value(); - if (transformed.name.value() != orig.name.value()) { - *out_msg << " (aka " << transformed.name.value() << ")"; - } - } else { - *out_msg << orig.id.value(); + *out_msg << ref.name.value(); + + Reference fully_qualified = ref; + xml::ResolvePackage(decls, &fully_qualified); + + ResourceName& full_name = fully_qualified.name.value(); + if (full_name.package.empty()) { + full_name.package = callsite.package; + } + + if (full_name != ref.name.value()) { + *out_msg << " (aka " << full_name << ")"; + } +} + +void ReferenceLinker::WriteAttributeName(const Reference& ref, const CallSite& callsite, + const xml::IPackageDeclStack* decls, + DiagMessage* out_msg) { + CHECK(out_msg != nullptr); + if (!ref.name) { + *out_msg << ref.id.value(); + return; + } + + const ResourceName& ref_name = ref.name.value(); + CHECK_EQ(ref_name.type, ResourceType::kAttr); + + if (!ref_name.package.empty()) { + *out_msg << ref_name.package << ":"; + } + *out_msg << ref_name.entry; + + Reference fully_qualified = ref; + xml::ResolvePackage(decls, &fully_qualified); + + ResourceName& full_name = fully_qualified.name.value(); + if (full_name.package.empty()) { + full_name.package = callsite.package; + } + + if (full_name != ref.name.value()) { + *out_msg << " (aka " << full_name.package << ":" << full_name.entry << ")"; } } bool ReferenceLinker::LinkReference(const CallSite& callsite, Reference* reference, IAaptContext* context, SymbolTable* symbols, - xml::IPackageDeclStack* decls) { + const xml::IPackageDeclStack* decls) { CHECK(reference != nullptr); if (!reference->name && !reference->id) { // This is @null. @@ -299,7 +328,7 @@ bool ReferenceLinker::LinkReference(const CallSite& callsite, Reference* referen } Reference transformed_reference = *reference; - TransformReferenceFromNamespace(decls, context->GetCompilationPackage(), &transformed_reference); + xml::ResolvePackage(decls, &transformed_reference); std::string err_str; const SymbolTable::Symbol* s = @@ -314,7 +343,7 @@ bool ReferenceLinker::LinkReference(const CallSite& callsite, Reference* referen DiagMessage error_msg(reference->GetSource()); error_msg << "resource "; - WriteResourceName(&error_msg, *reference, transformed_reference); + WriteResourceName(*reference, callsite, decls, &error_msg); error_msg << " " << err_str; context->GetDiagnostics()->Error(error_msg); return false; @@ -324,21 +353,24 @@ bool ReferenceLinker::Consume(IAaptContext* context, ResourceTable* table) { EmptyDeclStack decl_stack; bool error = false; for (auto& package : table->packages) { + // Since we're linking, each package must have a name. + CHECK(!package->name.empty()) << "all packages being linked must have a name"; + for (auto& type : package->types) { for (auto& entry : type->entries) { - // Symbol state information may be lost if there is no value for the - // resource. - if (entry->symbol_status.state != SymbolState::kUndefined && - entry->values.empty()) { - context->GetDiagnostics()->Error( - DiagMessage(entry->symbol_status.source) - << "no definition for declared symbol '" - << ResourceNameRef(package->name, type->type, entry->name) - << "'"); + // First, unmangle the name if necessary. + ResourceName name(package->name, type->type, entry->name); + NameMangler::Unmangle(&name.entry, &name.package); + + // Symbol state information may be lost if there is no value for the resource. + if (entry->symbol_status.state != SymbolState::kUndefined && entry->values.empty()) { + context->GetDiagnostics()->Error(DiagMessage(entry->symbol_status.source) + << "no definition for declared symbol '" << name << "'"); error = true; } - CallSite callsite = {ResourceNameRef(package->name, type->type, entry->name)}; + // The context of this resource is the package in which it is defined. + const CallSite callsite{name.package}; ReferenceLinkerVisitor visitor(callsite, context, context->GetExternalSymbols(), &table->string_pool, &decl_stack); diff --git a/tools/aapt2/link/ReferenceLinker.h b/tools/aapt2/link/ReferenceLinker.h index b3d0196d9e7c..3b11bee0acc9 100644 --- a/tools/aapt2/link/ReferenceLinker.h +++ b/tools/aapt2/link/ReferenceLinker.h @@ -29,83 +29,58 @@ namespace aapt { -/** - * Resolves all references to resources in the ResourceTable and assigns them - * IDs. - * The ResourceTable must already have IDs assigned to each resource. - * Once the ResourceTable is processed by this linker, it is ready to be - * flattened. - */ +// Resolves all references to resources in the ResourceTable and assigns them IDs. +// The ResourceTable must already have IDs assigned to each resource. +// Once the ResourceTable is processed by this linker, it is ready to be flattened. class ReferenceLinker : public IResourceTableConsumer { public: ReferenceLinker() = default; - /** - * Returns true if the symbol is visible by the reference and from the - * callsite. - */ - static bool IsSymbolVisible(const SymbolTable::Symbol& symbol, - const Reference& ref, const CallSite& callsite); - - /** - * Performs name mangling and looks up the resource in the symbol table. - * Returns nullptr if the symbol was not found. - */ - static const SymbolTable::Symbol* ResolveSymbol(const Reference& reference, SymbolTable* symbols); - - /** - * Performs name mangling and looks up the resource in the symbol table. If - * the symbol is not visible by the reference at the callsite, nullptr is - * returned. out_error holds the error message. - */ + // Performs name mangling and looks up the resource in the symbol table. Uses the callsite's + // package if the reference has no package name defined (implicit). + // Returns nullptr if the symbol was not found. + static const SymbolTable::Symbol* ResolveSymbol(const Reference& reference, + const CallSite& callsite, SymbolTable* symbols); + + // Performs name mangling and looks up the resource in the symbol table. If the symbol is not + // visible by the reference at the callsite, nullptr is returned. + // `out_error` holds the error message. static const SymbolTable::Symbol* ResolveSymbolCheckVisibility(const Reference& reference, const CallSite& callsite, SymbolTable* symbols, std::string* out_error); - /** - * Same as resolveSymbolCheckVisibility(), but also makes sure the symbol is - * an attribute. - * That is, the return value will have a non-null value for - * ISymbolTable::Symbol::attribute. - */ + // Same as ResolveSymbolCheckVisibility(), but also makes sure the symbol is an attribute. + // That is, the return value will have a non-null value for ISymbolTable::Symbol::attribute. static const SymbolTable::Symbol* ResolveAttributeCheckVisibility(const Reference& reference, const CallSite& callsite, SymbolTable* symbols, std::string* out_error); - /** - * Resolves the attribute reference and returns an xml::AaptAttribute if - * successful. - * If resolution fails, outError holds the error message. - */ + // Resolves the attribute reference and returns an xml::AaptAttribute if successful. + // If resolution fails, outError holds the error message. static Maybe<xml::AaptAttribute> CompileXmlAttribute(const Reference& reference, const CallSite& callsite, SymbolTable* symbols, std::string* out_error); - /** - * Writes the resource name to the DiagMessage, using the - * "orig_name (aka <transformed_name>)" syntax. - */ - static void WriteResourceName(DiagMessage* out_msg, const Reference& orig, - const Reference& transformed); - - /** - * Transforms the package name of the reference to the fully qualified package - * name using - * the xml::IPackageDeclStack, then mangles and looks up the symbol. If the - * symbol is visible - * to the reference at the callsite, the reference is updated with an ID. - * Returns false on failure, and an error message is logged to the - * IDiagnostics in the context. - */ + // Writes the resource name to the DiagMessage, using the + // "orig_name (aka <transformed_name>)" syntax. + static void WriteResourceName(const Reference& orig, const CallSite& callsite, + const xml::IPackageDeclStack* decls, DiagMessage* out_msg); + + // Same as WriteResourceName but omits the 'attr' part. + static void WriteAttributeName(const Reference& ref, const CallSite& callsite, + const xml::IPackageDeclStack* decls, DiagMessage* out_msg); + + // Transforms the package name of the reference to the fully qualified package name using + // the xml::IPackageDeclStack, then mangles and looks up the symbol. If the symbol is visible + // to the reference at the callsite, the reference is updated with an ID. + // Returns false on failure, and an error message is logged to the IDiagnostics in the context. static bool LinkReference(const CallSite& callsite, Reference* reference, IAaptContext* context, - SymbolTable* symbols, xml::IPackageDeclStack* decls); + SymbolTable* symbols, const xml::IPackageDeclStack* decls); - /** - * Links all references in the ResourceTable. - */ + // Links all references in the ResourceTable. bool Consume(IAaptContext* context, ResourceTable* table) override; private: diff --git a/tools/aapt2/link/ReferenceLinker_test.cpp b/tools/aapt2/link/ReferenceLinker_test.cpp index 72a91689e392..be38b967c986 100644 --- a/tools/aapt2/link/ReferenceLinker_test.cpp +++ b/tools/aapt2/link/ReferenceLinker_test.cpp @@ -18,7 +18,9 @@ #include "test/Test.h" -using android::ResTable_map; +using ::android::ResTable_map; +using ::testing::Eq; +using ::testing::IsNull; using ::testing::NotNull; namespace aapt { @@ -263,7 +265,7 @@ TEST(ReferenceLinkerTest, AppsWithSamePackageButDifferentIdAreVisibleNonPublic) .Build()); std::string error; - const CallSite call_site{ResourceNameRef("com.app.test", ResourceType::kString, "foo")}; + const CallSite call_site{"com.app.test"}; const SymbolTable::Symbol* symbol = ReferenceLinker::ResolveSymbolCheckVisibility( *test::BuildReference("com.app.test:string/foo"), call_site, &table, &error); ASSERT_THAT(symbol, NotNull()); @@ -281,7 +283,7 @@ TEST(ReferenceLinkerTest, AppsWithDifferentPackageCanNotUseEachOthersAttribute) .Build()); std::string error; - const CallSite call_site{ResourceNameRef("com.app.ext", ResourceType::kLayout, "foo")}; + const CallSite call_site{"com.app.ext"}; EXPECT_FALSE(ReferenceLinker::CompileXmlAttribute( *test::BuildReference("com.app.test:attr/foo"), call_site, &table, &error)); @@ -293,4 +295,27 @@ TEST(ReferenceLinkerTest, AppsWithDifferentPackageCanNotUseEachOthersAttribute) EXPECT_TRUE(error.empty()); } +TEST(ReferenceLinkerTest, ReferenceWithNoPackageUsesCallSitePackage) { + NameMangler mangler(NameManglerPolicy{"com.app.test"}); + SymbolTable table(&mangler); + table.AppendSource(test::StaticSymbolSourceBuilder() + .AddSymbol("com.app.test:string/foo", ResourceId(0x7f010000)) + .AddSymbol("com.app.lib:string/foo", ResourceId(0x7f010001)) + .Build()); + + const SymbolTable::Symbol* s = ReferenceLinker::ResolveSymbol(*test::BuildReference("string/foo"), + CallSite{"com.app.test"}, &table); + ASSERT_THAT(s, NotNull()); + EXPECT_THAT(s->id, Eq(make_value<ResourceId>(0x7f010000))); + + s = ReferenceLinker::ResolveSymbol(*test::BuildReference("string/foo"), CallSite{"com.app.lib"}, + &table); + ASSERT_THAT(s, NotNull()); + EXPECT_THAT(s->id, Eq(make_value<ResourceId>(0x7f010001))); + + EXPECT_THAT(ReferenceLinker::ResolveSymbol(*test::BuildReference("string/foo"), + CallSite{"com.app.bad"}, &table), + IsNull()); +} + } // namespace aapt diff --git a/tools/aapt2/link/TableMerger.cpp b/tools/aapt2/link/TableMerger.cpp index 10e837c725e5..93c904f1a743 100644 --- a/tools/aapt2/link/TableMerger.cpp +++ b/tools/aapt2/link/TableMerger.cpp @@ -24,7 +24,7 @@ #include "ValueVisitor.h" #include "util/Util.h" -using android::StringPiece; +using ::android::StringPiece; namespace aapt { @@ -32,27 +32,23 @@ TableMerger::TableMerger(IAaptContext* context, ResourceTable* out_table, const TableMergerOptions& options) : context_(context), master_table_(out_table), options_(options) { // Create the desired package that all tables will be merged into. - master_package_ = master_table_->CreatePackage( - context_->GetCompilationPackage(), context_->GetPackageId()); + master_package_ = + master_table_->CreatePackage(context_->GetCompilationPackage(), context_->GetPackageId()); CHECK(master_package_ != nullptr) << "package name or ID already taken"; } -bool TableMerger::Merge(const Source& src, ResourceTable* table, - io::IFileCollection* collection) { - return MergeImpl(src, table, collection, false /* overlay */, true /* allow new */); +bool TableMerger::Merge(const Source& src, ResourceTable* table, io::IFileCollection* collection) { + return MergeImpl(src, table, collection, false /*overlay*/, true /*allow_new*/); } bool TableMerger::MergeOverlay(const Source& src, ResourceTable* table, io::IFileCollection* collection) { - return MergeImpl(src, table, collection, true /* overlay */, options_.auto_add_overlay); + return MergeImpl(src, table, collection, true /*overlay*/, options_.auto_add_overlay); } -/** - * This will merge packages with the same package name (or no package name). - */ +// This will merge packages with the same package name (or no package name). bool TableMerger::MergeImpl(const Source& src, ResourceTable* table, - io::IFileCollection* collection, bool overlay, - bool allow_new) { + io::IFileCollection* collection, bool overlay, bool allow_new) { bool error = false; for (auto& package : table->packages) { // Only merge an empty package or the package we're building. @@ -62,9 +58,8 @@ bool TableMerger::MergeImpl(const Source& src, ResourceTable* table, if (package->name.empty() || context_->GetCompilationPackage() == package->name) { FileMergeCallback callback; if (collection) { - callback = [&](const ResourceNameRef& name, - const ConfigDescription& config, FileReference* new_file, - FileReference* old_file) -> bool { + callback = [&](const ResourceNameRef& name, const ConfigDescription& config, + FileReference* new_file, FileReference* old_file) -> bool { // The old file's path points inside the APK, so we can use it as is. io::IFile* f = collection->FindFile(*old_file->path); if (!f) { @@ -78,45 +73,38 @@ bool TableMerger::MergeImpl(const Source& src, ResourceTable* table, }; } - // Merge here. Once the entries are merged and mangled, any references to - // them are still valid. This is because un-mangled references are - // mangled, then looked up at resolution time. - // Also, when linking, we convert references with no package name to use - // the compilation package name. - error |= !DoMerge(src, table, package.get(), false /* mangle */, overlay, - allow_new, callback); + // Merge here. Once the entries are merged and mangled, any references to them are still + // valid. This is because un-mangled references are mangled, then looked up at resolution + // time. Also, when linking, we convert references with no package name to use the compilation + // package name. + error |= + !DoMerge(src, table, package.get(), false /* mangle */, overlay, allow_new, callback); } } return !error; } -/** - * This will merge and mangle resources from a static library. - */ -bool TableMerger::MergeAndMangle(const Source& src, - const StringPiece& package_name, - ResourceTable* table, - io::IFileCollection* collection) { +// This will merge and mangle resources from a static library. +bool TableMerger::MergeAndMangle(const Source& src, const StringPiece& package_name, + ResourceTable* table, io::IFileCollection* collection) { bool error = false; for (auto& package : table->packages) { // Warn of packages with an unrelated ID. if (package_name != package->name) { - context_->GetDiagnostics()->Warn(DiagMessage(src) << "ignoring package " - << package->name); + context_->GetDiagnostics()->Warn(DiagMessage(src) << "ignoring package " << package->name); continue; } bool mangle = package_name != context_->GetCompilationPackage(); merged_packages_.insert(package->name); - auto callback = [&]( - const ResourceNameRef& name, const ConfigDescription& config, - FileReference* new_file, FileReference* old_file) -> bool { + auto callback = [&](const ResourceNameRef& name, const ConfigDescription& config, + FileReference* new_file, FileReference* old_file) -> bool { // The old file's path points inside the APK, so we can use it as is. io::IFile* f = collection->FindFile(*old_file->path); if (!f) { - context_->GetDiagnostics()->Error( - DiagMessage(src) << "file '" << *old_file->path << "' not found"); + context_->GetDiagnostics()->Error(DiagMessage(src) + << "file '" << *old_file->path << "' not found"); return false; } @@ -124,21 +112,18 @@ bool TableMerger::MergeAndMangle(const Source& src, return true; }; - error |= !DoMerge(src, table, package.get(), mangle, false /* overlay */, - true /* allow new */, callback); + error |= !DoMerge(src, table, package.get(), mangle, false /*overlay*/, true /*allow_new*/, + callback); } return !error; } -static bool MergeType(IAaptContext* context, const Source& src, - ResourceTableType* dst_type, +static bool MergeType(IAaptContext* context, const Source& src, ResourceTableType* dst_type, ResourceTableType* src_type) { if (dst_type->symbol_status.state < src_type->symbol_status.state) { - // The incoming type's visibility is stronger, so we should override - // the visibility. + // The incoming type's visibility is stronger, so we should override the visibility. if (src_type->symbol_status.state == SymbolState::kPublic) { - // Only copy the ID if the source is public, or else the ID is - // meaningless. + // Only copy the ID if the source is public, or else the ID is meaningless. dst_type->id = src_type->id; } dst_type->symbol_status = std::move(src_type->symbol_status); @@ -155,14 +140,12 @@ static bool MergeType(IAaptContext* context, const Source& src, return true; } -static bool MergeEntry(IAaptContext* context, const Source& src, - ResourceEntry* dst_entry, ResourceEntry* src_entry) { +static bool MergeEntry(IAaptContext* context, const Source& src, ResourceEntry* dst_entry, + ResourceEntry* src_entry) { if (dst_entry->symbol_status.state < src_entry->symbol_status.state) { - // The incoming type's visibility is stronger, so we should override - // the visibility. + // The incoming type's visibility is stronger, so we should override the visibility. if (src_entry->symbol_status.state == SymbolState::kPublic) { - // Only copy the ID if the source is public, or else the ID is - // meaningless. + // Only copy the ID if the source is public, or else the ID is meaningless. dst_entry->id = src_entry->id; } dst_entry->symbol_status = std::move(src_entry->symbol_status); @@ -171,9 +154,8 @@ static bool MergeEntry(IAaptContext* context, const Source& src, dst_entry->id && src_entry->id && dst_entry->id.value() != src_entry->id.value()) { // Both entries are public and have different IDs. - context->GetDiagnostics()->Error( - DiagMessage(src) << "cannot merge entry '" << src_entry->name - << "': conflicting public IDs"); + context->GetDiagnostics()->Error(DiagMessage(src) << "cannot merge entry '" << src_entry->name + << "': conflicting public IDs"); return false; } return true; @@ -181,12 +163,10 @@ static bool MergeEntry(IAaptContext* context, const Source& src, // Modified CollisionResolver which will merge Styleables and Styles. Used with overlays. // -// Styleables are not actual resources, but they are treated as such during the -// compilation phase. +// Styleables are not actual resources, but they are treated as such during the compilation phase. // -// Styleables and Styles don't simply overlay each other, their definitions merge -// and accumulate. If both values are Styleables/Styles, we just merge them into the -// existing value. +// Styleables and Styles don't simply overlay each other, their definitions merge and accumulate. +// If both values are Styleables/Styles, we just merge them into the existing value. static ResourceTable::CollisionResult ResolveMergeCollision(Value* existing, Value* incoming, StringPool* pool) { if (Styleable* existing_styleable = ValueCast<Styleable>(existing)) { diff --git a/tools/aapt2/link/TableMerger.h b/tools/aapt2/link/TableMerger.h index c96b1b0b4dfb..81518ffb2441 100644 --- a/tools/aapt2/link/TableMerger.h +++ b/tools/aapt2/link/TableMerger.h @@ -33,81 +33,49 @@ namespace aapt { struct TableMergerOptions { - /** - * If true, resources in overlays can be added without previously having - * existed. - */ + // If true, resources in overlays can be added without previously having existed. bool auto_add_overlay = false; }; -/** - * TableMerger takes resource tables and merges all packages within the tables - * that have the same - * package ID. - * - * If a package has a different name, all the entries in that table have their - * names mangled - * to include the package name. This way there are no collisions. In order to do - * this correctly, - * the TableMerger needs to also mangle any FileReference paths. Once these are - * mangled, - * the original source path of the file, along with the new destination path is - * recorded in the - * queue returned from getFileMergeQueue(). - * - * Once the merging is complete, a separate process can go collect the files - * from the various - * source APKs and either copy or process their XML and put them in the correct - * location in - * the final APK. - */ +// TableMerger takes resource tables and merges all packages within the tables that have the same +// package ID. +// +// If a package has a different name, all the entries in that table have their names mangled +// to include the package name. This way there are no collisions. In order to do this correctly, +// the TableMerger needs to also mangle any FileReference paths. Once these are mangled, the +// `IFile` pointer in `FileReference` will point to the original file. +// +// Once the merging is complete, a separate phase can go collect the files from the various +// source APKs and either copy or process their XML and put them in the correct location in the +// final APK. class TableMerger { public: - /** - * Note: The out_table ResourceTable must live longer than this TableMerger. - * References are made to this ResourceTable for efficiency reasons. - */ - TableMerger(IAaptContext* context, ResourceTable* out_table, - const TableMergerOptions& options); - - const std::set<std::string>& merged_packages() const { + // Note: The out_table ResourceTable must live longer than this TableMerger. + // References are made to this ResourceTable for efficiency reasons. + TableMerger(IAaptContext* context, ResourceTable* out_table, const TableMergerOptions& options); + + inline const std::set<std::string>& merged_packages() const { return merged_packages_; } - /** - * Merges resources from the same or empty package. This is for local sources. - * An io::IFileCollection is optional and used to find the referenced Files - * and process them. - */ - bool Merge(const Source& src, ResourceTable* table, - io::IFileCollection* collection = nullptr); - - /** - * Merges resources from an overlay ResourceTable. - * An io::IFileCollection is optional and used to find the referenced Files - * and process them. - */ + // Merges resources from the same or empty package. This is for local sources. + // An io::IFileCollection is optional and used to find the referenced Files and process them. + bool Merge(const Source& src, ResourceTable* table, io::IFileCollection* collection = nullptr); + + // Merges resources from an overlay ResourceTable. + // An io::IFileCollection is optional and used to find the referenced Files and process them. bool MergeOverlay(const Source& src, ResourceTable* table, io::IFileCollection* collection = nullptr); - /** - * Merges resources from the given package, mangling the name. This is for - * static libraries. - * An io::IFileCollection is needed in order to find the referenced Files and - * process them. - */ + // Merges resources from the given package, mangling the name. This is for static libraries. + // An io::IFileCollection is needed in order to find the referenced Files and process them. bool MergeAndMangle(const Source& src, const android::StringPiece& package, ResourceTable* table, io::IFileCollection* collection); - /** - * Merges a compiled file that belongs to this same or empty package. This is - * for local sources. - */ + // Merges a compiled file that belongs to this same or empty package. This is for local sources. bool MergeFile(const ResourceFile& fileDesc, io::IFile* file); - /** - * Merges a compiled file from an overlay, overriding an existing definition. - */ + // Merges a compiled file from an overlay, overriding an existing definition. bool MergeFileOverlay(const ResourceFile& fileDesc, io::IFile* file); private: diff --git a/tools/aapt2/link/XmlReferenceLinker.cpp b/tools/aapt2/link/XmlReferenceLinker.cpp index bcecd2003846..6ebb80f4be8f 100644 --- a/tools/aapt2/link/XmlReferenceLinker.cpp +++ b/tools/aapt2/link/XmlReferenceLinker.cpp @@ -31,13 +31,9 @@ namespace aapt { namespace { -/** - * Visits all references (including parents of styles, references in styles, - * arrays, etc) and - * links their symbolic name to their Resource ID, performing mangling and - * package aliasing - * as needed. - */ +// Visits all references (including parents of styles, references in styles, arrays, etc) and +// links their symbolic name to their Resource ID, performing mangling and package aliasing +// as needed. class ReferenceVisitor : public ValueVisitor { public: using ValueVisitor::Visit; @@ -52,7 +48,9 @@ class ReferenceVisitor : public ValueVisitor { } } - bool HasError() const { return error_; } + bool HasError() const { + return error_; + } private: DISALLOW_COPY_AND_ASSIGN(ReferenceVisitor); @@ -64,9 +62,7 @@ class ReferenceVisitor : public ValueVisitor { bool error_; }; -/** - * Visits each xml Element and compiles the attributes within. - */ +// Visits each xml Element and compiles the attributes within. class XmlVisitor : public xml::PackageAwareVisitor { public: using xml::PackageAwareVisitor::Visit; @@ -92,18 +88,12 @@ class XmlVisitor : public xml::PackageAwareVisitor { // they were assigned to the default Attribute. const Attribute* attribute = &kDefaultAttribute; - std::string attribute_package; if (Maybe<xml::ExtractedPackage> maybe_package = xml::ExtractPackageFromNamespace(attr.namespace_uri)) { // There is a valid package name for this attribute. We will look this up. - attribute_package = maybe_package.value().package; - if (attribute_package.empty()) { - // Empty package means the 'current' or 'local' package. - attribute_package = context_->GetCompilationPackage(); - } - - Reference attr_ref(ResourceNameRef(attribute_package, ResourceType::kAttr, attr.name)); + Reference attr_ref( + ResourceNameRef(maybe_package.value().package, ResourceType::kAttr, attr.name)); attr_ref.private_reference = maybe_package.value().private_namespace; std::string err_str; @@ -111,9 +101,11 @@ class XmlVisitor : public xml::PackageAwareVisitor { ReferenceLinker::CompileXmlAttribute(attr_ref, callsite_, symbols_, &err_str); if (!attr.compiled_attribute) { - context_->GetDiagnostics()->Error(DiagMessage(source) << "attribute '" - << attribute_package << ":" - << attr.name << "' " << err_str); + DiagMessage error_msg(source); + error_msg << "attribute "; + ReferenceLinker::WriteAttributeName(attr_ref, callsite_, this, &error_msg); + error_msg << " " << err_str; + context_->GetDiagnostics()->Error(error_msg); error_ = true; continue; } @@ -129,12 +121,8 @@ class XmlVisitor : public xml::PackageAwareVisitor { } else if ((attribute->type_mask & android::ResTable_map::TYPE_STRING) == 0) { // We won't be able to encode this as a string. DiagMessage msg(source); - msg << "'" << attr.value << "' " - << "is incompatible with attribute "; - if (!attribute_package.empty()) { - msg << attribute_package << ":"; - } - msg << attr.name << " " << *attribute; + msg << "'" << attr.value << "' is incompatible with attribute " << attr.name << " " + << *attribute; context_->GetDiagnostics()->Error(msg); error_ = true; } @@ -163,7 +151,17 @@ class XmlVisitor : public xml::PackageAwareVisitor { } // namespace bool XmlReferenceLinker::Consume(IAaptContext* context, xml::XmlResource* resource) { - const CallSite callsite = {resource->file.name}; + CallSite callsite{resource->file.name.package}; + + std::string out_name = resource->file.name.entry; + NameMangler::Unmangle(&out_name, &callsite.package); + + if (callsite.package.empty()) { + // Assume an empty package means that the XML file is local. This is true of AndroidManifest.xml + // for example. + callsite.package = context->GetCompilationPackage(); + } + XmlVisitor visitor(resource->file.source, callsite, context, context->GetExternalSymbols()); if (resource->root) { resource->root->Accept(&visitor); diff --git a/tools/aapt2/optimize/MultiApkGenerator.cpp b/tools/aapt2/optimize/MultiApkGenerator.cpp new file mode 100644 index 000000000000..5ff890832371 --- /dev/null +++ b/tools/aapt2/optimize/MultiApkGenerator.cpp @@ -0,0 +1,251 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "MultiApkGenerator.h" + +#include <algorithm> +#include <string> + +#include "androidfw/StringPiece.h" + +#include "LoadedApk.h" +#include "configuration/ConfigurationParser.h" +#include "filter/AbiFilter.h" +#include "filter/Filter.h" +#include "flatten/Archive.h" +#include "optimize/VersionCollapser.h" +#include "process/IResourceTableConsumer.h" +#include "split/TableSplitter.h" +#include "util/Files.h" + +namespace aapt { + +using ::aapt::configuration::AndroidSdk; +using ::aapt::configuration::Artifact; +using ::aapt::configuration::PostProcessingConfiguration; +using ::android::StringPiece; + +/** + * Context wrapper that allows the min Android SDK value to be overridden. + */ +class ContextWrapper : public IAaptContext { + public: + explicit ContextWrapper(IAaptContext* context) + : context_(context), min_sdk_(context_->GetMinSdkVersion()) { + } + + PackageType GetPackageType() override { + return context_->GetPackageType(); + } + + SymbolTable* GetExternalSymbols() override { + return context_->GetExternalSymbols(); + } + + IDiagnostics* GetDiagnostics() override { + return context_->GetDiagnostics(); + } + + const std::string& GetCompilationPackage() override { + return context_->GetCompilationPackage(); + } + + uint8_t GetPackageId() override { + return context_->GetPackageId(); + } + + NameMangler* GetNameMangler() override { + return context_->GetNameMangler(); + } + + bool IsVerbose() override { + return context_->IsVerbose(); + } + + int GetMinSdkVersion() override { + return min_sdk_; + } + + void SetMinSdkVersion(int min_sdk) { + min_sdk_ = min_sdk; + } + + private: + IAaptContext* context_; + + int min_sdk_ = -1; +}; + +MultiApkGenerator::MultiApkGenerator(LoadedApk* apk, IAaptContext* context) + : apk_(apk), context_(context) { +} + +bool MultiApkGenerator::FromBaseApk(const MultiApkGeneratorOptions& options) { + // TODO(safarmer): Handle APK version codes for the generated APKs. + IDiagnostics* diag = context_->GetDiagnostics(); + const PostProcessingConfiguration& config = options.config; + + const std::string& apk_name = file::GetFilename(apk_->GetSource().path).to_string(); + const StringPiece ext = file::GetExtension(apk_name); + const std::string base_name = apk_name.substr(0, apk_name.rfind(ext.to_string())); + + // For now, just write out the stripped APK since ABI splitting doesn't modify anything else. + for (const Artifact& artifact : config.artifacts) { + FilterChain filters; + + if (!artifact.name && !config.artifact_format) { + diag->Error( + DiagMessage() << "Artifact does not have a name and no global name template defined"); + return false; + } + + Maybe<std::string> artifact_name = + (artifact.name) ? artifact.Name(apk_name, diag) + : artifact.ToArtifactName(config.artifact_format.value(), apk_name, diag); + + if (!artifact_name) { + diag->Error(DiagMessage() << "Could not determine split APK artifact name"); + return false; + } + + std::unique_ptr<ResourceTable> table = + FilterTable(artifact, config, *apk_->GetResourceTable(), &filters); + if (!table) { + return false; + } + + std::string out = options.out_dir; + if (!file::mkdirs(out)) { + context_->GetDiagnostics()->Warn(DiagMessage() << "could not create out dir: " << out); + } + file::AppendPath(&out, artifact_name.value()); + + if (context_->IsVerbose()) { + context_->GetDiagnostics()->Note(DiagMessage() << "Generating split: " << out); + } + + std::unique_ptr<IArchiveWriter> writer = + CreateZipFileArchiveWriter(context_->GetDiagnostics(), out); + + if (context_->IsVerbose()) { + diag->Note(DiagMessage() << "Writing output: " << out); + } + + if (!apk_->WriteToArchive(context_, table.get(), options.table_flattener_options, &filters, + writer.get())) { + return false; + } + } + + return true; +} + +std::unique_ptr<ResourceTable> MultiApkGenerator::FilterTable( + const configuration::Artifact& artifact, + const configuration::PostProcessingConfiguration& config, const ResourceTable& old_table, + FilterChain* filters) { + TableSplitterOptions splits; + AxisConfigFilter axis_filter; + ContextWrapper wrappedContext{context_}; + + if (artifact.abi_group) { + const std::string& group_name = artifact.abi_group.value(); + + auto group = config.abi_groups.find(group_name); + // TODO: Remove validation when configuration parser ensures referential integrity. + if (group == config.abi_groups.end()) { + context_->GetDiagnostics()->Error(DiagMessage() << "could not find referenced ABI group '" + << group_name << "'"); + return {}; + } + filters->AddFilter(AbiFilter::FromAbiList(group->second)); + } + + if (artifact.screen_density_group) { + const std::string& group_name = artifact.screen_density_group.value(); + + auto group = config.screen_density_groups.find(group_name); + // TODO: Remove validation when configuration parser ensures referential integrity. + if (group == config.screen_density_groups.end()) { + context_->GetDiagnostics()->Error(DiagMessage() << "could not find referenced group '" + << group_name << "'"); + return {}; + } + + const std::vector<ConfigDescription>& densities = group->second; + for(const auto& density_config : densities) { + splits.preferred_densities.push_back(density_config.density); + } + } + + if (artifact.locale_group) { + const std::string& group_name = artifact.locale_group.value(); + auto group = config.locale_groups.find(group_name); + // TODO: Remove validation when configuration parser ensures referential integrity. + if (group == config.locale_groups.end()) { + context_->GetDiagnostics()->Error(DiagMessage() << "could not find referenced group '" + << group_name << "'"); + return {}; + } + + const std::vector<ConfigDescription>& locales = group->second; + for (const auto& locale : locales) { + axis_filter.AddConfig(locale); + } + splits.config_filter = &axis_filter; + } + + if (artifact.android_sdk_group) { + const std::string& group_name = artifact.android_sdk_group.value(); + auto group = config.android_sdk_groups.find(group_name); + // TODO: Remove validation when configuration parser ensures referential integrity. + if (group == config.android_sdk_groups.end()) { + context_->GetDiagnostics()->Error(DiagMessage() << "could not find referenced group '" + << group_name << "'"); + return {}; + } + + const AndroidSdk& sdk = group->second; + if (!sdk.min_sdk_version) { + context_->GetDiagnostics()->Error(DiagMessage() + << "skipping SDK version. No min SDK: " << group_name); + return {}; + } + + ConfigDescription c; + const std::string& version = sdk.min_sdk_version.value(); + if (!ConfigDescription::Parse(version, &c)) { + context_->GetDiagnostics()->Error(DiagMessage() << "could not parse min SDK: " << version); + return {}; + } + + wrappedContext.SetMinSdkVersion(c.sdkVersion); + } + + std::unique_ptr<ResourceTable> table = old_table.Clone(); + + VersionCollapser collapser; + if (!collapser.Consume(context_, table.get())) { + context_->GetDiagnostics()->Error(DiagMessage() << "Failed to strip versioned resources"); + return {}; + } + + TableSplitter splitter{{}, splits}; + splitter.SplitTable(table.get()); + return table; +} + +} // namespace aapt diff --git a/tools/aapt2/optimize/MultiApkGenerator.h b/tools/aapt2/optimize/MultiApkGenerator.h new file mode 100644 index 000000000000..b06440014be6 --- /dev/null +++ b/tools/aapt2/optimize/MultiApkGenerator.h @@ -0,0 +1,63 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef AAPT2_APKSPLITTER_H +#define AAPT2_APKSPLITTER_H + +#include "Diagnostics.h" +#include "LoadedApk.h" +#include "configuration/ConfigurationParser.h" + +namespace aapt { + +struct MultiApkGeneratorOptions { + std::string out_dir; + configuration::PostProcessingConfiguration config; + TableFlattenerOptions table_flattener_options; +}; + +/** + * Generates a set of APKs that are a subset of the original base APKs. Each of the new APKs contain + * only the resources and assets for an artifact in the configuration file. + */ +class MultiApkGenerator { + public: + MultiApkGenerator(LoadedApk* apk, IAaptContext* context); + + /** + * Writes a set of APKs to the provided output directory. Each APK is a subset fo the base APK and + * represents an artifact in the post processing configuration. + */ + bool FromBaseApk(const MultiApkGeneratorOptions& options); + + protected: + virtual std::unique_ptr<ResourceTable> FilterTable( + const configuration::Artifact& artifact, + const configuration::PostProcessingConfiguration& config, const ResourceTable& old_table, + FilterChain* chain); + + private: + IDiagnostics* GetDiagnostics() { + return context_->GetDiagnostics(); + } + + LoadedApk* apk_; + IAaptContext* context_; +}; + +} // namespace aapt + +#endif // AAPT2_APKSPLITTER_H diff --git a/tools/aapt2/optimize/MultiApkGenerator_test.cpp b/tools/aapt2/optimize/MultiApkGenerator_test.cpp new file mode 100644 index 000000000000..23f573cd52a5 --- /dev/null +++ b/tools/aapt2/optimize/MultiApkGenerator_test.cpp @@ -0,0 +1,274 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "optimize/MultiApkGenerator.h" + +#include <string> + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +#include "LoadedApk.h" +#include "ResourceTable.h" +#include "configuration/ConfigurationParser.h" +#include "filter/Filter.h" +#include "flatten/Archive.h" +#include "flatten/TableFlattener.h" +#include "process/IResourceTableConsumer.h" +#include "test/Context.h" +#include "test/Test.h" + +namespace aapt { +namespace { + +using ::aapt::configuration::Abi; +using ::aapt::configuration::AndroidSdk; +using ::aapt::configuration::Artifact; +using ::aapt::configuration::PostProcessingConfiguration; +using ::aapt::test::GetValue; +using ::aapt::test::GetValueForConfig; +using ::aapt::test::ParseConfigOrDie; +using ::testing::Eq; +using ::testing::IsNull; +using ::testing::Not; +using ::testing::NotNull; +using ::testing::PrintToString; +using ::testing::Return; +using ::testing::Test; +using ::testing::_; + +/** Subclass the LoadedApk class so that we can mock the WriteToArchive method. */ +class MockApk : public LoadedApk { + public: + MockApk(std::unique_ptr<ResourceTable> table) : LoadedApk({"test.apk"}, {}, std::move(table)){}; + MOCK_METHOD5(WriteToArchive, bool(IAaptContext*, ResourceTable*, const TableFlattenerOptions&, + FilterChain*, IArchiveWriter*)); +}; + +/** + * Subclass the MultiApkGenerator class so that we can access the protected FilterTable method to + * directly test table filter. + */ +class MultiApkGeneratorWrapper : public MultiApkGenerator { + public: + MultiApkGeneratorWrapper(LoadedApk* apk, IAaptContext* context) + : MultiApkGenerator(apk, context) { + } + + std::unique_ptr<ResourceTable> FilterTable( + const configuration::Artifact& artifact, + const configuration::PostProcessingConfiguration& config, const ResourceTable& old_table, + FilterChain* pChain) override { + return MultiApkGenerator::FilterTable(artifact, config, old_table, pChain); + } +}; + +/** MultiApkGenerator test fixture. */ +class MultiApkGeneratorTest : public ::testing::Test { + public: + std::unique_ptr<ResourceTable> BuildTable() { + return test::ResourceTableBuilder() + .AddFileReference(kResourceName, "res/drawable-mdpi/icon.png", mdpi_) + .AddFileReference(kResourceName, "res/drawable-hdpi/icon.png", hdpi_) + .AddFileReference(kResourceName, "res/drawable-xhdpi/icon.png", xhdpi_) + .AddFileReference(kResourceName, "res/drawable-xxhdpi/icon.png", xxhdpi_) + .AddFileReference(kResourceName, "res/drawable-v19/icon.xml", v19_) + .AddFileReference(kResourceName, "res/drawable-v21/icon.xml", v21_) + .AddSimple("android:string/one") + .Build(); + } + + inline FileReference* ValueForConfig(ResourceTable* table, const ConfigDescription& config) { + return GetValueForConfig<FileReference>(table, kResourceName, config); + }; + + void SetUp() override { + } + + protected: + static constexpr const char* kResourceName = "android:drawable/icon"; + + ConfigDescription default_ = ParseConfigOrDie("").CopyWithoutSdkVersion(); + ConfigDescription mdpi_ = ParseConfigOrDie("mdpi").CopyWithoutSdkVersion(); + ConfigDescription hdpi_ = ParseConfigOrDie("hdpi").CopyWithoutSdkVersion(); + ConfigDescription xhdpi_ = ParseConfigOrDie("xhdpi").CopyWithoutSdkVersion(); + ConfigDescription xxhdpi_ = ParseConfigOrDie("xxhdpi").CopyWithoutSdkVersion(); + ConfigDescription xxxhdpi_ = ParseConfigOrDie("xxxhdpi").CopyWithoutSdkVersion(); + ConfigDescription v19_ = ParseConfigOrDie("v19"); + ConfigDescription v21_ = ParseConfigOrDie("v21"); +}; + +TEST_F(MultiApkGeneratorTest, FromBaseApk) { + std::unique_ptr<ResourceTable> table = BuildTable(); + + MockApk apk{std::move(table)}; + + EXPECT_CALL(apk, WriteToArchive(_, _, _, _, _)).Times(0); + + test::Context ctx; + PostProcessingConfiguration empty_config; + TableFlattenerOptions table_flattener_options; + + MultiApkGenerator generator{&apk, &ctx}; + EXPECT_TRUE(generator.FromBaseApk({"out", empty_config, table_flattener_options})); + + Artifact x64 = test::ArtifactBuilder() + .SetName("${basename}.x64.apk") + .SetAbiGroup("x64") + .SetLocaleGroup("en") + .SetDensityGroup("xhdpi") + .Build(); + + Artifact intel = test::ArtifactBuilder() + .SetName("${basename}.intel.apk") + .SetAbiGroup("intel") + .SetLocaleGroup("europe") + .SetDensityGroup("large") + .Build(); + + auto config = test::PostProcessingConfigurationBuilder() + .SetLocaleGroup("en", {"en"}) + .SetLocaleGroup("europe", {"en", "fr", "de", "es"}) + .SetAbiGroup("x64", {Abi::kX86_64}) + .SetAbiGroup("intel", {Abi::kX86_64, Abi::kX86}) + .SetDensityGroup("xhdpi", {"xhdpi"}) + .SetDensityGroup("large", {"xhdpi", "xxhdpi", "xxxhdpi"}) + .AddArtifact(x64) + .AddArtifact(intel) + .Build(); + + // Called once for each artifact. + EXPECT_CALL(apk, WriteToArchive(Eq(&ctx), _, _, _, _)).Times(2).WillRepeatedly(Return(true)); + EXPECT_TRUE(generator.FromBaseApk({"out", config, table_flattener_options})); +} + +TEST_F(MultiApkGeneratorTest, VersionFilterNewerVersion) { + std::unique_ptr<ResourceTable> table = BuildTable(); + + MockApk apk{std::move(table)}; + std::unique_ptr<IAaptContext> ctx = test::ContextBuilder().SetMinSdkVersion(19).Build(); + PostProcessingConfiguration empty_config; + TableFlattenerOptions table_flattener_options; + FilterChain chain; + + Artifact x64 = test::ArtifactBuilder() + .SetName("${basename}.${sdk}.apk") + .SetDensityGroup("xhdpi") + .SetAndroidSdk("v23") + .Build(); + + auto config = test::PostProcessingConfigurationBuilder() + .SetLocaleGroup("en", {"en"}) + .SetAbiGroup("x64", {Abi::kX86_64}) + .SetDensityGroup("xhdpi", {"xhdpi"}) + .SetAndroidSdk("v23", AndroidSdk::ForMinSdk("v23")) + .AddArtifact(x64) + .Build(); + + MultiApkGeneratorWrapper generator{&apk, ctx.get()}; + std::unique_ptr<ResourceTable> split = + generator.FilterTable(x64, config, *apk.GetResourceTable(), &chain); + + ResourceTable* new_table = split.get(); + EXPECT_THAT(ValueForConfig(new_table, mdpi_), IsNull()); + EXPECT_THAT(ValueForConfig(new_table, hdpi_), IsNull()); + EXPECT_THAT(ValueForConfig(new_table, xxhdpi_), IsNull()); + EXPECT_THAT(ValueForConfig(new_table, xxxhdpi_), IsNull()); + EXPECT_THAT(ValueForConfig(new_table, v19_), IsNull()); + + // xhdpi directly matches one of the required dimensions. + EXPECT_THAT(ValueForConfig(new_table, xhdpi_), NotNull()); + // drawable-v21 was converted to drawable. + EXPECT_THAT(ValueForConfig(new_table, default_), NotNull()); + EXPECT_THAT(GetValue<Id>(new_table, "android:string/one"), NotNull()); +} + +TEST_F(MultiApkGeneratorTest, VersionFilterOlderVersion) { + std::unique_ptr<ResourceTable> table = BuildTable(); + + MockApk apk{std::move(table)}; + std::unique_ptr<IAaptContext> ctx = test::ContextBuilder().SetMinSdkVersion(1).Build(); + PostProcessingConfiguration empty_config; + TableFlattenerOptions table_flattener_options; + FilterChain chain; + + Artifact x64 = test::ArtifactBuilder() + .SetName("${basename}.${sdk}.apk") + .SetDensityGroup("xhdpi") + .SetAndroidSdk("v4") + .Build(); + + auto config = test::PostProcessingConfigurationBuilder() + .SetLocaleGroup("en", {"en"}) + .SetAbiGroup("x64", {Abi::kX86_64}) + .SetDensityGroup("xhdpi", {"xhdpi"}) + .SetAndroidSdk("v4", AndroidSdk::ForMinSdk("v4")) + .AddArtifact(x64) + .Build(); + + MultiApkGeneratorWrapper generator{&apk, ctx.get()};; + std::unique_ptr<ResourceTable> split = + generator.FilterTable(x64, config, *apk.GetResourceTable(), &chain); + + ResourceTable* new_table = split.get(); + EXPECT_THAT(ValueForConfig(new_table, mdpi_), IsNull()); + EXPECT_THAT(ValueForConfig(new_table, hdpi_), IsNull()); + EXPECT_THAT(ValueForConfig(new_table, xxhdpi_), IsNull()); + EXPECT_THAT(ValueForConfig(new_table, xxxhdpi_), IsNull()); + + EXPECT_THAT(ValueForConfig(new_table, xhdpi_), NotNull()); + EXPECT_THAT(ValueForConfig(new_table, v19_), NotNull()); + EXPECT_THAT(ValueForConfig(new_table, v21_), NotNull()); + EXPECT_THAT(GetValue<Id>(new_table, "android:string/one"), NotNull()); +} + +TEST_F(MultiApkGeneratorTest, VersionFilterNoVersion) { + std::unique_ptr<ResourceTable> table = BuildTable(); + + MockApk apk{std::move(table)}; + std::unique_ptr<IAaptContext> ctx = test::ContextBuilder().SetMinSdkVersion(1).Build(); + PostProcessingConfiguration empty_config; + TableFlattenerOptions table_flattener_options; + FilterChain chain; + + Artifact x64 = + test::ArtifactBuilder().SetName("${basename}.${sdk}.apk").SetDensityGroup("xhdpi").Build(); + + auto config = test::PostProcessingConfigurationBuilder() + .SetLocaleGroup("en", {"en"}) + .SetAbiGroup("x64", {Abi::kX86_64}) + .SetDensityGroup("xhdpi", {"xhdpi"}) + .AddArtifact(x64) + .Build(); + + MultiApkGeneratorWrapper generator{&apk, ctx.get()}; + std::unique_ptr<ResourceTable> split = + generator.FilterTable(x64, config, *apk.GetResourceTable(), &chain); + + ResourceTable* new_table = split.get(); + EXPECT_THAT(ValueForConfig(new_table, mdpi_), IsNull()); + EXPECT_THAT(ValueForConfig(new_table, hdpi_), IsNull()); + EXPECT_THAT(ValueForConfig(new_table, xxhdpi_), IsNull()); + EXPECT_THAT(ValueForConfig(new_table, xxxhdpi_), IsNull()); + + EXPECT_THAT(ValueForConfig(new_table, xhdpi_), NotNull()); + EXPECT_THAT(ValueForConfig(new_table, v19_), NotNull()); + EXPECT_THAT(ValueForConfig(new_table, v21_), NotNull()); + EXPECT_THAT(GetValue<Id>(new_table, "android:string/one"), NotNull()); +} + +} // namespace +} // namespace aapt diff --git a/tools/aapt2/optimize/VersionCollapser.cpp b/tools/aapt2/optimize/VersionCollapser.cpp index d941b487e439..cc1fc1e6910b 100644 --- a/tools/aapt2/optimize/VersionCollapser.cpp +++ b/tools/aapt2/optimize/VersionCollapser.cpp @@ -63,11 +63,9 @@ FilterIterator<Iterator, Pred> make_filter_iterator(Iterator begin, } /** - * Every Configuration with an SDK version specified that is less than minSdk - * will be removed. - * The exception is when there is no exact matching resource for the minSdk. The - * next smallest - * one will be kept. + * Every Configuration with an SDK version specified that is less than minSdk will be removed. The + * exception is when there is no exact matching resource for the minSdk. The next smallest one will + * be kept. */ static void CollapseVersions(int min_sdk, ResourceEntry* entry) { // First look for all sdks less than minSdk. @@ -80,11 +78,9 @@ static void CollapseVersions(int min_sdk, ResourceEntry* entry) { const ConfigDescription& config = (*iter)->config; if (config.sdkVersion <= min_sdk) { - // This is the first configuration we've found with a smaller or equal SDK - // level - // to the minimum. We MUST keep this one, but remove all others we find, - // which get - // overridden by this one. + // This is the first configuration we've found with a smaller or equal SDK level to the + // minimum. We MUST keep this one, but remove all others we find, which get overridden by this + // one. ConfigDescription config_without_sdk = config.CopyWithoutSdkVersion(); auto pred = [&](const std::unique_ptr<ResourceConfigValue>& val) -> bool { @@ -115,11 +111,9 @@ static void CollapseVersions(int min_sdk, ResourceEntry* entry) { -> bool { return val == nullptr; }), entry->values.end()); - // Strip the version qualifiers for every resource with version <= minSdk. - // This will ensure - // that the resource entries are all packed together in the same ResTable_type - // struct - // and take up less space in the resources.arsc table. + // Strip the version qualifiers for every resource with version <= minSdk. This will ensure that + // the resource entries are all packed together in the same ResTable_type struct and take up less + // space in the resources.arsc table. bool modified = false; for (std::unique_ptr<ResourceConfigValue>& config_value : entry->values) { if (config_value->config.sdkVersion != 0 && @@ -137,8 +131,8 @@ static void CollapseVersions(int min_sdk, ResourceEntry* entry) { } if (modified) { - // We've modified the keys (ConfigDescription) by changing the sdkVersion to - // 0. We MUST re-sort to ensure ordering guarantees hold. + // We've modified the keys (ConfigDescription) by changing the sdkVersion to 0. We MUST re-sort + // to ensure ordering guarantees hold. std::sort(entry->values.begin(), entry->values.end(), [](const std::unique_ptr<ResourceConfigValue>& a, const std::unique_ptr<ResourceConfigValue>& b) -> bool { diff --git a/tools/aapt2/split/TableSplitter.cpp b/tools/aapt2/split/TableSplitter.cpp index 27e13d81ff49..9d49ca6c0aa9 100644 --- a/tools/aapt2/split/TableSplitter.cpp +++ b/tools/aapt2/split/TableSplitter.cpp @@ -32,8 +32,7 @@ namespace aapt { using ConfigClaimedMap = std::unordered_map<ResourceConfigValue*, bool>; -using ConfigDensityGroups = - std::map<ConfigDescription, std::vector<ResourceConfigValue*>>; +using ConfigDensityGroups = std::map<ConfigDescription, std::vector<ResourceConfigValue*>>; static ConfigDescription CopyWithoutDensity(const ConfigDescription& config) { ConfigDescription without_density = config; @@ -51,8 +50,7 @@ class SplitValueSelector { if (config.density == 0) { density_independent_configs_.insert(config); } else { - density_dependent_config_to_density_map_[CopyWithoutDensity(config)] = - config.density; + density_dependent_config_to_density_map_[CopyWithoutDensity(config)] = config.density; } } } @@ -94,9 +92,7 @@ class SplitValueSelector { ResourceConfigValue* best_value = nullptr; for (ResourceConfigValue* this_value : related_values) { - if (!best_value || - this_value->config.isBetterThan(best_value->config, - &target_density)) { + if (!best_value || this_value->config.isBetterThan(best_value->config, &target_density)) { best_value = this_value; } } @@ -120,9 +116,8 @@ class SplitValueSelector { }; /** - * Marking non-preferred densities as claimed will make sure the base doesn't - * include them, - * leaving only the preferred density behind. + * Marking non-preferred densities as claimed will make sure the base doesn't include them, leaving + * only the preferred density behind. */ static void MarkNonPreferredDensitiesAsClaimed( const std::vector<uint16_t>& preferred_densities, const ConfigDensityGroups& density_groups, @@ -161,8 +156,7 @@ bool TableSplitter::VerifySplitConstraints(IAaptContext* context) { for (size_t i = 0; i < split_constraints_.size(); i++) { for (size_t j = i + 1; j < split_constraints_.size(); j++) { for (const ConfigDescription& config : split_constraints_[i].configs) { - if (split_constraints_[j].configs.find(config) != - split_constraints_[j].configs.end()) { + if (split_constraints_[j].configs.find(config) != split_constraints_[j].configs.end()) { context->GetDiagnostics()->Error(DiagMessage() << "config '" << config << "' appears in multiple splits, " @@ -193,28 +187,22 @@ void TableSplitter::SplitTable(ResourceTable* original_table) { for (auto& entry : type->entries) { if (options_.config_filter) { // First eliminate any resource that we definitely don't want. - for (std::unique_ptr<ResourceConfigValue>& config_value : - entry->values) { + for (std::unique_ptr<ResourceConfigValue>& config_value : entry->values) { if (!options_.config_filter->Match(config_value->config)) { - // null out the entry. We will clean up and remove nulls at the - // end for performance reasons. + // null out the entry. We will clean up and remove nulls at the end for performance + // reasons. config_value.reset(); } } } - // Organize the values into two separate buckets. Those that are - // density-dependent - // and those that are density-independent. - // One density technically matches all density, it's just that some - // densities - // match better. So we need to be aware of the full set of densities to - // make this - // decision. + // Organize the values into two separate buckets. Those that are density-dependent and those + // that are density-independent. One density technically matches all density, it's just that + // some densities match better. So we need to be aware of the full set of densities to make + // this decision. ConfigDensityGroups density_groups; ConfigClaimedMap config_claimed_map; - for (const std::unique_ptr<ResourceConfigValue>& config_value : - entry->values) { + for (const std::unique_ptr<ResourceConfigValue>& config_value : entry->values) { if (config_value) { config_claimed_map[config_value.get()] = false; @@ -226,9 +214,8 @@ void TableSplitter::SplitTable(ResourceTable* original_table) { } } - // First we check all the splits. If it doesn't match one of the splits, - // we - // leave it in the base. + // First we check all the splits. If it doesn't match one of the splits, we leave it in the + // base. for (size_t idx = 0; idx < split_count; idx++) { const SplitConstraints& split_constraint = split_constraints_[idx]; ResourceTable* split_table = splits_[idx].get(); @@ -240,20 +227,16 @@ void TableSplitter::SplitTable(ResourceTable* original_table) { // No need to do any work if we selected nothing. if (!selected_values.empty()) { - // Create the same resource structure in the split. We do this - // lazily because we might not have actual values for each - // type/entry. - ResourceTablePackage* split_pkg = - split_table->FindPackage(pkg->name); - ResourceTableType* split_type = - split_pkg->FindOrCreateType(type->type); + // Create the same resource structure in the split. We do this lazily because we might + // not have actual values for each type/entry. + ResourceTablePackage* split_pkg = split_table->FindPackage(pkg->name); + ResourceTableType* split_type = split_pkg->FindOrCreateType(type->type); if (!split_type->id) { split_type->id = type->id; split_type->symbol_status = type->symbol_status; } - ResourceEntry* split_entry = - split_type->FindOrCreateEntry(entry->name); + ResourceEntry* split_entry = split_type->FindOrCreateEntry(entry->name); if (!split_entry->id) { split_entry->id = entry->id; split_entry->symbol_status = entry->symbol_status; @@ -262,8 +245,7 @@ void TableSplitter::SplitTable(ResourceTable* original_table) { // Copy the selected values into the new Split Entry. for (ResourceConfigValue* config_value : selected_values) { ResourceConfigValue* new_config_value = - split_entry->FindOrCreateValue(config_value->config, - config_value->product); + split_entry->FindOrCreateValue(config_value->config, config_value->product); new_config_value->value = std::unique_ptr<Value>( config_value->value->Clone(&split_table->string_pool)); } @@ -276,11 +258,9 @@ void TableSplitter::SplitTable(ResourceTable* original_table) { &config_claimed_map); } - // All splits are handled, now check to see what wasn't claimed and - // remove - // whatever exists in other splits. - for (std::unique_ptr<ResourceConfigValue>& config_value : - entry->values) { + // All splits are handled, now check to see what wasn't claimed and remove whatever exists + // in other splits. + for (std::unique_ptr<ResourceConfigValue>& config_value : entry->values) { if (config_value && config_claimed_map[config_value.get()]) { // Claimed, remove from base. config_value.reset(); diff --git a/tools/aapt2/test/Builders.cpp b/tools/aapt2/test/Builders.cpp index 8f9788e8a25d..e658664d8653 100644 --- a/tools/aapt2/test/Builders.cpp +++ b/tools/aapt2/test/Builders.cpp @@ -23,6 +23,9 @@ #include "test/Common.h" #include "util/Util.h" +using ::aapt::configuration::Abi; +using ::aapt::configuration::AndroidSdk; +using ::aapt::configuration::Artifact; using ::aapt::io::StringInputStream; using ::android::StringPiece; @@ -212,5 +215,74 @@ std::unique_ptr<xml::XmlResource> BuildXmlDomForPackageName(IAaptContext* contex return doc; } +PostProcessingConfigurationBuilder& PostProcessingConfigurationBuilder::SetAbiGroup( + const std::string& name, const std::vector<Abi>& abis) { + config_.abi_groups[name] = abis; + return *this; +} + +PostProcessingConfigurationBuilder& PostProcessingConfigurationBuilder::SetLocaleGroup( + const std::string& name, const std::vector<std::string>& locales) { + auto& group = config_.locale_groups[name]; + for (const auto& locale : locales) { + group.push_back(ParseConfigOrDie(locale)); + } + return *this; +} + +PostProcessingConfigurationBuilder& PostProcessingConfigurationBuilder::SetDensityGroup( + const std::string& name, const std::vector<std::string>& densities) { + auto& group = config_.screen_density_groups[name]; + for (const auto& density : densities) { + group.push_back(ParseConfigOrDie(density)); + } + return *this; +} + +PostProcessingConfigurationBuilder& PostProcessingConfigurationBuilder::SetAndroidSdk( + const std::string& name, const AndroidSdk& sdk) { + config_.android_sdk_groups[name] = sdk; + return *this; +} + +PostProcessingConfigurationBuilder& PostProcessingConfigurationBuilder::AddArtifact( + const Artifact& artifact) { + config_.artifacts.push_back(artifact); + return *this; +} + +configuration::PostProcessingConfiguration PostProcessingConfigurationBuilder::Build() { + return config_; +} + +ArtifactBuilder& ArtifactBuilder::SetName(const std::string& name) { + artifact_.name = {name}; + return *this; +} + +ArtifactBuilder& ArtifactBuilder::SetAbiGroup(const std::string& name) { + artifact_.abi_group = {name}; + return *this; +} + +ArtifactBuilder& ArtifactBuilder::SetDensityGroup(const std::string& name) { + artifact_.screen_density_group = {name}; + return *this; +} + +ArtifactBuilder& ArtifactBuilder::SetLocaleGroup(const std::string& name) { + artifact_.locale_group = {name}; + return *this; +} + +ArtifactBuilder& ArtifactBuilder::SetAndroidSdk(const std::string& name) { + artifact_.android_sdk_group = {name}; + return *this; +} + +configuration::Artifact ArtifactBuilder::Build() { + return artifact_; +} + } // namespace test } // namespace aapt diff --git a/tools/aapt2/test/Builders.h b/tools/aapt2/test/Builders.h index d9f3912fb4c6..263fb5562b25 100644 --- a/tools/aapt2/test/Builders.h +++ b/tools/aapt2/test/Builders.h @@ -24,7 +24,9 @@ #include "Resource.h" #include "ResourceTable.h" #include "ResourceValues.h" +#include "configuration/ConfigurationParser.h" #include "process/IResourceTableConsumer.h" +#include "test/Common.h" #include "util/Maybe.h" #include "xml/XmlDom.h" @@ -149,6 +151,40 @@ std::unique_ptr<xml::XmlResource> BuildXmlDom(const android::StringPiece& str); std::unique_ptr<xml::XmlResource> BuildXmlDomForPackageName(IAaptContext* context, const android::StringPiece& str); +class PostProcessingConfigurationBuilder { + public: + PostProcessingConfigurationBuilder() = default; + + PostProcessingConfigurationBuilder& SetAbiGroup(const std::string& name, + const std::vector<configuration::Abi>& abis); + PostProcessingConfigurationBuilder& SetLocaleGroup(const std::string& name, + const std::vector<std::string>& locales); + PostProcessingConfigurationBuilder& SetDensityGroup(const std::string& name, + const std::vector<std::string>& densities); + PostProcessingConfigurationBuilder& SetAndroidSdk(const std::string& name, + const configuration::AndroidSdk& sdk); + PostProcessingConfigurationBuilder& AddArtifact(const configuration::Artifact& artifact); + configuration::PostProcessingConfiguration Build(); + + private: + configuration::PostProcessingConfiguration config_; +}; + +class ArtifactBuilder { + public: + ArtifactBuilder() = default; + + ArtifactBuilder& SetName(const std::string& name); + ArtifactBuilder& SetAbiGroup(const std::string& name); + ArtifactBuilder& SetDensityGroup(const std::string& name); + ArtifactBuilder& SetLocaleGroup(const std::string& name); + ArtifactBuilder& SetAndroidSdk(const std::string& name); + configuration::Artifact Build(); + + private: + configuration::Artifact artifact_; +}; + } // namespace test } // namespace aapt diff --git a/tools/aapt2/test/Common.h b/tools/aapt2/test/Common.h index e6b38c0007b4..61d056309a69 100644 --- a/tools/aapt2/test/Common.h +++ b/tools/aapt2/test/Common.h @@ -41,13 +41,13 @@ IDiagnostics* GetDiagnostics(); inline ResourceName ParseNameOrDie(const android::StringPiece& str) { ResourceNameRef ref; - CHECK(ResourceUtils::ParseResourceName(str, &ref)) << "invalid resource name"; + CHECK(ResourceUtils::ParseResourceName(str, &ref)) << "invalid resource name: " << str; return ref.ToResourceName(); } inline ConfigDescription ParseConfigOrDie(const android::StringPiece& str) { ConfigDescription config; - CHECK(ConfigDescription::Parse(str, &config)) << "invalid configuration"; + CHECK(ConfigDescription::Parse(str, &config)) << "invalid configuration: " << str; return config; } diff --git a/tools/aapt2/util/Util.cpp b/tools/aapt2/util/Util.cpp index 51a75d7556ad..a9b49d9d9904 100644 --- a/tools/aapt2/util/Util.cpp +++ b/tools/aapt2/util/Util.cpp @@ -520,11 +520,10 @@ bool Tokenizer::iterator::operator!=(const iterator& rhs) const { return !(*this == rhs); } -Tokenizer::iterator::iterator(StringPiece s, char sep, StringPiece tok, - bool end) +Tokenizer::iterator::iterator(const StringPiece& s, char sep, const StringPiece& tok, bool end) : str_(s), separator_(sep), token_(tok), end_(end) {} -Tokenizer::Tokenizer(StringPiece str, char sep) +Tokenizer::Tokenizer(const StringPiece& str, char sep) : begin_(++iterator(str, sep, StringPiece(str.begin() - 1, 0), false)), end_(str, sep, StringPiece(str.end(), 0), true) {} diff --git a/tools/aapt2/util/Util.h b/tools/aapt2/util/Util.h index 8f021ab8cb8a..c928458786e4 100644 --- a/tools/aapt2/util/Util.h +++ b/tools/aapt2/util/Util.h @@ -246,7 +246,7 @@ class Tokenizer { private: friend class Tokenizer; - iterator(android::StringPiece s, char sep, android::StringPiece tok, bool end); + iterator(const android::StringPiece& s, char sep, const android::StringPiece& tok, bool end); android::StringPiece str_; char separator_; @@ -254,7 +254,7 @@ class Tokenizer { bool end_; }; - Tokenizer(android::StringPiece str, char sep); + Tokenizer(const android::StringPiece& str, char sep); iterator begin() const { return begin_; diff --git a/tools/aapt2/xml/XmlDom.cpp b/tools/aapt2/xml/XmlDom.cpp index cbb652ed9a1a..32ec7bc60c78 100644 --- a/tools/aapt2/xml/XmlDom.cpp +++ b/tools/aapt2/xml/XmlDom.cpp @@ -236,16 +236,22 @@ static void CopyAttributes(Element* el, android::ResXMLParser* parser, StringPoo attr.name = util::Utf16ToUtf8(StringPiece16(str16, len)); } + uint32_t res_id = parser->getAttributeNameResID(i); + if (res_id > 0) { + attr.compiled_attribute = AaptAttribute(::aapt::Attribute(), {res_id}); + } + str16 = parser->getAttributeStringValue(i, &len); if (str16) { attr.value = util::Utf16ToUtf8(StringPiece16(str16, len)); + } else { + android::Res_value res_value; + if (parser->getAttributeValue(i, &res_value) > 0) { + attr.compiled_value = ResourceUtils::ParseBinaryResValue( + ResourceType::kAnim, {}, parser->getStrings(), res_value, out_pool); + } } - android::Res_value res_value; - if (parser->getAttributeValue(i, &res_value) > 0) { - attr.compiled_value = ResourceUtils::ParseBinaryResValue( - ResourceType::kAnim, {}, parser->getStrings(), res_value, out_pool); - } el->attributes.push_back(std::move(attr)); } @@ -274,6 +280,8 @@ std::unique_ptr<XmlResource> Inflate(const void* data, size_t data_len, IDiagnos switch (code) { case ResXMLParser::START_NAMESPACE: { NamespaceDecl decl; + decl.line_number = tree.getLineNumber(); + size_t len; const char16_t* str16 = tree.getNamespacePrefix(&len); if (str16) { @@ -288,6 +296,7 @@ std::unique_ptr<XmlResource> Inflate(const void* data, size_t data_len, IDiagnos if (pending_element == nullptr) { pending_element = util::make_unique<Element>(); } + pending_element->namespace_decls.push_back(std::move(decl)); break; } @@ -297,8 +306,8 @@ std::unique_ptr<XmlResource> Inflate(const void* data, size_t data_len, IDiagnos el = std::move(pending_element); } else { el = util::make_unique<Element>(); - ; } + el->line_number = tree.getLineNumber(); size_t len; const char16_t* str16 = tree.getElementNamespace(&len); @@ -479,10 +488,9 @@ void PackageAwareVisitor::AfterVisitElement(Element* el) { package_decls_.pop_back(); } -Maybe<ExtractedPackage> PackageAwareVisitor::TransformPackageAlias( - const StringPiece& alias, const StringPiece& local_package) const { +Maybe<ExtractedPackage> PackageAwareVisitor::TransformPackageAlias(const StringPiece& alias) const { if (alias.empty()) { - return ExtractedPackage{local_package.to_string(), false /* private */}; + return ExtractedPackage{{}, false /*private*/}; } const auto rend = package_decls_.rend(); @@ -493,7 +501,7 @@ Maybe<ExtractedPackage> PackageAwareVisitor::TransformPackageAlias( const PackageDecl& decl = *iter2; if (alias == decl.prefix) { if (decl.package.package.empty()) { - return ExtractedPackage{local_package.to_string(), decl.package.private_namespace}; + return ExtractedPackage{{}, decl.package.private_namespace}; } return decl.package; } diff --git a/tools/aapt2/xml/XmlDom.h b/tools/aapt2/xml/XmlDom.h index 154224381626..9a9151da4ab7 100644 --- a/tools/aapt2/xml/XmlDom.h +++ b/tools/aapt2/xml/XmlDom.h @@ -185,8 +185,7 @@ class PackageAwareVisitor : public Visitor, public IPackageDeclStack { public: using Visitor::Visit; - Maybe<ExtractedPackage> TransformPackageAlias( - const android::StringPiece& alias, const android::StringPiece& local_package) const override; + Maybe<ExtractedPackage> TransformPackageAlias(const android::StringPiece& alias) const override; protected: PackageAwareVisitor() = default; diff --git a/tools/aapt2/xml/XmlDom_test.cpp b/tools/aapt2/xml/XmlDom_test.cpp index 6ed2d616f782..10a45870e556 100644 --- a/tools/aapt2/xml/XmlDom_test.cpp +++ b/tools/aapt2/xml/XmlDom_test.cpp @@ -86,19 +86,14 @@ class TestVisitor : public PackageAwareVisitor { void Visit(Element* el) override { if (el->name == "View1") { - EXPECT_THAT(TransformPackageAlias("one", "local"), - Eq(make_value(ExtractedPackage{"com.one", false}))); + EXPECT_THAT(TransformPackageAlias("one"), Eq(make_value(ExtractedPackage{"com.one", false}))); } else if (el->name == "View2") { - EXPECT_THAT(TransformPackageAlias("one", "local"), - Eq(make_value(ExtractedPackage{"com.one", false}))); - EXPECT_THAT(TransformPackageAlias("two", "local"), - Eq(make_value(ExtractedPackage{"com.two", false}))); + EXPECT_THAT(TransformPackageAlias("one"), Eq(make_value(ExtractedPackage{"com.one", false}))); + EXPECT_THAT(TransformPackageAlias("two"), Eq(make_value(ExtractedPackage{"com.two", false}))); } else if (el->name == "View3") { - EXPECT_THAT(TransformPackageAlias("one", "local"), - Eq(make_value(ExtractedPackage{"com.one", false}))); - EXPECT_THAT(TransformPackageAlias("two", "local"), - Eq(make_value(ExtractedPackage{"com.two", false}))); - EXPECT_THAT(TransformPackageAlias("three", "local"), + EXPECT_THAT(TransformPackageAlias("one"), Eq(make_value(ExtractedPackage{"com.one", false}))); + EXPECT_THAT(TransformPackageAlias("two"), Eq(make_value(ExtractedPackage{"com.two", false}))); + EXPECT_THAT(TransformPackageAlias("three"), Eq(make_value(ExtractedPackage{"com.three", false}))); } } @@ -112,7 +107,6 @@ TEST(XmlDomTest, PackageAwareXmlVisitor) { </View2> </View1>)"); - Debug::DumpXml(doc.get()); TestVisitor visitor; doc->root->Accept(&visitor); } diff --git a/tools/aapt2/xml/XmlPullParser.cpp b/tools/aapt2/xml/XmlPullParser.cpp index 30bdc507303b..402e5a459f4e 100644 --- a/tools/aapt2/xml/XmlPullParser.cpp +++ b/tools/aapt2/xml/XmlPullParser.cpp @@ -141,17 +141,16 @@ const std::string& XmlPullParser::namespace_uri() const { return event_queue_.front().data2; } -Maybe<ExtractedPackage> XmlPullParser::TransformPackageAlias( - const StringPiece& alias, const StringPiece& local_package) const { +Maybe<ExtractedPackage> XmlPullParser::TransformPackageAlias(const StringPiece& alias) const { if (alias.empty()) { - return ExtractedPackage{local_package.to_string(), false /* private */}; + return ExtractedPackage{{}, false /*private*/}; } const auto end_iter = package_aliases_.rend(); for (auto iter = package_aliases_.rbegin(); iter != end_iter; ++iter) { if (alias == iter->prefix) { if (iter->package.package.empty()) { - return ExtractedPackage{local_package.to_string(), iter->package.private_namespace}; + return ExtractedPackage{{}, iter->package.private_namespace}; } return iter->package; } diff --git a/tools/aapt2/xml/XmlPullParser.h b/tools/aapt2/xml/XmlPullParser.h index a00caa139061..63db66f0b2b7 100644 --- a/tools/aapt2/xml/XmlPullParser.h +++ b/tools/aapt2/xml/XmlPullParser.h @@ -119,8 +119,7 @@ class XmlPullParser : public IPackageDeclStack { * If xmlns:app="http://schemas.android.com/apk/res-auto", then * 'package' will be set to 'defaultPackage'. */ - Maybe<ExtractedPackage> TransformPackageAlias( - const android::StringPiece& alias, const android::StringPiece& local_package) const override; + Maybe<ExtractedPackage> TransformPackageAlias(const android::StringPiece& alias) const override; // // Remaining methods are for retrieving information about attributes diff --git a/tools/aapt2/xml/XmlUtil.cpp b/tools/aapt2/xml/XmlUtil.cpp index fb8cee8b5634..c1186e83369c 100644 --- a/tools/aapt2/xml/XmlUtil.cpp +++ b/tools/aapt2/xml/XmlUtil.cpp @@ -62,19 +62,15 @@ Maybe<ExtractedPackage> ExtractPackageFromNamespace( return {}; } -void TransformReferenceFromNamespace(IPackageDeclStack* decl_stack, - const StringPiece& local_package, - Reference* in_ref) { +void ResolvePackage(const IPackageDeclStack* decl_stack, Reference* in_ref) { if (in_ref->name) { if (Maybe<ExtractedPackage> transformed_package = - decl_stack->TransformPackageAlias(in_ref->name.value().package, - local_package)) { + decl_stack->TransformPackageAlias(in_ref->name.value().package)) { ExtractedPackage& extracted_package = transformed_package.value(); in_ref->name.value().package = std::move(extracted_package.package); // If the reference was already private (with a * prefix) and the - // namespace is public, - // we keep the reference private. + // namespace is public, we keep the reference private. in_ref->private_reference |= extracted_package.private_namespace; } } diff --git a/tools/aapt2/xml/XmlUtil.h b/tools/aapt2/xml/XmlUtil.h index 866b6dcd7a88..4eb359a9eed4 100644 --- a/tools/aapt2/xml/XmlUtil.h +++ b/tools/aapt2/xml/XmlUtil.h @@ -35,7 +35,7 @@ constexpr const char* kSchemaAapt = "http://schemas.android.com/aapt"; // Result of extracting a package name from a namespace URI declaration. struct ExtractedPackage { // The name of the package. This can be the empty string, which means that the package - // should be assumed to be the package being compiled. + // should be assumed to be the same as the CallSite it was defined in. std::string package; // True if the package's private namespace was declared. This means that private resources @@ -51,8 +51,8 @@ struct ExtractedPackage { // http://schemas.android.com/apk/res/<package> or // http://schemas.android.com/apk/prv/res/<package> // -// Special case: if namespaceUri is http://schemas.android.com/apk/res-auto, -// returns an empty package name. +// Special case: if namespaceUri is http://schemas.android.com/apk/res-auto, returns an empty +// package name. Maybe<ExtractedPackage> ExtractPackageFromNamespace(const std::string& namespace_uri); // Returns an XML Android namespace for the given package of the form: @@ -63,21 +63,20 @@ Maybe<ExtractedPackage> ExtractPackageFromNamespace(const std::string& namespace std::string BuildPackageNamespace(const android::StringPiece& package, bool private_reference = false); -// Interface representing a stack of XML namespace declarations. When looking up the package -// for a namespace prefix, the stack is checked from top to bottom. +// Interface representing a stack of XML namespace declarations. When looking up the package for a +// namespace prefix, the stack is checked from top to bottom. struct IPackageDeclStack { virtual ~IPackageDeclStack() = default; // Returns an ExtractedPackage struct if the alias given corresponds with a package declaration. virtual Maybe<ExtractedPackage> TransformPackageAlias( - const android::StringPiece& alias, const android::StringPiece& local_package) const = 0; + const android::StringPiece& alias) const = 0; }; // Helper function for transforming the original Reference inRef to a fully qualified reference // via the IPackageDeclStack. This will also mark the Reference as private if the namespace of the // package declaration was private. -void TransformReferenceFromNamespace(IPackageDeclStack* decl_stack, - const android::StringPiece& local_package, Reference* in_ref); +void ResolvePackage(const IPackageDeclStack* decl_stack, Reference* in_ref); } // namespace xml } // namespace aapt diff --git a/tools/apilint/apilint.py b/tools/apilint/apilint.py index 77c1c24b17eb..2eab22e8ec14 100644 --- a/tools/apilint/apilint.py +++ b/tools/apilint/apilint.py @@ -1127,6 +1127,80 @@ def verify_closable(clazz): return +def verify_member_name_not_kotlin_keyword(clazz): + """Prevent method names which are keywords in Kotlin.""" + + # https://kotlinlang.org/docs/reference/keyword-reference.html#hard-keywords + # This list does not include Java keywords as those are already impossible to use. + keywords = [ + 'as', + 'fun', + 'in', + 'is', + 'object', + 'typealias', + 'val', + 'var', + 'when', + ] + + for m in clazz.methods: + if m.name in keywords: + error(clazz, m, None, "Method name must not be a Kotlin keyword") + for f in clazz.fields: + if f.name in keywords: + error(clazz, f, None, "Field name must not be a Kotlin keyword") + + +def verify_method_name_not_kotlin_operator(clazz): + """Warn about method names which become operators in Kotlin.""" + + binary = set() + + def unique_binary_op(m, op): + if op in binary: + error(clazz, m, None, "Only one of '{0}' and '{0}Assign' methods should be present for Kotlin".format(op)) + binary.add(op) + + for m in clazz.methods: + if 'static' in m.split: + continue + + # https://kotlinlang.org/docs/reference/operator-overloading.html#unary-prefix-operators + if m.name in ['unaryPlus', 'unaryMinus', 'not'] and len(m.args) == 0: + warn(clazz, m, None, "Method can be invoked as a unary operator from Kotlin") + + # https://kotlinlang.org/docs/reference/operator-overloading.html#increments-and-decrements + if m.name in ['inc', 'dec'] and len(m.args) == 0 and m.typ != 'void': + # This only applies if the return type is the same or a subtype of the enclosing class, but we have no + # practical way of checking that relationship here. + warn(clazz, m, None, "Method can be invoked as a pre/postfix inc/decrement operator from Kotlin") + + # https://kotlinlang.org/docs/reference/operator-overloading.html#arithmetic + if m.name in ['plus', 'minus', 'times', 'div', 'rem', 'mod', 'rangeTo'] and len(m.args) == 1: + warn(clazz, m, None, "Method can be invoked as a binary operator from Kotlin") + unique_binary_op(m, m.name) + + # https://kotlinlang.org/docs/reference/operator-overloading.html#in + if m.name == 'contains' and len(m.args) == 1 and m.typ == 'boolean': + warn(clazz, m, None, "Method can be invoked as a 'in' operator from Kotlin") + + # https://kotlinlang.org/docs/reference/operator-overloading.html#indexed + if (m.name == 'get' and len(m.args) > 0) or (m.name == 'set' and len(m.args) > 1): + warn(clazz, m, None, "Method can be invoked with an indexing operator from Kotlin") + + # https://kotlinlang.org/docs/reference/operator-overloading.html#invoke + if m.name == 'invoke': + warn(clazz, m, None, "Method can be invoked with function call syntax from Kotlin") + + # https://kotlinlang.org/docs/reference/operator-overloading.html#assignments + if m.name in ['plusAssign', 'minusAssign', 'timesAssign', 'divAssign', 'remAssign', 'modAssign'] \ + and len(m.args) == 1 \ + and m.typ == 'void': + warn(clazz, m, None, "Method can be invoked as a compound assignment operator from Kotlin") + unique_binary_op(m, m.name[:-6]) # Remove 'Assign' suffix + + def examine_clazz(clazz): """Find all style issues in the given class.""" if clazz.pkg.name.startswith("java"): return @@ -1178,6 +1252,8 @@ def examine_clazz(clazz): verify_error(clazz) verify_units(clazz) verify_closable(clazz) + verify_member_name_not_kotlin_keyword(clazz) + verify_method_name_not_kotlin_operator(clazz) def examine_stream(stream): diff --git a/tools/incident_report/main.cpp b/tools/incident_report/main.cpp index 1d8809f6f603..250d1186c672 100644 --- a/tools/incident_report/main.cpp +++ b/tools/incident_report/main.cpp @@ -97,6 +97,8 @@ read_message(CodedInputStream* in, Descriptor const* descriptor, GenericMessage* message->addInt64(fieldId, value64); break; } else { + fprintf(stderr, "bad VARINT: 0x%x (%d) at index %d\n", tag, tag, + in->CurrentPosition()); return false; } case WireFormatLite::WIRETYPE_FIXED64: @@ -104,10 +106,14 @@ read_message(CodedInputStream* in, Descriptor const* descriptor, GenericMessage* message->addInt64(fieldId, value64); break; } else { + fprintf(stderr, "bad VARINT: 0x%x (%d) at index %d\n", tag, tag, + in->CurrentPosition()); return false; } case WireFormatLite::WIRETYPE_LENGTH_DELIMITED: if (!read_length_delimited(in, fieldId, descriptor, message)) { + fprintf(stderr, "bad LENGTH_DELIMITED: 0x%x (%d) at index %d\n", + tag, tag, in->CurrentPosition()); return false; } break; @@ -116,6 +122,8 @@ read_message(CodedInputStream* in, Descriptor const* descriptor, GenericMessage* message->addInt32(fieldId, value32); break; } else { + fprintf(stderr, "bad FIXED32: 0x%x (%d) at index %d\n", tag, tag, + in->CurrentPosition()); return false; } default: @@ -146,7 +154,7 @@ print_value(Out* out, FieldDescriptor const* field, GenericMessage::Node const& out->printf("%f", *(float*)&node.value32); break; default: - out->printf("(unexpected value %d (0x%x)", node.value32, node.value32); + out->printf("(unexpected value32 %d (0x%x)", node.value32, node.value32); break; } break; @@ -177,8 +185,11 @@ print_value(Out* out, FieldDescriptor const* field, GenericMessage::Node const& } break; case FieldDescriptor::TYPE_ENUM: + out->printf("%s", field->enum_type()->FindValueByNumber((int)node.value64) + ->name().c_str()); + break; default: - out->printf("(unexpected value %ld (0x%x))", node.value64, node.value64); + out->printf("(unexpected value64 %ld (0x%x))", node.value64, node.value64); break; } break; @@ -297,7 +308,7 @@ static int adb_incident_workaround(const char* adbSerial, const vector<string>& sections) { const int maxAllowedSize = 20 * 1024 * 1024; // 20MB - uint8_t* buffer = (uint8_t*)malloc(maxAllowedSize); + unique_ptr<uint8_t[]> buffer(new uint8_t[maxAllowedSize]); for (vector<string>::const_iterator it=sections.begin(); it!=sections.end(); it++) { Descriptor const* descriptor = IncidentProto::descriptor(); @@ -363,7 +374,7 @@ adb_incident_workaround(const char* adbSerial, const vector<string>& sections) size_t size = 0; while (size < maxAllowedSize) { - ssize_t amt = read(pfd[0], buffer + size, maxAllowedSize - size); + ssize_t amt = read(pfd[0], buffer.get() + size, maxAllowedSize - size); if (amt == 0) { break; } else if (amt == -1) { @@ -390,7 +401,7 @@ adb_incident_workaround(const char* adbSerial, const vector<string>& sections) fprintf(stderr, "write error: %s\n", strerror(err)); return 1; } - err = write_all(STDOUT_FILENO, buffer, size); + err = write_all(STDOUT_FILENO, buffer.get(), size); if (err != 0) { fprintf(stderr, "write error: %s\n", strerror(err)); return 1; @@ -401,7 +412,6 @@ adb_incident_workaround(const char* adbSerial, const vector<string>& sections) } } - free(buffer); return 0; } diff --git a/tools/incident_section_gen/main.cpp b/tools/incident_section_gen/main.cpp index 15f622cf9461..900690cf36d6 100644 --- a/tools/incident_section_gen/main.cpp +++ b/tools/incident_section_gen/main.cpp @@ -17,27 +17,34 @@ #include <frameworks/base/core/proto/android/os/incident.pb.h> - #include <map> +#include <string> +using namespace android; using namespace android::os; using namespace google::protobuf; using namespace google::protobuf::io; using namespace google::protobuf::internal; using namespace std; -int -main(int, const char**) -{ - map<string,FieldDescriptor const*> sections; - int N; +static inline void emptyline() { + printf("\n"); +} +static void generateHead(const char* header) { printf("// Auto generated file. Do not modify\n"); - printf("\n"); - printf("#include \"incident_sections.h\"\n"); - printf("\n"); + emptyline(); + printf("#include \"%s.h\"\n", header); + emptyline(); +} - Descriptor const* descriptor = IncidentProto::descriptor(); +// ================================================================================ +static bool generateIncidentSectionsCpp(Descriptor const* descriptor) +{ + generateHead("incident_sections"); + + map<string,FieldDescriptor const*> sections; + int N; N = descriptor->field_count(); for (int i=0; i<N; i++) { const FieldDescriptor* field = descriptor->field(i); @@ -63,5 +70,182 @@ main(int, const char**) printf("const int INCIDENT_SECTION_COUNT = %d;\n", N); - return 0; + return true; +} + +// ================================================================================ +static void splitAndPrint(const string& args) { + size_t base = 0; + size_t found; + while (true) { + found = args.find_first_of(" ", base); + if (found != base) { + string arg = args.substr(base, found - base); + printf(" \"%s\",", arg.c_str()); + } + if (found == args.npos) break; + base = found + 1; + } +} + +static const std::string replaceAll(const string& field_name, const char oldC, const string& newS) { + if (field_name.find_first_of(oldC) == field_name.npos) return field_name.c_str(); + size_t pos = 0, idx = 0; + char* res = new char[field_name.size() * newS.size() + 1]; // assign a larger buffer + while (pos != field_name.size()) { + char cur = field_name[pos++]; + if (cur != oldC) { + res[idx++] = cur; + continue; + } + + for (size_t i=0; i<newS.size(); i++) { + res[idx++] = newS[i]; + } + } + res[idx] = '\0'; + std::string result(res); + delete [] res; + return result; +} + +static inline bool isDefaultDest(const FieldDescriptor* field) { + return field->options().GetExtension(privacy).dest() == PrivacyFlags::default_instance().dest(); +} + +// Returns true if the descriptor doesn't have any non default privacy flags set, including its submessages +static bool generatePrivacyFlags(const Descriptor* descriptor, const char* alias, map<string, bool> &msgNames) { + bool hasDefaultFlags[descriptor->field_count()]; + // iterate though its field and generate sub flags first + for (int i=0; i<descriptor->field_count(); i++) { + hasDefaultFlags[i] = true; // set default to true + const FieldDescriptor* field = descriptor->field(i); + const std::string field_name_str = replaceAll(field->full_name(), '.', "__"); + const char* field_name = field_name_str.c_str(); + // check if the same name is already defined + if (msgNames.find(field_name) != msgNames.end()) { + hasDefaultFlags[i] = msgNames[field_name]; + continue; + }; + + PrivacyFlags p = field->options().GetExtension(privacy); + + switch (field->type()) { + case FieldDescriptor::TYPE_MESSAGE: + if (generatePrivacyFlags(field->message_type(), field_name, msgNames) && + isDefaultDest(field)) break; + + printf("Privacy %s { %d, %d, %s_LIST, %d, NULL };\n", field_name, field->number(), field->type(), field_name, p.dest()); + hasDefaultFlags[i] = false; + break; + case FieldDescriptor::TYPE_STRING: + if (isDefaultDest(field) && p.patterns_size() == 0) break; + + printf("const char* %s_patterns[] = {\n", field_name); + for (int i=0; i<p.patterns_size(); i++) { + // the generated string need to escape backslash as well, need to dup it here + printf(" \"%s\",\n", replaceAll(p.patterns(i), '\\', "\\\\").c_str()); + } + printf(" NULL };\n"); + printf("Privacy %s { %d, %d, NULL, %d, %s_patterns };\n", field_name, field->number(), field->type(), p.dest(), field_name); + hasDefaultFlags[i] = false; + break; + default: + if (isDefaultDest(field)) break; + printf("Privacy %s { %d, %d, NULL, %d, NULL };\n", field_name, field->number(), field->type(), p.dest()); + hasDefaultFlags[i] = false; + } + // add the field name to message map, true means it has default flags + msgNames[field_name] = hasDefaultFlags[i]; + } + + bool allDefaults = true; + for (int i=0; i<descriptor->field_count(); i++) { + allDefaults &= hasDefaultFlags[i]; + } + if (allDefaults) return true; + + emptyline(); + + bool needConst = strcmp(alias, "PRIVACY_POLICY") == 0; + int policyCount = 0; + + printf("%s Privacy* %s_LIST[] = {\n", needConst ? "const" : "", alias); + for (int i=0; i<descriptor->field_count(); i++) { + const FieldDescriptor* field = descriptor->field(i); + if (hasDefaultFlags[i]) continue; + printf(" &%s,\n", replaceAll(field->full_name(), '.', "__").c_str()); + policyCount++; + } + if (needConst) { + printf("};\n\n"); + printf("const int PRIVACY_POLICY_COUNT = %d;\n", policyCount); + } else { + printf(" NULL };\n"); + } + emptyline(); + return false; +} + +static bool generateSectionListCpp(Descriptor const* descriptor) { + generateHead("section_list"); + + // generates SECTION_LIST + printf("const Section* SECTION_LIST[] = {\n"); + for (int i=0; i<descriptor->field_count(); i++) { + const FieldDescriptor* field = descriptor->field(i); + + if (field->type() != FieldDescriptor::TYPE_MESSAGE) { + continue; + } + const SectionFlags s = field->options().GetExtension(section); + switch (s.type()) { + case SECTION_NONE: + continue; + case SECTION_FILE: + printf(" new FileSection(%d, \"%s\"),\n", field->number(), s.args().c_str()); + break; + case SECTION_COMMAND: + printf(" new CommandSection(%d,", field->number()); + splitAndPrint(s.args()); + printf(" NULL),\n"); + break; + case SECTION_DUMPSYS: + printf(" new DumpsysSection(%d,", field->number()); + splitAndPrint(s.args()); + printf(" NULL),\n"); + break; + } + } + printf(" NULL };\n"); + emptyline(); + + // generates PRIVACY_POLICY + map<string, bool> messageNames; + if (generatePrivacyFlags(descriptor, "PRIVACY_POLICY", messageNames)) { + // if no privacy options set at all, define an empty list + printf("const Privacy* PRIVACY_POLICY_LIST[] = {};\n"); + printf("const int PRIVACY_POLICY_COUNT = 0;\n"); + } + + return true; +} + +// ================================================================================ +int main(int argc, char const *argv[]) +{ + if (argc != 2) return 1; + const char* module = argv[1]; + + Descriptor const* descriptor = IncidentProto::descriptor(); + + if (strcmp(module, "incident") == 0) { + return !generateIncidentSectionsCpp(descriptor); + } + if (strcmp(module, "incidentd") == 0 ) { + return !generateSectionListCpp(descriptor); + } + + // return failure if not called by the whitelisted modules + return 1; } diff --git a/tools/validatekeymaps/Main.cpp b/tools/validatekeymaps/Main.cpp index 5b45c551c930..bbfcba6272b2 100644 --- a/tools/validatekeymaps/Main.cpp +++ b/tools/validatekeymaps/Main.cpp @@ -20,13 +20,15 @@ #include <utils/PropertyMap.h> #include <utils/String8.h> +#include <stdarg.h> #include <stdio.h> #include <stdlib.h> #include <string.h> using namespace android; -static const char* gProgName = "validatekeymaps"; +static const char* kProgName = "validatekeymaps"; +static bool gQuiet = false; enum FileType { FILETYPE_UNKNOWN, @@ -36,15 +38,32 @@ enum FileType { FILETYPE_INPUTDEVICECONFIGURATION, }; +static void log(const char* fmt, ...) { + if (gQuiet) { + return; + } + va_list args; + va_start(args, fmt); + vfprintf(stdout, fmt, args); + va_end(args); +} + +static void error(const char* fmt, ...) { + va_list args; + va_start(args, fmt); + vfprintf(stderr, fmt, args); + va_end(args); +} static void usage() { - fprintf(stderr, "Keymap Validation Tool\n\n"); - fprintf(stderr, "Usage:\n"); - fprintf(stderr, - " %s [*.kl] [*.kcm] [*.idc] [virtualkeys.*] [...]\n" + error("Keymap Validation Tool\n\n"); + error("Usage:\n"); + error( + " %s [-q] [*.kl] [*.kcm] [*.idc] [virtualkeys.*] [...]\n" " Validates the specified key layouts, key character maps, \n" - " input device configurations, or virtual key definitions.\n\n", - gProgName); + " input device configurations, or virtual key definitions.\n\n" + " -q Quiet; do not write anything to standard out.\n", + kProgName); } static FileType getFileType(const char* filename) { @@ -69,19 +88,19 @@ static FileType getFileType(const char* filename) { } static bool validateFile(const char* filename) { - fprintf(stdout, "Validating file '%s'...\n", filename); + log("Validating file '%s'...\n", filename); FileType fileType = getFileType(filename); switch (fileType) { case FILETYPE_UNKNOWN: - fprintf(stderr, "Supported file types: *.kl, *.kcm, virtualkeys.*\n\n"); + error("Supported file types: *.kl, *.kcm, virtualkeys.*\n\n"); return false; case FILETYPE_KEYLAYOUT: { sp<KeyLayoutMap> map; status_t status = KeyLayoutMap::load(String8(filename), &map); if (status) { - fprintf(stderr, "Error %d parsing key layout file.\n\n", status); + error("Error %d parsing key layout file.\n\n", status); return false; } break; @@ -92,7 +111,7 @@ static bool validateFile(const char* filename) { status_t status = KeyCharacterMap::load(String8(filename), KeyCharacterMap::FORMAT_ANY, &map); if (status) { - fprintf(stderr, "Error %d parsing key character map file.\n\n", status); + error("Error %d parsing key character map file.\n\n", status); return false; } break; @@ -102,7 +121,7 @@ static bool validateFile(const char* filename) { PropertyMap* map; status_t status = PropertyMap::load(String8(filename), &map); if (status) { - fprintf(stderr, "Error %d parsing input device configuration file.\n\n", status); + error("Error %d parsing input device configuration file.\n\n", status); return false; } delete map; @@ -113,7 +132,7 @@ static bool validateFile(const char* filename) { VirtualKeyMap* map; status_t status = VirtualKeyMap::load(String8(filename), &map); if (status) { - fprintf(stderr, "Error %d parsing virtual key definition file.\n\n", status); + error("Error %d parsing virtual key definition file.\n\n", status); return false; } delete map; @@ -121,7 +140,7 @@ static bool validateFile(const char* filename) { } } - fputs("No errors.\n\n", stdout); + log("No errors.\n\n"); return true; } @@ -133,15 +152,19 @@ int main(int argc, const char** argv) { int result = 0; for (int i = 1; i < argc; i++) { + if (i == 1 && !strcmp(argv[1], "-q")) { + gQuiet = true; + continue; + } if (!validateFile(argv[i])) { result = 1; } } if (result) { - fputs("Failed!\n", stderr); + error("Failed!\n"); } else { - fputs("Success.\n", stdout); + log("Success.\n"); } return result; } |
