AAPT2: Few tweaks to get shared-libraries working
Test: manual (building shared support library demo)
Change-Id: I4730645aa92ba1893baf67ffe35fbd4aac0f8e46
diff --git a/tools/aapt2/Resource.h b/tools/aapt2/Resource.h
index cffe8d5..493f238 100644
--- a/tools/aapt2/Resource.h
+++ b/tools/aapt2/Resource.h
@@ -118,6 +118,9 @@
bool is_valid() const;
};
+constexpr const uint8_t kAppPackageId = 0x7fu;
+constexpr const uint8_t kFrameworkPackageId = 0x01u;
+
/**
* A binary identifier representing a resource. Internally it
* is a 32bit integer split as follows:
diff --git a/tools/aapt2/ResourceUtils.cpp b/tools/aapt2/ResourceUtils.cpp
index 2d8e5a21..ca6738b 100644
--- a/tools/aapt2/ResourceUtils.cpp
+++ b/tools/aapt2/ResourceUtils.cpp
@@ -533,6 +533,7 @@
case android::Res_value::TYPE_REFERENCE:
case android::Res_value::TYPE_ATTRIBUTE:
case android::Res_value::TYPE_DYNAMIC_REFERENCE:
+ case android::Res_value::TYPE_DYNAMIC_ATTRIBUTE:
return android::ResTable_map::TYPE_REFERENCE;
case android::Res_value::TYPE_STRING:
diff --git a/tools/aapt2/ResourceValues.cpp b/tools/aapt2/ResourceValues.cpp
index f75ed7a..2868b2a 100644
--- a/tools/aapt2/ResourceValues.cpp
+++ b/tools/aapt2/ResourceValues.cpp
@@ -88,10 +88,24 @@
}
bool Reference::Flatten(android::Res_value* out_value) const {
- out_value->dataType = (reference_type == Reference::Type::kResource)
- ? android::Res_value::TYPE_REFERENCE
- : android::Res_value::TYPE_ATTRIBUTE;
- out_value->data = util::HostToDevice32(id ? id.value().id : 0);
+ const ResourceId resid = id.value_or_default(ResourceId(0));
+ const bool dynamic =
+ (resid.package_id() != kFrameworkPackageId && resid.package_id() != kAppPackageId);
+
+ if (reference_type == Reference::Type::kResource) {
+ if (dynamic) {
+ out_value->dataType = android::Res_value::TYPE_DYNAMIC_REFERENCE;
+ } else {
+ out_value->dataType = android::Res_value::TYPE_REFERENCE;
+ }
+ } else {
+ if (dynamic) {
+ out_value->dataType = android::Res_value::TYPE_DYNAMIC_ATTRIBUTE;
+ } else {
+ out_value->dataType = android::Res_value::TYPE_ATTRIBUTE;
+ }
+ }
+ out_value->data = util::HostToDevice32(resid.id);
return true;
}
diff --git a/tools/aapt2/java/JavaClassGenerator.cpp b/tools/aapt2/java/JavaClassGenerator.cpp
index 9c0f13c..68bdb95 100644
--- a/tools/aapt2/java/JavaClassGenerator.cpp
+++ b/tools/aapt2/java/JavaClassGenerator.cpp
@@ -533,9 +533,14 @@
std::unique_ptr<MethodDefinition> rewrite_method;
// Generate an onResourcesLoaded() callback if requested.
- if (options_.generate_rewrite_callback) {
+ if (options_.rewrite_callback_options) {
rewrite_method =
util::make_unique<MethodDefinition>("public static void onResourcesLoaded(int p)");
+ for (const std::string& package_to_callback :
+ options_.rewrite_callback_options.value().packages_to_callback) {
+ rewrite_method->AppendStatement(
+ StringPrintf("%s.R.onResourcesLoaded(p);", package_to_callback.data()));
+ }
}
for (const auto& package : table_->packages) {
diff --git a/tools/aapt2/java/JavaClassGenerator.h b/tools/aapt2/java/JavaClassGenerator.h
index 178f558..4510430 100644
--- a/tools/aapt2/java/JavaClassGenerator.h
+++ b/tools/aapt2/java/JavaClassGenerator.h
@@ -33,13 +33,19 @@
class ClassDefinition;
class MethodDefinition;
+// Options for generating onResourcesLoaded callback in R.java.
+struct OnResourcesLoadedCallbackOptions {
+ // Other R classes to delegate the same callback to (with the same package ID).
+ std::vector<std::string> packages_to_callback;
+};
+
struct JavaClassGeneratorOptions {
// Specifies whether to use the 'final' modifier on resource entries. Default is true.
bool use_final = true;
- // Whether to generate code to rewrite the package ID of resources.
- // Implies use_final == true. Default is false.
- bool generate_rewrite_callback = false;
+ // If set, generates code to rewrite the package ID of resources.
+ // Implies use_final == true. Default is unset.
+ Maybe<OnResourcesLoadedCallbackOptions> rewrite_callback_options;
enum class SymbolTypes {
kAll,
diff --git a/tools/aapt2/java/JavaClassGenerator_test.cpp b/tools/aapt2/java/JavaClassGenerator_test.cpp
index bcb2d4f..271279f 100644
--- a/tools/aapt2/java/JavaClassGenerator_test.cpp
+++ b/tools/aapt2/java/JavaClassGenerator_test.cpp
@@ -381,7 +381,9 @@
JavaClassGeneratorOptions options;
options.use_final = false;
- options.generate_rewrite_callback = true;
+ options.rewrite_callback_options = OnResourcesLoadedCallbackOptions{
+ {"com.foo", "com.boo"},
+ };
JavaClassGenerator generator(context.get(), table.get(), options);
std::stringstream out;
@@ -389,7 +391,9 @@
std::string actual = out.str();
- EXPECT_NE(std::string::npos, actual.find("onResourcesLoaded"));
+ 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"));
}
} // namespace aapt
diff --git a/tools/aapt2/link/Link.cpp b/tools/aapt2/link/Link.cpp
index 4905216..dd8e14b 100644
--- a/tools/aapt2/link/Link.cpp
+++ b/tools/aapt2/link/Link.cpp
@@ -1097,18 +1097,15 @@
return false;
}
- std::unique_ptr<ResourceTable> table =
- LoadTablePbFromCollection(collection.get());
+ std::unique_ptr<ResourceTable> table = LoadTablePbFromCollection(collection.get());
if (!table) {
- context_->GetDiagnostics()->Error(DiagMessage(input)
- << "invalid static library");
+ context_->GetDiagnostics()->Error(DiagMessage(input) << "invalid static library");
return false;
}
ResourceTablePackage* pkg = table->FindPackageById(0x7f);
if (!pkg) {
- context_->GetDiagnostics()->Error(DiagMessage(input)
- << "static library has no package");
+ context_->GetDiagnostics()->Error(DiagMessage(input) << "static library has no package");
return false;
}
@@ -1125,11 +1122,9 @@
pkg->name = "";
if (override) {
- result = table_merger_->MergeOverlay(Source(input), table.get(),
- collection.get());
+ result = table_merger_->MergeOverlay(Source(input), table.get(), collection.get());
} else {
- result =
- table_merger_->Merge(Source(input), table.get(), collection.get());
+ result = table_merger_->Merge(Source(input), table.get(), collection.get());
}
} else {
@@ -1782,17 +1777,21 @@
}
if (options_.generate_java_class_path) {
- JavaClassGeneratorOptions options;
- options.types = JavaClassGeneratorOptions::SymbolTypes::kAll;
- options.javadoc_annotations = options_.javadoc_annotations;
+ // 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 (options_.package_type == PackageType::kStaticLib || options_.generate_non_final_ids) {
- options.use_final = false;
+ template_options.use_final = false;
}
if (options_.package_type == PackageType::kSharedLib) {
- options.use_final = false;
- options.generate_rewrite_callback = true;
+ template_options.use_final = false;
+ template_options.rewrite_callback_options = OnResourcesLoadedCallbackOptions{};
}
const StringPiece actual_package = context_->GetCompilationPackage();
@@ -1802,36 +1801,51 @@
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.
-
- options.types = JavaClassGeneratorOptions::SymbolTypes::kPublic;
- if (!WriteJavaFile(&final_table_, context_->GetCompilationPackage(),
- output_package, options)) {
- return 1;
- }
-
+ JavaClassGeneratorOptions options = template_options;
options.types = JavaClassGeneratorOptions::SymbolTypes::kPublicPrivate;
- output_package = options_.private_symbols.value();
- }
-
- if (!WriteJavaFile(&final_table_, actual_package, output_package,
- options)) {
- return 1;
- }
-
- for (const std::string& extra_package : options_.extra_java_packages) {
- if (!WriteJavaFile(&final_table_, actual_package, extra_package,
+ 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)) {
+ return 1;
+ }
}
- if (!WriteProguardFile(options_.generate_proguard_rules_path,
- proguard_keep_set)) {
+ if (!WriteProguardFile(options_.generate_proguard_rules_path, proguard_keep_set)) {
return 1;
}
diff --git a/tools/aapt2/link/ManifestFixer.cpp b/tools/aapt2/link/ManifestFixer.cpp
index d5c0dc4..313fe45 100644
--- a/tools/aapt2/link/ManifestFixer.cpp
+++ b/tools/aapt2/link/ManifestFixer.cpp
@@ -79,6 +79,17 @@
return false;
}
+static xml::XmlNodeAction::ActionFuncWithDiag RequiredAndroidAttribute(const std::string& attr) {
+ return [=](xml::Element* el, SourcePathDiagnostics* diag) -> bool {
+ if (el->FindAttribute(xml::kSchemaAndroid, attr) == nullptr) {
+ diag->Error(DiagMessage(el->line_number)
+ << "<" << el->name << "> is missing required attribute 'android:" << attr << "'");
+ return false;
+ }
+ return true;
+ };
+}
+
static bool VerifyManifest(xml::Element* el, SourcePathDiagnostics* diag) {
xml::Attribute* attr = el->FindAttribute({}, "package");
if (!attr) {
@@ -272,6 +283,16 @@
application_action["uses-library"].Action(RequiredNameIsJavaPackage);
application_action["library"].Action(RequiredNameIsJavaPackage);
+
+ xml::XmlNodeAction& static_library_action = application_action["static-library"];
+ static_library_action.Action(RequiredNameIsJavaPackage);
+ static_library_action.Action(RequiredAndroidAttribute("version"));
+
+ xml::XmlNodeAction& uses_static_library_action = application_action["uses-static-library"];
+ uses_static_library_action.Action(RequiredNameIsJavaPackage);
+ uses_static_library_action.Action(RequiredAndroidAttribute("version"));
+ uses_static_library_action.Action(RequiredAndroidAttribute("certDigest"));
+
application_action["meta-data"] = meta_data_action;
application_action["activity"] = component_action;
application_action["activity-alias"] = component_action;
diff --git a/tools/aapt2/link/ReferenceLinker.cpp b/tools/aapt2/link/ReferenceLinker.cpp
index 4a42826..0331313 100644
--- a/tools/aapt2/link/ReferenceLinker.cpp
+++ b/tools/aapt2/link/ReferenceLinker.cpp
@@ -122,8 +122,7 @@
DiagMessage msg(entry.key.GetSource());
// Call the matches method again, this time with a DiagMessage so we
- // fill
- // in the actual error message.
+ // fill in the actual error message.
symbol->attribute->Matches(entry.value.get(), &msg);
context_->GetDiagnostics()->Error(msg);
error_ = true;
diff --git a/tools/aapt2/link/TableMerger.cpp b/tools/aapt2/link/TableMerger.cpp
index 7e7b9fb..9311091 100644
--- a/tools/aapt2/link/TableMerger.cpp
+++ b/tools/aapt2/link/TableMerger.cpp
@@ -39,14 +39,12 @@
bool TableMerger::Merge(const Source& src, ResourceTable* table,
io::IFileCollection* collection) {
- return MergeImpl(src, table, collection, false /* overlay */,
- true /* allow new */);
+ 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);
}
/**
@@ -55,25 +53,13 @@
bool TableMerger::MergeImpl(const Source& src, ResourceTable* table,
io::IFileCollection* collection, bool overlay,
bool allow_new) {
- const uint8_t desired_package_id = context_->GetPackageId();
-
bool error = false;
for (auto& package : table->packages) {
- // Warn of packages with an unrelated ID.
- const Maybe<ResourceId>& id = package->id;
- if (id && id.value() != 0x0 && id.value() != desired_package_id) {
- context_->GetDiagnostics()->Warn(DiagMessage(src) << "ignoring package "
- << package->name);
- continue;
- }
-
// Only merge an empty package or the package we're building.
// Other packages may exist, which likely contain attribute definitions.
// This is because at compile time it is unknown if the attributes are
- // simply
- // uses of the attribute or definitions.
- if (package->name.empty() ||
- context_->GetCompilationPackage() == package->name) {
+ // simply uses of the attribute or definitions.
+ if (package->name.empty() || context_->GetCompilationPackage() == package->name) {
FileMergeCallback callback;
if (collection) {
callback = [&](const ResourceNameRef& name,
@@ -83,8 +69,7 @@
io::IFile* f = collection->FindFile(*old_file->path);
if (!f) {
context_->GetDiagnostics()->Error(DiagMessage(src)
- << "file '" << *old_file->path
- << "' not found");
+ << "file '" << *old_file->path << "' not found");
return false;
}
diff --git a/tools/aapt2/process/SymbolTable.cpp b/tools/aapt2/process/SymbolTable.cpp
index 5d75e76..bcafbca 100644
--- a/tools/aapt2/process/SymbolTable.cpp
+++ b/tools/aapt2/process/SymbolTable.cpp
@@ -293,6 +293,11 @@
std::unique_ptr<SymbolTable::Symbol> AssetManagerSymbolSource::FindById(
ResourceId id) {
+ if (!id.is_valid()) {
+ // Exit early and avoid the error logs from AssetManager.
+ return {};
+ }
+
const android::ResTable& table = assets_.getResources(false);
Maybe<ResourceName> maybe_name = GetResourceName(table, id);
if (!maybe_name) {
diff --git a/tools/aapt2/unflatten/BinaryResourceParser.cpp b/tools/aapt2/unflatten/BinaryResourceParser.cpp
index 29a921c..9158bdd 100644
--- a/tools/aapt2/unflatten/BinaryResourceParser.cpp
+++ b/tools/aapt2/unflatten/BinaryResourceParser.cpp
@@ -458,10 +458,14 @@
}
if (value->dataType == Res_value::TYPE_REFERENCE ||
- value->dataType == Res_value::TYPE_ATTRIBUTE) {
- const Reference::Type type = (value->dataType == Res_value::TYPE_REFERENCE)
- ? Reference::Type::kResource
- : Reference::Type::kAttribute;
+ value->dataType == Res_value::TYPE_ATTRIBUTE ||
+ value->dataType == Res_value::TYPE_DYNAMIC_REFERENCE ||
+ value->dataType == Res_value::TYPE_DYNAMIC_ATTRIBUTE) {
+ Reference::Type type = Reference::Type::kResource;
+ if (value->dataType == Res_value::TYPE_ATTRIBUTE ||
+ value->dataType == Res_value::TYPE_DYNAMIC_ATTRIBUTE) {
+ type = Reference::Type::kAttribute;
+ }
if (data == 0) {
// A reference of 0, must be the magic @null reference.