Revert "[SuperSize] Add multiple Container support using .ssargs files."

This reverts commit 19d7a780.

Reason for revert: Breaking CQ (see bug)

Original change's description:
> [SuperSize] Add multiple Container support using .ssargs files.
> 
> This CL adds SuperSize support for multiple containers. Details:
> * Introduce .ssargs file, which is a text file to specify multiple
>   containers for SuperSize-archive to process into a single .size file.
>   * Container file types are auto-detected.
>   * Container files are relative to .ssargs file location, unless
>     absolute paths are given.
> * Introduce Version 1.1 .size file format:
>   * Header fields contain Container specs, now absorbing metadata and
>     section_sizes.
>   * Per-section data become per-container-per-section.
> * For compatibility: Running SuperSize-archive on one container still
>   generates Version 1.0 .size file format.
> * Update SuperSize-console output (including diffs) to accommodate.
> 
> After this CL, basic Trichrome support is available. A recommended
> .ssargs file (3 lines) for the official build is as follows:
> 
> TrichromeLibraryGoogle.apk --name Library
> TrichromeChromeGoogle.minimal.apks --name Chrome --java-only
> TrichromeWebViewGoogle.apk --name WebView --java-only
> 
> Bug: 1040645
> Change-Id: I67df00731d07660163410b59c57854bf56ea651e
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2248860
> Commit-Queue: Samuel Huang <huangs@chromium.org>
> Reviewed-by: Andrew Grieve <agrieve@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#780594}

TBR=huangs@chromium.org,agrieve@chromium.org

Change-Id: Ic3175ea3e8ede41e3183c12b5a00a4a09be55a75
No-Try: True
Bug: 1040645, 1097572
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2256593Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#780639}

tools/binary_size/libsupersize/archive.py

@@ -15,7 +15,6 @@ import logging
 import os
 import posixpath
 import re
-import shlex
 import string
 import subprocess
 import sys
@@ -1132,8 +1131,6 @@ def _ParsePakSymbols(symbols_by_id, object_paths_by_pak_id):
           full_name=symbol.full_name, object_path=path, aliases=aliases)
       aliases.append(new_sym)
       raw_symbols.append(new_sym)
-  # Sorting can ignore containers because symbols created here are all in the
-  # same container.
   raw_symbols.sort(key=lambda s: (s.section_name, s.address, s.object_path))
   raw_total = 0.0
   int_total = 0
@@ -1750,47 +1747,50 @@ def _AutoIdentifyInputFile(args):
   elif args.f.endswith('.map') or args.f.endswith('.map.gz'):
     logging.info('Auto-identified --map-file.')
     args.map_file = args.f
-  elif args.f.endswith('.ssargs'):
-    logging.info('Auto-identified --ssargs-file.')
-    args.ssargs_file = args.f
   else:
     return False
   return True
 
 
-def _AddContainerArguments(parser):
-  """Add arguments applicable to a single container."""
-
-  # Main container file arguments. These are mutually-exclusive.
+def AddMainPathsArguments(parser):
+  """Add arguments for _DeduceMainPaths()."""
   parser.add_argument('-f', metavar='FILE',
                       help='Auto-identify input file type.')
   parser.add_argument('--apk-file',
                       help='.apk file to measure. Other flags can generally be '
                            'derived when this is used.')
+  parser.add_argument(
+      '--resources-pathmap-file',
+      help='.pathmap.txt file that contains a maping from '
+      'original resource paths to shortened resource paths.')
   parser.add_argument('--minimal-apks-file',
                       help='.minimal.apks file to measure. Other flags can '
                            'generally be derived when this is used.')
+  parser.add_argument('--mapping-file',
+                      help='Proguard .mapping file for deobfuscation.')
   parser.add_argument('--elf-file',
                       help='Path to input ELF file. Currently used for '
                            'capturing metadata.')
-
-  # Auxiliary file arguments.
   parser.add_argument('--map-file',
                       help='Path to input .map(.gz) file. Defaults to '
                            '{{elf_file}}.map(.gz)?. If given without '
                            '--elf-file, no size metadata will be recorded.')
-  parser.add_argument('--mapping-file',
-                      help='Proguard .mapping file for deobfuscation.')
-  parser.add_argument('--resources-pathmap-file',
-                      help='.pathmap.txt file that contains a maping from '
-                      'original resource paths to shortened resource paths.')
+  parser.add_argument('--no-source-paths', action='store_true',
+                      help='Do not use .ninja files to map '
+                           'object_path -> source_path')
+  parser.add_argument('--output-directory',
+                      help='Path to the root build directory.')
+  parser.add_argument('--tool-prefix',
+                      help='Path prefix for c++filt, nm, readelf.')
+
+
+def AddArguments(parser):
+  parser.add_argument('size_file', help='Path to output .size file.')
   parser.add_argument('--pak-file', action='append',
                       help='Paths to pak files.')
   parser.add_argument('--pak-info-file',
                       help='This file should contain all ids found in the pak '
                            'files that have been passed in.')
-
-  # Non-file argument.
   parser.add_argument('--no-string-literals', dest='track_string_literals',
                       default=True, action='store_false',
                       help='Disable breaking down "** merge strings" into more '
@@ -1800,10 +1800,8 @@ def _AddContainerArguments(parser):
       action='store_true',
       help='Instead of counting binary size, count number of relative'
       'relocation instructions in ELF code.')
-  parser.add_argument('--no-source-paths',
-                      action='store_true',
-                      help='Do not use .ninja files to map '
-                      'object_path -> source_path')
+  parser.add_argument('--source-directory',
+                      help='Custom path to the root source directory.')
   parser.add_argument(
       '--java-only', action='store_true', help='Run on only Java symbols')
   parser.add_argument(
@@ -1817,63 +1815,7 @@ def _AddContainerArguments(parser):
       action='store_true',
       help='Include a padding field for each symbol, instead of rederiving '
       'from consecutive symbols on file load.')
