Merge "AAPT2: Fix <add-resource> tag for overlays" into oc-dev

am: 8255ced3f5

Change-Id: Icc420eea48a8379723c0bc84a5f30c03ac2a2492
This commit is contained in:
Adam Lesinski
2017-05-30 20:53:38 +00:00
committed by android-build-merger
12 changed files with 70 additions and 68 deletions

View File

@@ -69,6 +69,7 @@ message SymbolStatus {
optional Visibility visibility = 1;
optional Source source = 2;
optional string comment = 3;
optional bool allow_new = 4;
}
message Entry {

View File

@@ -92,14 +92,14 @@ struct ParsedResource {
Source source;
ResourceId id;
Maybe<SymbolState> symbol_state;
bool allow_new = false;
std::string comment;
std::unique_ptr<Value> value;
std::list<ParsedResource> child_resources;
};
// Recursively adds resources to the ResourceTable.
static bool AddResourcesToTable(ResourceTable* table, IDiagnostics* diag,
ParsedResource* res) {
static bool AddResourcesToTable(ResourceTable* table, IDiagnostics* diag, ParsedResource* res) {
StringPiece trimmed_comment = util::TrimWhitespace(res->comment);
if (trimmed_comment.size() != res->comment.size()) {
// Only if there was a change do we re-assign.
@@ -111,6 +111,7 @@ static bool AddResourcesToTable(ResourceTable* table, IDiagnostics* diag,
symbol.state = res->symbol_state.value();
symbol.source = res->source;
symbol.comment = res->comment;
symbol.allow_new = res->allow_new;
if (!table->SetSymbolState(res->name, res->id, symbol, diag)) {
return false;
}
@@ -121,8 +122,8 @@ static bool AddResourcesToTable(ResourceTable* table, IDiagnostics* diag,
res->value->SetComment(std::move(res->comment));
res->value->SetSource(std::move(res->source));
if (!table->AddResource(res->name, res->id, res->config, res->product,
std::move(res->value), diag)) {
if (!table->AddResource(res->name, res->id, res->config, res->product, std::move(res->value),
diag)) {
return false;
}
}
@@ -849,6 +850,7 @@ bool ResourceParser::ParseAddResource(xml::XmlPullParser* parser,
ParsedResource* out_resource) {
if (ParseSymbolImpl(parser, out_resource)) {
out_resource->symbol_state = SymbolState::kUndefined;
out_resource->allow_new = true;
return true;
}
return false;

View File

@@ -777,8 +777,7 @@ TEST_F(ResourceParserTest, ExternalTypesShouldOnlyBeReferences) {
ASSERT_FALSE(TestParse(input));
}
TEST_F(ResourceParserTest,
AddResourcesElementShouldAddEntryWithUndefinedSymbol) {
TEST_F(ResourceParserTest, AddResourcesElementShouldAddEntryWithUndefinedSymbol) {
std::string input = R"EOF(<add-resource name="bar" type="string" />)EOF";
ASSERT_TRUE(TestParse(input));
@@ -788,6 +787,7 @@ TEST_F(ResourceParserTest,
const ResourceEntry* entry = result.value().entry;
ASSERT_NE(nullptr, entry);
EXPECT_EQ(SymbolState::kUndefined, entry->symbol_status.state);
EXPECT_TRUE(entry->symbol_status.allow_new);
}
TEST_F(ResourceParserTest, ParseItemElementWithFormat) {

View File

@@ -440,8 +440,7 @@ bool ResourceTable::AddResourceImpl(const ResourceNameRef& name, const ResourceI
return true;
}
bool ResourceTable::SetSymbolState(const ResourceNameRef& name,
const ResourceId& res_id,
bool ResourceTable::SetSymbolState(const ResourceNameRef& name, const ResourceId& res_id,
const Symbol& symbol, IDiagnostics* diag) {
return SetSymbolStateImpl(name, res_id, symbol, ValidateName, diag);
}
@@ -489,8 +488,7 @@ bool ResourceTable::SetSymbolStateImpl(const ResourceNameRef& name, const Resour
diag->Error(DiagMessage(symbol.source)
<< "trying to add resource '" << name << "' with ID " << res_id
<< " but resource already has ID "
<< ResourceId(package->id.value(), type->id.value(),
entry->id.value()));
<< ResourceId(package->id.value(), type->id.value(), entry->id.value()));
return false;
}
@@ -505,6 +503,11 @@ bool ResourceTable::SetSymbolStateImpl(const ResourceNameRef& name, const Resour
type->symbol_status.state = SymbolState::kPublic;
}
if (symbol.allow_new) {
// This symbol can be added as a new resource when merging (if it belongs to an overlay).
entry->symbol_status.allow_new = true;
}
if (symbol.state == SymbolState::kUndefined &&
entry->symbol_status.state != SymbolState::kUndefined) {
// We can't undefine a symbol (remove its visibility). Ignore.
@@ -517,7 +520,10 @@ bool ResourceTable::SetSymbolStateImpl(const ResourceNameRef& name, const Resour
return true;
}
entry->symbol_status = std::move(symbol);
// This symbol definition takes precedence, replace.
entry->symbol_status.state = symbol.state;
entry->symbol_status.source = symbol.source;
entry->symbol_status.comment = symbol.comment;
return true;
}

View File

@@ -50,6 +50,10 @@ enum class SymbolState {
struct Symbol {
SymbolState state = SymbolState::kUndefined;
Source source;
// Whether this entry (originating from an overlay) can be added as a new resource.
bool allow_new = false;
std::string comment;
};
@@ -223,8 +227,7 @@ class ResourceTable {
bool SetSymbolState(const ResourceNameRef& name, const ResourceId& res_id,
const Symbol& symbol, IDiagnostics* diag);
bool SetSymbolStateAllowMangled(const ResourceNameRef& name,
const ResourceId& res_id,
bool SetSymbolStateAllowMangled(const ResourceNameRef& name, const ResourceId& res_id,
const Symbol& symbol, IDiagnostics* diag);
struct SearchResult {

View File

@@ -243,8 +243,7 @@ bool TableMerger::DoMerge(const Source& src, ResourceTable* src_table,
bool error = false;
for (auto& src_type : src_package->types) {
ResourceTableType* dst_type =
master_package_->FindOrCreateType(src_type->type);
ResourceTableType* dst_type = master_package_->FindOrCreateType(src_type->type);
if (!MergeType(context_, src, dst_type, src_type.get())) {
error = true;
continue;
@@ -253,27 +252,24 @@ bool TableMerger::DoMerge(const Source& src, ResourceTable* src_table,
for (auto& src_entry : src_type->entries) {
std::string entry_name = src_entry->name;
if (mangle_package) {
entry_name =
NameMangler::MangleEntry(src_package->name, src_entry->name);
entry_name = NameMangler::MangleEntry(src_package->name, src_entry->name);
}
ResourceEntry* dst_entry;
if (allow_new_resources) {
if (allow_new_resources || src_entry->symbol_status.allow_new) {
dst_entry = dst_type->FindOrCreateEntry(entry_name);
} else {
dst_entry = dst_type->FindEntry(entry_name);
}
const ResourceNameRef res_name(src_package->name, src_type->type,
src_entry->name);
const ResourceNameRef res_name(src_package->name, src_type->type, src_entry->name);
if (!dst_entry) {
context_->GetDiagnostics()->Error(
DiagMessage(src) << "resource " << res_name
<< " does not override an existing resource");
context_->GetDiagnostics()->Note(
DiagMessage(src) << "define an <add-resource> tag or use "
<< "--auto-add-overlay");
context_->GetDiagnostics()->Error(DiagMessage(src)
<< "resource " << res_name
<< " does not override an existing resource");
context_->GetDiagnostics()->Note(DiagMessage(src) << "define an <add-resource> tag or use "
<< "--auto-add-overlay");
error = true;
continue;
}

View File

@@ -248,18 +248,18 @@ TEST_F(TableMergerTest, FailToOverrideConflictingEntryIdsWithOverlay) {
TEST_F(TableMergerTest, MergeAddResourceFromOverlay) {
std::unique_ptr<ResourceTable> table_a =
test::ResourceTableBuilder()
.SetPackageId("", 0x7f)
.SetSymbolState("bool/foo", {}, SymbolState::kUndefined)
.Build();
test::ResourceTableBuilder().SetPackageId("", 0x7f).Build();
std::unique_ptr<ResourceTable> table_b =
test::ResourceTableBuilder()
.SetPackageId("", 0x7f)
.SetSymbolState("bool/foo", {}, SymbolState::kUndefined, true /*allow new overlay*/)
.AddValue("bool/foo", ResourceUtils::TryParseBool("true"))
.Build();
ResourceTable final_table;
TableMerger merger(context_.get(), &final_table, TableMergerOptions{});
TableMergerOptions options;
options.auto_add_overlay = false;
TableMerger merger(context_.get(), &final_table, options);
ASSERT_TRUE(merger.Merge({}, table_a.get()));
ASSERT_TRUE(merger.MergeOverlay({}, table_b.get()));

View File

@@ -32,8 +32,7 @@ class ReferenceIdToNameVisitor : public ValueVisitor {
public:
using ValueVisitor::Visit;
explicit ReferenceIdToNameVisitor(
const std::map<ResourceId, ResourceNameRef>* mapping)
explicit ReferenceIdToNameVisitor(const std::map<ResourceId, ResourceNameRef>* mapping)
: mapping_(mapping) {
CHECK(mapping_ != nullptr);
}
@@ -75,13 +74,11 @@ class PackagePbDeserializer {
std::map<ResourceId, ResourceNameRef> idIndex;
ResourceTablePackage* pkg =
table->CreatePackage(pbPackage.package_name(), id);
ResourceTablePackage* pkg = table->CreatePackage(pbPackage.package_name(), id);
for (const pb::Type& pbType : pbPackage.types()) {
const ResourceType* resType = ParseResourceType(pbType.name());
if (!resType) {
diag_->Error(DiagMessage(source_) << "unknown type '" << pbType.name()
<< "'");
diag_->Error(DiagMessage(source_) << "unknown type '" << pbType.name() << "'");
return {};
}
@@ -95,21 +92,20 @@ class PackagePbDeserializer {
if (pbEntry.has_symbol_status()) {
const pb::SymbolStatus& pbStatus = pbEntry.symbol_status();
if (pbStatus.has_source()) {
DeserializeSourceFromPb(pbStatus.source(), *source_pool_,
&entry->symbol_status.source);
DeserializeSourceFromPb(pbStatus.source(), *source_pool_, &entry->symbol_status.source);
}
if (pbStatus.has_comment()) {
entry->symbol_status.comment = pbStatus.comment();
}
SymbolState visibility =
DeserializeVisibilityFromPb(pbStatus.visibility());
entry->symbol_status.allow_new = pbStatus.allow_new();
SymbolState visibility = DeserializeVisibilityFromPb(pbStatus.visibility());
entry->symbol_status.state = visibility;
if (visibility == SymbolState::kPublic) {
// This is a public symbol, we must encode the ID now if there is
// one.
// This is a public symbol, we must encode the ID now if there is one.
if (pbEntry.has_id()) {
entry->id = static_cast<uint16_t>(pbEntry.id());
}
@@ -142,16 +138,15 @@ class PackagePbDeserializer {
return {};
}
ResourceConfigValue* configValue =
entry->FindOrCreateValue(config, pbConfig.product());
ResourceConfigValue* configValue = entry->FindOrCreateValue(config, pbConfig.product());
if (configValue->value) {
// Duplicate config.
diag_->Error(DiagMessage(source_) << "duplicate configuration");
return {};
}
configValue->value = DeserializeValueFromPb(
pbConfigValue.value(), config, &table->string_pool);
configValue->value =
DeserializeValueFromPb(pbConfigValue.value(), config, &table->string_pool);
if (!configValue->value) {
return {};
}

View File

@@ -250,19 +250,16 @@ std::unique_ptr<pb::ResourceTable> SerializeTableToPb(ResourceTable* table) {
// Write the SymbolStatus struct.
pb::SymbolStatus* pb_status = pb_entry->mutable_symbol_status();
pb_status->set_visibility(
SerializeVisibilityToPb(entry->symbol_status.state));
SerializeSourceToPb(entry->symbol_status.source, &source_pool,
pb_status->mutable_source());
pb_status->set_visibility(SerializeVisibilityToPb(entry->symbol_status.state));
SerializeSourceToPb(entry->symbol_status.source, &source_pool, pb_status->mutable_source());
pb_status->set_comment(entry->symbol_status.comment);
pb_status->set_allow_new(entry->symbol_status.allow_new);
for (auto& config_value : entry->values) {
pb::ConfigValue* pb_config_value = pb_entry->add_config_values();
SerializeConfig(config_value->config,
pb_config_value->mutable_config());
SerializeConfig(config_value->config, pb_config_value->mutable_config());
if (!config_value->product.empty()) {
pb_config_value->mutable_config()->set_product(
config_value->product);
pb_config_value->mutable_config()->set_product(config_value->product);
}
pb::Value* pb_value = pb_config_value->mutable_value();

View File

@@ -28,12 +28,11 @@ TEST(TableProtoSerializer, SerializeSinglePackage) {
std::unique_ptr<ResourceTable> table =
test::ResourceTableBuilder()
.SetPackageId("com.app.a", 0x7f)
.AddFileReference("com.app.a:layout/main", ResourceId(0x7f020000),
"res/layout/main.xml")
.AddReference("com.app.a:layout/other", ResourceId(0x7f020001),
"com.app.a:layout/main")
.AddFileReference("com.app.a:layout/main", ResourceId(0x7f020000), "res/layout/main.xml")
.AddReference("com.app.a:layout/other", ResourceId(0x7f020001), "com.app.a:layout/main")
.AddString("com.app.a:string/text", {}, "hi")
.AddValue("com.app.a:id/foo", {}, util::make_unique<Id>())
.SetSymbolState("com.app.a:bool/foo", {}, SymbolState::kUndefined, true /*allow_new*/)
.Build();
Symbol public_symbol;
@@ -94,21 +93,23 @@ TEST(TableProtoSerializer, SerializeSinglePackage) {
EXPECT_EQ(SymbolState::kPublic, result.value().type->symbol_status.state);
EXPECT_EQ(SymbolState::kPublic, result.value().entry->symbol_status.state);
result = new_table->FindResource(test::ParseNameOrDie("com.app.a:bool/foo"));
ASSERT_TRUE(result);
EXPECT_EQ(SymbolState::kUndefined, result.value().entry->symbol_status.state);
EXPECT_TRUE(result.value().entry->symbol_status.allow_new);
// Find the product-dependent values
BinaryPrimitive* prim = test::GetValueForConfigAndProduct<BinaryPrimitive>(
new_table.get(), "com.app.a:integer/one", test::ParseConfigOrDie("land"),
"");
new_table.get(), "com.app.a:integer/one", test::ParseConfigOrDie("land"), "");
ASSERT_NE(nullptr, prim);
EXPECT_EQ(123u, prim->value.data);
prim = test::GetValueForConfigAndProduct<BinaryPrimitive>(
new_table.get(), "com.app.a:integer/one", test::ParseConfigOrDie("land"),
"tablet");
new_table.get(), "com.app.a:integer/one", test::ParseConfigOrDie("land"), "tablet");
ASSERT_NE(nullptr, prim);
EXPECT_EQ(321u, prim->value.data);
Reference* actual_ref =
test::GetValue<Reference>(new_table.get(), "com.app.a:layout/abc");
Reference* actual_ref = test::GetValue<Reference>(new_table.get(), "com.app.a:layout/abc");
ASSERT_NE(nullptr, actual_ref);
AAPT_ASSERT_TRUE(actual_ref->name);
AAPT_ASSERT_TRUE(actual_ref->id);

View File

@@ -3,6 +3,7 @@
## Version 2.17
### `aapt2 compile ...`
- Fixed an issue where symlinks would not be followed when compiling PNGs. (bug 62144459)
- Fixed issue where overlays that declared `<add-resource>` did not compile. (bug 38355988)
## Version 2.16
### `aapt2 link ...`

View File

@@ -117,12 +117,12 @@ class ResourceTableBuilder {
}
ResourceTableBuilder& SetSymbolState(const android::StringPiece& name, const ResourceId& id,
SymbolState state) {
SymbolState state, bool allow_new = false) {
ResourceName res_name = ParseNameOrDie(name);
Symbol symbol;
symbol.state = state;
CHECK(table_->SetSymbolStateAllowMangled(res_name, id, symbol,
&diagnostics_));
symbol.allow_new = allow_new;
CHECK(table_->SetSymbolStateAllowMangled(res_name, id, symbol, &diagnostics_));
return *this;
}