diagnose_bloat.py: Add features for large rolls.

This CL adds the --step flag to make debugging large rolls more
efficient.

Also adds the --apply-patch flag to give users the ability to apply
local patches before performing each build (not compatible with
--cloud). Note that this doesn't work for patches to supersize; we copy
supersize to a temp dir before modifying local state.

Side note: using diagnose_bloat.py with CLs before and after the blink
rename require "git config merge.renamelimit 20000".

Bug: 831601
Change-Id: Ida5d0a879355e48c942dab07afd5489c36bca1ca
Reviewed-on: https://chromium-review.googlesource.com/1011681Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: Eric Stevenson <estevenson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550714}

tools/binary_size/diagnose_bloat.py

@@ -197,16 +197,17 @@ class ResourceSizesDiff(BaseDiff):
 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
     self.max_jobs = args.max_jobs
     self.max_load_average = args.max_load_average
     self.output_directory = args.output_directory
     self.target = args.target
     self.target_os = args.target_os
     self.use_goma = args.use_goma
-    self.clean = args.clean
     self._SetDefaults()
 
   @property
@@ -508,6 +509,7 @@ class _Metadata(object):
     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,
@@ -521,19 +523,11 @@ class _Metadata(object):
     }
 
   def Exists(self):
-    old_metadata = {}
     path = self.data['path']
     if os.path.exists(path):
       with open(path, 'r') as f:
-        old_metadata = json.load(f)
-        # For local builds, all keys need to be the same. Differing GN args will
-        # make diffs noisy and inaccurate. GN args do not matter for --cloud
-        # since we download prebuilt build artifacts.
-        keys = self.data.keys()
-        if self.is_cloud:
-          keys.remove('gn_args')
-        return all(v == old_metadata[k]
-                   for k, v in self.data.iteritems() if k in keys)
+        return self.data == json.load(f)
+    return False
 
   def Write(self):
     with open(self.data['path'], 'w') as f:
@@ -587,10 +581,9 @@ def _GclientSyncCmd(rev, subrepo):
   return retcode
 
 
-def _SyncAndBuild(archive, build, subrepo, no_gclient):
+def _SyncAndBuild(archive, build, subrepo, no_gclient, extra_rev):
   """Sync, build and return non 0 if any commands failed."""
   # Simply do a checkout if subrepo is used.
-  retcode = 0
   if _CurrentGitHash(subrepo) == archive.rev:
     if subrepo != _SRC_ROOT:
       logging.info('Skipping git checkout since already at desired rev')
@@ -603,11 +596,26 @@ def _SyncAndBuild(archive, build, subrepo, no_gclient):
     # commits on a branch.
     _GitCmd(['checkout', '--detach'], subrepo)
     logging.info('Syncing to %s', archive.rev)
