[depot_tools] New "git cl upload" flag to traverse dependent branches and re-upload them

git_cl.py

 #!/usr/bin/env python
 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 # Copyright (C) 2008 Evan Martin <martine@danga.com>
 
 """A git-command for integrating reviews on Rietveld."""
 
 from distutils.version import LooseVersion
@@ skipping 1487 matching lines @@
               else len(branches_to_fetch))
       for x in pool.imap_unordered(fetch, branches_to_fetch):
         yield x
   else:
     # Do not use GetApprovingReviewers(), since it requires an HTTP request.
     for b in branches:
       c = Changelist(branchref=b, auth_config=auth_config)
       url = c.GetIssueURL()
       yield (b, url, Fore.BLUE if url else Fore.WHITE)
 
+
+def CMDupload_branch_deps(unused_parser, args):
+  """Uploads CLs of local branches that are dependents of the current branch.
+
+  If the local branch dependency tree looks like:
+  test1 -> test2.1 -> test3.1
+                   -> test3.2
+        -> test2.2 -> test3.3
+
+  and you run this command from test1 then "git cl upload" is run on the
+  dependent branches in this order:
+  test2.1, test3.1, test3.2, test2.2, test3.3
+
+  Note: This command does not rebase your local dependent branches. Use it when
+        you make a change to the parent branch that will not conflict with its
+        dependent branches, and you would like their dependencies updated in
+        Rietveld.

[nodir, 2015/06/15 23:50:22]

Rebasing would be very nice, for example if I want to test a change locally
against code in another CL. Do you plan to provide this functionality as an
option or a separate command?

[iannucci, 2015/06/15 23:55:12]

git rebase-update can/does already do this.

[nodir, 2015/06/15 23:58:43]

Acknowledged.

+  """
+
+  branchref = RunGit(['symbolic-ref', 'HEAD'],
+                     stderr=subprocess2.VOID, error_ok=True).strip()
+  parent_branch = ShortBranchName(branchref)

[nodir, 2015/06/15 23:50:21]

nit: call it root_branch?

[rmistry, 2015/06/17 17:58:46]

Done.

+  if parent_branch is None:
+      DieWithError('Can\'t find dependent branches from detached HEAD state. '
+                   'Get on a branch!')
+
+  branches = RunGit(['for-each-ref',
+                     '--format=%(refname:short) %(upstream:short)',
+                     'refs/heads'])
+  if not branches:
+    print('No local branches found.')
+    return 0
+
+  # Create a dictionary of all local branches to the branches that are dependent
+  # on it.
+  tracked_to_dependents = {}

[nodir, 2015/06/15 23:50:21]

collections.defaultdict([])
lines 1546-1547 can be removed

[rmistry, 2015/06/17 17:58:45]

Cool. Done.

+  for b in branches.splitlines():
+    branch_name, tracked = b.split()

[nodir, 2015/06/15 23:50:22]

this will crash on a non-tracked branch: b.split() will return a list of one
element

[rmistry, 2015/06/17 17:58:45]

Done.

+    if tracked not in tracked_to_dependents:
+      tracked_to_dependents[tracked] = []
+    tracked_to_dependents[tracked].append(branch_name)

[iannucci, 2015/06/15 23:55:12]

It would be worth consolidating this branch hierarchy calculation with the same
calculation which is done in rebase-update. I can't remember if there's already
a function that does this in git_common, or if it's implemented in
git_rebase_update directly.

[rmistry, 2015/06/17 17:58:45]

I looked at git_rebase_update and tried using some of the git_common methods it
used like:
git_common.get_branch_tree and git_common.topo_iter(branch_tree)

I prefer the code here because it is specific and optimized for this particular
usecase.
get_branch_tree and topo_iter would work for this if we could specify a root
instead of it considering all roots, but I could not find a clean way to do this
in the existing implementation without rewrites (which I would like to avoid).

+
+  print
+  print 'The dependent local branches of %s are:' % parent_branch
+  inorder_dependents = []