-
-
-def AddArguments(parser):
-  parser.add_argument('size_file', help='Path to output .size file.')
-  parser.add_argument('--source-directory',
-                      help='Custom path to the root source directory.')
-  parser.add_argument('--output-directory',
-                      help='Path to the root build directory.')
-  parser.add_argument('--tool-prefix',
-                      help='Path prefix for c++filt, nm, readelf.')
-
-  _AddContainerArguments(parser)
-  parser.add_argument('--ssargs-file',
-                      help='Path to SuperSize multi-container arguments file.')
-
-
-def _ParseSsargs(lines):
-  """Parses .ssargs data.
-
-  An .ssargs file is a text file to specify multiple containers as input to
-  SuperSize-archive. After '#'-based comments, start / end whitespaces, and
-  empty lines are stripped, each line specifies a distinct container. Format:
-  * Positional argument: |name| for the container.
-  * Main input file specified by -f, --apk-file, --elf-file, etc.:
-    * Can be an absolute path.
-    * Can be a relative path. In this case, it's up to the caller to supply the
-      base directory.
-    * -f switch must not specify another .ssargs file.
-  * For supported switches: See _AddContainerArguments().
-
-  Args:
-    lines: An iterator containing lines of .ssargs data.
-  Returns:
-    A list of arguments, one for each container.
-  Raises:
-    ValueError: Parse error, including input line number.
-  """
-  container_args_list = []
-  parser = argparse.ArgumentParser(add_help=False)
-  parser.error = lambda msg: (_ for _ in ()).throw(ValueError(msg))
-  parser.add_argument('name')
-  _AddContainerArguments(parser)
-  try:
-    for lineno, line in enumerate(lines, 1):
-      toks = shlex.split(line, comments=True)
-      if not toks:  # Skip if line is empty after stripping comments.
-        continue
-      container_args = parser.parse_args(toks)
-      if set(container_args.name) & set('<>'):
-        parser.error('container name cannot have characters in "<>"')
-      if container_args.f and container_args.f.endswith('.ssargs'):
-        parser.error('cannot nest .ssargs files')
-      container_args_list.append(container_args)
-  except ValueError as e:
-    e.args = ('Line %d: %s' % (lineno, e.args[0]), )
-    raise e
-  return container_args_list
+  AddMainPathsArguments(parser)
 
 
 def _DeduceNativeInfo(tentative_output_dir, apk_path, elf_path, map_path,
@@ -1941,57 +1883,6 @@ def _DeduceAuxPaths(args, apk_prefix):
   return mapping_path, resources_pathmap_path
 
 
-def _DeduceDerivedArgs(args, is_top_level_args, on_config_error):
-  setattr(args, 'is_bundle', args.minimal_apks_file is not None)
-  if is_top_level_args:
-    any_path = (args.apk_file or args.minimal_apks_file or args.elf_file
-                or args.map_file or args.ssargs_file)
-    if any_path is None:
-      on_config_error(
-          'Must pass at least one of --apk-file, --minimal-apks-file, '
-          '--elf-file, --map-file, --ssargs-file')
-    setattr(args, 'any_path_within_output_directory', any_path)
-
-
-def _ReadMultipleArgsFromStream(lines, base_dir, err_prefix, args,
-                                on_config_error):
-  try:
-    container_args_list = _ParseSsargs(lines)
-  except ValueError as e:
-    on_config_error('%s: %s' % (err_prefix, e.args[0]))
-  sub_args_list = []
-  for container_args in container_args_list:
-    # Clone |args| keys but assign empty values.
-    sub_args = argparse.Namespace(**{k: None for k in vars(args)})
-    # Copy parsed values to |sub_args|.
-    for k, v in container_args.__dict__.items():
-      # Translate ile arguments to be relative to |sub_dir|.
-      if (k.endswith('_file') or k == 'f') and v is not None:
-        v = os.path.join(base_dir, v)
-      sub_args.__dict__[k] = v
-    if sub_args.f is not None:
-      _AutoIdentifyInputFile(sub_args)
-    _DeduceDerivedArgs(sub_args,
-                       is_top_level_args=False,
-                       on_config_error=on_config_error)
-    logging.info('Container: %r' %
-                 {k: v
-                  for k, v in sub_args.__dict__.items() if v is not None})
-    sub_args_list.append(sub_args)
-  return sub_args_list
-
-
-def _ReadMultipleArgsFromFile(args, on_config_error):
-  with open(args.ssargs_file, 'r') as fh:
-    lines = list(fh)
-  err_prefix = 'In file ' + args.ssargs_file
-  # Supply |base_dir| as the directory containing the .ssargs file, to ensure
-  # consistent behavior wherever SuperSize-archive runs.
-  base_dir = os.path.dirname(os.path.abspath(args.ssargs_file))
-  return _ReadMultipleArgsFromStream(lines, base_dir, err_prefix, args,
-                                     on_config_error)
-
-
 def _DeduceMainPaths(args, on_config_error):
   """Generates main paths (may be deduced) for each containers given by input.
 
@@ -2013,12 +1904,13 @@ def _DeduceMainPaths(args, on_config_error):
       apk_path: Path to .apk file that can be opened for processing, but whose
         filename is unimportant (e.g., can be a temp file).
     """
+    # TODO(huangs): Assign distinct names for multiple containers.
+    assert idx == 0
+    container_name = ''
+
     output_directory = output_directory_finder.Tentative()
     opts = ContainerArchiveOptions(sub_args, output_directory=output_directory)
-    container_name = sub_args.name if hasattr(sub_args, 'name') else None
     if apk_prefix:
-      if not container_name:
-        container_name = apk_prefix
       # Allow either .minimal.apks or just .apks.
       apk_prefix = apk_prefix.replace('.minimal.apks', '.aab')
       apk_prefix = apk_prefix.replace('.apks', '.aab')
@@ -2039,8 +1931,6 @@ def _DeduceMainPaths(args, on_config_error):
             output_directory_finder=output_directory_finder,
             linker_name=linker_name)
         tool_prefix = tool_prefix_finder.Finalized()
