[SuperSize] Extract build_config from metadata.

SuperSize metadata fields can be specific to containers (e.g., filename)
or be common among them (e.g., GN args). This CL changes SuperSize to
represent these separately:
* Add BaseSizeInfo.build_config to represent common fields, which are:
  {git_revision, gn_args, linker_name, tool_prefix}.
* Keep SizeInfo.metadata (with above fields moved). It will be moved
  once we introduce the Container class.

The change affects internal data structure, but *not* .size files.
* Add SizeInfo.metadata_legacy to support this: It fuses build_config
  with metadata, for the (current) one-container case.

Other changes:
* Replace describe.DescribeMetadata() with describe.DescribeDict(), to
  be used for build_config and metadata.
* Fix lint warnings in diff_tests.py (assertEquals() -> assertEqual()).

Bug: 1040645
Change-Id: I5db9e91a72abf3b7c4a0606adcd85db4afbddd29
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2224637
Commit-Queue: Samuel Huang <huangs@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#773798}

tools/binary_size/libsupersize/archive.py

@@ -725,9 +725,9 @@ def _ExtendSectionRange(section_range_by_name, section_name, delta_size):
   section_range_by_name[section_name] = (prev_address, prev_size + delta_size)
 
 
-def CreateMetadata(map_path, elf_path, apk_path, minimal_apks_path,
-                   tool_prefix, output_directory, linker_name):
-  """Creates metadata dict.
+def CreateMetadata(map_path, elf_path, apk_path, minimal_apks_path, tool_prefix,
+                   output_directory, linker_name, build_config):
+  """Creates metadata dict while updating |build_config|.
 
   Args:
     map_path: Path to the linker .map(.gz) file to parse.
