Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 4cb4624f authored by Brandon Liu's avatar Brandon Liu Committed by branliu
Browse files

Fail framework build when staged type id overlap public type id 

Added more elegant error message to fail the build when staged type id
overlap public type id. And also added logic to handle the case when
staging type is assigned the same type with same id as public resource.

Bug: b/268793902
Test: Added and verified affected atests pass
Change-Id: Ic245d703bc34a8f7c2c4c266085a7a09c89918bf
parent 74527f0c

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;