-      if not container_name and elf_path:
-        container_name = elf_path
     else:
       # Trust that these values will not be used, and set to None.
       elf_path = None
@@ -2057,19 +1947,14 @@ def _DeduceMainPaths(args, on_config_error):
       size_info_prefix = os.path.join(output_directory, 'size-info',
                                       os.path.basename(apk_prefix))
 
-    if not container_name:
-      container_name = 'Container %d' % idx
-
     return (opts, output_directory, tool_prefix, container_name, apk_path,
             mapping_path, apk_so_path, elf_path, map_path,
             resources_pathmap_path, linker_name, size_info_prefix)
 
-  if args.ssargs_file:
-    sub_args_list = _ReadMultipleArgsFromFile(args, on_config_error)
-  else:
-    sub_args_list = [args]
+  # One for each container.
+  # TODO(huangs): Add support for multiple containers
+  sub_args_list = [args]
 
-  # Each element in |sub_args_list| specifies a container.
   for idx, sub_args in enumerate(sub_args_list):
     # If needed, extract .apk file to a temp file and process that instead.
     if sub_args.minimal_apks_file:
@@ -2089,9 +1974,17 @@ def Run(args, on_config_error):
       on_config_error('Cannot identify file %s' % args.f)
   if args.apk_file and args.minimal_apks_file:
     on_config_error('Cannot use both --apk-file and --minimal-apks-file.')
-  _DeduceDerivedArgs(args,
-                     is_top_level_args=True,
-                     on_config_error=on_config_error)
+
+  # Deduce arguments.
+  setattr(args, 'is_bundle', args.minimal_apks_file is not None)
+  any_path = (args.apk_file or args.minimal_apks_file or args.elf_file
+              or args.map_file)
+  if any_path is None:
+    on_config_error(
+        'Must pass at least one of --apk-file, --minimal-apks-file, '
+        '--elf-file, --map-file')
+  setattr(args, 'any_path_within_output_directory', any_path)
+
   knobs = SectionSizeKnobs()
 
   build_config = {}

tools/binary_size/libsupersize/describe.py

@@ -177,33 +177,30 @@ class DescriberText(Describer):
     self.recursive = recursive
     self.summarize = summarize
 
-  def _DescribeSectionSizes(self,
-                            unsummed_sections,
-                            summed_sections,
-                            section_sizes,
-                            indent=''):
+  def _DescribeSectionSizes(self, unsummed_sections, summed_sections,
+                            section_sizes):
     total_bytes, section_names = _GetSectionSizeInfo(unsummed_sections,
                                                      summed_sections,
                                                      section_sizes)
     yield ''
-    yield '{}Section Sizes (Total={} ({} bytes)):'.format(
-        indent, _PrettySize(total_bytes), total_bytes)
+    yield 'Section Sizes (Total={} ({} bytes)):'.format(
+        _PrettySize(total_bytes), total_bytes)
     for name in section_names:
       size = section_sizes[name]
       if name in unsummed_sections:
-        yield '{}    {}: {} ({} bytes) (not included in totals)'.format(
-            indent, name, _PrettySize(size), size)
+        yield '    {}: {} ({} bytes) (not included in totals)'.format(
+            name, _PrettySize(size), size)
       else:
         notes = ''
         if name not in summed_sections:
           notes = ' (counted in .other)'
         percent = _Divide(size, total_bytes)
-        yield '{}    {}: {} ({} bytes) ({:.1%}){}'.format(
-            indent, name, _PrettySize(size), size, percent, notes)
+        yield '    {}: {} ({} bytes) ({:.1%}){}'.format(name, _PrettySize(size),
+                                                        size, percent, notes)
 
     if self.verbose:
       yield ''
-      yield '{}Other section sizes:'.format(indent)
+      yield 'Other section sizes:'
       section_names = sorted(
           k for k in section_sizes.keys() if k not in section_names)
       for name in section_names:
@@ -212,15 +209,12 @@ class DescriberText(Describer):
           notes = ' (not included in totals)'
         elif name not in summed_sections:
           notes = ' (counted in .other)'
-        yield '{}    {}: {} ({} bytes){}'.format(
-            indent, name, _PrettySize(section_sizes[name]), section_sizes[name],
-            notes)
+        yield '    {}: {} ({} bytes){}'.format(name,
+                                               _PrettySize(section_sizes[name]),
+                                               section_sizes[name], notes)
 
   def _DescribeSymbol(self, sym, single_line=False):
-    container_str = sym.container_short_name
-    if container_str:
-      container_str = '<{}>'.format(container_str)
-
+    # TODO(huangs): Render and display container name.
     address = 'Group' if sym.IsGroup() else hex(sym.address)
 
     last_field = ''
@@ -248,13 +242,11 @@ class DescriberText(Describer):
       if last_field:
         last_field = '  ' + last_field
       if sym.IsDelta():
-        yield '{}{}@{:<9s}  {}{}'.format(container_str, sym.section, address,
-                                         pss_field, last_field)
+        yield '{}@{:<9s}  {}{}'.format(
+            sym.section, address, pss_field, last_field)
       else:
-        l = '{}{}@{:<9s}  pss={}  padding={}{}'.format(container_str,
-                                                       sym.section, address,
-                                                       pss_field, sym.padding,
-                                                       last_field)
+        l = '{}@{:<9s}  pss={}  padding={}{}'.format(
+            sym.section, address, pss_field, sym.padding, last_field)
         yield l
       yield '    source_path={} \tobject_path={}'.format(
           sym.source_path, sym.object_path)
@@ -273,17 +265,16 @@ class DescriberText(Describer):
       else:
         pss_field = '{:<14}'.format(pss_field)
       if single_line:
-        yield '{}{}@{:<9s}  {}  {}{}'.format(container_str, sym.section,
-                                             address, pss_field, sym.name,
-                                             last_field)
+        yield '{}@{:<9s}  {}  {}{}'.format(
+            sym.section, address, pss_field, sym.name, last_field)
       else:
         path = sym.source_path or sym.object_path
         if path and sym.generated_source:
           path = '$root_gen_dir/' + path
         path = path or '{no path}'
 
