diagnose_bloat.py: Remove --cloud option.

The builder used for this option has been changed and does not
archive all artifacts needed.

Now that the android-binary-size trybot exists, downloading builds
locally generally isn't necessary.

Change-Id: Ibf24646dce4db1ae926885937dbe11fb7728a9c7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1676676
Commit-Queue: Eric Stevenson <estevenson@chromium.org>
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Auto-Submit: Eric Stevenson <estevenson@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#672174}

tools/binary_size/README.md

@@ -217,11 +217,6 @@ Collect size information and dump it into a `.size` file.
 **Note:** Refer to
 [diagnose_bloat.py](https://cs.chromium.org/search/?q=file:diagnose_bloat.py+gn_args)
 for list of GN args to build a Release binary (or just use the tool with --single).
-
-**Googlers:** If you just want a `.size` for a commit on master:
-
-    GIT_REV="HEAD~200"
-    tools/binary_size/diagnose_bloat.py --single --cloud --unstripped $GIT_REV
 ***
 
 Example Usage:
@@ -327,8 +322,6 @@ and Linux (although Linux symbol diffs have issues, as noted below).
 
 1. Builds multiple revisions using release GN args.
    * Default is to build just two revisions (before & after commit)
-   * Rather than building, can fetch build artifacts and `.size` files from perf
-     bots (`--cloud`)
 1. Measures all outputs using `resource_size.py` and `supersize`.
 1. Saves & displays a breakdown of the difference in binary sizes.
 
@@ -344,12 +337,6 @@ tools/binary_size/diagnose_bloat.py HEAD --enable-chrome-android-internal -v
 # Build and diff monochrome_public_apk HEAD^ and HEAD without is_official_build.
 tools/binary_size/diagnose_bloat.py HEAD --gn-args="is_official_build=false" -v
 
-# Diff BEFORE_REV and AFTER_REV using build artifacts downloaded from perf bots.
-tools/binary_size/diagnose_bloat.py AFTER_REV --reference-rev BEFORE_REV --cloud -v
-
-# Fetch a .size, libmonochrome.so, and MonochromePublic.apk from perf bots (Googlers only):
-tools/binary_size/diagnose_bloat.py AFTER_REV --cloud --unstripped --single
-
 # Build and diff all contiguous revs in range BEFORE_REV..AFTER_REV for src/v8.
 tools/binary_size/diagnose_bloat.py AFTER_REV --reference-rev BEFORE_REV --subrepo v8 --all -v
 

tools/binary_size/diagnose_bloat.py

@@ -7,8 +7,7 @@
 
 See //tools/binary_size/README.md for example usage.
 
-Note: this tool will perform gclient sync/git checkout on your local repo if
-you don't use the --cloud option.
+Note: this tool will perform gclient sync/git checkout on your local repo.
 """
 
 import atexit
@@ -220,7 +219,6 @@ class _BuildHelper(object):
   """Helper class for generating and building targets."""
   def __init__(self, args):
     self.clean = args.clean
-    self.cloud = args.cloud
     self.enable_chrome_android_internal = args.enable_chrome_android_internal
     self.extra_gn_args_str = args.gn_args
     self.apply_patch = args.extra_rev
@@ -263,8 +261,7 @@ class _BuildHelper(object):
 
   @property
   def main_lib_path(self):
-    # Cannot extract this from GN because --cloud needs to work without GN.
-    # TODO(agrieve): Could maybe extract from .apk?
+    # TODO(agrieve): Could maybe extract from .apk or GN?
     if self.IsLinux():
       return 'chrome'
     if 'monochrome' in self.target or 'trichrome' in self.target:
@@ -279,15 +276,6 @@ class _BuildHelper(object):
   def abs_main_lib_path(self):
     return os.path.join(self.output_directory, self.main_lib_path)
 
-  @property
-  def builder_url(self):
-    url = 'https://build.chromium.org/p/chromium.perf/builders/%s%%20Builder'
-    return url % self.target_os.title()
-
-  @property
-  def download_bucket(self):
-    return 'gs://chrome-perf/%s Builder/' % self.target_os.title()
-
   @property
   def map_file_path(self):
     return self.main_lib_path + '.map.gz'
@@ -368,18 +356,12 @@ class _BuildHelper(object):
     return _RunCmd(
         self._GenNinjaCmd(), verbose=True, exit_on_failure=False)[1]
 
-  def DownloadUrl(self, rev):
-    return self.download_bucket + 'full-build-linux_%s.zip' % rev
-
   def IsAndroid(self):
     return self.target_os == 'android'
 
   def IsLinux(self):
     return self.target_os == 'linux'
 
-  def IsCloud(self):
-    return self.cloud
-
 
 class _BuildArchive(object):
   """Class for managing a directory with build results and build metadata."""
@@ -422,10 +404,10 @@ class _BuildArchive(object):
     return os.path.join(self.dir, self.build.size_name)
 
   def _ArchiveResourceSizes(self):
-    cmd = [_RESOURCE_SIZES_PATH, self.build.abs_apk_path,'--output-dir',
-           self.dir, '--chartjson']
-    if not self.build.IsCloud():
-      cmd += ['--chromium-output-dir', self.build.output_directory]
+    cmd = [
+        _RESOURCE_SIZES_PATH, self.build.abs_apk_path, '--output-dir', self.dir,
+        '--chartjson', '--chromium-output-dir', self.build.output_directory
+    ]
     if self._slow_options:
       cmd += ['--estimate-patch-size', '--dump-static-initializers']
     _RunCmd(cmd)
@@ -441,14 +423,13 @@ class _BuildArchive(object):
       logging.info('Found existing .size file')
       shutil.copy(existing_size_file, self.archived_size_path)
     else:
-      supersize_cmd = [supersize_path, 'archive', self.archived_size_path,
-                       '--elf-file', self.build.abs_main_lib_path]
+      supersize_cmd = [
+          supersize_path, 'archive', self.archived_size_path, '--elf-file',
+          self.build.abs_main_lib_path, '--output-directory',
+          self.build.output_directory
+      ]
       if tool_prefix:
         supersize_cmd += ['--tool-prefix', tool_prefix]
-      if self.build.IsCloud():
-        supersize_cmd += ['--no-source-paths']
-      else:
-        supersize_cmd += ['--output-directory', self.build.output_directory]
       if self.build.IsAndroid():
         supersize_cmd += ['-f', self.build.abs_apk_path]
       logging.info('Creating .size file')
@@ -586,14 +567,12 @@ class _DiffArchiveManager(object):
 class _Metadata(object):
 
   def __init__(self, archives, build, path, subrepo):
-    self.is_cloud = build.IsCloud()
     self.data = {
       'revs': [a.rev for a in archives],
       'apply_patch': build.apply_patch,
       'archive_dirs': [a.dir for a in archives],
       'target': build.target,
       'target_os': build.target_os,
-      'is_cloud': build.IsCloud(),
       'subrepo': subrepo,
       'path': path,
       'gn_args': {
@@ -757,93 +736,6 @@ def _Die(s, *args):
   sys.exit(1)
 
 
-def _DownloadBuildArtifacts(archive, build, supersize_path, depot_tools_path):
-  """Download artifacts from arm32 chromium perf builder."""
-  if depot_tools_path:
-    gsutil_path = os.path.join(depot_tools_path, 'gsutil.py')
-  else:
-    gsutil_path = distutils.spawn.find_executable('gsutil.py')
-
-  if not gsutil_path:
-    _Die('gsutil.py not found, please provide path to depot_tools via '
-         '--depot-tools-path or add it to your PATH')
-
-  download_dir = tempfile.mkdtemp(dir=_SRC_ROOT)
-  try:
-    _DownloadAndArchive(
-        gsutil_path, archive, download_dir, build, supersize_path)
-  finally:
-    shutil.rmtree(download_dir)
-
-
-def _DownloadAndArchive(gsutil_path, archive, dl_dir, build, supersize_path):
-  # Wraps gsutil calls and returns stdout + stderr.
-  def gsutil_cmd(args, fail_msg=None):
-    fail_msg = fail_msg or ''
-    proc = subprocess.Popen([gsutil_path] + args, stdout=subprocess.PIPE,
-                            stderr=subprocess.STDOUT)
-    output = proc.communicate()[0].rstrip()
-    if proc.returncode or not output:
-      _Die(fail_msg + ' Process output:\n%s' % output)
-    return output
-
-  # Fails if gsutil isn't configured.
-  gsutil_cmd(['version'],
-             'gsutil error. Please file a bug in Tools>BinarySize.')
-  dl_dst = os.path.join(dl_dir, archive.rev)
-  logging.info('Downloading build artifacts for %s', archive.rev)
-
-  # Fails if archive isn't found.
-  output = gsutil_cmd(['stat', build.DownloadUrl(archive.rev)],
-      'Unexpected error while downloading %s. It may no longer exist on the '
-      'server or it may not have been uploaded yet (check %s). Otherwise, you '
-      'may not have the correct access permissions.' % (
-      build.DownloadUrl(archive.rev), build.builder_url))
-  size = re.search(r'Content-Length:\s+([0-9]+)', output).group(1)
-  logging.info('File size: %s', _ReadableBytes(int(size)))
-
-  # Download archive. Any failures here are unexpected.
-  gsutil_cmd(['cp', build.DownloadUrl(archive.rev), dl_dst])
-
-  # Files needed for supersize and resource_sizes. Paths relative to out dir.
-  to_extract = [build.main_lib_path, build.map_file_path, 'args.gn']
-  if build.IsAndroid():
-    to_extract += ['build_vars.txt', build.apk_path,
-                   build.mapping_path, build.apk_path + '.size']
-  extract_dir = dl_dst + '_' + 'unzipped'
-  logging.info('Extracting build artifacts')
-  with zipfile.ZipFile(dl_dst, 'r') as z:
-    dl_out = _ExtractFiles(to_extract, extract_dir, z)
-    build.output_directory, output_directory = dl_out, build.output_directory
-    archive.ArchiveBuildResults(supersize_path)
-    build.output_directory = output_directory
-
-
-def _ReadableBytes(b):
-  val = b
-  units = ['Bytes','KB', 'MB', 'GB']
-  for unit in units:
-    if val < 1024:
-      return '%.2f %s' % (val, unit)
-    val /= 1024.0
-  else:
-    return '%d %s' % (b, 'Bytes')
-
-
-def _ExtractFiles(to_extract, dst, z):
-  """Extract a list of files. Returns the common prefix of the extracted files.
-
-  Paths in |to_extract| should be relative to the output directory.
-  """
-  zipped_paths = z.namelist()
-  output_dir = os.path.commonprefix(zipped_paths)
-  for f in to_extract:
-    path = os.path.join(output_dir, f)
-    if path in zipped_paths:
-      z.extract(path, path=dst)
-  return os.path.join(dst, output_dir)
-
-
 def _WriteToFile(logfile, s, *args, **kwargs):
   if isinstance(s, basestring):
     data = s.format(*args, **kwargs) + '\n'
@@ -918,24 +810,17 @@ def main():
                       help='Run some extra steps that take longer to complete. '
                            'This includes apk-patch-size estimation and '
                            'static-initializer counting.')
-  parser.add_argument('--cloud',
-                      action='store_true',
-                      help='Download build artifacts from perf builders '
-                      '(Googlers only).')
   parser.add_argument('--single',
                       action='store_true',
                       help='Sets --reference-rev=rev.')
   parser.add_argument('--unstripped',
                       action='store_true',
                       help='Save the unstripped native library when archiving.')
-  parser.add_argument('--depot-tools-path',
-                      help='Custom path to depot tools. Needed for --cloud if '
-                           'depot tools isn\'t in your PATH.')
-  parser.add_argument('--subrepo',
+  parser.add_argument(
+      '--subrepo',
       help='Specify a subrepo directory to use. Implies '
       '--no-gclient. All git commands will be executed '
-                           'from the subrepo directory. Does not work with '
-                           '--cloud.')
+      'from the subrepo directory.')
   parser.add_argument('--no-gclient',
                       action='store_true',
                       help='Do not perform gclient sync steps.')
@@ -995,18 +880,9 @@ def main():
   log_level = logging.DEBUG if args.verbose else logging.INFO
   logging.basicConfig(level=log_level,
                       format='%(levelname).1s %(relativeCreated)6d %(message)s')
-  build = _BuildHelper(args)
-  if build.IsCloud():
-    if args.subrepo:
-      parser.error('--subrepo doesn\'t work with --cloud')
-    if build.IsLinux():
-      parser.error('--target-os linux doesn\'t work with --cloud because map '
-                   'files aren\'t generated by builders (crbug.com/716209).')
-    if args.extra_rev:
-      parser.error('--apply-patch doesn\'t work with --cloud')
 
+  build = _BuildHelper(args)
   subrepo = args.subrepo or _SRC_ROOT
-  if not build.IsCloud():
   _EnsureDirectoryClean(subrepo)
   _SetRestoreFunc(subrepo)
 
@@ -1032,16 +908,11 @@ def main():
     i = 0
     for i, archive in enumerate(diff_mngr.build_archives):
       if archive.Exists():
-        step = 'download' if build.IsCloud() else 'build'
-        logging.info('Found matching metadata for %s, skipping %s step.',
-                     archive.rev, step)
-      else:
-        if build.IsCloud():
-          _DownloadBuildArtifacts(
-              archive, build, supersize_path, args.depot_tools_path)
+        logging.info('Found matching metadata for %s, skipping build step.',
+                     archive.rev)
       else:
-          build_failure = _SyncAndBuild(archive, build, subrepo,
-                                        args.no_gclient, args.extra_rev)
+        build_failure = _SyncAndBuild(archive, build, subrepo, args.no_gclient,
+                                      args.extra_rev)
         if build_failure:
           logging.info(
               'Build failed for %s, diffs using this rev will be skipped.',