[SuperSize] Fix default src root to find OWNERS files.

Two notions of "src root" in SuperSize are:

(1) SuperSize script's src root |path_util.SRC_ROOT|. This is useful
    for finding tools used by SuperSize.
(2) The src root of the input binary (i.e., Chrome) given for
    SuperSize-archive, and can be specified by the --source-directory
    switch. This is used to find OWNER files to extract symbol
    component values.

Previously SuperSize uses (1) as the default value of (2). Normally this
is okay since (1) and (2) coincide. However, for development we may need
to compare the results of two versions of SuperSize (with different (1)
src root) on common binaries (common (2) src root). In this case,
defaulting (2) to (1) would lead to inconsistent default results
(fixable by --source-directory, but this is error-prone manually).

This CL fixes the problem by changing the default value for (2), by
having SuperSize search for src root using the output directory as the
starting point, i.e., map
  /.../src/out/Debug/ -> /.../src ,
and falls back to (1) only if results are inconclusive. Details:

* path_util.py:
  * For (1): Rename |SRC_ROOT| to |TOOLS_SRC_ROOT|.
  * Add GetSrcRootFromOutputDirectory().
  * Rename {From, To}SrcRootRelative() to
    {From, To}ToolsSrcRootRelative().
* archive.py:
  * ContainerArchiveOptions: Take optional |output_directory| in
    __init__() to compute default (2) |self.src_root|.
  * _ParseComponentFromOwners(): Take ContainerArchiveOptions param,
    and clean up to avoid recursion.
* Update tests:
  * Specify --source-directory.
  * Mock OWNER files no longer refer to actual Chrome directories.

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

tools/binary_size/libsupersize/archive.py

@@ -38,7 +38,7 @@ import string_extract
 import zip_util
 
 
-sys.path.insert(1, os.path.join(path_util.SRC_ROOT, 'tools', 'grit'))
+sys.path.insert(1, os.path.join(path_util.TOOLS_SRC_ROOT, 'tools', 'grit'))
 from grit.format import data_pack
 
 _OWNERS_FILENAME = 'OWNERS'
@@ -116,8 +116,9 @@ class SectionSizeKnobs(object):
 
 # Parameters and states for archiving a container.
 class ContainerArchiveOptions:
-  def __init__(self, sub_args):
-    self.src_root = sub_args.source_directory or path_util.SRC_ROOT
+  def __init__(self, sub_args, output_directory=''):
+    self.src_root = (sub_args.source_directory or
+                     path_util.GetSrcRootFromOutputDirectory(output_directory))
 
     # An estimate of pak translation compression ratio to make comparisons
     # between .size files reasonable. Otherwise this can differ every pak
