AAPT2: add flag for strict visibility
Will only detect whether a resource was defined as both 'public' and
'private' (but will allow overriding 'undefined' visiblity for now).
Test: TableMerger_test + manual
Bug: 72735798
Change-Id: If0749559c91c4d8820a6286fc9ddc80209c1e5e9
diff --git a/tools/aapt2/link/TableMerger.cpp b/tools/aapt2/link/TableMerger.cpp
index e819f51..91a55b3 100644
--- a/tools/aapt2/link/TableMerger.cpp
+++ b/tools/aapt2/link/TableMerger.cpp
@@ -102,7 +102,17 @@
}
static bool MergeEntry(IAaptContext* context, const Source& src, bool overlay,
- ResourceEntry* dst_entry, ResourceEntry* src_entry) {
+ ResourceEntry* dst_entry, ResourceEntry* src_entry,
+ bool strict_visibility) {
+ if (strict_visibility
+ && dst_entry->visibility.level != Visibility::Level::kUndefined
+ && src_entry->visibility.level != dst_entry->visibility.level) {
+ context->GetDiagnostics()->Error(
+ DiagMessage(src) << "cannot merge resource '" << dst_entry->name << "' with conflicting visibilities: "
+ << "public and private");
+ return false;
+ }
+
// Copy over the strongest visibility.
if (src_entry->visibility.level > dst_entry->visibility.level) {
// Only copy the ID if the source is public, or else the ID is meaningless.
@@ -234,7 +244,7 @@
continue;
}
- if (!MergeEntry(context_, src, overlay, dst_entry, src_entry.get())) {
+ if (!MergeEntry(context_, src, overlay, dst_entry, src_entry.get(), options_.strict_visibility)) {
error = true;
continue;
}
diff --git a/tools/aapt2/link/TableMerger.h b/tools/aapt2/link/TableMerger.h
index 47e23dd..24c5e13 100644
--- a/tools/aapt2/link/TableMerger.h
+++ b/tools/aapt2/link/TableMerger.h
@@ -35,6 +35,8 @@
struct TableMergerOptions {
// If true, resources in overlays can be added without previously having existed.
bool auto_add_overlay = false;
+ // If true, resource overlays with conflicting visibility are not allowed.
+ bool strict_visibility = false;
};
// TableMerger takes resource tables and merges all packages within the tables that have the same
diff --git a/tools/aapt2/link/TableMerger_test.cpp b/tools/aapt2/link/TableMerger_test.cpp
index 34461c6..cf504c4 100644
--- a/tools/aapt2/link/TableMerger_test.cpp
+++ b/tools/aapt2/link/TableMerger_test.cpp
@@ -241,6 +241,37 @@
ASSERT_FALSE(merger.Merge({}, overlay.get(), true /*overlay*/));
}
+TEST_F(TableMergerTest, FailConflictingVisibility) {
+ std::unique_ptr<ResourceTable> base =
+ test::ResourceTableBuilder()
+ .SetPackageId("", 0x7f)
+ .SetSymbolState("bool/foo", ResourceId(0x7f, 0x01, 0x0001), Visibility::Level::kPublic)
+ .Build();
+ std::unique_ptr<ResourceTable> overlay =
+ test::ResourceTableBuilder()
+ .SetPackageId("", 0x7f)
+ .SetSymbolState("bool/foo", ResourceId(0x7f, 0x01, 0x0001), Visibility::Level::kPrivate)
+ .Build();
+
+ // It should fail if the "--strict-visibility" flag is set.
+ ResourceTable final_table;
+ TableMergerOptions options;
+ options.auto_add_overlay = false;
+ options.strict_visibility = true;
+ TableMerger merger(context_.get(), &final_table, options);
+
+ ASSERT_TRUE(merger.Merge({}, base.get(), false /*overlay*/));
+ ASSERT_FALSE(merger.Merge({}, overlay.get(), true /*overlay*/));
+
+ // But it should still pass if the flag is not set.
+ ResourceTable final_table2;
+ options.strict_visibility = false;
+ TableMerger merger2(context_.get(), &final_table2, options);
+
+ ASSERT_TRUE(merger2.Merge({}, base.get(), false /*overlay*/));
+ ASSERT_TRUE(merger2.Merge({}, overlay.get(), true /*overlay*/));
+}
+
TEST_F(TableMergerTest, MergeAddResourceFromOverlay) {
std::unique_ptr<ResourceTable> table_a =
test::ResourceTableBuilder().SetPackageId("", 0x7f).Build();