-    retcode = _GclientSyncCmd(archive.rev, subrepo)
-  return retcode or build.Run()
+    if _GclientSyncCmd(archive.rev, subrepo):
+      return False
+  with _ApplyPatch(extra_rev, subrepo):
+    return build.Run()
+
+
+@contextmanager
+def _ApplyPatch(rev, subrepo):
+  if not rev:
+    yield
+  else:
+    restore_func = _GenRestoreFunc(subrepo)
+    try:
+      _GitCmd(['cherry-pick', rev, '--strategy-option', 'theirs'], subrepo)
+      yield
+    finally:
+      restore_func()
 
 
-def _GenerateRevList(rev, reference_rev, all_in_range, subrepo):
+def _GenerateRevList(rev, reference_rev, all_in_range, subrepo, step):
   """Normalize and optionally generate a list of commits in the given range.
 
   Returns:
@@ -616,17 +624,21 @@ def _GenerateRevList(rev, reference_rev, all_in_range, subrepo):
   rev_seq = '%s^..%s' % (reference_rev, rev)
   stdout = _GitCmd(['rev-list', rev_seq], subrepo)
   all_revs = stdout.splitlines()[::-1]
-  if all_in_range or len(all_revs) < 2:
+  if all_in_range or len(all_revs) < 2 or step:
     revs = all_revs
+    if step:
+      revs = revs[::step]
   else:
     revs = [all_revs[0], all_revs[-1]]
-  if len(revs) >= _COMMIT_COUNT_WARN_THRESHOLD:
+  num_revs = len(revs)
+  if num_revs >= _COMMIT_COUNT_WARN_THRESHOLD:
     _VerifyUserAccepts(
-        'You\'ve provided a commit range that contains %d commits.' % len(revs))
+        'You\'ve provided a commit range that contains %d commits.' % num_revs)
+  logging.info('Processing %d commits', num_revs)
   return revs
 
 
-def _ValidateRevs(rev, reference_rev, subrepo):
+def _ValidateRevs(rev, reference_rev, subrepo, extra_rev):
   def git_fatal(args, message):
     devnull = open(os.devnull, 'wb')
     retcode = subprocess.call(
@@ -638,9 +650,10 @@ def _ValidateRevs(rev, reference_rev, subrepo):
                     'date, try "git fetch origin master"')
   git_fatal(['cat-file', '-e', rev], no_obj_message % rev)
   git_fatal(['cat-file', '-e', reference_rev], no_obj_message % reference_rev)
+  if extra_rev:
+    git_fatal(['cat-file', '-e', extra_rev], no_obj_message % extra_rev)
   git_fatal(['merge-base', '--is-ancestor', reference_rev, rev],
             'reference-rev is newer than rev')
-  return rev, reference_rev
 
 
 def _VerifyUserAccepts(message):
@@ -781,7 +794,7 @@ def _CurrentGitHash(subrepo):
   return _GitCmd(['rev-parse', 'HEAD'], subrepo)
 
 
-def _SetRestoreFunc(subrepo):
+def _GenRestoreFunc(subrepo):
   branch = _GitCmd(['rev-parse', '--abbrev-ref', 'HEAD'], subrepo)
   # Happens when the repo didn't start on a named branch.
   if branch == 'HEAD':
@@ -789,7 +802,11 @@ def _SetRestoreFunc(subrepo):
   def _RestoreFunc():
     logging.warning('Restoring original git checkout')
     _GitCmd(['checkout', branch], subrepo)
-  atexit.register(_RestoreFunc)
+  return _RestoreFunc
+
+
+def _SetRestoreFunc(subrepo):
+  atexit.register(_GenRestoreFunc(subrepo))
 
 
 def main():
@@ -818,7 +835,7 @@ def main():
                       '(Googlers only).')
   parser.add_argument('--single',
                       action='store_true',
-                      help='Sets --reference-rev=rev')
+                      help='Sets --reference-rev=rev.')
   parser.add_argument('--unstripped',
                       action='store_true',
                       help='Save the unstripped native library when archiving.')
@@ -833,11 +850,18 @@ def main():
   parser.add_argument('--no-gclient',
                       action='store_true',
                       help='Do not perform gclient sync steps.')
+  parser.add_argument('--apply-patch', dest='extra_rev',
+                      help='A local commit to cherry-pick before each build. '
+                           'This can leave your repo in a broken state if '
+                           'the cherry-pick fails.')
+  parser.add_argument('--step', type=int,
+                      help='Assumes --all and only builds/downloads every '
+                           '--step\'th revision.')
   parser.add_argument('-v',
                       '--verbose',
                       action='store_true',
-                      help='Show  commands executed, extra debugging output'
-                           ', and Ninja/GN output')
+                      help='Show commands executed, extra debugging output'
+                           ', and Ninja/GN output.')
 
   build_group = parser.add_argument_group('build arguments')
   build_group.add_argument('-j',
@@ -873,7 +897,7 @@ def main():
                            help='GN target to build. Linux default: chrome. '
                                 'Android default: monochrome_public_apk or '
                                 'monochrome_apk (depending on '
-                                '--enable-chrome-android-internal)')
+                                '--enable-chrome-android-internal).')
   if len(sys.argv) == 1:
     parser.print_help()
     sys.exit()
@@ -888,6 +912,8 @@ def main():
     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')
 
   subrepo = args.subrepo or _SRC_ROOT
   if not build.IsCloud():
@@ -900,8 +926,8 @@ def main():
   reference_rev = args.reference_rev or args.rev + '^'
   if args.single:
     reference_rev = args.rev
-  rev, reference_rev = _ValidateRevs(args.rev, reference_rev, subrepo)
-  revs = _GenerateRevList(rev, reference_rev, args.all, subrepo)
+  _ValidateRevs(args.rev, reference_rev, subrepo, args.extra_rev)
+  revs = _GenerateRevList(args.rev, reference_rev, args.all, subrepo, args.step)
   with _TmpCopyBinarySizeDir() as supersize_path:
     diffs = [NativeDiff(build.size_name, supersize_path)]
     if build.IsAndroid():
@@ -923,7 +949,7 @@ def main():
               archive, build, supersize_path, args.depot_tools_path)
         else:
           build_failure = _SyncAndBuild(archive, build, subrepo,
-                                        args.no_gclient)
+                                        args.no_gclient, args.extra_rev)
           if build_failure:
             logging.info(
                 'Build failed for %s, diffs using this rev will be skipped.',