Formatting and general cleanup of merge_target_files.

Also adds a new validation that IMAGES/<partition>.img must come from
the same build that provides <PARTITION>/*.

Bug: 171431774
Test: python3 -m unittest test_merge_target_files
Test: Use merge_target_files to merge a build.
Change-Id: Ia1f3f2e65a0ca90962216fb0c5cdd24c4c150cae

tools/releasetools/merge_target_files.py

@@ -16,11 +16,15 @@
 #
 """This script merges two partial target files packages.
 
-One package contains framework files, and the other contains vendor files.
+One input package contains framework files, and the other contains vendor files.
-It produces a complete target files package that can be used to generate an
-OTA package.
 
-Usage: merge_target_files.py [args]
+This script produces a complete, merged target files package:
+  - This package can be used to generate a flashable IMG package.
+    See --output-img.
+  - This package can be used to generate an OTA package. See --output-ota.
+  - The merged package is checked for compatibility between the two inputs.
+
+Usage: merge_target_files [args]
 
   --framework-target-files framework-target-files-zip-archive
       The input target files package containing framework bits. This is a zip
@@ -155,16 +159,9 @@ DEFAULT_FRAMEWORK_ITEM_LIST = (
     'SYSTEM/*',
 )
 
-# FRAMEWORK_EXTRACT_SPECIAL_ITEM_LIST is a list of items to extract from the
-# partial framework target files package that need some special processing, such
-# as some sort of combination with items from the partial vendor target files
-# package.
-
-FRAMEWORK_EXTRACT_SPECIAL_ITEM_LIST = ('META/*',)
-
 # DEFAULT_FRAMEWORK_MISC_INFO_KEYS is a list of keys to obtain from the
-# framework instance of META/misc_info.txt. The remaining keys from the
+# framework instance of META/misc_info.txt. The remaining keys should come
-# vendor instance.
+# from the vendor instance.
 
 DEFAULT_FRAMEWORK_MISC_INFO_KEYS = (
     'avb_system_hashtree_enable',
@@ -205,13 +202,6 @@ DEFAULT_VENDOR_ITEM_LIST = (
     'VENDOR/*',
 )
 
-# VENDOR_EXTRACT_SPECIAL_ITEM_LIST is a list of items to extract from the
-# partial vendor target files package that need some special processing, such as
-# some sort of combination with items from the partial framework target files
-# package.
-
-VENDOR_EXTRACT_SPECIAL_ITEM_LIST = ('META/*',)
-
 # The merge config lists should not attempt to extract items from both
 # builds for any of the following partitions. The partitions in
 # SINGLE_BUILD_PARTITIONS should come entirely from a single build (either
@@ -320,8 +310,8 @@ def validate_config_lists(framework_item_list, framework_misc_info_keys,
     framework_item_list: The list of items to extract from the partial framework
       target files package as is.
     framework_misc_info_keys: A list of keys to obtain from the framework
-      instance of META/misc_info.txt. The remaining keys from the vendor
+      instance of META/misc_info.txt. The remaining keys should come from the
-      instance.
+      vendor instance.
     vendor_item_list: The list of items to extract from the partial vendor
       target files package as is.
 
@@ -346,10 +336,15 @@ def validate_config_lists(framework_item_list, framework_misc_info_keys,
                  'this script.')
     has_error = True
 
+  # Check that partitions only come from one input.
   for partition in SINGLE_BUILD_PARTITIONS:
-    in_framework = any(
+    image_path = 'IMAGES/{}.img'.format(partition.lower().replace('/', ''))
-        item.startswith(partition) for item in framework_item_list)
+    in_framework = (
-    in_vendor = any(item.startswith(partition) for item in vendor_item_list)
+        any(item.startswith(partition) for item in framework_item_list) or
+        image_path in framework_item_list)
+    in_vendor = (
+        any(item.startswith(partition) for item in vendor_item_list) or
+        image_path in vendor_item_list)
     if in_framework and in_vendor:
       logger.error(
           'Cannot extract items from %s for both the framework and vendor'
@@ -375,8 +370,8 @@ def process_ab_partitions_txt(framework_target_files_temp_dir,
   framework directory and the vendor directory, placing the merged result in the
   output directory. The precondition in that the files are already extracted.
   The post condition is that the output META/ab_partitions.txt contains the
-  merged content. The format for each ab_partitions.txt a one partition name per
+  merged content. The format for each ab_partitions.txt is one partition name
-  line. The output file contains the union of the parition names.
+  per line. The output file contains the union of the partition names.
 
   Args:
     framework_target_files_temp_dir: The name of a directory containing the
@@ -429,8 +424,8 @@ def process_misc_info_txt(framework_target_files_temp_dir,
       create the output target files package after all the special cases are
       processed.
     framework_misc_info_keys: A list of keys to obtain from the framework
-      instance of META/misc_info.txt. The remaining keys from the vendor
+      instance of META/misc_info.txt. The remaining keys should come from the
-      instance.
+      vendor instance.
   """
 
   misc_info_path = ['META', 'misc_info.txt']
@@ -674,8 +669,8 @@ def process_special_cases(framework_target_files_temp_dir,
       create the output target files package after all the special cases are
       processed.
     framework_misc_info_keys: A list of keys to obtain from the framework
-      instance of META/misc_info.txt. The remaining keys from the vendor
+      instance of META/misc_info.txt. The remaining keys should come from the
-      instance.
+      vendor instance.
     framework_partition_set: Partitions that are considered framework
       partitions. Used to filter apexkeys.txt and apkcerts.txt.
     vendor_partition_set: Partitions that are considered vendor partitions. Used
@@ -721,26 +716,6 @@ def process_special_cases(framework_target_files_temp_dir,
       file_name='apexkeys.txt')
 
 
-def files_from_path(target_path, extra_args=None):
-  """Gets files under given path.
-
-  Get (sub)files from given target path and return sorted list.
-
-  Args:
-    target_path: Target path to get subfiles.
-    extra_args: List of extra argument for find command. Optional.
-
-  Returns:
-    Sorted files and directories list.
-  """
-
-  find_command = ['find', target_path] + (extra_args or [])
-  find_process = common.Run(find_command, stdout=subprocess.PIPE, verbose=False)
-  return common.RunAndCheckOutput(['sort'],
-                                  stdin=find_process.stdout,
-                                  verbose=False)
-
-
 def create_merged_package(temp_dir, framework_target_files, framework_item_list,
                           vendor_target_files, vendor_item_list,
                           framework_misc_info_keys, rebuild_recovery):
@@ -762,64 +737,42 @@ def create_merged_package(temp_dir, framework_target_files, framework_item_list,
       target files package as is, meaning these items will land in the output
       target files package exactly as they appear in the input partial vendor
       target files package.
-    framework_misc_info_keys: The list of keys to obtain from the framework
+    framework_misc_info_keys: A list of keys to obtain from the framework
-      instance of META/misc_info.txt. The remaining keys from the vendor
+      instance of META/misc_info.txt. The remaining keys should come from the
-      instance.
+      vendor instance.
     rebuild_recovery: If true, rebuild the recovery patch used by non-A/B
       devices and write it to the system image.
 
   Returns:
     Path to merged package under temp directory.
   """
+  # Extract "as is" items from the input framework and vendor partial target
+  # files packages directly into the output temporary directory, since these items
+  # do not need special case processing.
 
-  # Create directory names that we'll use when we extract files from framework,
-  # and vendor, and for zipping the final output.
-
-  framework_target_files_temp_dir = os.path.join(temp_dir, 'framework')
-  vendor_target_files_temp_dir = os.path.join(temp_dir, 'vendor')
   output_target_files_temp_dir = os.path.join(temp_dir, 'output')
-
-  # Extract "as is" items from the input framework partial target files package.
-  # We extract them directly into the output temporary directory since the
-  # items do not need special case processing.
-
   extract_items(
       target_files=framework_target_files,
       target_files_temp_dir=output_target_files_temp_dir,
       extract_item_list=framework_item_list)
-
-  # Extract "as is" items from the input vendor partial target files package. We
-  # extract them directly into the output temporary directory since the items
-  # do not need special case processing.
-
   extract_items(
       target_files=vendor_target_files,
       target_files_temp_dir=output_target_files_temp_dir,
       extract_item_list=vendor_item_list)
 
-  # Extract "special" items from the input framework partial target files
+  # Perform special case processing on META/* items.
-  # package. We extract these items to different directory since they require
+  # After this function completes successfully, all the files we need to create
-  # special processing before they will end up in the output directory.
+  # the output target files package are in place.
-
+  framework_target_files_temp_dir = os.path.join(temp_dir, 'framework')
+  vendor_target_files_temp_dir = os.path.join(temp_dir, 'vendor')
   extract_items(
       target_files=framework_target_files,
       target_files_temp_dir=framework_target_files_temp_dir,
-      extract_item_list=FRAMEWORK_EXTRACT_SPECIAL_ITEM_LIST)
+      extract_item_list=('META/*',))
-
-  # Extract "special" items from the input vendor partial target files package.
-  # We extract these items to different directory since they require special
-  # processing before they will end up in the output directory.
-
   extract_items(
       target_files=vendor_target_files,
       target_files_temp_dir=vendor_target_files_temp_dir,
-      extract_item_list=VENDOR_EXTRACT_SPECIAL_ITEM_LIST)
+      extract_item_list=('META/*',))
-
-  # Now that the temporary directories contain all the extracted files, perform
-  # special case processing on any items that need it. After this function
-  # completes successfully, all the files we need to create the output target
-  # files package are in place.
-
   process_special_cases(
       framework_target_files_temp_dir=framework_target_files_temp_dir,
       vendor_target_files_temp_dir=vendor_target_files_temp_dir,
@@ -845,8 +798,10 @@ def generate_images(target_files_dir, rebuild_recovery):
 
   # Regenerate IMAGES in the target directory.
 
-  add_img_args = ['--verbose']
+  add_img_args = [
-  add_img_args.append('--add_missing')
+      '--verbose',
+      '--add_missing',
+  ]
   # TODO(b/132730255): Remove this if statement.
   if rebuild_recovery:
     add_img_args.append('--rebuild_recovery')
@@ -899,6 +854,15 @@ def create_target_files_archive(output_file, source_dir, temp_dir):
   output_zip = os.path.abspath(output_file)
   output_target_files_meta_dir = os.path.join(source_dir, 'META')
 
+  def files_from_path(target_path, extra_args=None):
+    """Gets files under the given path and return a sorted list."""
+    find_command = ['find', target_path] + (extra_args or [])
+    find_process = common.Run(
+        find_command, stdout=subprocess.PIPE, verbose=False)
+    return common.RunAndCheckOutput(['sort'],
+                                    stdin=find_process.stdout,
+                                    verbose=False)
+
   meta_content = files_from_path(output_target_files_meta_dir)
   other_content = files_from_path(
       source_dir,
@@ -947,9 +911,9 @@ def merge_target_files(temp_dir, framework_target_files, framework_item_list,
       target files package as is, meaning these items will land in the output
       target files package exactly as they appear in the input partial framework
       target files package.
-    framework_misc_info_keys: The list of keys to obtain from the framework
+    framework_misc_info_keys: A list of keys to obtain from the framework
-      instance of META/misc_info.txt. The remaining keys from the vendor
+      instance of META/misc_info.txt. The remaining keys should come from the
-      instance.
+      vendor instance.
     vendor_target_files: The name of the zip archive containing the vendor
       partial target files package.
     vendor_item_list: The list of items to extract from the partial vendor

tools/releasetools/test_merge_target_files.py

@@ -117,6 +117,15 @@ class MergeTargetFilesTest(test_utils.ReleaseToolsTestCase):
                               DEFAULT_FRAMEWORK_MISC_INFO_KEYS,
                               vendor_item_list))
 
+  def test_validate_config_lists_ReturnsFalseIfSharedExtractedPartitionImage(
+      self):
+    vendor_item_list = list(DEFAULT_VENDOR_ITEM_LIST)
+    vendor_item_list.append('IMAGES/system.img')
+    self.assertFalse(
+        validate_config_lists(DEFAULT_FRAMEWORK_ITEM_LIST,
+                              DEFAULT_FRAMEWORK_MISC_INFO_KEYS,
+                              vendor_item_list))
+
   def test_validate_config_lists_ReturnsFalseIfBadSystemMiscInfoKeys(self):
     for bad_key in ['dynamic_partition_list', 'super_partition_groups']:
       framework_misc_info_keys = list(DEFAULT_FRAMEWORK_MISC_INFO_KEYS)
@@ -144,8 +153,7 @@ class MergeTargetFilesTest(test_utils.ReleaseToolsTestCase):
 
     process_apex_keys_apk_certs_common(framework_dir, vendor_dir, output_dir,
                                        set(['product', 'system', 'system_ext']),
-                                       set(['odm', 'vendor']),
+                                       set(['odm', 'vendor']), 'apexkeys.txt')
-                                       'apexkeys.txt')
 
     merged_entries = []
     merged_path = os.path.join(self.testdata_dir, 'apexkeys_merge.txt')
@@ -180,8 +188,7 @@ class MergeTargetFilesTest(test_utils.ReleaseToolsTestCase):
     self.assertRaises(ValueError, process_apex_keys_apk_certs_common,
                       framework_dir, conflict_dir, output_dir,
                       set(['product', 'system', 'system_ext']),
-                      set(['odm', 'vendor']),
+                      set(['odm', 'vendor']), 'apexkeys.txt')
-                      'apexkeys.txt')
 
   def test_process_apex_keys_apk_certs_HandlesApkCertsSyntax(self):
     output_dir = common.MakeTempDir()
@@ -201,8 +208,7 @@ class MergeTargetFilesTest(test_utils.ReleaseToolsTestCase):
 
     process_apex_keys_apk_certs_common(framework_dir, vendor_dir, output_dir,
                                        set(['product', 'system', 'system_ext']),
-                                       set(['odm', 'vendor']),
+                                       set(['odm', 'vendor']), 'apkcerts.txt')
-                                       'apkcerts.txt')
 
     merged_entries = []
     merged_path = os.path.join(self.testdata_dir, 'apkcerts_merge.txt')