Merge changes from topic "revert-3040304-IYDQWEWFVW" into main

* changes:
  Revert "Dynamically combine dynamic partitions and groups"
  Revert "Extend misc_info.txt merging logic"

3 files changed, 89 insertions, 221 deletions

diff --git a/host/commands/assemble_cvd/misc_info.cc b/host/commands/assemble_cvd/misc_info.cc
index 834cf7389..292258b96 100644
--- a/host/commands/assemble_cvd/misc_info.cc
+++ b/host/commands/assemble_cvd/misc_info.cc
@@ -15,20 +15,14 @@
 
 #include "misc_info.h"
 
-#include <array>
-#include <memory>
-#include <set>
+#include <algorithm>
 #include <string>
-#include <unordered_set>
 #include <vector>
 
 #include <android-base/logging.h>
-#include <android-base/parseint.h>
+#include <android-base/stringprintf.h>
 #include <android-base/strings.h>
-#include <fmt/format.h>
 
-#include "common/libs/fs/shared_buf.h"
-#include "common/libs/fs/shared_fd.h"
 #include "common/libs/utils/contains.h"
 #include "common/libs/utils/result.h"
 
@@ -37,75 +31,7 @@ namespace {
 
 constexpr char kDynamicPartitions[] = "dynamic_partition_list";
 constexpr char kGoogleDynamicPartitions[] = "google_dynamic_partitions";
-constexpr char kRollbackIndexSuffix[] = "_rollback_index_location";
-constexpr char kSuperBlockDevices[] = "super_block_devices";
 constexpr char kSuperPartitionGroups[] = "super_partition_groups";
-constexpr char kUseDynamicPartitions[] = "use_dynamic_partitions";
-constexpr std::array kNonPartitionKeysToMerge = {
-    "ab_update", "default_system_dev_certificate"};
-
-Result<std::string> GetExpected(const MiscInfo& misc_info,
-                                const std::string& key) {
-  auto lookup = misc_info.find(key);
-  CF_EXPECTF(lookup != misc_info.end(),
-             "Unable to retrieve expected value from key: {}", key);
-  return lookup->second;
-}
-
-std::string GetOrDefault(const MiscInfo& misc_info, const std::string& key,
-                         const std::string& default_value) {
-  auto result = GetExpected(misc_info, key);
-  return result.ok() ? *result : default_value;
-}
-
-std::string MergePartitionLists(const std::string& vendor,
-                                const std::string& system) {
-  const std::string full_string = fmt::format("{} {}", vendor, system);
-  auto full_list = android::base::Tokenize(full_string, " ");
-  // std::set removes duplicates and orders the elements, which we want
-  std::set<std::string> full_set(full_list.begin(), full_list.end());
-  return android::base::Join(full_set, " ");
-}
-
-std::string GetPartitionList(const MiscInfo& vendor_info,
-                             const MiscInfo& system_info,
-                             const std::string& key) {
-  std::string vendor_list = GetOrDefault(vendor_info, key, "");
-  std::string system_list = GetOrDefault(system_info, key, "");
-  return MergePartitionLists(vendor_list, system_list);
-}
-
-std::vector<std::string> GeneratePartitionKeys(const std::string& name) {
-  std::vector<std::string> result;
-  result.emplace_back("avb_" + name);
-  result.emplace_back("avb_" + name + "_algorithm");
-  result.emplace_back("avb_" + name + "_key_path");
-  result.emplace_back("avb_" + name + kRollbackIndexSuffix);
-  result.emplace_back("avb_" + name + "_hashtree_enable");
-  result.emplace_back("avb_" + name + "_add_hashtree_footer_args");
-  result.emplace_back(name + "_disable_sparse");
-  result.emplace_back("building_" + name + "_image");
-  auto fs_type_key = name + "_fs_type";
-  if (name == "system") {
-    fs_type_key = "fs_type";
-  }
-  result.emplace_back(fs_type_key);
-  return result;
-}
-
-Result<int> ResolveRollbackIndexConflicts(
-    const std::string& index_string,
-    const std::unordered_set<int> used_indices) {
-  int index;
-  CF_EXPECTF(android::base::ParseInt(index_string, &index),
-             "Unable to parse value {} to string.  Maybe a wrong or bad value "
-             "read for the rollback index?",
-             index_string);
-  while (Contains(used_indices, index)) {
-    ++index;
-  }
-  return index;
-}
 
 }  // namespace
 
