Use CLs more consistently instead of branch names

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 and Gerrit."""
 
 from distutils.version import LooseVersion
 from multiprocessing.pool import ThreadPool
 import base64
 import collections
 import glob
 import httplib
 import json
 import logging
 import multiprocessing
 import optparse
 import os
-import Queue
 import re
 import stat
 import sys
-import tempfile
 import textwrap
 import time
 import traceback
 import urllib
 import urllib2
 import urlparse
 import uuid
 import webbrowser
 import zlib
 
@@ skipping 89 matching lines @@
                                         stdout=subprocess2.PIPE,
                                         stderr=stderr)
     return code, out[0]
   except ValueError:
     # When the subprocess fails, it returns None.  That triggers a ValueError
     # when trying to unpack the return value into (out, code).
     return 1, ''
 
 
 def RunGitSilent(args):
-  """Returns stdout, suppresses stderr and ingores the return code."""
+  """Returns stdout, suppresses stderr and ignores the return code."""
   return RunGitWithCode(args, suppress_stderr=True)[1]
 
 
 def IsGitVersionAtLeast(min_version):
   prefix = 'git version '
   version = RunGit(['--version']).strip()
   return (version.startswith(prefix) and
       LooseVersion(version[len(prefix):]) >= LooseVersion(min_version))
 
 
@@ skipping 776 matching lines @@
     global settings
     if not settings:
       # Happens when git_cl.py is used as a utility library.
       settings = Settings()
 
     if issue:
       assert codereview, 'codereview must be known, if issue is known'
 
     self.branchref = branchref
     if self.branchref:
+      assert branchref.startswith('refs/heads/')
       self.branch = ShortBranchName(self.branchref)
     else:
       self.branch = None
     self.upstream_branch = None
     self.lookedup_issue = False
     self.issue = issue or None
     self.has_description = False
     self.description = None
     self.lookedup_patchset = False
     self.patchset = None
@@ skipping 500 matching lines @@
 
   def GetApprovingReviewers(self):
     return self._codereview_impl.GetApprovingReviewers()
 
   def GetMostRecentPatchset(self):
     return self._codereview_impl.GetMostRecentPatchset()
 
   def __getattr__(self, attr):
     # This is because lots of untested code accesses Rietveld-specific stuff
     # directly, and it's hard to fix for sure. So, just let it work, and fix
-    # on a cases by case basis.
+    # on a case by case basis.
     return getattr(self._codereview_impl, attr)
 
 
 class _ChangelistCodereviewBase(object):
   """Abstract base class encapsulating codereview specifics of a changelist."""
   def __init__(self, changelist):
     self._changelist = changelist  # instance of Changelist
 
   def __getattr__(self, attr):
     # Forward methods to changelist.
@@ skipping 465 matching lines @@
       target_ref = GetTargetRef(remote, remote_branch, options.target_branch,
                                 settings.GetPendingRefPrefix())
       if target_ref:
         upload_args.extend(['--target_ref', target_ref])
 
       # Look for dependent patchsets. See crbug.com/480453 for more details.
       remote, upstream_branch = self.FetchUpstreamTuple(self.GetBranch())
       upstream_branch = ShortBranchName(upstream_branch)
       if remote is '.':
         # A local branch is being tracked.
-        local_branch = ShortBranchName(upstream_branch)
+        local_branch = upstream_branch
         if settings.GetIsSkipDependencyUpload(local_branch):
           print
           print ('Skipping dependency patchset upload because git config '
                  'branch.%s.skip-deps-uploads is set to True.' % local_branch)
           print
         else:
           auth_config = auth.extract_auth_config_from_options(options)
-          branch_cl = Changelist(branchref=local_branch,
+          branch_cl = Changelist(branchref='refs/heads/'+local_branch,
                                  auth_config=auth_config)
           branch_cl_issue_url = branch_cl.GetIssueURL()
           branch_cl_issue = branch_cl.GetIssue()
           branch_cl_patchset = branch_cl.GetPatchset()
           if branch_cl_issue_url and branch_cl_issue and branch_cl_patchset:
             upload_args.extend(
                 ['--depends_on_patchset', '%s:%s' % (
                      branch_cl_issue, branch_cl_patchset)])
             print(
                 '\n'
@@ skipping 955 matching lines @@
   return {
     'unsent': Fore.RED,
     'waiting': Fore.BLUE,
     'reply': Fore.YELLOW,
     'lgtm': Fore.GREEN,
     'commit': Fore.MAGENTA,
     'closed': Fore.CYAN,
     'error': Fore.WHITE,
   }.get(status, Fore.WHITE)
 
-def fetch_cl_status(branch, auth_config=None):
-  """Fetches information for an issue and returns (branch, issue, status)."""
-  cl = Changelist(branchref=branch, auth_config=auth_config)
-  url = cl.GetIssueURL()
-  status = cl.GetStatus()
-
-  if url and (not status or status == 'error'):
-    # The issue probably doesn't exist anymore.
-    url += ' (broken)'
-
-  return (branch, url, status)
 
 def get_cl_statuses(

[tandrii(chromium), 2016/05/20 20:56:06]

nit: merge two lines.

-    branches, fine_grained, max_processes=None, auth_config=None):
-  """Returns a blocking iterable of (branch, issue, status) for given branches.
+    changes, fine_grained, max_processes=None):
+  """Returns a blocking iterable of (cl, status) for given branches.
 
   If fine_grained is true, this will fetch CL statuses from the server.
   Otherwise, simply indicate if there's a matching url for the given branches.
 
   If max_processes is specified, it is used as the maximum number of processes
   to spawn to fetch CL status from the server. Otherwise 1 process per branch is
   spawned.
 
   See GetStatus() for a list of possible statuses.
   """