@@ -532,36 +533,41 @@ def _CreateMergeStringsReplacements(merge_string_syms,
   return ret
 
 
-def _ParseComponentFromOwners(filename):
+def _ParseComponentFromOwners(filename, opts):
   """Searches an OWNERS file for lines that start with `# COMPONENT:`.
 
-  If an OWNERS file has no COMPONENT but references another OWNERS file, follow
-  the reference and check that file instead.
+  If an OWNERS file has no COMPONENT but references exactly one other OWNERS
+  file, follows the reference and checks that file instead.
 
   Args:
     filename: Path to the file to parse.
+    opts: Instance of ContainerArchiveOptions.
   Returns:
     The text that follows the `# COMPONENT:` prefix, such as 'component>name'.
-    Empty string if no component found or the file didn't exist.
+    Empty string if no component found or the file doesn't exist.
   """
-  reference_paths = []
-  try:
-    with open(filename) as f:
-      for line in f:
-        component_matches = _COMPONENT_REGEX.match(line)
-        path_matches = _FILE_PATH_REGEX.match(line)
-        if component_matches:
-          return component_matches.group(1)
-        elif path_matches:
-          reference_paths.append(path_matches.group(1))
-  except IOError:
-    return ''
-
-  if len(reference_paths) == 1:
-    newpath = os.path.join(path_util.SRC_ROOT, reference_paths[0])
-    return _ParseComponentFromOwners(newpath)
-  else:
-    return ''
+  seen = set()
+  while True:
+    seen.add(filename)
+    reference_paths = []
+    try:
+      with open(filename) as f:
+        for line in f:
+          component_matches = _COMPONENT_REGEX.match(line)
+          path_matches = _FILE_PATH_REGEX.match(line)
+          if component_matches:
+            return component_matches.group(1)
+          elif path_matches:
+            reference_paths.append(path_matches.group(1))
+    except IOError:
+      break
+    if len(reference_paths) != 1:
+      break
+    filename = os.path.join(opts.src_root, reference_paths[0])
+    if filename in seen:
+      logging.warning('OWNER dependence loop found for %s' % filename)
+      break
+  return ''
 
 
 def _FindComponentRoot(start_path, cache, opts):
@@ -569,7 +575,7 @@ def _FindComponentRoot(start_path, cache, opts):
 
   Args:
     start_path: Path of directory to start searching from. Must be relative to
-      SRC_ROOT.
+      |opts.src_root|.
     cache: Dict of OWNERS paths. Used instead of filesystem if paths are present
       in the dict.
     opts: Instance of ContainerArchiveOptions.
@@ -580,14 +586,15 @@ def _FindComponentRoot(start_path, cache, opts):
   prev_dir = None
   test_dir = start_path
   # This loop will traverse the directory structure upwards until reaching
-  # SRC_ROOT, where test_dir and prev_dir will both equal an empty string.
+  # |opts.src_root|, where |test_dir| and |prev_dir| will both equal an empty
+  # string.
   while test_dir != prev_dir:
     cached_component = cache.get(test_dir)
     if cached_component:
       return cached_component
-    elif cached_component is None:
+    if cached_component is None:  # Excludes ''.
       owners_path = os.path.join(opts.src_root, test_dir, _OWNERS_FILENAME)
-      component = _ParseComponentFromOwners(owners_path)
+      component = _ParseComponentFromOwners(owners_path, opts)
       cache[test_dir] = component
       if component:
         return component
@@ -751,7 +758,7 @@ def CreateMetadata(map_path, elf_path, apk_path, minimal_apks_path,
     shorten_path = os.path.basename
 
   if tool_prefix:
-    relative_tool_prefix = path_util.ToSrcRootRelative(tool_prefix)
+    relative_tool_prefix = path_util.ToToolsSrcRootRelative(tool_prefix)
     metadata[models.METADATA_TOOL_PREFIX] = relative_tool_prefix
 
   if linker_name:
@@ -1871,7 +1878,8 @@ 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).
     """
-    opts = ContainerArchiveOptions(sub_args)
+    output_directory = output_directory_finder.Tentative()
+    opts = ContainerArchiveOptions(sub_args, output_directory=output_directory)
     if apk_prefix:
       # Allow either .minimal.apks or just .apks.
       apk_prefix = apk_prefix.replace('.minimal.apks', '.aab')
@@ -1882,8 +1890,8 @@ def _DeduceMainPaths(args, on_config_error):
     tool_prefix = None
     if opts.analyze_native:
       elf_path, map_path, apk_so_path = _DeduceNativeInfo(
-          output_directory_finder.Tentative(), apk_path, sub_args.elf_file,
-          sub_args.map_file, on_config_error)
+          output_directory, apk_path, sub_args.elf_file, sub_args.map_file,
+          on_config_error)
       if map_path:
         linker_name = _DetectLinkerName(map_path)
         logging.info('Linker name: %s' % linker_name)

tools/binary_size/libsupersize/console.py

@@ -245,7 +245,7 @@ class _Session(object):
     tool_prefix = self._tool_prefix_finder.Tentative()
     orig_tool_prefix = size_info.metadata.get(models.METADATA_TOOL_PREFIX)
     if orig_tool_prefix:
-      orig_tool_prefix = path_util.FromSrcRootRelative(orig_tool_prefix)
+      orig_tool_prefix = path_util.FromToolsSrcRootRelative(orig_tool_prefix)
       if os.path.exists(path_util.GetObjDumpPath(orig_tool_prefix)):
         tool_prefix = orig_tool_prefix
 

tools/binary_size/libsupersize/html_report.py

@@ -293,8 +293,9 @@ def Run(args, on_config_error):
       '    https://chrome-supersize.firebaseapp.com/viewer.html'
       '?load_url=oneoffs/{2}.ndjson',
   ]
-  supersize_path = os.path.relpath(os.path.join(
-      path_util.SRC_ROOT, 'tools', 'binary_size', 'supersize'))
+  supersize_path = os.path.relpath(
+      os.path.join(path_util.TOOLS_SRC_ROOT, 'tools', 'binary_size',
+                   'supersize'))
   # Use a random UUID as the filename so user can copy-and-paste command
   # directly without a name collision.
   upload_id = uuid.uuid4()

tools/binary_size/libsupersize/path_util.py

@@ -12,9 +12,13 @@ import os
 _STATUS_DETECTED = 1
 _STATUS_VERIFIED = 2
 
-SRC_ROOT = os.environ.get('CHECKOUT_SOURCE_ROOT',
-    os.path.abspath(os.path.join(os.path.dirname(__file__),
-                                 os.pardir, os.pardir, os.pardir)))
+# Src root of SuperSize being run. Not to be confused with src root of the input
+# binary being archived.
+TOOLS_SRC_ROOT = os.environ.get(
+    'CHECKOUT_SOURCE_ROOT',
+    os.path.abspath(
+        os.path.join(
+            os.path.dirname(__file__), os.pardir, os.pardir, os.pardir)))
 
 _SAMPLE_TOOL_SUFFIX = 'readelf'
 
@@ -93,7 +97,7 @@ class ToolPrefixFinder(_PathFinder):
     if output_directory:
       ret = None
       if self.IsLld():
-        ret = os.path.join(SRC_ROOT, 'third_party', 'llvm-build',
+        ret = os.path.join(TOOLS_SRC_ROOT, 'third_party', 'llvm-build',
                            'Release+Asserts', 'bin', 'llvm-')
       else:
         # Auto-detect from build_vars.txt
@@ -137,16 +141,42 @@ def _LoadBuildVars(output_directory):
   return dict()
 
 
-def FromSrcRootRelative(path):
-  ret = os.path.relpath(os.path.join(SRC_ROOT, path))
+def GetSrcRootFromOutputDirectory(output_directory):
+  """Returns the source root directory from output directory.
+
+  Typical case: '/.../chromium/src/out/Release' -> '/.../chromium/src/'.
+  Heuristic: Look for .gn in the current and successive parent directories.
+
+  Args:
+    output_directory: Starting point of search. This may be relative to CWD.
+
+  Returns:
+    Source root directory.
+  """
+  if output_directory:
+    cur_dir = os.path.abspath(output_directory)
+    while True:
+      gn_path = os.path.join(cur_dir, '.gn')
+      if os.path.isfile(gn_path):
+        return cur_dir
+      cur_dir, prev_dir = os.path.dirname(cur_dir), cur_dir
+      if cur_dir == prev_dir:  # Reached root.
+        break
+  logging.warning('Cannot deduce src root from output directory. Falling back '
+                  'to tools src root.')
+  return TOOLS_SRC_ROOT
+
+
+def FromToolsSrcRootRelative(path):
+  ret = os.path.relpath(os.path.join(TOOLS_SRC_ROOT, path))
   # Need to maintain a trailing /.
   if path.endswith(os.path.sep):
     ret += os.path.sep
   return ret
 
 
-def ToSrcRootRelative(path):
-  ret = os.path.relpath(path, SRC_ROOT)
+def ToToolsSrcRootRelative(path):
+  ret = os.path.relpath(path, TOOLS_SRC_ROOT)
   # Need to maintain a trailing /.
   if path.endswith(os.path.sep):
     ret += os.path.sep
@@ -164,14 +194,14 @@ def GetNmPath(tool_prefix):
 
 
 def GetApkAnalyzerPath():
-  default_path = FromSrcRootRelative(
+  default_path = FromToolsSrcRootRelative(
       os.path.join('third_party', 'android_sdk', 'public', 'cmdline-tools',
                    'latest', 'bin', 'apkanalyzer'))
   return os.environ.get('APK_ANALYZER', default_path)
 
 
 def GetJavaHome():
-  return FromSrcRootRelative(os.path.join('third_party', 'jdk', 'current'))
+  return FromToolsSrcRootRelative(os.path.join('third_party', 'jdk', 'current'))
 
 
 def GetObjDumpPath(tool_prefix):

tools/binary_size/libsupersize/testdata/TEST_OWNERS

-# COMPONENT: UI>Browser
\ No newline at end of file

tools/binary_size/libsupersize/testdata/mock_source_directory/base/OWNERS

-file://tools/binary_size/libsupersize/testdata/OWNERS
+file://OWNERS
 
-# COMPONENT: Blink>Internal
+# COMPONENT: Blink>Internal
\ No newline at end of file

tools/binary_size/libsupersize/testdata/mock_source_directory/third_party/OWNERS

 3p@email.com
 
-# COMPONENT: Internal>Android
+# COMPONENT: Internal>Android
\ No newline at end of file

tools/binary_size/libsupersize/testdata/mock_source_directory/third_party/blink/OWNERS

+# COMPONENT: UI>Browser

tools/binary_size/libsupersize/testdata/mock_source_directory/third_party/container/OWNERS

-file://tools/binary_size/libsupersize/testdata/TEST_OWNERS
+file://third_party/blink/OWNERS
\ No newline at end of file