@@ -135,112 +61,73 @@ Result<MiscInfo> ParseMiscInfo(const std::string& misc_info_contents) {
   return misc_info;
 }
 
-Result<void> WriteMiscInfo(const MiscInfo& misc_info,
-                           const std::string& output_path) {
-  std::stringstream file_content;
+std::string WriteMiscInfo(const MiscInfo& misc_info) {
+  std::stringstream out;
   for (const auto& entry : misc_info) {
-    file_content << entry.first << "=" << entry.second << "\n";
+    out << entry.first << "=" << entry.second << "\n";
   }
-
-  SharedFD output_file = SharedFD::Creat(output_path.c_str(), 0644);
-  CF_EXPECT(output_file->IsOpen(),
-            "Failed to open output misc file: " << output_file->StrError());
-
-  CF_EXPECT(
-      WriteAll(output_file, file_content.str()) >= 0,
-      "Failed to write output misc file contents: " << output_file->StrError());
-  return {};
+  return out.str();
 }
 
-// based on build/make/tools/releasetools/merge/merge_target_files.py
-Result<MiscInfo> GetCombinedDynamicPartitions(const MiscInfo& vendor_info,
-                                              const MiscInfo& system_info) {
-  CF_EXPECTF(CF_EXPECT(GetExpected(vendor_info, kDynamicPartitions)) == "true",
-             "Vendor build must have {}=true", kUseDynamicPartitions);
-  CF_EXPECTF(CF_EXPECT(GetExpected(system_info, kDynamicPartitions)) == "true",
-             "System build must have {}=true", kUseDynamicPartitions);
-  MiscInfo result;
-  // copy where both files are equal
-  for (const auto& key_val : vendor_info) {
-    const auto value_result = GetExpected(system_info, key_val.first);
-    if (value_result.ok() && *value_result == key_val.second) {
-      result[key_val.first] = key_val.second;
-    }
-  }
-
-  result[kDynamicPartitions] =
-      GetPartitionList(vendor_info, system_info, kDynamicPartitions);
-
-  const auto block_devices_result =
-      GetExpected(vendor_info, kSuperBlockDevices);
-  if (block_devices_result.ok()) {
-    result[kSuperBlockDevices] = *block_devices_result;
-    for (const auto& block_device :
-         android::base::Tokenize(result[kSuperBlockDevices], " ")) {
-      const auto key = "super_" + block_device + "_device_size";
-      result[key] = CF_EXPECT(GetExpected(vendor_info, key));
-    }
+std::vector<std::string> SuperPartitionComponents(const MiscInfo& info) {
+  auto value_it = info.find(kDynamicPartitions);
+  if (value_it == info.end()) {
+    return {};
   }
-
-  result[kSuperPartitionGroups] =
-      CF_EXPECT(GetExpected(vendor_info, kSuperPartitionGroups));
-  for (const auto& group :
-       android::base::Tokenize(result[kSuperPartitionGroups], " ")) {
-    const auto group_size_key = "super_" + group + "_group_size";
-    result[group_size_key] =
-        CF_EXPECT(GetExpected(vendor_info, group_size_key));
-
-    const auto partition_list_key = "super_" + group + "_partition_list";
-    result[partition_list_key] =
-        GetPartitionList(vendor_info, system_info, partition_list_key);
+  auto components = android::base::Split(value_it->second, " ");
+  for (auto& component : components) {
+    component = android::base::Trim(component);
   }
+  components.erase(std::remove(components.begin(), components.end(), ""),
+                   components.end());
+  return components;
+}
 
-  // TODO(chadreynolds): add vabc_cow_version logic if we need to support older
-  // builds
-  for (const auto& key :
-       {"virtual_ab", "virtual_ab_retrofit", "lpmake", "super_metadata_device",
-        "super_partition_error_limit", "super_partition_size"}) {
-    const auto value_result = GetExpected(vendor_info, key);
-    if (value_result.ok()) {
-      result[key] = *value_result;
-    }
+bool SetSuperPartitionComponents(const std::vector<std::string>& components,
+                                 MiscInfo* misc_info) {
+  auto super_partition_groups = misc_info->find(kSuperPartitionGroups);
+  if (super_partition_groups == misc_info->end()) {
+    LOG(ERROR) << "Failed to find super partition groups in misc_info";
+    return false;
   }
-  return std::move(result);
-}
 
-Result<MiscInfo> MergeMiscInfos(
-    const MiscInfo& vendor_info, const MiscInfo& system_info,
-    const MiscInfo& combined_dp_info,
-    const std::vector<std::string>& system_partitions) {
-  // the combined misc info uses the vendor values as defaults
-  MiscInfo result = vendor_info;
-  std::unordered_set<int> used_indices;
-  for (const auto& partition : system_partitions) {
-    for (const auto& key : GeneratePartitionKeys(partition)) {
-      if (!Contains(system_info, key)) {
-        continue;
-      }
-      auto system_value = system_info.find(key)->second;
-      // avb_<partition>_rollback_index_location values can conflict across
-      // different builds
-      if (android::base::EndsWith(key, kRollbackIndexSuffix)) {
-        const auto index = CF_EXPECT(
-            ResolveRollbackIndexConflicts(system_value, used_indices));
-        used_indices.insert(index);
-        system_value = std::to_string(index);
+  // Remove all existing update groups in misc_info
+  auto update_groups =
+      android::base::Split(super_partition_groups->second, " ");
+  for (const auto& group_name : update_groups) {
+    auto partition_list = android::base::StringPrintf("super_%s_partition_list",
+                                                      group_name.c_str());
+    auto partition_size =
+        android::base::StringPrintf("super_%s_group_size", group_name.c_str());
+    for (const auto& key : {partition_list, partition_size}) {
+      auto it = misc_info->find(key);
+      if (it == misc_info->end()) {
+        LOG(ERROR) << "Failed to find " << key << " in misc_info";
+        return false;
       }
-      result[key] = system_value;
+      misc_info->erase(it);
     }
   }
-  for (const auto& key : kNonPartitionKeysToMerge) {
-    if (Contains(system_info, key)) {
-      result[key] = system_info.find(key)->second;
-    }
-  }
-  for (const auto& key_val : combined_dp_info) {
-    result[key_val.first] = key_val.second;
+
+  // For merged target-file, put all dynamic partitions under the
+  // google_dynamic_partitions update group.
+  // TODO(xunchang) use different update groups for system and vendor images.
+  (*misc_info)[kDynamicPartitions] = android::base::Join(components, " ");
+  (*misc_info)[kSuperPartitionGroups] = kGoogleDynamicPartitions;
+  std::string partitions_list_key = android::base::StringPrintf(
+      "super_%s_partition_list", kGoogleDynamicPartitions);
+  (*misc_info)[partitions_list_key] = android::base::Join(components, " ");
+
+  // Use the entire super partition as the group size
+  std::string group_size_key = android::base::StringPrintf(
+      "super_%s_group_size", kGoogleDynamicPartitions);
+  auto super_size_it = misc_info->find("super_partition_size");
+  if (super_size_it == misc_info->end()) {
+    LOG(ERROR) << "Failed to find super partition size";
+    return false;
   }
-  return std::move(result);
+  (*misc_info)[group_size_key] = super_size_it->second;
+  return true;
 }
 
 } // namespace cuttlefish
diff --git a/host/commands/assemble_cvd/misc_info.h b/host/commands/assemble_cvd/misc_info.h
index 9b018bd43..f832a4c5f 100644
--- a/host/commands/assemble_cvd/misc_info.h
+++ b/host/commands/assemble_cvd/misc_info.h
@@ -23,18 +23,13 @@
 
 namespace cuttlefish {
 
-// TODO(chadreynolds): rename MiscInfo to more generic KeyValueFile since this
-// logic is processing multiple filetypes now
 using MiscInfo = std::map<std::string, std::string>;
 
 Result<MiscInfo> ParseMiscInfo(const std::string& file_contents);
-Result<void> WriteMiscInfo(const MiscInfo& misc_info,
-                           const std::string& output_path);
-Result<MiscInfo> GetCombinedDynamicPartitions(const MiscInfo& vendor_info,
-                                              const MiscInfo& system_info);
-Result<MiscInfo> MergeMiscInfos(
-    const MiscInfo& vendor_info, const MiscInfo& system_info,
-    const MiscInfo& combined_dp_info,
-    const std::vector<std::string>& system_partitions);
+std::string WriteMiscInfo(const MiscInfo& info);
+
+std::vector<std::string> SuperPartitionComponents(const MiscInfo&);
+bool SetSuperPartitionComponents(const std::vector<std::string>& components,
+                                 MiscInfo* misc_info);
 
 } // namespace cuttlefish
diff --git a/host/commands/assemble_cvd/super_image_mixer.cc b/host/commands/assemble_cvd/super_image_mixer.cc
index 0cf46707d..b59d163fb 100644
--- a/host/commands/assemble_cvd/super_image_mixer.cc
+++ b/host/commands/assemble_cvd/super_image_mixer.cc
@@ -22,11 +22,11 @@
 #include <memory>
 #include <string>
 #include <unordered_set>
-#include <vector>
 
 #include <android-base/strings.h>
 #include <android-base/logging.h>
 
+#include "common/libs/fs/shared_buf.h"
 #include "common/libs/utils/archive.h"
 #include "common/libs/utils/contains.h"
 #include "common/libs/utils/files.h"
@@ -41,7 +41,6 @@ namespace cuttlefish {
 namespace {
 
 constexpr char kMiscInfoPath[] = "META/misc_info.txt";
-constexpr char kDynamicPartitionsPath[] = "META/dynamic_partitions_info.txt";
 constexpr std::array kVendorTargetImages = {
     "IMAGES/boot.img",
     "IMAGES/dtbo.img",
@@ -93,11 +92,6 @@ bool IsTargetFilesBuildProp(const std::string& filename) {
   return android::base::EndsWith(filename, "build.prop");
 }
 
-std::string GetPartitionNameFromPath(const std::string& path) {
-  // "IMAGES/<name>.img" -> "<name>"
-  return path.substr(7, path.length() - 11);
-}
-
 Result<TargetFiles> GetTargetFiles(const std::string& vendor_zip_path,
                                    const std::string& system_zip_path) {
   auto result = TargetFiles{
@@ -113,47 +107,42 @@ Result<TargetFiles> GetTargetFiles(const std::string& vendor_zip_path,
   return result;
 }
 
-Result<MiscInfo> CombineDynamicPartitionsInfo(TargetFiles& target_files) {
-  CF_EXPECTF(Contains(target_files.vendor_contents, kDynamicPartitionsPath),
-             "Vendor target files zip does not contain {}",
-             kDynamicPartitionsPath);
-  CF_EXPECTF(Contains(target_files.system_contents, kDynamicPartitionsPath),
-             "System target files zip does not contain {}",
-             kDynamicPartitionsPath);
-
-  const MiscInfo vendor_dp_info = CF_EXPECT(ParseMiscInfo(
-      target_files.vendor_zip.ExtractToMemory(kDynamicPartitionsPath)));
-  const MiscInfo system_dp_info = CF_EXPECT(ParseMiscInfo(
-      target_files.system_zip.ExtractToMemory(kDynamicPartitionsPath)));
-
-  return CF_EXPECT(
-      GetCombinedDynamicPartitions(vendor_dp_info, system_dp_info));
-}
-
-Result<void> CombineMiscInfo(
-    TargetFiles& target_files, const std::string& misc_output_path,
-    const std::vector<std::string>& system_partitions) {
+Result<void> CombineMiscInfo(TargetFiles& target_files,
+                             const std::string& misc_output_path) {
   CF_EXPECTF(Contains(target_files.vendor_contents, kMiscInfoPath),
-             "Vendor target files zip does not contain {}", kMiscInfoPath);
+             "Default target files zip does not contain {}", kMiscInfoPath);
   CF_EXPECTF(Contains(target_files.system_contents, kMiscInfoPath),
              "System target files zip does not contain {}", kMiscInfoPath);
 
-  const MiscInfo vendor_misc = CF_EXPECT(
+  const MiscInfo default_misc = CF_EXPECT(
       ParseMiscInfo(target_files.vendor_zip.ExtractToMemory(kMiscInfoPath)));
   const MiscInfo system_misc = CF_EXPECT(
       ParseMiscInfo(target_files.system_zip.ExtractToMemory(kMiscInfoPath)));
 
-  const auto combined_dp_info =
-      CF_EXPECT(CombineDynamicPartitionsInfo(target_files));
-  const auto output_misc = CF_EXPECT(MergeMiscInfos(
-      vendor_misc, system_misc, combined_dp_info, system_partitions));
+  auto output_misc = default_misc;
+  auto system_super_partitions = SuperPartitionComponents(system_misc);
+  // Ensure specific skipped partitions end up in the misc_info.txt
+  for (auto partition :
+       {"odm", "odm_dlkm", "vendor", "vendor_dlkm", "system_dlkm"}) {
+    if (!Contains(system_super_partitions, partition)) {
+      system_super_partitions.push_back(partition);
+    }
+  }
+  CF_EXPECT(SetSuperPartitionComponents(system_super_partitions, &output_misc),
+            "Failed to update super partitions components for misc_info");
 
-  CF_EXPECT(WriteMiscInfo(output_misc, misc_output_path));
+  SharedFD misc_output_file = SharedFD::Creat(misc_output_path.c_str(), 0644);
+  CF_EXPECT(misc_output_file->IsOpen(), "Failed to open output misc file: "
+                                            << misc_output_file->StrError());
+
+  CF_EXPECT(WriteAll(misc_output_file, WriteMiscInfo(output_misc)) >= 0,
+            "Failed to write output misc file contents: "
+                << misc_output_file->StrError());
   return {};
 }
 
-Result<std::vector<std::string>> ExtractTargetFiles(
-    TargetFiles& target_files, const std::string& combined_output_path) {
+Result<void> ExtractTargetFiles(TargetFiles& target_files,
+                                const std::string& combined_output_path) {
   for (const auto& name : target_files.vendor_contents) {
     if (!IsTargetFilesImage(name)) {
       continue;
@@ -178,7 +167,6 @@ Result<std::vector<std::string>> ExtractTargetFiles(
         "Failed to extract " << name << " from the vendor target zip");
   }
 
-  std::vector<std::string> system_partitions;
   for (const auto& name : target_files.system_contents) {
     if (!IsTargetFilesImage(name)) {
       continue;
@@ -189,7 +177,6 @@ Result<std::vector<std::string>> ExtractTargetFiles(
     CF_EXPECT(
         target_files.system_zip.ExtractFiles({name}, combined_output_path),
         "Failed to extract " << name << " from the system target zip");
-    system_partitions.emplace_back(GetPartitionNameFromPath(name));
   }
   for (const auto& name : target_files.system_contents) {
     if (!IsTargetFilesBuildProp(name)) {
@@ -203,7 +190,7 @@ Result<std::vector<std::string>> ExtractTargetFiles(
         target_files.system_zip.ExtractFiles({name}, combined_output_path),
         "Failed to extract " << name << " from the system target zip");
   }
-  return std::move(system_partitions);
+  return {};
 }
 
 Result<void> CombineTargetZipFiles(const std::string& vendor_zip_path,
@@ -213,10 +200,9 @@ Result<void> CombineTargetZipFiles(const std::string& vendor_zip_path,
   CF_EXPECT(EnsureDirectoryExists(output_path + "/META"));
   auto target_files =
       CF_EXPECT(GetTargetFiles(vendor_zip_path, system_zip_path));
-  const auto system_partitions =
-      CF_EXPECT(ExtractTargetFiles(target_files, output_path));
+  CF_EXPECT(ExtractTargetFiles(target_files, output_path));
   const auto misc_output_path = output_path + "/" + kMiscInfoPath;
-  CF_EXPECT(CombineMiscInfo(target_files, misc_output_path, system_partitions));
+  CF_EXPECT(CombineMiscInfo(target_files, misc_output_path));
   return {};
 }