summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--tools/aapt2/Format.proto1
-rw-r--r--tools/aapt2/ResourceParser.cpp10
-rw-r--r--tools/aapt2/ResourceParser_test.cpp4
-rw-r--r--tools/aapt2/ResourceTable.cpp16
-rw-r--r--tools/aapt2/ResourceTable.h7
-rw-r--r--tools/aapt2/link/TableMerger.cpp22
-rw-r--r--tools/aapt2/link/TableMerger_test.cpp10
-rw-r--r--tools/aapt2/proto/TableProtoDeserializer.cpp27
-rw-r--r--tools/aapt2/proto/TableProtoSerializer.cpp13
-rw-r--r--tools/aapt2/proto/TableProtoSerializer_test.cpp21
-rw-r--r--tools/aapt2/readme.md1
-rw-r--r--tools/aapt2/test/Builders.h6
12 files changed, 70 insertions, 68 deletions
diff --git a/tools/aapt2/Format.proto b/tools/aapt2/Format.proto
index 0917129ba90d..870b735f70a7 100644
--- a/tools/aapt2/Format.proto
+++ b/tools/aapt2/Format.proto
@@ -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 {
diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp
index 0d1850fed26a..57ae2700127c 100644
--- a/tools/aapt2/ResourceParser.cpp
+++ b/tools/aapt2/ResourceParser.cpp
@@ -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;
diff --git a/tools/aapt2/ResourceParser_test.cpp b/tools/aapt2/ResourceParser_test.cpp
index faa660729237..e3abde6bef29 100644
--- a/tools/aapt2/ResourceParser_test.cpp
+++ b/tools/aapt2/ResourceParser_test.cpp
@@ -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) {
diff --git a/tools/aapt2/ResourceTable.cpp b/tools/aapt2/ResourceTable.cpp
index 1947628e9267..168004f0b721 100644
--- a/tools/aapt2/ResourceTable.cpp
+++ b/tools/aapt2/ResourceTable.cpp
@@ -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;
}
diff --git a/tools/aapt2/ResourceTable.h b/tools/aapt2/ResourceTable.h
index b0321214c6cc..4295d0674774 100644
--- a/tools/aapt2/ResourceTable.h
+++ b/tools/aapt2/ResourceTable.h
@@ -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 {
diff --git a/tools/aapt2/link/TableMerger.cpp b/tools/aapt2/link/TableMerger.cpp
index 931109125263..cce750acc2d6 100644
--- a/tools/aapt2/link/TableMerger.cpp
+++ b/tools/aapt2/link/TableMerger.cpp
@@ -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;
}
diff --git a/tools/aapt2/link/TableMerger_test.cpp b/tools/aapt2/link/TableMerger_test.cpp
index 742f5a7a570a..147d857e1447 100644
--- a/tools/aapt2/link/TableMerger_test.cpp
+++ b/tools/aapt2/link/TableMerger_test.cpp
@@ -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()));
diff --git a/tools/aapt2/proto/TableProtoDeserializer.cpp b/tools/aapt2/proto/TableProtoDeserializer.cpp
index d93d495b106f..1d0041b2ac5a 100644
--- a/tools/aapt2/proto/TableProtoDeserializer.cpp
+++ b/tools/aapt2/proto/TableProtoDeserializer.cpp
@@ -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 {};
}
diff --git a/tools/aapt2/proto/TableProtoSerializer.cpp b/tools/aapt2/proto/TableProtoSerializer.cpp
index 7230b55e10e3..de10bb80bf7e 100644
--- a/tools/aapt2/proto/TableProtoSerializer.cpp
+++ b/tools/aapt2/proto/TableProtoSerializer.cpp
@@ -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();
diff --git a/tools/aapt2/proto/TableProtoSerializer_test.cpp b/tools/aapt2/proto/TableProtoSerializer_test.cpp
index fdd5197167ef..e6ce6d37b879 100644
--- a/tools/aapt2/proto/TableProtoSerializer_test.cpp
+++ b/tools/aapt2/proto/TableProtoSerializer_test.cpp
@@ -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);
diff --git a/tools/aapt2/readme.md b/tools/aapt2/readme.md
index ac09b5933ef6..0290e30dfced 100644
--- a/tools/aapt2/readme.md
+++ b/tools/aapt2/readme.md
@@ -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 ...`
diff --git a/tools/aapt2/test/Builders.h b/tools/aapt2/test/Builders.h
index 6888cf32fab8..02acedbd8ac5 100644
--- a/tools/aapt2/test/Builders.h
+++ b/tools/aapt2/test/Builders.h
@@ -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;
}
generated by cgit v1.2.3-59-g8ed1b (git 2.39.0) at 2025-10-21 14:12:58 +0000