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/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp
index 75eba9e..74edf6e 100644
--- a/tools/aapt2/cmd/Link.cpp
+++ b/tools/aapt2/cmd/Link.cpp
@@ -137,6 +137,9 @@
// In order to work around this limitation, we allow the use of traditionally reserved
// resource IDs [those between 0x02 and 0x7E].
bool allow_reserved_package_id = false;
+
+ // Whether we should fail on definitions of a resource with conflicting visibility.
+ bool strict_visibility = false;
};
class LinkContext : public IAaptContext {
@@ -1713,6 +1716,7 @@
TableMergerOptions table_merger_options;
table_merger_options.auto_add_overlay = options_.auto_add_overlay;
+ table_merger_options.strict_visibility = options_.strict_visibility;
table_merger_ = util::make_unique<TableMerger>(context_, &final_table_, table_merger_options);
if (context_->IsVerbose()) {
@@ -2209,7 +2213,10 @@
.OptionalSwitch("--debug-mode",
"Inserts android:debuggable=\"true\" in to the application node of the\n"
"manifest, making the application debuggable even on production devices.",
- &options.manifest_fixer_options.debug_mode);
+ &options.manifest_fixer_options.debug_mode)
+ .OptionalSwitch("--strict-visibility",
+ "Do not allow overlays with different visibility levels.",
+ &options.strict_visibility);
if (!flags.Parse("aapt2 link", args, &std::cerr)) {
return 1;
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();
