diff options
-rw-r--r-- | tools/aapt2/Debug.cpp | 17 | ||||
-rw-r--r-- | tools/aapt2/JavaClassGenerator.cpp | 39 | ||||
-rw-r--r-- | tools/aapt2/JavaClassGenerator.h | 19 | ||||
-rw-r--r-- | tools/aapt2/JavaClassGenerator_test.cpp | 81 | ||||
-rw-r--r-- | tools/aapt2/ResourceParser.cpp | 37 | ||||
-rw-r--r-- | tools/aapt2/ResourceParser.h | 1 | ||||
-rw-r--r-- | tools/aapt2/ResourceTable.cpp | 49 | ||||
-rw-r--r-- | tools/aapt2/ResourceTable.h | 32 | ||||
-rw-r--r-- | tools/aapt2/flatten/ResourceTypeExtensions.h | 9 | ||||
-rw-r--r-- | tools/aapt2/flatten/TableFlattener.cpp | 26 | ||||
-rw-r--r-- | tools/aapt2/link/Link.cpp | 51 | ||||
-rw-r--r-- | tools/aapt2/link/PrivateAttributeMover.cpp | 4 | ||||
-rw-r--r-- | tools/aapt2/link/PrivateAttributeMover_test.cpp | 10 | ||||
-rw-r--r-- | tools/aapt2/link/ReferenceLinker.cpp | 11 | ||||
-rw-r--r-- | tools/aapt2/link/TableMerger.cpp | 39 | ||||
-rw-r--r-- | tools/aapt2/test/Builders.h | 5 | ||||
-rw-r--r-- | tools/aapt2/unflatten/BinaryResourceParser.cpp | 19 |
17 files changed, 359 insertions, 90 deletions
diff --git a/tools/aapt2/Debug.cpp b/tools/aapt2/Debug.cpp index 84f438520e90..d864f664f9db 100644 --- a/tools/aapt2/Debug.cpp +++ b/tools/aapt2/Debug.cpp @@ -124,17 +124,18 @@ void Debug::printTable(ResourceTable* table) { } for (const ResourceEntry* entry : sortedEntries) { - ResourceId id = { - package->id ? package->id.value() : uint8_t(0), - type->id ? type->id.value() : uint8_t(0), - entry->id ? entry->id.value() : uint16_t(0) - }; + ResourceId id(package->id ? package->id.value() : uint8_t(0), + type->id ? type->id.value() : uint8_t(0), + entry->id ? entry->id.value() : uint16_t(0)); + ResourceName name(package->name, type->type, entry->name); - ResourceName name = { package->name, type->type, entry->name }; std::cout << " spec resource " << id << " " << name; - if (entry->publicStatus.isPublic) { - std::cout << " PUBLIC"; + switch (entry->symbolStatus.state) { + case SymbolState::kPublic: std::cout << " PUBLIC"; break; + case SymbolState::kPrivate: std::cout << " _PRIVATE_"; break; + default: break; } + std::cout << std::endl; PrintVisitor visitor; diff --git a/tools/aapt2/JavaClassGenerator.cpp b/tools/aapt2/JavaClassGenerator.cpp index 84a412545dd8..cdf1b6ad600b 100644 --- a/tools/aapt2/JavaClassGenerator.cpp +++ b/tools/aapt2/JavaClassGenerator.cpp @@ -77,6 +77,18 @@ static std::u16string transform(const StringPiece16& symbol) { return output; } +bool JavaClassGenerator::skipSymbol(SymbolState state) { + switch (mOptions.types) { + case JavaClassGeneratorOptions::SymbolTypes::kAll: + return false; + case JavaClassGeneratorOptions::SymbolTypes::kPublicPrivate: + return state == SymbolState::kUndefined; + case JavaClassGeneratorOptions::SymbolTypes::kPublic: + return state != SymbolState::kPublic; + } + return true; +} + void JavaClassGenerator::generateStyleable(const StringPiece16& packageNameToGenerate, const std::u16string& entryName, const Styleable* styleable, @@ -121,7 +133,7 @@ void JavaClassGenerator::generateStyleable(const StringPiece16& packageNameToGen // We may reference IDs from other packages, so prefix the entry name with // the package. const ResourceNameRef& itemName = sortedAttributes[i].second; - if (packageNameToGenerate != itemName.package) { + if (!itemName.package.empty() && packageNameToGenerate != itemName.package) { *out << "_" << transform(itemName.package); } *out << "_" << transform(itemName.entry) << " = " << i << ";\n"; @@ -137,7 +149,11 @@ bool JavaClassGenerator::generateType(const StringPiece16& packageNameToGenerate std::u16string unmangledPackage; std::u16string unmangledName; for (const auto& entry : type->entries) { - ResourceId id = { package->id.value(), type->id.value(), entry->id.value() }; + if (skipSymbol(entry->symbolStatus.state)) { + continue; + } + + ResourceId id(package->id.value(), type->id.value(), entry->id.value()); assert(id.isValid()); unmangledName = entry->name; @@ -157,7 +173,7 @@ bool JavaClassGenerator::generateType(const StringPiece16& packageNameToGenerate } if (!isValidSymbol(unmangledName)) { - ResourceNameRef resourceName = { packageNameToGenerate, type->type, unmangledName }; + ResourceNameRef resourceName(packageNameToGenerate, type->type, unmangledName); std::stringstream err; err << "invalid symbol name '" << resourceName << "'"; mError = err.str(); @@ -177,13 +193,24 @@ bool JavaClassGenerator::generateType(const StringPiece16& packageNameToGenerate } bool JavaClassGenerator::generate(const StringPiece16& packageNameToGenerate, std::ostream* out) { - generateHeader(packageNameToGenerate, out); + return generate(packageNameToGenerate, packageNameToGenerate, out); +} + +bool JavaClassGenerator::generate(const StringPiece16& packageNameToGenerate, + const StringPiece16& outPackageName, std::ostream* out) { + generateHeader(outPackageName, out); *out << "public final class R {\n"; for (const auto& package : mTable->packages) { for (const auto& type : package->types) { - *out << " public static final class " << type->type << " {\n"; + StringPiece16 typeStr; + if (type->type == ResourceType::kAttrPrivate) { + typeStr = toString(ResourceType::kAttr); + } else { + typeStr = toString(type->type); + } + *out << " public static final class " << typeStr << " {\n"; if (!generateType(packageNameToGenerate, package.get(), type.get(), out)) { return false; } @@ -196,4 +223,6 @@ bool JavaClassGenerator::generate(const StringPiece16& packageNameToGenerate, st return true; } + + } // namespace aapt diff --git a/tools/aapt2/JavaClassGenerator.h b/tools/aapt2/JavaClassGenerator.h index 682bacfb0db4..e53a765ae0d8 100644 --- a/tools/aapt2/JavaClassGenerator.h +++ b/tools/aapt2/JavaClassGenerator.h @@ -33,6 +33,17 @@ struct JavaClassGeneratorOptions { * on resource entries. Default is true. */ bool useFinal = true; + + enum class SymbolTypes { + kAll, + kPublicPrivate, + kPublic, + }; + + /* + * + */ + SymbolTypes types = SymbolTypes::kAll; }; /* @@ -49,7 +60,11 @@ public: * We need to generate these symbols in a separate file. * Returns true on success. */ - bool generate(const StringPiece16& package, std::ostream* out); + bool generate(const StringPiece16& packageNameToGenerate, std::ostream* out); + + bool generate(const StringPiece16& packageNameToGenerate, + const StringPiece16& outputPackageName, + std::ostream* out); const std::string& getError() const; @@ -64,6 +79,8 @@ private: const Styleable* styleable, std::ostream* out); + bool skipSymbol(SymbolState state); + ResourceTable* mTable; JavaClassGeneratorOptions mOptions; std::string mError; diff --git a/tools/aapt2/JavaClassGenerator_test.cpp b/tools/aapt2/JavaClassGenerator_test.cpp index 48fcf8cb748f..becf99b521ed 100644 --- a/tools/aapt2/JavaClassGenerator_test.cpp +++ b/tools/aapt2/JavaClassGenerator_test.cpp @@ -65,6 +65,87 @@ TEST(JavaClassGeneratorTest, TransformInvalidJavaIdentifierCharacter) { output.find("public static final int hey_dude_cool_attr = 0;")); } +TEST(JavaClassGeneratorTest, CorrectPackageNameIsUsed) { + std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() + .setPackageId(u"android", 0x01) + .addSimple(u"@android:id/one", ResourceId(0x01020000)) + .addSimple(u"@android:id/com.foo$two", ResourceId(0x01020001)) + .build(); + + JavaClassGenerator generator(table.get(), {}); + std::stringstream out; + ASSERT_TRUE(generator.generate(u"android", u"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")); +} + +TEST(JavaClassGeneratorTest, AttrPrivateIsWrittenAsAttr) { + std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() + .setPackageId(u"android", 0x01) + .addSimple(u"@android:^attr-private/one", ResourceId(0x01010000)) + .build(); + + JavaClassGenerator generator(table.get(), {}); + std::stringstream out; + ASSERT_TRUE(generator.generate(u"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")); +} + +TEST(JavaClassGeneratorTest, OnlyWritePublicResources) { + StdErrDiagnostics diag; + std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() + .setPackageId(u"android", 0x01) + .addSimple(u"@android:id/one", ResourceId(0x01020000)) + .addSimple(u"@android:id/two", ResourceId(0x01020001)) + .addSimple(u"@android:id/three", ResourceId(0x01020002)) + .build(); + ASSERT_TRUE(table->setSymbolState(test::parseNameOrDie(u"@android:id/one"), {}, {}, + SymbolState::kPublic, &diag)); + ASSERT_TRUE(table->setSymbolState(test::parseNameOrDie(u"@android:id/two"), {}, {}, + SymbolState::kPrivate, &diag)); + + JavaClassGeneratorOptions options; + options.types = JavaClassGeneratorOptions::SymbolTypes::kPublic; + { + JavaClassGenerator generator(table.get(), options); + std::stringstream out; + ASSERT_TRUE(generator.generate(u"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")); + } + + options.types = JavaClassGeneratorOptions::SymbolTypes::kPublicPrivate; + { + JavaClassGenerator generator(table.get(), options); + std::stringstream out; + ASSERT_TRUE(generator.generate(u"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")); + } + + options.types = JavaClassGeneratorOptions::SymbolTypes::kAll; + { + JavaClassGenerator generator(table.get(), options); + std::stringstream out; + ASSERT_TRUE(generator.generate(u"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;")); + } +} + /* * TODO(adamlesinski): Re-enable this once we get merging working again. * TEST(JavaClassGeneratorTest, EmitPackageMangledSymbols) { diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp index 63629f0d6c10..bfef9d06d742 100644 --- a/tools/aapt2/ResourceParser.cpp +++ b/tools/aapt2/ResourceParser.cpp @@ -18,10 +18,11 @@ #include "ResourceTable.h" #include "ResourceUtils.h" #include "ResourceValues.h" -#include "util/Util.h" #include "ValueVisitor.h" #include "XmlPullParser.h" +#include "util/Util.h" + #include <sstream> namespace aapt { @@ -184,7 +185,7 @@ struct ParsedResource { ResourceName name; Source source; ResourceId id; - bool markPublic = false; + SymbolState symbolState = SymbolState::kUndefined; std::unique_ptr<Value> value; std::list<ParsedResource> childResources; }; @@ -192,8 +193,10 @@ struct ParsedResource { // Recursively adds resources to the ResourceTable. static bool addResourcesToTable(ResourceTable* table, const ConfigDescription& config, IDiagnostics* diag, ParsedResource* res) { - if (res->markPublic && !table->markPublic(res->name, res->id, res->source, diag)) { - return false; + if (res->symbolState != SymbolState::kUndefined) { + if (!table->setSymbolState(res->name, res->id, res->source, res->symbolState, diag)) { + return false; + } } if (!res->value) { @@ -318,6 +321,8 @@ bool ResourceParser::parseResources(XmlPullParser* parser) { result = parseAttr(parser, &parsedResource); } else if (elementName == u"public") { result = parsePublic(parser, &parsedResource); + } else if (elementName == u"java-symbol" || elementName == u"symbol") { + result = parseSymbol(parser, &parsedResource); } else { mDiag->warn(DiagMessage(mSource.withLine(parser->getLineNumber())) << "unknown resource type '" << elementName << "'"); @@ -506,7 +511,29 @@ bool ResourceParser::parsePublic(XmlPullParser* parser, ParsedResource* outResou outResource->value = util::make_unique<Id>(); } - outResource->markPublic = true; + outResource->symbolState = SymbolState::kPublic; + return true; +} + +bool ResourceParser::parseSymbol(XmlPullParser* parser, ParsedResource* outResource) { + const Source source = mSource.withLine(parser->getLineNumber()); + + Maybe<StringPiece16> maybeType = findNonEmptyAttribute(parser, u"type"); + if (!maybeType) { + mDiag->error(DiagMessage(source) << "<" << parser->getElementName() << "> must have a " + "'type' attribute"); + return false; + } + + const ResourceType* parsedType = parseResourceType(maybeType.value()); + if (!parsedType) { + mDiag->error(DiagMessage(source) << "invalid resource type '" << maybeType.value() + << "' in <" << parser->getElementName() << ">"); + return false; + } + + outResource->name.type = *parsedType; + outResource->symbolState = SymbolState::kPrivate; return true; } diff --git a/tools/aapt2/ResourceParser.h b/tools/aapt2/ResourceParser.h index 5ccd47f2b6a7..34c68d7257a3 100644 --- a/tools/aapt2/ResourceParser.h +++ b/tools/aapt2/ResourceParser.h @@ -77,6 +77,7 @@ private: bool parseColor(XmlPullParser* parser, ParsedResource* outResource); bool parsePrimitive(XmlPullParser* parser, ParsedResource* outResource); bool parsePublic(XmlPullParser* parser, ParsedResource* outResource); + bool parseSymbol(XmlPullParser* parser, ParsedResource* outResource); bool parseAttr(XmlPullParser* parser, ParsedResource* outResource); bool parseAttrImpl(XmlPullParser* parser, ParsedResource* outResource, bool weak); Maybe<Attribute::Symbol> parseEnumOrFlagItem(XmlPullParser* parser, const StringPiece16& tag); diff --git a/tools/aapt2/ResourceTable.cpp b/tools/aapt2/ResourceTable.cpp index e32fb5ee22ea..84674e841063 100644 --- a/tools/aapt2/ResourceTable.cpp +++ b/tools/aapt2/ResourceTable.cpp @@ -220,6 +220,16 @@ bool ResourceTable::addResourceAllowMangled(const ResourceNameRef& name, kValidNameMangledChars, diag); } +bool ResourceTable::addResourceAllowMangled(const ResourceNameRef& name, + const ResourceId id, + const ConfigDescription& config, + const Source& source, + std::unique_ptr<Value> value, + IDiagnostics* diag) { + return addResourceImpl(name, id, config, source, std::move(value), + kValidNameMangledChars, diag); +} + bool ResourceTable::addResourceImpl(const ResourceNameRef& name, const ResourceId resId, const ConfigDescription& config, const Source& source, std::unique_ptr<Value> value, const char16_t* validChars, @@ -305,19 +315,25 @@ bool ResourceTable::addResourceImpl(const ResourceNameRef& name, const ResourceI return true; } -bool ResourceTable::markPublic(const ResourceNameRef& name, const ResourceId resId, - const Source& source, IDiagnostics* diag) { - return markPublicImpl(name, resId, source, kValidNameChars, diag); +bool ResourceTable::setSymbolState(const ResourceNameRef& name, const ResourceId resId, + const Source& source, SymbolState state, IDiagnostics* diag) { + return setSymbolStateImpl(name, resId, source, state, kValidNameChars, diag); } -bool ResourceTable::markPublicAllowMangled(const ResourceNameRef& name, const ResourceId resId, - const Source& source, IDiagnostics* diag) { - return markPublicImpl(name, resId, source, kValidNameMangledChars, diag); +bool ResourceTable::setSymbolStateAllowMangled(const ResourceNameRef& name, const ResourceId resId, + const Source& source, SymbolState state, + IDiagnostics* diag) { + return setSymbolStateImpl(name, resId, source, state, kValidNameMangledChars, diag); } -bool ResourceTable::markPublicImpl(const ResourceNameRef& name, const ResourceId resId, - const Source& source, const char16_t* validChars, - IDiagnostics* diag) { +bool ResourceTable::setSymbolStateImpl(const ResourceNameRef& name, const ResourceId resId, + const Source& source, SymbolState state, + const char16_t* validChars, IDiagnostics* diag) { + if (state == SymbolState::kUndefined) { + // Nothing to do. + return true; + } + auto badCharIter = util::findNonAlphaNumericAndNotInSet(name.entry, validChars); if (badCharIter != name.entry.end()) { diag->error(DiagMessage(source) @@ -371,9 +387,18 @@ bool ResourceTable::markPublicImpl(const ResourceNameRef& name, const ResourceId return false; } - type->publicStatus.isPublic = true; - entry->publicStatus.isPublic = true; - entry->publicStatus.source = source; + // Only mark the type state as public, it doesn't care about being private. + if (state == SymbolState::kPublic) { + type->symbolStatus.state = SymbolState::kPublic; + } + + // Downgrading to a private symbol from a public one is not allowed. + if (entry->symbolStatus.state != SymbolState::kPublic) { + if (entry->symbolStatus.state != state) { + entry->symbolStatus.state = state; + entry->symbolStatus.source = source; + } + } if (resId.isValid()) { package->id = resId.packageId(); diff --git a/tools/aapt2/ResourceTable.h b/tools/aapt2/ResourceTable.h index 60fed2f0e26b..be909361bef1 100644 --- a/tools/aapt2/ResourceTable.h +++ b/tools/aapt2/ResourceTable.h @@ -31,11 +31,17 @@ namespace aapt { +enum class SymbolState { + kUndefined, + kPublic, + kPrivate +}; + /** * The Public status of a resource. */ -struct Public { - bool isPublic = false; +struct Symbol { + SymbolState state = SymbolState::kUndefined; Source source; std::u16string comment; }; @@ -71,7 +77,7 @@ struct ResourceEntry { * Whether this resource is public (and must maintain the same * entry ID across builds). */ - Public publicStatus; + Symbol symbolStatus; /** * The resource's values for each configuration. @@ -100,7 +106,7 @@ struct ResourceTableType { * Whether this type is public (and must maintain the same * type ID across builds). */ - Public publicStatus; + Symbol symbolStatus; /** * List of resources for this type. @@ -171,10 +177,15 @@ public: const Source& source, std::unique_ptr<Value> value, IDiagnostics* diag); - bool markPublic(const ResourceNameRef& name, const ResourceId resId, const Source& source, - IDiagnostics* diag); - bool markPublicAllowMangled(const ResourceNameRef& name, const ResourceId resId, - const Source& source, IDiagnostics* diag); + bool addResourceAllowMangled(const ResourceNameRef& name, const ResourceId id, + const ConfigDescription& config, + const Source& source, std::unique_ptr<Value> value, + IDiagnostics* diag); + + bool setSymbolState(const ResourceNameRef& name, const ResourceId resId, const Source& source, + SymbolState state, IDiagnostics* diag); + bool setSymbolStateAllowMangled(const ResourceNameRef& name, const ResourceId resId, + const Source& source, SymbolState state, IDiagnostics* diag); struct SearchResult { ResourceTablePackage* package; ResourceTableType* type; @@ -217,8 +228,9 @@ private: const ConfigDescription& config, const Source& source, std::unique_ptr<Value> value, const char16_t* validChars, IDiagnostics* diag); - bool markPublicImpl(const ResourceNameRef& name, const ResourceId resId, - const Source& source, const char16_t* validChars, IDiagnostics* diag); + bool setSymbolStateImpl(const ResourceNameRef& name, const ResourceId resId, + const Source& source, SymbolState state, const char16_t* validChars, + IDiagnostics* diag); }; } // namespace aapt diff --git a/tools/aapt2/flatten/ResourceTypeExtensions.h b/tools/aapt2/flatten/ResourceTypeExtensions.h index af0afefe1676..38cda097c927 100644 --- a/tools/aapt2/flatten/ResourceTypeExtensions.h +++ b/tools/aapt2/flatten/ResourceTypeExtensions.h @@ -134,7 +134,14 @@ struct Public_header { struct Public_entry { uint16_t entryId; - uint16_t res0; + + enum : uint16_t { + kUndefined = 0, + kPublic = 1, + kPrivate = 2, + }; + + uint16_t state; android::ResStringPool_ref key; android::ResStringPool_ref source; uint32_t sourceLine; diff --git a/tools/aapt2/flatten/TableFlattener.cpp b/tools/aapt2/flatten/TableFlattener.cpp index 75cbd5d7baa1..095552aa7f31 100644 --- a/tools/aapt2/flatten/TableFlattener.cpp +++ b/tools/aapt2/flatten/TableFlattener.cpp @@ -300,7 +300,7 @@ private: T* result = buffer->nextBlock<T>(); ResTable_entry* outEntry = (ResTable_entry*)(result); - if (entry->entry->publicStatus.isPublic) { + if (entry->entry->symbolStatus.state == SymbolState::kPublic) { outEntry->flags |= ResTable_entry::FLAG_PUBLIC; } @@ -455,7 +455,7 @@ private: // Populate the config masks for this entry. - if (entry->publicStatus.isPublic) { + if (entry->symbolStatus.state == SymbolState::kPublic) { configMasks[entry->id.value()] |= util::hostToDevice32(ResTable_typeSpec::SPEC_PUBLIC); } @@ -480,17 +480,31 @@ private: publicHeader->typeId = type->id.value(); for (ResourceEntry* entry : *sortedEntries) { - if (entry->publicStatus.isPublic) { + if (entry->symbolStatus.state != SymbolState::kUndefined) { // Write the public status of this entry. Public_entry* publicEntry = publicWriter.nextBlock<Public_entry>(); publicEntry->entryId = util::hostToDevice32(entry->id.value()); publicEntry->key.index = util::hostToDevice32(mKeyPool.makeRef( entry->name).getIndex()); publicEntry->source.index = util::hostToDevice32(mSourcePool->makeRef( - util::utf8ToUtf16(entry->publicStatus.source.path)).getIndex()); - if (entry->publicStatus.source.line) { + util::utf8ToUtf16(entry->symbolStatus.source.path)).getIndex()); + if (entry->symbolStatus.source.line) { publicEntry->sourceLine = util::hostToDevice32( - entry->publicStatus.source.line.value()); + entry->symbolStatus.source.line.value()); + } + + switch (entry->symbolStatus.state) { + case SymbolState::kPrivate: + publicEntry->state = Public_entry::kPrivate; + break; + + case SymbolState::kPublic: + publicEntry->state = Public_entry::kPublic; + break; + + default: + assert(false && "should not serialize any other state"); + break; } // Don't hostToDevice until the last step. diff --git a/tools/aapt2/link/Link.cpp b/tools/aapt2/link/Link.cpp index fa321a01d303..b84f2e068dc2 100644 --- a/tools/aapt2/link/Link.cpp +++ b/tools/aapt2/link/Link.cpp @@ -52,7 +52,7 @@ struct LinkOptions { bool staticLib = false; bool verbose = false; bool outputToDirectory = false; - Maybe<std::string> privateSymbols; + Maybe<std::u16string> privateSymbols; }; struct LinkContext : public IAaptContext { @@ -328,13 +328,14 @@ struct LinkCommand { return true; } - bool writeJavaFile(ResourceTable* table, const StringPiece16& package) { + bool writeJavaFile(ResourceTable* table, const StringPiece16& packageNameToGenerate, + const StringPiece16& outPackage, JavaClassGeneratorOptions javaOptions) { if (!mOptions.generateJavaClassPath) { return true; } std::string outPath = mOptions.generateJavaClassPath.value(); - file::appendPath(&outPath, file::packageToPath(util::utf16ToUtf8(package))); + file::appendPath(&outPath, file::packageToPath(util::utf16ToUtf8(outPackage))); file::mkdirs(outPath); file::appendPath(&outPath, "R.java"); @@ -344,13 +345,8 @@ struct LinkCommand { return false; } - JavaClassGeneratorOptions javaOptions; - if (mOptions.staticLib) { - javaOptions.useFinal = false; - } - JavaClassGenerator generator(table, javaOptions); - if (!generator.generate(mContext.getCompilationPackage(), &fout)) { + if (!generator.generate(packageNameToGenerate, outPackage, &fout)) { mContext.getDiagnostics()->error(DiagMessage(outPath) << generator.getError()); return false; } @@ -652,8 +648,34 @@ struct LinkCommand { } if (mOptions.generateJavaClassPath) { - if (!writeJavaFile(&mergedTable, mContext.getCompilationPackage())) { - return 1; + JavaClassGeneratorOptions options; + if (mOptions.staticLib) { + options.useFinal = false; + } + + if (mOptions.privateSymbols) { + // 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(&mergedTable, mContext.getCompilationPackage(), + mContext.getCompilationPackage(), options)) { + return 1; + } + + options.types = JavaClassGeneratorOptions::SymbolTypes::kPublicPrivate; + if (!writeJavaFile(&mergedTable, mContext.getCompilationPackage(), + mOptions.privateSymbols.value(), options)) { + return 1; + } + + } else { + // Emit Everything. + + if (!writeJavaFile(&mergedTable, mContext.getCompilationPackage(), + mContext.getCompilationPackage(), options)) { + return 1; + } } } @@ -680,6 +702,7 @@ struct LinkCommand { int link(const std::vector<StringPiece>& args) { LinkOptions options; + Maybe<std::string> privateSymbolsPackage; Flags flags = Flags() .requiredFlag("-o", "Output path", &options.outputPath) .requiredFlag("--manifest", "Path to the Android manifest to build", @@ -698,13 +721,17 @@ int link(const std::vector<StringPiece>& args) { .optionalSwitch("--static-lib", "Generate a static Android library", &options.staticLib) .optionalFlag("--private-symbols", "Package name to use when generating R.java for " "private symbols. If not specified, public and private symbols will " - "use the application's package name", &options.privateSymbols) + "use the application's package name", &privateSymbolsPackage) .optionalSwitch("-v", "Enables verbose logging", &options.verbose); if (!flags.parse("aapt2 link", args, &std::cerr)) { return 1; } + if (privateSymbolsPackage) { + options.privateSymbols = util::utf8ToUtf16(privateSymbolsPackage.value()); + } + LinkCommand cmd = { options }; return cmd.run(flags.getArgs()); } diff --git a/tools/aapt2/link/PrivateAttributeMover.cpp b/tools/aapt2/link/PrivateAttributeMover.cpp index db20bcb09425..5a2f5f0771d2 100644 --- a/tools/aapt2/link/PrivateAttributeMover.cpp +++ b/tools/aapt2/link/PrivateAttributeMover.cpp @@ -61,7 +61,7 @@ bool PrivateAttributeMover::consume(IAaptContext* context, ResourceTable* table) continue; } - if (!type->publicStatus.isPublic) { + if (type->symbolStatus.state != SymbolState::kPublic) { // No public attributes, so we can safely leave these private attributes where they are. return true; } @@ -71,7 +71,7 @@ bool PrivateAttributeMover::consume(IAaptContext* context, ResourceTable* table) moveIf(type->entries, std::back_inserter(privAttrType->entries), [](const std::unique_ptr<ResourceEntry>& entry) -> bool { - return !entry->publicStatus.isPublic; + return entry->symbolStatus.state != SymbolState::kPublic; }); break; } diff --git a/tools/aapt2/link/PrivateAttributeMover_test.cpp b/tools/aapt2/link/PrivateAttributeMover_test.cpp index 8173c30b25fd..a2f8d19b57cc 100644 --- a/tools/aapt2/link/PrivateAttributeMover_test.cpp +++ b/tools/aapt2/link/PrivateAttributeMover_test.cpp @@ -31,10 +31,12 @@ TEST(PrivateAttributeMoverTest, MovePrivateAttributes) { .addSimple(u"@android:attr/publicB") .addSimple(u"@android:attr/privateB") .build(); - ASSERT_TRUE(table->markPublic(test::parseNameOrDie(u"@android:attr/publicA"), - ResourceId(0x01010000), {}, context->getDiagnostics())); - ASSERT_TRUE(table->markPublic(test::parseNameOrDie(u"@android:attr/publicB"), - ResourceId(0x01010002), {}, context->getDiagnostics())); + ASSERT_TRUE(table->setSymbolState(test::parseNameOrDie(u"@android:attr/publicA"), + ResourceId(0x01010000), {}, SymbolState::kPublic, + context->getDiagnostics())); + ASSERT_TRUE(table->setSymbolState(test::parseNameOrDie(u"@android:attr/publicB"), + ResourceId(0x01010002), {}, SymbolState::kPublic, + context->getDiagnostics())); PrivateAttributeMover mover; ASSERT_TRUE(mover.consume(context.get(), table.get())); diff --git a/tools/aapt2/link/ReferenceLinker.cpp b/tools/aapt2/link/ReferenceLinker.cpp index c0356e50da3a..b4fb9961f0c5 100644 --- a/tools/aapt2/link/ReferenceLinker.cpp +++ b/tools/aapt2/link/ReferenceLinker.cpp @@ -249,10 +249,13 @@ bool ReferenceLinker::consume(IAaptContext* context, ResourceTable* table) { for (auto& package : table->packages) { for (auto& type : package->types) { for (auto& entry : type->entries) { - // A public entry with no values will not be encoded properly. - if (entry->publicStatus.isPublic && entry->values.empty()) { - context->getDiagnostics()->error(DiagMessage(entry->publicStatus.source) - << "No value for public resource"); + // Symbol state information may be lost if there is no value for the resource. + if (entry->symbolStatus.state != SymbolState::kUndefined && entry->values.empty()) { + context->getDiagnostics()->error( + DiagMessage(entry->symbolStatus.source) + << "no definition for declared symbol '" + << ResourceNameRef(package->name, type->type, entry->name) + << "'"); error = true; } diff --git a/tools/aapt2/link/TableMerger.cpp b/tools/aapt2/link/TableMerger.cpp index 0d63b976936e..db525467933f 100644 --- a/tools/aapt2/link/TableMerger.cpp +++ b/tools/aapt2/link/TableMerger.cpp @@ -69,8 +69,8 @@ bool TableMerger::doMerge(const Source& src, ResourceTable* srcTable, for (auto& srcType : srcPackage->types) { ResourceTableType* dstType = mMasterPackage->findOrCreateType(srcType->type); - if (srcType->publicStatus.isPublic) { - if (dstType->publicStatus.isPublic && dstType->id && srcType->id + if (srcType->symbolStatus.state == SymbolState::kPublic) { + if (dstType->symbolStatus.state == SymbolState::kPublic && dstType->id && srcType->id && dstType->id.value() == srcType->id.value()) { // Both types are public and have different IDs. mContext->getDiagnostics()->error(DiagMessage(src) @@ -81,7 +81,7 @@ bool TableMerger::doMerge(const Source& src, ResourceTable* srcTable, continue; } - dstType->publicStatus = std::move(srcType->publicStatus); + dstType->symbolStatus = std::move(srcType->symbolStatus); dstType->id = srcType->id; } @@ -94,20 +94,29 @@ bool TableMerger::doMerge(const Source& src, ResourceTable* srcTable, dstEntry = dstType->findOrCreateEntry(srcEntry->name); } - if (srcEntry->publicStatus.isPublic) { - if (dstEntry->publicStatus.isPublic && dstEntry->id && srcEntry->id - && dstEntry->id.value() != srcEntry->id.value()) { - // Both entries are public and have different IDs. - mContext->getDiagnostics()->error(DiagMessage(src) - << "can not merge entry '" - << srcEntry->name - << "': conflicting public IDs"); - error = true; - continue; + if (srcEntry->symbolStatus.state != SymbolState::kUndefined) { + if (srcEntry->symbolStatus.state == SymbolState::kPublic) { + if (dstEntry->symbolStatus.state == SymbolState::kPublic && + dstEntry->id && srcEntry->id && + dstEntry->id.value() != srcEntry->id.value()) { + // Both entries are public and have different IDs. + mContext->getDiagnostics()->error(DiagMessage(src) + << "can not merge entry '" + << srcEntry->name + << "': conflicting public IDs"); + error = true; + continue; + } + + if (srcEntry->id) { + dstEntry->id = srcEntry->id; + } } - dstEntry->publicStatus = std::move(srcEntry->publicStatus); - dstEntry->id = srcEntry->id; + if (dstEntry->symbolStatus.state != SymbolState::kPublic && + dstEntry->symbolStatus.state != srcEntry->symbolStatus.state) { + dstEntry->symbolStatus = std::move(srcEntry->symbolStatus); + } } for (ResourceConfigValue& srcValue : srcEntry->values) { diff --git a/tools/aapt2/test/Builders.h b/tools/aapt2/test/Builders.h index 0d8d8b5d49aa..0383c443d4c1 100644 --- a/tools/aapt2/test/Builders.h +++ b/tools/aapt2/test/Builders.h @@ -87,8 +87,9 @@ public: ResourceTableBuilder& addValue(const StringPiece16& name, ResourceId id, const ConfigDescription& config, std::unique_ptr<Value> value) { - bool result = mTable->addResource(parseNameOrDie(name), id, config, {}, std::move(value), - &mDiagnostics); + ResourceName resName = parseNameOrDie(name); + bool result = mTable->addResourceAllowMangled(resName, id, config, {}, std::move(value), + &mDiagnostics); assert(result); return *this; } diff --git a/tools/aapt2/unflatten/BinaryResourceParser.cpp b/tools/aapt2/unflatten/BinaryResourceParser.cpp index ac91865e9e39..c7a715e7d2e2 100644 --- a/tools/aapt2/unflatten/BinaryResourceParser.cpp +++ b/tools/aapt2/unflatten/BinaryResourceParser.cpp @@ -429,7 +429,19 @@ bool BinaryResourceParser::parsePublic(const ResourceTablePackage* package, source.line = util::deviceToHost32(entry->sourceLine); } - if (!mTable->markPublicAllowMangled(name, resId, source, mContext->getDiagnostics())) { + SymbolState state = SymbolState::kUndefined; + switch (util::deviceToHost16(entry->state)) { + case Public_entry::kPrivate: + state = SymbolState::kPrivate; + break; + + case Public_entry::kPublic: + state = SymbolState::kPublic; + break; + } + + if (!mTable->setSymbolStateAllowMangled(name, resId, source, state, + mContext->getDiagnostics())) { return false; } @@ -564,8 +576,9 @@ bool BinaryResourceParser::parseType(const ResourceTablePackage* package, } if ((entry->flags & ResTable_entry::FLAG_PUBLIC) != 0) { - if (!mTable->markPublicAllowMangled(name, resId, mSource.withLine(0), - mContext->getDiagnostics())) { + if (!mTable->setSymbolStateAllowMangled(name, resId, mSource.withLine(0), + SymbolState::kPublic, + mContext->getDiagnostics())) { return false; } } |