@@ -738,6 +738,8 @@ def CreateMetadata(map_path, elf_path, apk_path, minimal_apks_path,
     tool_prefix: Prefix for c++filt & nm.
     output_directory: Build output directory.
     linker_name: A coded linker name (see linker_map_parser.py).
+    builg_config: Common build configurations to update or to undergo
+        consistency checks.
 
   Returns:
     A dict mapping string costants to values, or None if empty. Performs
@@ -748,21 +750,31 @@ def CreateMetadata(map_path, elf_path, apk_path, minimal_apks_path,
   """
   assert not (apk_path and minimal_apks_path)
   logging.debug('Constructing metadata')
+
+  def update_build_config(key, value):
+    if key in build_config:
+      old_value = build_config[key]
+      if value != old_value:
+        raise ValueError('Inconsistent {}: {} (was {})'.format(
+            key, value, old_value))
+    else:
+      build_config[key] = value
+
   metadata = {}
 
   if output_directory:
     shorten_path = lambda path: os.path.relpath(path, output_directory)
     gn_args = _ParseGnArgs(os.path.join(output_directory, 'args.gn'))
-    metadata[models.METADATA_GN_ARGS] = gn_args
+    update_build_config(models.BUILD_CONFIG_GN_ARGS, gn_args)
   else:
     shorten_path = os.path.basename
 
   if tool_prefix:
     relative_tool_prefix = path_util.ToToolsSrcRootRelative(tool_prefix)
-    metadata[models.METADATA_TOOL_PREFIX] = relative_tool_prefix
+    update_build_config(models.BUILD_CONFIG_TOOL_PREFIX, relative_tool_prefix)
 
   if linker_name:
-    metadata[models.METADATA_LINKER_NAME] = linker_name
+    update_build_config(models.BUILD_CONFIG_LINKER_NAME, linker_name)
 
   # Deduce GIT revision.
   path_candidates = [elf_path, apk_path, minimal_apks_path]
@@ -774,8 +786,7 @@ def CreateMetadata(map_path, elf_path, apk_path, minimal_apks_path,
       if dirname:
         git_rev = _DetectGitRevision(dirname)
         if git_rev:
-          metadata[models.METADATA_GIT_REVISION] = git_rev
-          break
+          update_build_config(models.BUILD_CONFIG_GIT_REVISION, git_rev)
 
   if elf_path:
     metadata[models.METADATA_ELF_FILENAME] = shorten_path(elf_path)
@@ -1405,6 +1416,7 @@ def _AddUnattributedSectionSymbols(raw_symbols, section_ranges):
 
 def CreateSectionSizesAndSymbols(knobs=None,
                                  opts=None,
+                                 metadata=None,
                                  map_path=None,
                                  tool_prefix=None,
                                  output_directory=None,
@@ -1413,7 +1425,6 @@ def CreateSectionSizesAndSymbols(knobs=None,
                                  mapping_path=None,
                                  resources_pathmap_path=None,
                                  track_string_literals=True,
-                                 metadata=None,
                                  apk_so_path=None,
                                  pak_files=None,
                                  pak_info_file=None,
@@ -1424,6 +1435,7 @@ def CreateSectionSizesAndSymbols(knobs=None,
   Args:
     knobs: Instance of SectionSizeKnobs.
     opts: Instance of ContainerArchiveOptions.
+    metadata: Metadata dict from CreateMetadata().
     map_path: Path to the linker .map(.gz) file to parse.
     tool_prefix: Prefix for c++filt & nm (required).
     output_directory: Build output directory. If None, source_paths and symbol
@@ -1436,7 +1448,6 @@ def CreateSectionSizesAndSymbols(knobs=None,
         resource paths to shortened resource paths.
     track_string_literals: Whether to break down "** merge string" sections into
         smaller symbols (requires output_directory).
-    metadata: Metadata dict from CreateMetadata().
     apk_so_path: Path to an .so file within an APK file.
     pak_files: List of paths to .pak files.
     pak_info_file: Path to a .pak.info file.
@@ -1600,7 +1611,8 @@ def CreateSectionSizesAndSymbols(knobs=None,
   return section_sizes, raw_symbols
 
 
-def CreateSizeInfo(section_sizes_list,
+def CreateSizeInfo(build_config,
+                   section_sizes_list,
                    raw_symbols_list,
                    metadata_list,
                    normalize_names=True):
@@ -1621,7 +1633,10 @@ def CreateSizeInfo(section_sizes_list,
   raw_symbols = raw_symbols_list[0]
   metadata = metadata_list[0]
 
-  return models.SizeInfo(section_sizes, raw_symbols, metadata=metadata)
+  return models.SizeInfo(build_config,
+                         section_sizes,
+                         raw_symbols,
+                         metadata=metadata)
 
 
 def _DetectGitRevision(directory):
@@ -1927,7 +1942,6 @@ def _Inner(sub_args, apk_prefix, apk_path):
             apk_so_path, elf_path, map_path, resources_pathmap_path,
             linker_name, size_info_prefix)
 
-  # Process each container.
   # If needed, extract .apk file to a temp file and process that instead.
   if args.minimal_apks_file:
     with zip_util.UnzipToTemp(args.minimal_apks_file, _APKS_MAIN_APK) as temp:
@@ -1958,6 +1972,7 @@ def Run(args, on_config_error):
 
   knobs = SectionSizeKnobs()
 
+  build_config = {}
   metadata_list = []
   section_sizes_list = []
   raw_symbols_list = []
@@ -1969,10 +1984,11 @@ def Run(args, on_config_error):
     # may be an extracted temporary file.
     metadata = CreateMetadata(map_path, elf_path, args.apk_file,
                               args.minimal_apks_file, tool_prefix,
-                              output_directory, linker_name)
+                              output_directory, linker_name, build_config)
     section_sizes, raw_symbols = CreateSectionSizesAndSymbols(
         knobs=knobs,
         opts=opts,
+        metadata=metadata,
         map_path=map_path,
         tool_prefix=tool_prefix,
         elf_path=elf_path,
@@ -1981,7 +1997,6 @@ def Run(args, on_config_error):
         output_directory=output_directory,
         resources_pathmap_path=resources_pathmap_path,
         track_string_literals=args.track_string_literals,
-        metadata=metadata,
         apk_so_path=apk_so_path,
         pak_files=args.pak_file,
         pak_info_file=args.pak_info_file,
@@ -1992,18 +2007,18 @@ def Run(args, on_config_error):
     section_sizes_list.append(section_sizes)
     raw_symbols_list.append(raw_symbols)
 
-  size_info = CreateSizeInfo(
-      section_sizes_list,
-      raw_symbols_list,
-      metadata_list,
-      normalize_names=False)
+  size_info = CreateSizeInfo(build_config,
+                             section_sizes_list,
+                             raw_symbols_list,
+                             metadata_list,
+                             normalize_names=False)
 
   if logging.getLogger().isEnabledFor(logging.DEBUG):
     for line in describe.DescribeSizeInfoCoverage(size_info):
       logging.debug(line)
   logging.info('Recorded info for %d symbols', len(size_info.raw_symbols))
-  logging.info('Recording metadata: \n  %s', '\n  '.join(
-      describe.DescribeMetadata(size_info.metadata)))
+  logging.info('Recording metadata: \n  %s',
+               '\n  '.join(describe.DescribeDict(size_info.metadata)))
 
   logging.info('Saving result to %s', args.size_file)
   file_format.SaveSizeInfo(

tools/binary_size/libsupersize/console.py

@@ -126,8 +126,7 @@ def _ReadStringLiterals(self, thing=None, all_rodata=False, elf_path=None):
       return []
     size_info = self._SizeInfoForSymbol(first_sym)
     tool_prefix = self._ToolPrefixForSymbol(size_info)
-    elf_path = self._ElfPathForSymbol(
-        size_info, tool_prefix, elf_path)
+    elf_path = self._ElfPathForSymbol(size_info, tool_prefix, elf_path)
 
     return string_extract.ReadStringLiterals(
         thing, elf_path, tool_prefix, all_rodata=all_rodata)
@@ -243,7 +242,8 @@ def _CsvFunc(self, obj=None, verbose=False, use_pager=None, to_file=None):
 
   def _ToolPrefixForSymbol(self, size_info):
     tool_prefix = self._tool_prefix_finder.Tentative()
-    orig_tool_prefix = size_info.metadata.get(models.METADATA_TOOL_PREFIX)
+    orig_tool_prefix = size_info.build_config.get(
+        models.BUILD_CONFIG_TOOL_PREFIX)
     if orig_tool_prefix:
       orig_tool_prefix = path_util.FromToolsSrcRootRelative(orig_tool_prefix)
       if os.path.exists(path_util.GetObjDumpPath(orig_tool_prefix)):
@@ -489,7 +489,7 @@ def Run(args, on_config_error):
   output_directory_finder = path_util.OutputDirectoryFinder(
       value=args.output_directory,
       any_path_within_output_directory=args.inputs[0])
-  linker_name = size_infos[-1].metadata.get(models.METADATA_LINKER_NAME)
+  linker_name = size_infos[-1].build_config.get(models.BUILD_CONFIG_LINKER_NAME)
   tool_prefix_finder = path_util.ToolPrefixFinder(
       value=args.tool_prefix,
       output_directory_finder=output_directory_finder,

tools/binary_size/libsupersize/describe.py

@@ -489,27 +489,28 @@ def _DescribeDeltaSymbolGroup(self, delta_group):
     group_desc = self._DescribeSymbolGroup(delta_group)
     return itertools.chain(diff_summary_desc, path_delta_desc, group_desc)
 
-  def _DescribeDeltaSizeInfo(self, diff):
-    common_metadata = {
-        k: v
-        for k, v in diff.before.metadata.items()
-        if diff.after.metadata.get(k) == v
-    }
-    before_metadata = {
+  def _DescribeDeltaDict(self, data_name, before_dict, after_dict):
+    common_items = {
         k: v
-        for k, v in diff.before.metadata.items() if k not in common_metadata
+        for k, v in before_dict.items() if after_dict.get(k) == v
     }
-    after_metadata = {
+    before_items = {
         k: v
-        for k, v in diff.after.metadata.items() if k not in common_metadata
+        for k, v in before_dict.items() if k not in common_items
     }
-    metadata_desc = itertools.chain(
-        ('Common Metadata:',),
-        ('    %s' % line for line in DescribeMetadata(common_metadata)),
-        ('Old Metadata:',),
-        ('    %s' % line for line in DescribeMetadata(before_metadata)),
-        ('New Metadata:',),
-        ('    %s' % line for line in DescribeMetadata(after_metadata)))
+    after_items = {k: v for k, v in after_dict.items() if k not in common_items}
+    return itertools.chain(
+        ('Common %s:' % data_name, ), ('    %s' % line
+                                       for line in DescribeDict(common_items)),
+        ('Old %s:' % data_name, ), ('    %s' % line
+                                    for line in DescribeDict(before_items)),
+        ('New %s:' % data_name, ), ('    %s' % line
+                                    for line in DescribeDict(after_items)))
+
+  def _DescribeDeltaSizeInfo(self, diff):
+    metadata_desc = self._DescribeDeltaDict('Metadata',
+                                            diff.before.metadata_legacy,
+                                            diff.after.metadata_legacy)
     unsummed_sections, summed_sections = diff.ClassifySections()
     section_desc = self._DescribeSectionSizes(unsummed_sections,
                                               summed_sections,
@@ -518,9 +519,10 @@ def _DescribeDeltaSizeInfo(self, diff):
     return itertools.chain(metadata_desc, section_desc, ('',), group_desc)
 
   def _DescribeSizeInfo(self, size_info):
+    # Support legacy output by reporting |build_config| as part of metadata.
     metadata_desc = itertools.chain(
-        ('Metadata:',),
-        ('    %s' % line for line in DescribeMetadata(size_info.metadata)))
+        ('Metadata:', ),
+        ('    %s' % line for line in DescribeDict(size_info.metadata_legacy)))
     unsummed_sections, summed_sections = size_info.ClassifySections()
     section_desc = self._DescribeSectionSizes(unsummed_sections,
                                               summed_sections,
@@ -744,16 +746,25 @@ def _UtcToLocal(utc):
   return utc + offset
 
 
-def DescribeMetadata(metadata):
-  display_dict = metadata.copy()
-  timestamp = display_dict.get(models.METADATA_ELF_MTIME)
-  if timestamp:
-    timestamp_obj = datetime.datetime.utcfromtimestamp(timestamp)
-    display_dict[models.METADATA_ELF_MTIME] = (
-        _UtcToLocal(timestamp_obj).strftime('%Y-%m-%d %H:%M:%S'))
-  gn_args = display_dict.get(models.METADATA_GN_ARGS)
-  if gn_args:
-    display_dict[models.METADATA_GN_ARGS] = ' '.join(gn_args)
+def DescribeDict(input_dict):
+  display_dict = {}
+  for k, v in input_dict.items():
+    if k == models.METADATA_ELF_MTIME:
+      timestamp_obj = datetime.datetime.utcfromtimestamp(v)
+      display_dict[k] = (
+          _UtcToLocal(timestamp_obj).strftime('%Y-%m-%d %H:%M:%S'))
+    elif isinstance(v, str):
+      display_dict[k] = v
+    elif isinstance(v, list):
+      if v:
+        if isinstance(v[0], str):
+          display_dict[k] = ' '.join(str(t) for t in v)
+        else:
+          display_dict[k] = repr(v)
+      else:
+        display_dict[k] = ''
+    else:
+      display_dict[k] = repr(v)
   return sorted('%s=%s' % t for t in display_dict.items())