Merge "Fail framework build when staged type id overlap public type id" (2c7dc561) · Commits · e / os / android_frameworks_base

tools/aapt2/cmd/Link_test.cpp

+6 −6

Original line number	Diff line number	Diff line
		@@ -441,8 +441,8 @@ static void BuildNonFinalizedSDK(const std::string& apk_path, const std::string&
		R"(<resources>
		<public type="attr" name="finalized_res" id="0x01010001"/>

		<!-- S staged attributes (support staged resources in the same type id) -->
		<staging-public-group type="attr" first-id="0x01010050">
		<!-- S staged attributes (Not support staged resources in the same type id) -->
		<staging-public-group type="attr" first-id="0x01fc0050">
		<public name="staged_s_res" />
		</staging-public-group>

		@@ -480,8 +480,8 @@ static void BuildFinalizedSDK(const std::string& apk_path, const std::string& ja
		<public type="attr" name="staged_s2_res" id="0x01010003"/>
		<public type="string" name="staged_s_string" id="0x01020000"/>

		<!-- S staged attributes (support staged resources in the same type id) -->
		<staging-public-group-final type="attr" first-id="0x01010050">
		<!-- S staged attributes (Not support staged resources in the same type id) -->
		<staging-public-group-final type="attr" first-id="0x01fc0050">
		<public name="staged_s_res" />
		</staging-public-group-final>

		@@ -551,7 +551,7 @@ TEST_F(LinkTest, StagedAndroidApi) {
		EXPECT_THAT(android_r_contents, HasSubstr("public static final int finalized_res=0x01010001;"));
		EXPECT_THAT(
		android_r_contents,
		HasSubstr("public static final int staged_s_res; static { staged_s_res=0x01010050; }"));
		HasSubstr("public static final int staged_s_res; static { staged_s_res=0x01fc0050; }"));
		EXPECT_THAT(
		android_r_contents,
		HasSubstr("public static final int staged_s_string; static { staged_s_string=0x01fd0080; }"));
		@@ -583,7 +583,7 @@ TEST_F(LinkTest, StagedAndroidApi) {

		result = am.GetResourceId("android:attr/staged_s_res");
		ASSERT_TRUE(result.has_value());
		EXPECT_THAT(*result, Eq(0x01010050));
		EXPECT_THAT(*result, Eq(0x01fc0050));

		result = am.GetResourceId("android:string/staged_s_string");
		ASSERT_TRUE(result.has_value());

tools/aapt2/compile/IdAssigner.cpp

+35 −4

Original line number	Diff line number	Diff line
		@@ -37,6 +37,7 @@ using Result = expected<T, std::string>;

		template <typename Id, typename Key>
		struct NextIdFinder {
		std::map<Id, Key> pre_assigned_ids_;
		explicit NextIdFinder(Id start_id = 0u) : next_id_(start_id){};

		// Attempts to reserve an identifier for the specified key.
		@@ -55,7 +56,6 @@ struct NextIdFinder {
		Id next_id_;
		bool next_id_called_ = false;
		bool exhausted_ = false;
		std::map<Id, Key> pre_assigned_ids_;
		typename std::map<Id, Key>::iterator next_preassigned_id_;
		};

		@@ -158,7 +158,7 @@ bool IdAssigner::Consume(IAaptContext* context, ResourceTable* table) {
		}

		if (assigned_id_map_) {
		// Reserve all the IDs mentioned in the stable ID map. That way we won't assig IDs that were
		// Reserve all the IDs mentioned in the stable ID map. That way we won't assign IDs that were
		// listed in the map if they don't exist in the table.
		for (const auto& stable_id_entry : *assigned_id_map_) {
		const ResourceName& pre_assigned_name = stable_id_entry.first;
		@@ -191,6 +191,11 @@ bool IdAssigner::Consume(IAaptContext* context, ResourceTable* table) {
		}

		namespace {
		static const std::string_view staged_type_overlap_error =
		"Staged public resource type IDs have conflict with non staged public resources type "
		"IDs, please restart staged resource type ID assignment at 0xff in public-staging.xml "
		"and also delete all the overlapping groups in public-final.xml";

		template <typename Id, typename Key>
		Result<Id> NextIdFinder<Id, Key>::ReserveId(Key key, Id id) {
		CHECK(!next_id_called_) << "ReserveId cannot be called after NextId";
		@@ -282,8 +287,20 @@ bool IdAssignerContext::ReserveId(const ResourceName& name, ResourceId id,
		// another type.
		auto assign_result = type_id_finder_.ReserveId(key, id.type_id());
		if (!assign_result.has_value()) {
		diag->Error(android::DiagMessage() << "can't assign ID " << id << " to resource " << name
		<< " because type " << assign_result.error());
		auto pre_assigned_type = type_id_finder_.pre_assigned_ids_[id.type_id()].type;
		bool pre_assigned_type_staged =
		non_staged_type_ids_.find(pre_assigned_type) == non_staged_type_ids_.end();
		auto hex_type_id = fmt::format("{:#04x}", (int)id.type_id());
		bool current_type_staged = visibility.staged_api;
		diag->Error(android::DiagMessage()
		<< "can't assign type ID " << hex_type_id << " to "
		<< (current_type_staged ? "staged type " : "non staged type ") << name.type.type
		<< " because this type ID have been assigned to "
		<< (pre_assigned_type_staged ? "staged type " : "non staged type ")
		<< pre_assigned_type);
		if (pre_assigned_type_staged \|\| current_type_staged) {
		diag->Error(android::DiagMessage() << staged_type_overlap_error);
		}
		return false;
		}
		type = types_.emplace(key, TypeGroup(package_id_, id.type_id())).first;
		@@ -298,6 +315,20 @@ bool IdAssignerContext::ReserveId(const ResourceName& name, ResourceId id,
		<< " because type already has ID " << std::hex << (int)id.type_id());
		return false;
		}
		} else {
		// Ensure that staged public resources cannot have the same type name and type id with
		// non staged public resources.
		auto non_staged_type = non_staged_type_ids_.find(name.type.type);
		if (non_staged_type != non_staged_type_ids_.end() && non_staged_type->second == id.type_id()) {
		diag->Error(
		android::DiagMessage()
		<< "can`t assign type ID " << fmt::format("{:#04x}", (int)id.type_id())
		<< " to staged type " << name.type.type << " because type ID "
		<< fmt::format("{:#04x}", (int)id.type_id())
		<< " already has been assigned to a non staged resource type with the same type name");
		diag->Error(android::DiagMessage() << staged_type_overlap_error);
		return false;
		}
		}

		auto assign_result = type->second.ReserveId(name, id);

tools/aapt2/compile/IdAssigner_test.cpp

+22 −8

Original line number	Diff line number	Diff line
		@@ -117,12 +117,26 @@ TEST_F(IdAssignerTests, FailWhenTypeHasTwoNonStagedIds) {
		}

		TEST_F(IdAssignerTests, FailWhenTypeHasTwoNonStagedIdsRegardlessOfStagedId) {
		auto table = test::ResourceTableBuilder()
		auto table =
		test::ResourceTableBuilder()
		.AddSimple("android:attr/foo", ResourceId(0x01050000))
		.AddSimple("android:attr/bar", ResourceId(0x01ff0006))
		.Add(NewResourceBuilder("android:attr/staged_baz")
		.SetId(0x01ff0000)
		.SetVisibility({.staged_api = true})
		.SetVisibility({.staged_api = true, .level = Visibility::Level::kPublic})
		.Build())
		.Build();
		IdAssigner assigner;
		ASSERT_FALSE(assigner.Consume(context.get(), table.get()));
		}

		TEST_F(IdAssignerTests, FailWhenTypeHaveBothStagedAndNonStagedIds) {
		auto table =
		test::ResourceTableBuilder()
		.AddSimple("android:attr/foo", ResourceId(0x01010000))
		.Add(NewResourceBuilder("android:bool/staged_baz")
		.SetId(0x01010001)
		.SetVisibility({.staged_api = true, .level = Visibility::Level::kPublic})
		.Build())
		.Build();
		IdAssigner assigner;