Use CLs more consistently instead of branch names

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=300132

Review-Url: https://codereview.chromium.org/1893563002

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@300673 0039d316-1c4b-4281-b951-d872f2087c98

git_cl.py

@@ -18,11 +18,9 @@ import logging
 import multiprocessing
 import optparse
 import os
-import Queue
 import re
 import stat
 import sys
-import tempfile
 import textwrap
 import time
 import traceback
@@ -132,7 +130,7 @@ def RunGitWithCode(args, suppress_stderr=False):
 
 
 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]
 
 
@@ -929,6 +927,7 @@ class Changelist(object):
 
     self.branchref = branchref
     if self.branchref:
+      assert branchref.startswith('refs/heads/')
       self.branch = ShortBranchName(self.branchref)
     else:
       self.branch = None
@@ -1449,7 +1448,7 @@ class Changelist(object):
   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)
 
 
@@ -1935,7 +1934,7 @@ class _RietveldChangelistImpl(_ChangelistCodereviewBase):
       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 '
@@ -1943,7 +1942,7 @@ class _RietveldChangelistImpl(_ChangelistCodereviewBase):
           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()
@@ -2925,21 +2924,9 @@ def color_for_status(status):
     '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(
-    branches, fine_grained, max_processes=None, auth_config=None):
-  """Returns a blocking iterable of (branch, issue, status) for given branches.
+def get_cl_statuses(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.
@@ -2950,50 +2937,41 @@ def get_cl_statuses(
 
   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:
-
-      yield fetch(branches[0])
+    if changes:
+      fetch = lambda cl: (cl, cl.GetStatus())
+      yield fetch(changes[0])
 
-      branches_to_fetch = branches[1:]
+      changes_to_fetch = changes[1:]
       pool = ThreadPool(
-          min(max_processes, len(branches_to_fetch))
+          min(max_processes, len(changes_to_fetch))
               if max_processes is not None
-              else len(branches_to_fetch))
+              else len(changes_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):
@@ -3146,25 +3124,27 @@ def CMDstatus(parser, args):
     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:
@@ -3172,8 +3152,8 @@ def CMDstatus(parser, args):
       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

git_map_branches.py

@@ -128,17 +128,19 @@ class BranchMapper(object):
         include_tracking_status=self.verbosity >= 1)
     if (self.verbosity >= 2):
       # Avoid heavy import unless necessary.
-      from git_cl import get_cl_statuses, color_for_status
+      from git_cl import get_cl_statuses, color_for_status, Changelist
 
-      status_info = get_cl_statuses(self.__branches_info.keys(),
+      change_cls = [Changelist(branchref='refs/heads/'+b)
+                    for b in self.__branches_info.keys() if b]
+      status_info = get_cl_statuses(change_cls,
                                     fine_grained=self.verbosity > 2,
                                     max_processes=self.maxjobs)
 
-      for _ in xrange(len(self.__branches_info)):
-        # This is a blocking get which waits for the remote CL status to be
-        # retrieved.
-        (branch, url, status) = status_info.next()
-        self.__status_info[branch] = (url, color_for_status(status))
+      # This is a blocking get which waits for the remote CL status to be
+      # retrieved.
+      for cl, status in status_info:
+        self.__status_info[cl.GetBranch()] = (cl.GetIssueURL(),
+                                              color_for_status(status))
 
     roots = set()
 
@@ -258,9 +260,9 @@ class BranchMapper(object):
 
     # The Rietveld issue associated with the branch.
     if self.verbosity >= 2:
-      none_text = '' if self.__is_invalid_parent(branch) else 'None'
-      (url, color) = self.__status_info[branch]
-      line.append(url or none_text, color=color)
+      (url, color) = ('', '') if self.__is_invalid_parent(branch) \
+                              else self.__status_info[branch]
+      line.append(url or '', color=color)
 
     # The subject of the most recent commit on the branch.
     if self.show_subject: