AAPT2: Fixup namespace implementation
A few pieces were missing in the namespace mangling implementation.
Namespace aware libraries now work, along with R class generation.
Bug: 64706588
Test: make AaptTestNamespace_App
Change-Id: I12f78d6aa909e782c0faf7ceaa36058f2e6c864a
diff --git a/tools/aapt2/link/Linkers.h b/tools/aapt2/link/Linkers.h
index 5527f90..3c9c476 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 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 @@
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 @@
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 @@
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 414e56e..71e828b 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 @@
}
}
- /**
- * 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 @@
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 @@
// 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 @@
} 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 @@
}
}
- 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 @@
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 @@
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 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;
- }
+// 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;
}
- return true;
+
+ 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;
+ }
+
+ 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 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 @@
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 @@
}
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 @@
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 @@
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 b3d0196..3b11bee 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. 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.
- * 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. 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);
+ // 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);
- /**
- * 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.
- */
+ // 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 72a9168..be38b96 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 @@
.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 @@
.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 @@
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 10e837c..93c904f 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 @@
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 @@
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 @@
};
}
- // 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 @@
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 @@
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 @@
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 @@
// 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 c96b1b0..81518ff 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);
+ // 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 {
+ 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 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 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 bcecd20..6ebb80f 100644
--- a/tools/aapt2/link/XmlReferenceLinker.cpp
+++ b/tools/aapt2/link/XmlReferenceLinker.cpp
@@ -31,13 +31,9 @@
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 @@
}
}
- bool HasError() const { return error_; }
+ bool HasError() const {
+ return error_;
+ }
private:
DISALLOW_COPY_AND_ASSIGN(ReferenceVisitor);
@@ -64,9 +62,7 @@
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 @@
// 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 @@
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 @@
} 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 @@
} // 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);