[nodir, 2015/06/15 23:50:22]

nit: "inorder" is somewhat confusing because it is about a different type of
traversal, see https://en.wikipedia.org/wiki/Tree_traversal 
I think what you meant is topological order
https://en.wikipedia.org/wiki/Topological_sorting

[rmistry, 2015/06/17 17:58:46]

Oops. Yea I meant preorder here.

                          test2
              test3               test3.1
          test4   test4.1
       test5


prints ['test2', 'test3', 'test4', 'test5', 'test4.1', 'test3.1']

+  def traverse_dependents_inorder(branch, padding=''):
+    dependents_to_process = tracked_to_dependents.get(branch, [])
+    padding += '  '
+    for dependent in dependents_to_process:
+      print '%s%s' % (padding, dependent)
+      inorder_dependents.append(dependent)
+      traverse_dependents_inorder(dependent, padding)
+  traverse_dependents_inorder(parent_branch)
+  print
+
+  print ('This command will checkout all dependent branches and run '
+         '"git cl upload".')
+  ask_for_data('[Press enter to dcommit or ctrl-C to quit]')
+
+  # Go through all dependents, checkout the branch and upload.
+  if inorder_dependents:

[nodir, 2015/06/15 23:50:22]

inverse the condition, return early and deindent the rest of code in this func

[rmistry, 2015/06/17 17:58:45]

Done.

+    try:
+      for dependent_branch in inorder_dependents:
+        print
+        print '--------------------------------------'
+        print 'Running "git cl upload" from %s:' % dependent_branch
+        RunGit(['checkout', '-q', dependent_branch])

[nodir, 2015/06/15 23:50:22]

can CMDupload_branch_deps be called when the work tree is dirty?

[rmistry, 2015/06/17 17:58:45]

At this point you have checked out a branch so AFAIK the work tree there cannot
be dirty because you were out of that branch originally.

But the root branch could be dirty, I added a check for that.

+        print
+        if CMDupload(OptionParser(), args) != 0:

[nodir, 2015/06/15 23:50:22]

Will it ask for pathset title every time? 
I think this tool should give a standard title, like "Updated patchset
dependency 123/456"

[rmistry, 2015/06/17 17:58:45]

I wanted to do this when I started this CL but forgot to.
Thanks for the reminder! Done.

+          print 'Upload failed for %s!' % dependent_branch

[nodir, 2015/06/15 23:50:21]

I think users would thank you if you check that the CL being updated already
depends on the latest patchset. For example, if upload of one of patchsets
failed, they wouldn't have to upload identical patchsets again

[rmistry, 2015/06/17 17:58:45]

Yea I think that is an optimization that can be made in the future based on
feedback.
Here I prefer to upload patches instead of adding more checks and having to deal
with more edge cases (it does not hurt to upload many patches as discussed
before).

+          return -1
+        print
+    finally:
+      # Swap back to the original parent branch.
+      RunGit(['checkout', '-q', parent_branch])
+
+    print
+    print 'All dependent branches %s have been updated!' % inorder_dependents
+    print
+  else:
+    print 'There are no dependent local branches for %s' % parent_branch
+
+  return 0
+
+
 def CMDstatus(parser, args):
   """Show status of changelists.
 
   Colors are used to tell the state of the CL unless --fast is used:
     - Red      not sent for review or broken
     - Blue     waiting for review
     - Yellow   waiting for you to reply to review
     - Green    LGTM'ed
     - Magenta  in the commit queue
     - Cyan     was committed, branch can be deleted
@@ skipping 1847 matching lines @@
 if __name__ == '__main__':
   # These affect sys.stdout so do it outside of main() to simplify mocks in
   # unit testing.
   fix_encoding.fix_encoding()
   colorama.init()
   try:
     sys.exit(main(sys.argv[1:]))
   except KeyboardInterrupt:
     sys.stderr.write('interrupted\n')
     sys.exit(1)