-  def fetch(branch):
-    if not branch:
-      return None
-
-    return fetch_cl_status(branch, auth_config=auth_config)
-
   # Silence upload.py otherwise it becomes unwieldly.
   upload.verbosity = 0
 
   if fine_grained:
     # Process one branch synchronously to work through authentication, then
     # spawn processes to process all the other branches in parallel.
-    if branches:
+    if changes:
+      fetch = lambda cl: (cl, cl.GetStatus())
+      yield fetch(changes[0])
 
-      yield fetch(branches[0])
+      changes_to_fetch = changes[1:]
+      pool = ThreadPool(
+          min(max_processes, len(changes_to_fetch))
+              if max_processes is not None
+              else len(changes_to_fetch))
 
-      branches_to_fetch = branches[1:]
-      pool = ThreadPool(
-          min(max_processes, len(branches_to_fetch))
-              if max_processes is not None
-              else len(branches_to_fetch))
-
-      fetched_branches = set()
-      it = pool.imap_unordered(fetch, branches_to_fetch).__iter__()
+      fetched_cls = set()
+      it = pool.imap_unordered(fetch, changes_to_fetch).__iter__()
       while True:
         try:
           row = it.next(timeout=5)
         except multiprocessing.TimeoutError:
           break
 
-        fetched_branches.add(row[0])
+        fetched_cls.add(row[0])
         yield row
 
       # Add any branches that failed to fetch.
-      for b in set(branches_to_fetch) - fetched_branches:
-        cl = Changelist(branchref=b, auth_config=auth_config)
-        yield (b, cl.GetIssueURL() if b else None, 'error')
+      for cl in set(changes_to_fetch) - fetched_cls:
+        yield (cl, 'error')
 
   else:
     # Do not use GetApprovingReviewers(), since it requires an HTTP request.
-    for b in branches:
-      cl = Changelist(branchref=b, auth_config=auth_config)
-      url = cl.GetIssueURL() if b else None
-      yield (b, url, 'waiting' if url else 'error')
+    for cl in changes:
+      yield (cl, 'waiting' if cl.GetIssueURL() else 'error')
 
 
 def upload_branch_deps(cl, 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
 
@@ skipping 132 matching lines @@
       url = cl.GetIssueURL()
       if url:
         print url
     return 0
 
   branches = RunGit(['for-each-ref', '--format=%(refname)', 'refs/heads'])
   if not branches:
     print('No local branch found.')
     return 0
 
-  changes = (
+  changes = [
       Changelist(branchref=b, auth_config=auth_config)
-      for b in branches.splitlines())
-  # TODO(tandrii): refactor to use CLs list instead of branches list.
-  branches = [c.GetBranch() for c in changes]
-  alignment = max(5, max(len(b) for b in branches))
+      for b in branches.splitlines()]
   print 'Branches associated with reviews:'
-  output = get_cl_statuses(branches,
+  output = get_cl_statuses(changes,
                            fine_grained=not options.fast,
-                           max_processes=options.maxjobs,
-                           auth_config=auth_config)
+                           max_processes=options.maxjobs)
 
   branch_statuses = {}
-  alignment = max(5, max(len(ShortBranchName(b)) for b in branches))
-  for branch in sorted(branches):
+  alignment = max(5, max(len(ShortBranchName(c.GetBranch())) for c in changes))
+  for cl in sorted(changes, key=lambda c: c.GetBranch()):
+    branch = cl.GetBranch()
     while branch not in branch_statuses:
-      b, i, status = output.next()
-      branch_statuses[b] = (i, status)
-    issue_url, status = branch_statuses.pop(branch)
+      c, status = output.next()
+      branch_statuses[c.GetBranch()] = status
+    status = branch_statuses.pop(branch)
+    url = cl.GetIssueURL()
+    if url and (not status or status == 'error'):
+      # The issue probably doesn't exist anymore.
+      url += ' (broken)'
+
     color = color_for_status(status)
     reset = Fore.RESET
     if not setup_color.IS_TTY:
       color = ''
       reset = ''
     status_str = '(%s)' % status if status else ''
     print '  %*s : %s%s %s%s' % (
-          alignment, ShortBranchName(branch), color, issue_url, status_str,
-          reset)
+          alignment, ShortBranchName(branch), color, url,
+          status_str, reset)
 
   cl = Changelist(auth_config=auth_config)
   print
   print 'Current branch:',
   print cl.GetBranch()
   if not cl.GetIssue():
     print 'No issue assigned.'
     return 0
   print 'Issue number: %s (%s)' % (cl.GetIssue(), cl.GetIssueURL())
   if not options.fast:
@@ skipping 1720 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()
   setup_color.init()
   try:
     sys.exit(main(sys.argv[1:]))
   except KeyboardInterrupt:
     sys.stderr.write('interrupted\n')
     sys.exit(1)