-        yield '{}{}@{:<9s}  {} {}'.format(container_str, sym.section, address,
-                                          pss_field, path)
+        yield '{}@{:<9s}  {} {}'.format(
+            sym.section, address, pss_field, path)
         if sym.name:
           yield '    {}{}'.format(sym.name, last_field)
 
@@ -407,7 +398,7 @@ class DescriberText(Describer):
     if group.container_name == '':
       title_parts.append('Section@Address')
     else:
-      title_parts.append('<Container>Section@Address')
+      raise ValueError('Multiple container not yet supported.')
     if self.verbose:
       title_parts.append('...')
     else:
@@ -506,7 +497,7 @@ class DescriberText(Describer):
     group_desc = self._DescribeSymbolGroup(delta_group)
     return itertools.chain(diff_summary_desc, path_delta_desc, group_desc)
 
-  def _DescribeDeltaDict(self, data_name, before_dict, after_dict, indent=''):
+  def _DescribeDeltaDict(self, data_name, before_dict, after_dict):
     common_items = {
         k: v
         for k, v in before_dict.items() if after_dict.get(k) == v
@@ -517,41 +508,17 @@ class DescriberText(Describer):
     }
     after_items = {k: v for k, v in after_dict.items() if k not in common_items}
     return itertools.chain(
-        (indent + 'Common %s:' % data_name, ),
-        (indent + '    %s' % line for line in DescribeDict(common_items)),
-        (indent + 'Old %s:' % data_name, ),
-        (indent + '    %s' % line for line in DescribeDict(before_items)),
-        (indent + 'New %s:' % data_name, ),
-        (indent + '    %s' % line for line in DescribeDict(after_items)))
+        ('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):
     desc_list = []
-    # Describe |build_config| and each container. If there is only one container
-    # then support legacy output by reporting |build_config| as part of the
-    # first container's metadata.
     if len(diff.containers) > 1:
-      desc_list.append(
-          self._DescribeDeltaDict('Build config', diff.before.build_config,
-                                  diff.after.build_config))
-      for c in diff.containers:
-        name = c.name
-        desc_list.append(('', ))
-        desc_list.append(('Container: <%s>' % name, ))
-        c_before = diff.before.ContainerForName(
-            name, default=models.Container.Empty())
-        c_after = diff.after.ContainerForName(name,
-                                              default=models.Container.Empty())
-        desc_list.append(
-            self._DescribeDeltaDict('Metadata',
-                                    c_before.metadata,
-                                    c_after.metadata,
-                                    indent='    '))
-        unsummed_sections, summed_sections = c.ClassifySections()
-        desc_list.append(
-            self._DescribeSectionSizes(unsummed_sections,
-                                       summed_sections,
-                                       c.section_sizes,
-                                       indent='    '))
+      raise ValueError('Multiple container not yet supported.')
     else:  # Legacy output for single Container case.
       desc_list.append(
           self._DescribeDeltaDict('Metadata', diff.before.metadata_legacy,
@@ -567,26 +534,14 @@ class DescriberText(Describer):
 
   def _DescribeSizeInfo(self, size_info):
     desc_list = []
-    # Describe |build_config| and each container. If there is only one container
-    # then support legacy output by reporting |build_config| as part of the
-    # first container's metadata.
     if len(size_info.containers) > 1:
-      desc_list.append(('Build Configs:', ))
-      desc_list.append('    %s' % line
-                       for line in DescribeDict(size_info.build_config))
-      containers = size_info.containers
+      raise ValueError('Multiple container not yet supported.')
     else:
-      containers = [
-          models.Container(name='',
-                           metadata=size_info.metadata_legacy,
-                           section_sizes=size_info.containers[0].section_sizes)
-      ]
-    for c in containers:
-      if c.name:
-        desc_list.append(('', ))
-        desc_list.append(('Container <%s>' % c.name, ))
+      # Support legacy output by reporting |build_config| as part of metadata.
       desc_list.append(('Metadata:', ))
-      desc_list.append('    %s' % line for line in DescribeDict(c.metadata))
+      desc_list.append('    %s' % line
+                       for line in DescribeDict(size_info.metadata_legacy))
+      c = size_info.containers[0]
       unsummed_sections, summed_sections = c.ClassifySections()
       desc_list.append(
           self._DescribeSectionSizes(unsummed_sections, summed_sections,
@@ -604,7 +559,8 @@ def _DescribeSizeInfoContainerCoverage(raw_symbols, container):
   """Yields lines describing how accurate |size_info| is."""
   for section, section_name in models.SECTION_TO_SECTION_NAME.items():
     expected_size = container.section_sizes.get(section_name)
-    in_section = raw_symbols.WhereInSection(section_name, container=container)
+    # TODO(huangs): Also filter by container.
+    in_section = raw_symbols.WhereInSection(section_name)
     actual_size = in_section.size
 
     if expected_size is None:
@@ -694,15 +650,12 @@ def _DescribeSizeInfoContainerCoverage(raw_symbols, container):
 
 
 def DescribeSizeInfoCoverage(size_info):
-  for i, container in enumerate(size_info.containers):
-    if i > 0:
-      yield ''
-    if container.name:
-      yield 'Container <%s>' % container.name
-    # TODO(huangs): Change to use "yield from" once linters allow this.
-    for line in _DescribeSizeInfoContainerCoverage(size_info.raw_symbols,
-                                                   container):
-      yield line
+  # TODO(huangs): Add support for multiple containers.
+  assert len(size_info.containers) == 1
+  c = size_info.containers[0]
+  # TODO(huangs): Change to use "yield from" once linters allow this.
+  for line in _DescribeSizeInfoContainerCoverage(size_info.raw_symbols, c):
+    yield line
 
 
 class DescriberCsv(Describer):

tools/binary_size/libsupersize/diff.py

@@ -150,25 +150,19 @@ def _DiffObj(before_obj, after_obj):
 
 
 def _DiffContainerLists(before_containers, after_containers):
-  """Computes diff of Containers lists, matching names."""
-  # Find ordered unique names, preferring order of |container_after|.
-  pairs = collections.OrderedDict()
-  for c in after_containers:
-    pairs[c.name] = [models.Container.Empty(), c]
-  for c in before_containers:
-    if c.name in pairs:
-      pairs[c.name][0] = c
-    else:
-      pairs[c.name] = [c, models.Container.Empty()]
+  """Computes diffs between two lists of Containers."""
+  # TODO(huangs): Add support for multiple containers (needs name matching).
+  assert len(before_containers) == 1
+  assert len(after_containers) == 1
   ret = []
-  for name, [before_c, after_c] in pairs.items():
+  for (before_c, after_c) in zip(before_containers, after_containers):
+    name = after_c.name
+    assert before_c.name == name
     ret.append(
         models.Container(name=name,
                          metadata=_DiffObj(before_c.metadata, after_c.metadata),
                          section_sizes=_DiffObj(before_c.section_sizes,
                                                 after_c.section_sizes)))
-  # This update newly created diff Containers, not existing ones or EMPTY.
-  models.Container.AssignShortNames(ret)
   return ret
 
 

tools/binary_size/libsupersize/diff_test.py

@@ -37,7 +37,6 @@ def _CreateSizeInfo(aliases=None):
   containers = [
       models.Container(name='', metadata=metadata, section_sizes=section_sizes)
   ]
-  models.Container.AssignShortNames(containers)
   TEXT = models.SECTION_TEXT
   symbols = [
       _MakeSym(models.SECTION_DEX_METHOD, 10, 'a', 'com.Foo#bar()'),

tools/binary_size/libsupersize/file_format.py

@@ -5,9 +5,9 @@
 """Deals with loading & saving .size and .sizediff files.
 
 The .size file is written in the following format. There are no section
-delimiters, instead the end of a section is usually determined by a row count on
-the first line of a section, followed by that number of rows. In other cases,
-the sections have a known size.
+delimiters, instead the end of a section is usually determined by a row count
+on the first line of a section, followed by that number of rows. In other
+cases, the sections have a known size.
 
 Header
 ------
@@ -116,8 +116,7 @@ import parallel
 
 
 # File format version for .size files.
-_SERIALIZATION_VERSION_SINGLE_CONTAINER = 'Size File Format v1'
-_SERIALIZATION_VERSION_MULTI_CONTAINER = 'Size File Format v1.1'
+_SERIALIZATION_VERSION = 'Size File Format v1'
 
 # Header for .sizediff files
 _SIZEDIFF_HEADER = '# Created by //tools/binary_size\nDIFF\n'
@@ -172,18 +171,18 @@ def CalculatePadding(raw_symbols):
 
   # Padding not really required, but it is useful to check for large padding and
   # log a warning.
-  seen_container_and_sections = set()
+  # TODO(huangs): Add support for multiple containers: Need to group by
+  #     container names and sections.
+  seen_sections = set()
   for i, symbol in enumerate(raw_symbols[1:]):
     prev_symbol = raw_symbols[i]
     if symbol.IsOverhead():
       # Overhead symbols are not actionable so should be padding-only.
       symbol.padding = symbol.size
-    if (prev_symbol.container.name != symbol.container.name
-        or prev_symbol.section_name != symbol.section_name):
-      container_and_section = (symbol.container.name, symbol.section_name)
-      assert container_and_section not in seen_container_and_sections, (
-          'Input symbols must be sorted by container, section, then address.')
-      seen_container_and_sections.add(container_and_section)
+    if prev_symbol.section_name != symbol.section_name:
+      assert symbol.section_name not in seen_sections, (
+          'Input symbols must be sorted by section, then address.')
+      seen_sections.add(symbol.section_name)
       continue
     if (symbol.address <= 0 or prev_symbol.address <= 0
         or not symbol.IsNative() or not prev_symbol.IsNative()):
@@ -249,34 +248,21 @@ def _SaveSizeInfoToFile(size_info,
   else:
     raw_symbols = size_info.raw_symbols
 
-  num_containers = len(size_info.containers)
-  has_multi_containers = (num_containers > 1)
-
   w = _Writer(file_obj)
 
-  # "Created by SuperSize" header
+  # Created by supersize header
   w.WriteLine('# Created by //tools/binary_size')
-  if has_multi_containers:
-    w.WriteLine(_SERIALIZATION_VERSION_MULTI_CONTAINER)
-  else:
-    w.WriteLine(_SERIALIZATION_VERSION_SINGLE_CONTAINER)
-
+  w.WriteLine(_SERIALIZATION_VERSION)
   # JSON header fields
   fields = {
       'has_components': True,
       'has_padding': include_padding,
   }
+  num_containers = len(size_info.containers)
+  has_multi_containers = (num_containers > 1)
 
   if has_multi_containers:
-    # Write using new format.
-    assert len(set(c.name for c in size_info.containers)) == num_containers, (
-        'Container names must be distinct.')
-    fields['build_config'] = size_info.build_config
-    fields['containers'] = [{
-        'name': c.name,
-        'metadata': c.metadata,
-        'section_sizes': c.section_sizes,
-    } for c in size_info.containers]
+    raise ValueError('Multiple container not yet supported.')
   else:
     # Write using old format.
     fields['metadata'] = size_info.metadata_legacy
@@ -304,18 +290,12 @@ def _SaveSizeInfoToFile(size_info,
     w.WriteLine(comp)
   w.LogSize('components')
 
-  # Symbol counts by container and section.
-  symbol_group_by_section = raw_symbols.GroupedByContainerAndSectionName()
+  # Symbol counts by section.
+  symbol_group_by_section = raw_symbols.GroupedBySectionName()
   if has_multi_containers:
-    container_name_to_index = {
-        c.name: i
-        for i, c in enumerate(size_info.containers)
-    }
-    w.WriteLine('\t'.join('<%d>%s' %
-                          (container_name_to_index[g.name[0]], g.name[1])
-                          for g in symbol_group_by_section))
+    raise ValueError('Multiple container not yet supported.')
   else:
-    w.WriteLine('\t'.join(g.name[1] for g in symbol_group_by_section))
+    w.WriteLine('\t'.join(g.name for g in symbol_group_by_section))
   w.WriteLine('\t'.join(str(len(g)) for g in symbol_group_by_section))
 
   def gen_delta(gen, prev_value=0):
@@ -404,35 +384,23 @@ def _LoadSizeInfoFromFile(file_obj, size_path):
   """
   # Split lines on '\n', since '\r' can appear in some lines!
   lines = io.TextIOWrapper(file_obj, newline='\n')
-  _ReadLine(lines)  # Line 0: "Created by SuperSize" header
+  _ReadLine(lines)  # Line 0: Created by supersize header
   actual_version = _ReadLine(lines)
-  if actual_version == _SERIALIZATION_VERSION_SINGLE_CONTAINER:
-    has_multi_containers = False
-  elif actual_version == _SERIALIZATION_VERSION_MULTI_CONTAINER:
-    has_multi_containers = True
-  else:
-    raise ValueError('Version mismatch. Need to write some upgrade code.')
-
+  assert actual_version == _SERIALIZATION_VERSION, (
+      'Version mismatch. Need to write some upgrade code.')
   # JSON header fields
   json_len = int(_ReadLine(lines))
   json_str = lines.read(json_len)
 
   fields = json.loads(json_str)
-  assert ('containers' in fields) == has_multi_containers
-  assert ('build_config' in fields) == has_multi_containers
-  assert ('containers' in fields) == has_multi_containers
-  assert ('metadata' not in fields) == has_multi_containers
-  assert ('section_sizes' not in fields) == has_multi_containers
+
+  has_multi_containers = False
 
   containers = []
   if has_multi_containers:  # New format.
-    build_config = fields['build_config']
-    for cfield in fields['containers']:
-      c = models.Container(name=cfield['name'],
-                           metadata=cfield['metadata'],
-                           section_sizes=cfield['section_sizes'])
-      containers.append(c)
-  else:  # Old format.
+    raise ValueError('Multiple container not yet supported.')
+  else:
+    # Parse old format, but separate data into build_config and metadata.
     build_config = {}
     metadata = fields.get('metadata')
     if metadata:
@@ -445,7 +413,6 @@ def _LoadSizeInfoFromFile(file_obj, size_path):
         models.Container(name='',
                          metadata=metadata,
                          section_sizes=section_sizes))
-  models.Container.AssignShortNames(containers)
 
   has_components = fields.get('has_components', False)
   has_padding = fields.get('has_padding', False)
@@ -466,7 +433,7 @@ def _LoadSizeInfoFromFile(file_obj, size_path):
     components = [_ReadLine(lines) for _ in range(num_components)]
 
   # Symbol counts by section.
-  container_and_section_names = _ReadValuesFromLine(lines, split='\t')
+  section_names = _ReadValuesFromLine(lines, split='\t')
   symbol_counts = [int(c) for c in _ReadValuesFromLine(lines, split='\t')]
 
   # Addresses, sizes, paddings, path indices, component indices
@@ -493,28 +460,23 @@ def _LoadSizeInfoFromFile(file_obj, size_path):
   if has_padding:
     paddings = read_numeric(delta=False)
   else:
-    paddings = [None] * len(container_and_section_names)
+    paddings = [None] * len(section_names)
   path_indices = read_numeric(delta=True)
   if has_components:
     component_indices = read_numeric(delta=True)
   else:
-    component_indices = [None] * len(container_and_section_names)
+    component_indices = [None] * len(section_names)
 
   raw_symbols = [None] * sum(symbol_counts)
   symbol_idx = 0
-  for (cur_container_and_section_name, cur_symbol_count, cur_addresses,
-       cur_sizes, cur_paddings, cur_path_indices,
-       cur_component_indices) in zip(container_and_section_names, symbol_counts,
-                                     addresses, sizes, paddings, path_indices,
+  for (cur_section_name, cur_symbol_count, cur_addresses, cur_sizes,
+       cur_paddings, cur_path_indices,
+       cur_component_indices) in zip(section_names, symbol_counts, addresses,
+                                     sizes, paddings, path_indices,
                                      component_indices):
     if has_multi_containers:
-      # Extract '<cur_container_idx_str>cur_section_name'.
-      assert cur_container_and_section_name.startswith('<')
-      cur_container_idx_str, cur_section_name = (
-          cur_container_and_section_name[1:].split('>', 1))
-      cur_container = containers[int(cur_container_idx_str)]
+      raise ValueError('Multiple container not yet supported.')
     else:
-      cur_section_name = cur_container_and_section_name
       cur_container = containers[0]
     alias_counter = 0
     for i in range(cur_symbol_count):

tools/binary_size/libsupersize/integration_test.py

@@ -309,10 +309,7 @@ class IntegrationTest(unittest.TestCase):
       merged_data_desc = describe.DescribeDict(size_info.metadata_legacy)
       return itertools.chain(merged_data_desc, stats, sym_strs)
     else:
-      build_config = describe.DescribeDict(size_info.build_config)
-      metadata = itertools.chain.from_iterable(
-          describe.DescribeDict(c.metadata) for c in size_info.containers)
-      return itertools.chain(build_config, metadata, stats, sym_strs)
+      raise ValueError('Multiple container not yet supported.')
 
   @_CompareWithGolden()
   def test_Archive(self):

tools/binary_size/libsupersize/models.py

@@ -102,8 +102,6 @@ PAK_SECTIONS = (
     SECTION_PAK_TRANSLATIONS,
 )
 
-CONTAINER_MULTIPLE = '*'
-
 SECTION_NAME_TO_SECTION = {
     SECTION_BSS: 'b',
     SECTION_BSS_REL_RO: 'b',
@@ -203,19 +201,16 @@ def ClassifySections(section_names):
 
 
 class Container(object):
-  """Info for a single SuperSize input file (e.g., APK file).
+  """Info for a a single input file for SuperSize, e.g., an APK file.
 
   Fields:
     name: Container name. Must be unique among containers, and can be ''.
-    short_name: Short container name for compact display. This, also needs to be
-        unique among containers in the same SizeInfo, and can be ''.
     metadata: A dict.
     section_sizes: A dict of section_name -> size.
     classified_sections: Cache for ClassifySections().
   """
   __slots__ = (
       'name',
-      'short_name',
       'metadata',
       'section_sizes',
       '_classified_sections',
@@ -225,31 +220,15 @@ class Container(object):
     # name == '' hints that only one container exists, and there's no need to
     # distinguish them. This can affect console output.
     self.name = name
-    self.short_name = None  # Assigned by AssignShortNames().
     self.metadata = metadata or {}
     self.section_sizes = section_sizes  # E.g. {SECTION_TEXT: 0}
     self._classified_sections = None
 
-  @staticmethod
-  def AssignShortNames(containers):
-    for i, c in enumerate(containers):
-      c.short_name = str(i) if c.name else ''
-
   def ClassifySections(self):
     if not self._classified_sections:
       self._classified_sections = ClassifySections(self.section_sizes.keys())
     return self._classified_sections
 
-  @staticmethod
-  def Empty():
-    """Returns a placeholder Container that should be read-only.
-
-    For simplicity, we're not enforcing read-only checks (frozenmap does not
-    exist, unfortunately). Creating a new instance instead of using a global
-    singleton for robustness.
-    """
-    return Container(name='(empty)', metadata={}, section_sizes={})
-
 
 class BaseSizeInfo(object):
   """Base class for SizeInfo and DeltaSizeInfo.
@@ -282,7 +261,6 @@ class BaseSizeInfo(object):
     self._symbols = symbols
     self._native_symbols = None
     self._pak_symbols = None
-    Container.AssignShortNames(self.containers)
 
   @property
   def symbols(self):
@@ -318,9 +296,6 @@ class BaseSizeInfo(object):
   def metadata(self):
     return [c.metadata for c in self.containers]
 
-  def ContainerForName(self, name, default=None):
-    return next((c for c in containers if c.name == name), default)
-
 
 class SizeInfo(BaseSizeInfo):
   """Represents all size information for a single binary.
@@ -387,10 +362,6 @@ class BaseSymbol(object):
   def container_name(self):
     return self.container.name if self.container else ''
 
-  @property
-  def container_short_name(self):
-    return self.container.short_name if self.container else ''
-
   @property
   def section(self):
     """Returns the one-letter section."""
@@ -545,17 +516,14 @@ class Symbol(BaseSymbol):
     self.component = ''
 
   def __repr__(self):
-    if self.container and self.container.name:
-      container_str = '<{}>'.format(self.container.name)
-    else:
-      container_str = ''
-    template = ('{}{}@{:x}(size_without_padding={},padding={},full_name={},'
+    # TODO(huangs): If container name is nonempty then display it.
+    template = ('{}@{:x}(size_without_padding={},padding={},full_name={},'
                 'object_path={},source_path={},flags={},num_aliases={},'
                 'component={})')
-    return template.format(container_str, self.section_name, self.address,
-                           self.size_without_padding, self.padding,
-                           self.full_name, self.object_path, self.source_path,
-                           self.FlagsString(), self.num_aliases, self.component)
+    return template.format(
+        self.section_name, self.address, self.size_without_padding,
+        self.padding, self.full_name, self.object_path, self.source_path,
+        self.FlagsString(), self.num_aliases, self.component)
 
   def SetName(self, full_name, template_name=None, name=None):
     # Note that _NormalizeNames() will clobber these values.
@@ -594,21 +562,14 @@ class DeltaSymbol(BaseSymbol):
     self.after_symbol = after_symbol
 
   def __repr__(self):
-    before_container_name = self.before_symbol.container_name
-    after_container_name = self.after_symbol.container_name
-    if before_container_name != after_container_name:
-      container_str = '<~{}>'.format(after_container_name)
-    elif after_container_name != '':
-      container_str = '<{}>'.format(after_container_name)
-    else:
-      container_str = ''
-    template = ('{}{}{}@{:x}(size_without_padding={},padding={},full_name={},'
+    # TODO(huangs): If container name is nonempty then display it.
+    template = ('{}{}@{:x}(size_without_padding={},padding={},full_name={},'
                 'object_path={},source_path={},flags={})')
-    return template.format(DIFF_PREFIX_BY_STATUS[self.diff_status],
-                           container_str, self.section_name, self.address,
-                           self.size_without_padding, self.padding,
-                           self.full_name, self.object_path, self.source_path,
-                           self.FlagsString())
+    return template.format(
+        DIFF_PREFIX_BY_STATUS[self.diff_status], self.section_name,
+        self.address, self.size_without_padding, self.padding,
+        self.full_name, self.object_path, self.source_path,
+        self.FlagsString())
 
   def IsDelta(self):
     return True
@@ -751,11 +712,11 @@ class SymbolGroup(BaseSymbol):
       'full_name',
       'template_name',
       'name',
+      'container_name',
       'section_name',
       'is_default_sorted',  # True for groups created by Sorted()
   )
 
-
   # template_name and full_name are useful when clustering symbol clones.
   def __init__(self,
                symbols,
@@ -774,6 +735,8 @@ class SymbolGroup(BaseSymbol):
     self.full_name = full_name if full_name is not None else name
     self.template_name = template_name if template_name is not None else name
     self.name = name or ''
+    # TODO(huangs): Add support for multiple containers.
+    self.container_name = ''
     self.section_name = section_name or SECTION_MULTIPLE
     self.is_default_sorted = is_default_sorted
 
@@ -820,20 +783,6 @@ class SymbolGroup(BaseSymbol):
   def index(self, item):
     return self._symbols.index(item)
 
-  @property
-  def container_name(self):
-    ret = set(s.container_name for s in self._symbols)
-    if ret:
-      return CONTAINER_MULTIPLE if len(ret) > 1 else (ret.pop() or '')
-    return ''
-
-  @property
-  def container_short_name(self):
-    ret = set(s.container_short_name for s in self._symbols)
-    if ret:
-      return CONTAINER_MULTIPLE if len(ret) > 1 else (ret.pop() or '')
-    return ''
-
   @property
   def address(self):
     first = self._symbols[0].address if self else 0
@@ -989,23 +938,13 @@ class SymbolGroup(BaseSymbol):
   def WherePssBiggerThan(self, min_pss):
     return self.Filter(lambda s: s.pss >= min_pss)
 
-  def WhereInSection(self, section, container=None):
+  def WhereInSection(self, section):
     """|section| can be section_name ('.bss'), or section chars ('bdr')."""
     if section.startswith('.'):
-      if container:
-        short_name = container.short_name
-        ret = self.Filter(lambda s: (s.container.short_name == short_name and s.
-                                     section_name == section))
-      else:
-        ret = self.Filter(lambda s: s.section_name == section)
+      ret = self.Filter(lambda s: s.section_name == section)
       ret.section_name = section
     else:
-      if container:
-        short_name = container.short_name
-        ret = self.Filter(lambda s: (s.container.short_name == short_name and s.
-                                     section in section))
-      else:
-        ret = self.Filter(lambda s: s.section in section)
+      ret = self.Filter(lambda s: s.section in section)
       if section in SECTION_TO_SECTION_NAME:
         ret.section_name = SECTION_TO_SECTION_NAME[section]
     return ret
@@ -1254,9 +1193,6 @@ class SymbolGroup(BaseSymbol):
         lambda s: (same_name_only and s.full_name, id(s.aliases or s)),
         min_count=min_count, group_factory=group_factory)
 
-  def GroupedByContainerAndSectionName(self):
-    return self.GroupedBy(lambda s: (s.container.name, s.section_name))
-
   def GroupedBySectionName(self):
     return self.GroupedBy(lambda s: s.section_name)
 

tools/binary_size/libsupersize/testdata/Console.golden

 ********************************************************************************
 Entering interactive Python shell. Quick reference:
 
-SizeInfo: ContainerForName, all_section_sizes, build_config, containers, metadata, metadata_legacy, native_symbols, pak_symbols, raw_symbols, size_path, symbols
-Symbol: FlagsString, IsBss, IsDelta, IsDex, IsGeneratedByToolchain, IsGroup, IsNameUnique, IsNative, IsOther, IsOverhead, IsPak, IsStringLiteral, IterLeafSymbols, SetName, address, aliases, component, container, container_name, container_short_name, end_address, flags, full_name, generated_source, is_anonymous, name, num_aliases, object_path, padding, padding_pss, pss, pss_without_padding, section, section_name, size, size_without_padding, source_path, template_name
+SizeInfo: all_section_sizes, build_config, containers, metadata, metadata_legacy, native_symbols, pak_symbols, raw_symbols, size_path, symbols
+Symbol: FlagsString, IsBss, IsDelta, IsDex, IsGeneratedByToolchain, IsGroup, IsNameUnique, IsNative, IsOther, IsOverhead, IsPak, IsStringLiteral, IterLeafSymbols, SetName, address, aliases, component, container, container_name, end_address, flags, full_name, generated_source, is_anonymous, name, num_aliases, object_path, padding, padding_pss, pss, pss_without_padding, section, section_name, size, size_without_padding, source_path, template_name
 
-SymbolGroup (extends Symbol): CountUniqueSymbols, Filter, GroupedBy, GroupedByAliases, GroupedByComponent, GroupedByContainerAndSectionName, GroupedByFullName, GroupedByName, GroupedByPath, GroupedBySectionName, Inverted, IterUniqueSymbols, Sorted, SortedByAddress, SortedByCount, SortedByName, WhereAddressInRange, WhereComponentMatches, WhereFullNameMatches, WhereGeneratedByToolchain, WhereHasAnyAttribution, WhereHasComponent, WhereHasFlag, WhereHasPath, WhereInSection, WhereIsDex, WhereIsGroup, WhereIsNative, WhereIsPak, WhereIsTemplate, WhereMatches, WhereNameMatches, WhereObjectPathMatches, WherePathMatches, WherePssBiggerThan, WhereSizeBiggerThan, WhereSourceIsGenerated, WhereSourcePathMatches, WhereTemplateNameMatches, index, is_default_sorted
+SymbolGroup (extends Symbol): CountUniqueSymbols, Filter, GroupedBy, GroupedByAliases, GroupedByComponent, GroupedByFullName, GroupedByName, GroupedByPath, GroupedBySectionName, Inverted, IterUniqueSymbols, Sorted, SortedByAddress, SortedByCount, SortedByName, WhereAddressInRange, WhereComponentMatches, WhereFullNameMatches, WhereGeneratedByToolchain, WhereHasAnyAttribution, WhereHasComponent, WhereHasFlag, WhereHasPath, WhereInSection, WhereIsDex, WhereIsGroup, WhereIsNative, WhereIsPak, WhereIsTemplate, WhereMatches, WhereNameMatches, WhereObjectPathMatches, WherePathMatches, WherePssBiggerThan, WhereSizeBiggerThan, WhereSourceIsGenerated, WhereSourcePathMatches, WhereTemplateNameMatches, index, is_default_sorted
 
-DeltaSizeInfo: ContainerForName, after, all_section_sizes, before, build_config, containers, metadata, native_symbols, pak_symbols, raw_symbols, symbols
+DeltaSizeInfo: after, all_section_sizes, before, build_config, containers, metadata, native_symbols, pak_symbols, raw_symbols, symbols
 DeltaSymbol (extends Symbol): after_symbol, before_symbol, diff_status
 DeltaSymbolGroup (extends SymbolGroup): CountsByDiffStatus, WhereDiffStatusIs, diff_status