Commit 1033efd0 authored by maruel@chromium.org's avatar maruel@chromium.org

Add color support to git cl and fetch issue properties in parallel.

Add cache for issue properties.

Add --fast option in case the HTTP requests become an issue.

Everyone loves colors:
- White means no issue is associated with the branch.
- Red means no email was sent to request a review. Also red if the issue doesn't
  exist anymore.
- Blue means the issue was not LGTM'ed yet.
- Green means the issue was LGTM'ed and is likely ready to commit or be CQ'ed.

R=iannucci@chromium.org
BUG=

Review URL: https://chromiumcodereview.appspot.com/19967004

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@213265 0039d316-1c4b-4281-b951-d872f2087c98
parent e25c75b0
...@@ -13,10 +13,12 @@ import json ...@@ -13,10 +13,12 @@ import json
import logging import logging
import optparse import optparse
import os import os
import Queue
import re import re
import stat import stat
import sys import sys
import textwrap import textwrap
import threading
import urllib2 import urllib2
import urlparse import urlparse
...@@ -412,7 +414,7 @@ def ShortBranchName(branch): ...@@ -412,7 +414,7 @@ def ShortBranchName(branch):
class Changelist(object): class Changelist(object):
def __init__(self, branchref=None): def __init__(self, branchref=None, issue=None):
# Poke settings so we get the "configure your server" message if necessary. # Poke settings so we get the "configure your server" message if necessary.
global settings global settings
if not settings: if not settings:
...@@ -426,16 +428,17 @@ class Changelist(object): ...@@ -426,16 +428,17 @@ class Changelist(object):
self.branch = None self.branch = None
self.rietveld_server = None self.rietveld_server = None
self.upstream_branch = None self.upstream_branch = None
self.has_issue = False self.lookedup_issue = False
self.issue = None self.issue = issue or None
self.has_description = False self.has_description = False
self.description = None self.description = None
self.has_patchset = False self.lookedup_patchset = False
self.patchset = None self.patchset = None
self._rpc_server = None self._rpc_server = None
self.cc = None self.cc = None
self.watchers = () self.watchers = ()
self._remote = None self._remote = None
self._props = None
def GetCCList(self): def GetCCList(self):
"""Return the users cc'd on this CL. """Return the users cc'd on this CL.
...@@ -601,13 +604,10 @@ or verify this branch is set up to track another (via the --track argument to ...@@ -601,13 +604,10 @@ or verify this branch is set up to track another (via the --track argument to
def GetIssue(self): def GetIssue(self):
"""Returns the issue number as a int or None if not set.""" """Returns the issue number as a int or None if not set."""
if not self.has_issue: if self.issue is None and not self.lookedup_issue:
issue = RunGit(['config', self._IssueSetting()], error_ok=True).strip() issue = RunGit(['config', self._IssueSetting()], error_ok=True).strip()
if issue: self.issue = int(issue) or None if issue else None
self.issue = int(issue) self.lookedup_issue = True
else:
self.issue = None
self.has_issue = True
return self.issue return self.issue
def GetRietveldServer(self): def GetRietveldServer(self):
...@@ -656,47 +656,53 @@ or verify this branch is set up to track another (via the --track argument to ...@@ -656,47 +656,53 @@ or verify this branch is set up to track another (via the --track argument to
def GetPatchset(self): def GetPatchset(self):
"""Returns the patchset number as a int or None if not set.""" """Returns the patchset number as a int or None if not set."""
if not self.has_patchset: if self.patchset is None and not self.lookedup_patchset:
patchset = RunGit(['config', self._PatchsetSetting()], patchset = RunGit(['config', self._PatchsetSetting()],
error_ok=True).strip() error_ok=True).strip()
if patchset: self.patchset = int(patchset) or None if patchset else None
self.patchset = int(patchset) self.lookedup_patchset = True
else:
self.patchset = None
self.has_patchset = True
return self.patchset return self.patchset
def SetPatchset(self, patchset): def SetPatchset(self, patchset):
"""Set this branch's patchset. If patchset=0, clears the patchset.""" """Set this branch's patchset. If patchset=0, clears the patchset."""
if patchset: if patchset:
RunGit(['config', self._PatchsetSetting(), str(patchset)]) RunGit(['config', self._PatchsetSetting(), str(patchset)])
self.patchset = patchset
else: else:
RunGit(['config', '--unset', self._PatchsetSetting()], RunGit(['config', '--unset', self._PatchsetSetting()],
stderr=subprocess2.PIPE, error_ok=True) stderr=subprocess2.PIPE, error_ok=True)
self.has_patchset = False self.patchset = None
def GetMostRecentPatchset(self, issue): def GetMostRecentPatchset(self):
return self.RpcServer().get_issue_properties( return self.GetIssueProperties()['patchsets'][-1]
int(issue), False)['patchsets'][-1]
def GetPatchSetDiff(self, issue, patchset): def GetPatchSetDiff(self, issue, patchset):
return self.RpcServer().get( return self.RpcServer().get(
'/download/issue%s_%s.diff' % (issue, patchset)) '/download/issue%s_%s.diff' % (issue, patchset))
def GetIssueProperties(self):
if self._props is None:
issue = self.GetIssue()
if not issue:
self._props = {}
else:
self._props = self.RpcServer().get_issue_properties(issue, True)
return self._props
def GetApprovingReviewers(self): def GetApprovingReviewers(self):
return get_approving_reviewers( return get_approving_reviewers(self.GetIssueProperties())
self.RpcServer().get_issue_properties(self.GetIssue(), True))
def SetIssue(self, issue): def SetIssue(self, issue):
"""Set this branch's issue. If issue=0, clears the issue.""" """Set this branch's issue. If issue=0, clears the issue."""
if issue: if issue:
self.issue = issue
RunGit(['config', self._IssueSetting(), str(issue)]) RunGit(['config', self._IssueSetting(), str(issue)])
if self.rietveld_server: if self.rietveld_server:
RunGit(['config', self._RietveldServer(), self.rietveld_server]) RunGit(['config', self._RietveldServer(), self.rietveld_server])
else: else:
RunGit(['config', '--unset', self._IssueSetting()]) RunGit(['config', '--unset', self._IssueSetting()])
self.SetPatchset(0) self.issue = None
self.has_issue = False self.SetPatchset(None)
def GetChange(self, upstream_branch, author): def GetChange(self, upstream_branch, author):
if not self.GitSanityChecks(upstream_branch): if not self.GitSanityChecks(upstream_branch):
...@@ -1064,6 +1070,8 @@ def CMDstatus(parser, args): ...@@ -1064,6 +1070,8 @@ def CMDstatus(parser, args):
"""show status of changelists""" """show status of changelists"""
parser.add_option('--field', parser.add_option('--field',
help='print only specific field (desc|id|patch|url)') help='print only specific field (desc|id|patch|url)')
parser.add_option('-f', '--fast', action='store_true',
help='Do not retrieve review status')
(options, args) = parser.parse_args(args) (options, args) = parser.parse_args(args)
if options.field: if options.field:
...@@ -1093,8 +1101,57 @@ def CMDstatus(parser, args): ...@@ -1093,8 +1101,57 @@ def CMDstatus(parser, args):
branches = dict((c.GetBranch(), c.GetIssueURL()) for c in changes) branches = dict((c.GetBranch(), c.GetIssueURL()) for c in changes)
alignment = max(5, max(len(b) for b in branches)) alignment = max(5, max(len(b) for b in branches))
print 'Branches associated with reviews:' print 'Branches associated with reviews:'
# Adhoc thread pool to request data concurrently.
output = Queue.Queue()
# Silence upload.py otherwise it becomes unweldly.
upload.verbosity = 0
if not options.fast:
def fetch(b):
c = Changelist(branchref=b)
i = c.GetIssueURL()
try:
props = c.GetIssueProperties()
r = c.GetApprovingReviewers() if i else None
if not props.get('messages'):
r = None
except urllib2.HTTPError:
# The issue probably doesn't exist anymore.
i += ' (broken)'
r = None
output.put((b, i, r))
threads = [threading.Thread(target=fetch, args=(b,)) for b in branches]
for t in threads:
t.daemon = True
t.start()
else:
# Do not use GetApprovingReviewers(), since it requires an HTTP request.
for b in branches:
c = Changelist(branchref=b)
output.put((b, c.GetIssue(), None))
tmp = {}
alignment = max(5, max(len(ShortBranchName(b)) for b in branches))
for branch in sorted(branches): for branch in sorted(branches):
print " %*s: %s" % (alignment, branch, branches[branch]) while branch not in tmp:
b, i, r = output.get()
tmp[b] = (i, r)
issue, reviewers = tmp.pop(branch)
if not issue:
color = Fore.WHITE
elif reviewers:
# Was approved.
color = Fore.GREEN
elif reviewers is None:
# No message was sent.
color = Fore.RED
else:
color = Fore.BLUE
print ' %*s: %s%s%s' % (
alignment, ShortBranchName(branch), color, issue, Fore.RESET)
cl = Changelist() cl = Changelist()
print print
print 'Current branch:', print 'Current branch:',
...@@ -1136,7 +1193,7 @@ def CMDcomments(parser, args): ...@@ -1136,7 +1193,7 @@ def CMDcomments(parser, args):
cl = Changelist() cl = Changelist()
if cl.GetIssue(): if cl.GetIssue():
data = cl.RpcServer().get_issue_properties(cl.GetIssue(), True) data = cl.GetIssueProperties()
for message in sorted(data['messages'], key=lambda x: x['date']): for message in sorted(data['messages'], key=lambda x: x['date']):
if message['disapproval']: if message['disapproval']:
color = Fore.RED color = Fore.RED
...@@ -1434,7 +1491,7 @@ def CMDupload(parser, args): ...@@ -1434,7 +1491,7 @@ def CMDupload(parser, args):
options.reviewers = hook_results.reviewers.split(',') options.reviewers = hook_results.reviewers.split(',')
if cl.GetIssue(): if cl.GetIssue():
latest_patchset = cl.GetMostRecentPatchset(cl.GetIssue()) latest_patchset = cl.GetMostRecentPatchset()
local_patchset = cl.GetPatchset() local_patchset = cl.GetPatchset()
if latest_patchset and local_patchset and local_patchset != latest_patchset: if latest_patchset and local_patchset and local_patchset != latest_patchset:
print ('The last upload made from this repository was patchset #%d but ' print ('The last upload made from this repository was patchset #%d but '
...@@ -1673,12 +1730,12 @@ def SendUpstream(parser, args, cmd): ...@@ -1673,12 +1730,12 @@ def SendUpstream(parser, args, cmd):
'(you may be prompted for your codereview password)...') '(you may be prompted for your codereview password)...')
cl.UpdateDescription(change_desc.description) cl.UpdateDescription(change_desc.description)
cl.CloseIssue() cl.CloseIssue()
props = cl.RpcServer().get_issue_properties(cl.GetIssue(), False) props = cl.GetIssueProperties()
patch_num = len(props['patchsets']) patch_num = len(props['patchsets'])
comment = "Committed patchset #%d manually as r%s" % (patch_num, revision) comment = "Committed patchset #%d manually as r%s" % (patch_num, revision)
comment += ' (presubmit successful).' if not options.bypass_hooks else '.' comment += ' (presubmit successful).' if not options.bypass_hooks else '.'
cl.RpcServer().add_comment(cl.GetIssue(), comment) cl.RpcServer().add_comment(cl.GetIssue(), comment)
cl.SetIssue(0) cl.SetIssue(None)
if retcode == 0: if retcode == 0:
hook = POSTUPSTREAM_HOOK_PATTERN % cmd hook = POSTUPSTREAM_HOOK_PATTERN % cmd
...@@ -1737,9 +1794,9 @@ def CMDpatch(parser, args): ...@@ -1737,9 +1794,9 @@ def CMDpatch(parser, args):
if issue_arg.isdigit(): if issue_arg.isdigit():
# Input is an issue id. Figure out the URL. # Input is an issue id. Figure out the URL.
cl = Changelist()
issue = int(issue_arg) issue = int(issue_arg)
patchset = cl.GetMostRecentPatchset(issue) cl = Changelist(issue)
patchset = cl.GetMostRecentPatchset()
patch_data = cl.GetPatchSetDiff(issue, patchset) patch_data = cl.GetPatchSetDiff(issue, patchset)
else: else:
# Assume it's a URL to the patch. Default to https. # Assume it's a URL to the patch. Default to https.
...@@ -1946,7 +2003,7 @@ def CMDtry(parser, args): ...@@ -1946,7 +2003,7 @@ def CMDtry(parser, args):
patchset = cl.GetPatchset() patchset = cl.GetPatchset()
if not cl.GetPatchset(): if not cl.GetPatchset():
patchset = cl.GetMostRecentPatchset(cl.GetIssue()) patchset = cl.GetMostRecentPatchset()
cl.RpcServer().trigger_try_jobs( cl.RpcServer().trigger_try_jobs(
cl.GetIssue(), patchset, options.name, options.clobber, options.revision, cl.GetIssue(), patchset, options.name, options.clobber, options.revision,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment