Commit 45495249 authored by Russ Cox's avatar Russ Cox

code review fixes

* clean up error handling: show Exception info
* white space fixes
* clean up output when creating CL
* simplify hg change command; add hg file
* fix stale cookie bug (thanks iant)
* in LoadAllCL, load each CL in a different thread,
  to parallelize the slow web fetches
* throw away support for Mercurial before version 1.3
* add @CL-number file pattern for commands like diff
* make hg sync show files being sync'ed

R=r
http://go/go-review/1016016
parent b7215331
...@@ -23,22 +23,27 @@ your repository's .hg/hgrc file. ...@@ -23,22 +23,27 @@ your repository's .hg/hgrc file.
codereview = path/to/codereview.py codereview = path/to/codereview.py
[codereview] [codereview]
project = project-url # optional server = codereview.appspot.com
If the project URL is specified, codereview will fetch The server should be running Rietveld; see http://code.google.com/p/rietveld/.
default the reviewer and cc list from that URL each time
it runs an "upload" command.
''' '''
# TODO(rsc):
# fix utf-8 upload bug
# look for and clear submitted CLs during sync / add "adopt" command?
# creating an issue prints the URL twice
# better documentation
from mercurial import cmdutil, commands, hg, util, error, match from mercurial import cmdutil, commands, hg, util, error, match
from mercurial.node import nullrev, hex, nullid, short from mercurial.node import nullrev, hex, nullid, short
import os, re import os, re
import stat import stat
import threading
from HTMLParser import HTMLParser from HTMLParser import HTMLParser
try: try:
hgversion = util.version() hgversion = util.version()
except Exception, e: except:
from mercurial.version import version as v from mercurial.version import version as v
hgversion = v.get_version() hgversion = v.get_version()
...@@ -150,7 +155,7 @@ class CL(object): ...@@ -150,7 +155,7 @@ class CL(object):
] ]
# NOTE(rsc): This duplicates too much of RealMain, # NOTE(rsc): This duplicates too much of RealMain,
# but RealMain doesn't have the nicest interface in the world. # but RealMain doesn't have the most reusable interface.
if self.name != "new": if self.name != "new":
form_fields.append(("issue", self.name)) form_fields.append(("issue", self.name))
vcs = GuessVCS(upload_options) vcs = GuessVCS(upload_options)
...@@ -170,12 +175,14 @@ class CL(object): ...@@ -170,12 +175,14 @@ class CL(object):
msg = lines[0] msg = lines[0]
patchset = lines[1].strip() patchset = lines[1].strip()
patches = [x.split(" ", 1) for x in lines[2:]] patches = [x.split(" ", 1) for x in lines[2:]]
ui.status("uploaded: " + msg + "\n") ui.status(msg + "\n")
if not response_body.startswith("Issue created.") and not response_body.startswith("Issue updated."): if not response_body.startswith("Issue created.") and not response_body.startswith("Issue updated."):
print response_body print response_body
raise "failed to update issue" raise "failed to update issue"
issue = msg[msg.rfind("/")+1:] issue = msg[msg.rfind("/")+1:]
self.name = issue self.name = issue
if not self.url:
self.url = server_url_base + self.name
if not uploaded_diff_file: if not uploaded_diff_file:
patches = UploadSeparatePatches(issue, rpc, patchset, data, upload_options) patches = UploadSeparatePatches(issue, rpc, patchset, data, upload_options)
vcs.UploadBaseFiles(issue, rpc, patches, patchset, upload_options, files) vcs.UploadBaseFiles(issue, rpc, patches, patchset, upload_options, files)
...@@ -244,27 +251,40 @@ def SplitCommaSpace(s): ...@@ -244,27 +251,40 @@ def SplitCommaSpace(s):
def JoinComma(l): def JoinComma(l):
return ", ".join(l) return ", ".join(l)
def ExceptionDetail():
s = str(sys.exc_info()[0])
if s.startswith("<type '") and s.endswith("'>"):
s = s[7:-2]
elif s.startswith("<class '") and s.endswith("'>"):
s = s[8:-2]
arg = str(sys.exc_info()[1])
if len(arg) > 0:
s += ": " + arg
return s
# Load CL from disk and/or the web. # Load CL from disk and/or the web.
def LoadCL(ui, repo, name, web=True): def LoadCL(ui, repo, name, web=True):
if not GoodCLName(name): if not GoodCLName(name):
return None, "invalid CL name" return None, "invalid CL name"
dir = CodeReviewDir(ui, repo) dir = CodeReviewDir(ui, repo)
path = dir + "cl." + name path = dir + "cl." + name
try: if os.access(path, 0):
ff = open(path) ff = open(path)
text = ff.read() text = ff.read()
ff.close() ff.close()
cl, lineno, err = ParseCL(text, name) cl, lineno, err = ParseCL(text, name)
if err != "": if err != "":
return None, "malformed CL data" return None, "malformed CL data: "+err
cl.local = True cl.local = True
except Exception, e: else:
cl = CL(name) cl = CL(name)
if web: if web:
try: try:
f = GetSettings(name) f = GetSettings(name)
except Exception, e: except:
return None, "cannot load CL data from code review server" return None, "cannot load CL data from code review server: "+ExceptionDetail()
if 'reviewers' not in f:
return None, "malformed response loading CL data from code review server"
cl.reviewer = SplitCommaSpace(f['reviewers']) cl.reviewer = SplitCommaSpace(f['reviewers'])
cl.cc = SplitCommaSpace(f['cc']) cl.cc = SplitCommaSpace(f['cc'])
cl.desc = f['description'] cl.desc = f['description']
...@@ -272,17 +292,40 @@ def LoadCL(ui, repo, name, web=True): ...@@ -272,17 +292,40 @@ def LoadCL(ui, repo, name, web=True):
cl.web = True cl.web = True
return cl, '' return cl, ''
class LoadCLThread(threading.Thread):
def __init__(self, ui, repo, dir, f, web):
threading.Thread.__init__(self)
self.ui = ui
self.repo = repo
self.f = f
self.web = web
self.cl = None
def run(self):
cl, err = LoadCL(self.ui, self.repo, self.f[3:], web=self.web)
if err != '':
self.ui.warn("loading "+self.dir+self.f+": " + err + "\n")
return
self.cl = cl
# Load all the CLs from this repository. # Load all the CLs from this repository.
def LoadAllCL(ui, repo, web=True): def LoadAllCL(ui, repo, web=True):
dir = CodeReviewDir(ui, repo) dir = CodeReviewDir(ui, repo)
m = {} m = {}
for f in os.listdir(dir): files = [f for f in os.listdir(dir) if f.startswith('cl.')]
if f.startswith('cl.'): if not files:
cl, err = LoadCL(ui, repo, f[3:], web=web) return m
if err != '': if web:
ui.warn("loading "+dir+f+": " + err + "\n") # Authenticate now, so we can use threads below
continue MySend(None)
m[cl.name] = cl active = []
for f in files:
t = LoadCLThread(ui, repo, dir, f, web)
t.start()
active.append(t)
for t in active:
t.join()
if t.cl:
m[t.cl.name] = t.cl
return m return m
# Find repository root. On error, ui.warn and return None # Find repository root. On error, ui.warn and return None
...@@ -305,8 +348,8 @@ def CodeReviewDir(ui, repo): ...@@ -305,8 +348,8 @@ def CodeReviewDir(ui, repo):
if not os.path.isdir(dir): if not os.path.isdir(dir):
try: try:
os.mkdir(dir, 0700) os.mkdir(dir, 0700)
except Exception, e: except:
ui.warn('cannot mkdir %s: %s\n' % (dir, e)) ui.warn('cannot mkdir %s: %s\n' % (dir, ExceptionDetail()))
return None return None
return dir return dir
...@@ -364,21 +407,18 @@ _change_prolog = """# Change list. ...@@ -364,21 +407,18 @@ _change_prolog = """# Change list.
# Return list of changed files in repository that match pats. # Return list of changed files in repository that match pats.
def ChangedFiles(ui, repo, pats, opts): def ChangedFiles(ui, repo, pats, opts):
# Find list of files being operated on. # Find list of files being operated on.
# TODO(rsc): The cutoff might not be 1.3.
# Definitely after 1.0.2.
try:
matcher = cmdutil.match(repo, pats, opts) matcher = cmdutil.match(repo, pats, opts)
node1, node2 = cmdutil.revpair(repo, None) node1, node2 = cmdutil.revpair(repo, None)
modified, added, removed = repo.status(node1, node2, matcher)[:3] modified, added, removed = repo.status(node1, node2, matcher)[:3]
except AttributeError, e: l = modified + added + removed
# Probably in earlier Mercurial, say 1.0.2. l.sort()
_, matcher, _ = cmdutil.matchpats(repo, pats, opts) return l
node1, node2 = cmdutil.revpair(repo, None)
modified, added, removed = repo.status(node1, node2, match=matcher)[:3]
return modified + added + removed
# Return list of files claimed by existing CLs # Return list of files claimed by existing CLs
def TakenFiles(ui, repo): def TakenFiles(ui, repo):
return Taken(ui, repo).keys()
def Taken(ui, repo):
all = LoadAllCL(ui, repo, web=False) all = LoadAllCL(ui, repo, web=False)
taken = {} taken = {}
for _, cl in all.items(): for _, cl in all.items():
...@@ -394,19 +434,17 @@ def Sub(l1, l2): ...@@ -394,19 +434,17 @@ def Sub(l1, l2):
return [l for l in l1 if l not in l2] return [l for l in l1 if l not in l2]
def Add(l1, l2): def Add(l1, l2):
return l1 + Sub(l2, l1) l = l1 + Sub(l2, l1)
l.sort()
return l
def Intersect(l1, l2): def Intersect(l1, l2):
return [l for l in l1 if l in l2] return [l for l in l1 if l in l2]
def Incoming(ui, repo, opts, op): def Incoming(ui, repo, opts, op):
source, _, _ = hg.parseurl(ui.expandpath("default"), None) source, _, _ = hg.parseurl(ui.expandpath("default"), None)
try:
other = hg.repository(cmdutil.remoteui(repo, opts), source) other = hg.repository(cmdutil.remoteui(repo, opts), source)
_, incoming, _ = repo.findcommonincoming(other) _, incoming, _ = repo.findcommonincoming(other)
except AttributeError, e:
other = hg.repository(ui, source)
incoming = repo.findincoming(other)
return incoming return incoming
def EditCL(ui, repo, cl): def EditCL(ui, repo, cl):
...@@ -415,7 +453,6 @@ def EditCL(ui, repo, cl): ...@@ -415,7 +453,6 @@ def EditCL(ui, repo, cl):
s = ui.edit(s, ui.username()) s = ui.edit(s, ui.username())
clx, line, err = ParseCL(s, cl.name) clx, line, err = ParseCL(s, cl.name)
if err != '': if err != '':
# TODO(rsc): another 1.3 inconsistency
if ui.prompt("error parsing change list: line %d: %s\nre-edit (y/n)?" % (line, err), ["&yes", "&no"], "y") == "n": if ui.prompt("error parsing change list: line %d: %s\nre-edit (y/n)?" % (line, err), ["&yes", "&no"], "y") == "n":
return "change list not modified" return "change list not modified"
continue continue
...@@ -458,6 +495,26 @@ def CommandLineCL(ui, repo, pats, opts): ...@@ -458,6 +495,26 @@ def CommandLineCL(ui, repo, pats, opts):
return None, err return None, err
return cl, "" return cl, ""
# reposetup replaces cmdutil.match with this wrapper,
# which expands the syntax @clnumber to mean the files
# in that CL.
original_match = None
def ReplacementForCmdutilMatch(repo, pats=[], opts={}, globbed=False, default='relpath'):
taken = []
files = []
for p in pats:
if p.startswith('@'):
taken.append(p)
clname = p[1:]
if not GoodCLName(clname):
raise util.Abort("invalid CL name " + clname)
cl, err = LoadCL(repo.ui, repo, clname, web=False)
if err != '':
raise util.Abort("loading CL " + clname + ": " + err)
files = Add(files, cl.files)
pats = Sub(pats, taken) + ['path:'+f for f in files]
return original_match(repo, pats=pats, opts=opts, globbed=globbed, default=default)
####################################################################### #######################################################################
# Mercurial commands # Mercurial commands
...@@ -475,7 +532,6 @@ server_url_base = None ...@@ -475,7 +532,6 @@ server_url_base = None
# they are required. # they are required.
# #
# Change command.
def change(ui, repo, *pats, **opts): def change(ui, repo, *pats, **opts):
"""create or edit a change list """create or edit a change list
...@@ -490,32 +546,37 @@ def change(ui, repo, *pats, **opts): ...@@ -490,32 +546,37 @@ def change(ui, repo, *pats, **opts):
change list for editing in the default editor. change list for editing in the default editor.
""" """
if opts["add"] and opts["delete"]:
return "cannot use -a with -d"
if (opts["add"] or opts["delete"]) and (opts["stdin"] or opts["stdout"]):
return "cannot use -a/-d with -i/-o"
dirty = {} dirty = {}
if len(pats) > 0 and GoodCLName(pats[0]): if len(pats) > 0 and GoodCLName(pats[0]):
name = pats[0] name = pats[0]
if len(pats) != 1:
return "cannot specify CL name and file patterns"
pats = pats[1:] pats = pats[1:]
cl, err = LoadCL(ui, repo, name, web=True) cl, err = LoadCL(ui, repo, name, web=True)
if err != '': if err != '':
return err return err
if not cl.local and (opts["add"] or opts["delete"] or opts["stdin"] or not opts["stdout"]): if not cl.local and (opts["stdin"] or not opts["stdout"]):
return "cannot change non-local CL " + name return "cannot change non-local CL " + name
else: else:
if opts["add"] or opts["delete"]:
return "cannot use -a/-d when creating CL"
name = "new" name = "new"
cl = CL("new") cl = CL("new")
dirty[cl] = True dirty[cl] = True
files = ChangedFiles(ui, repo, pats, opts) files = ChangedFiles(ui, repo, pats, opts)
taken = TakenFiles(ui, repo) taken = TakenFiles(ui, repo)
files = Sub(files, taken) files = Sub(files, taken)
if opts["delete"]:
if name == "new":
return "cannot use -d with file patterns"
if opts["stdin"] or opts["stdout"]:
return "cannot use -d with -i or -o"
if not cl.local:
return "cannot change non-local CL " + name
PostMessage(cl.name, "*** Abandoned ***", send_mail="checked")
EditDesc(cl.name, closed="checked")
cl.Delete(ui, repo)
return
if opts["stdin"]: if opts["stdin"]:
s = sys.stdin.read() s = sys.stdin.read()
clx, line, err = ParseCL(s, name) clx, line, err = ParseCL(s, name)
...@@ -534,35 +595,7 @@ def change(ui, repo, *pats, **opts): ...@@ -534,35 +595,7 @@ def change(ui, repo, *pats, **opts):
cl.files = clx.files cl.files = clx.files
dirty[cl] = True dirty[cl] = True
if opts["add"]: if not opts["stdin"] and not opts["stdout"]:
newfiles = Sub(files, cl.files)
stolen = Intersect(newfiles, taken)
if stolen:
ui.status("# Taking files from other CLs. To undo:\n")
for f in stolen:
ocl = taken[f]
ui.status("# hg change -a %s %s\n" % (ocl.name, f))
ocl.files = Sub(ocl.files, [f])
dirty[ocl] = True
not_stolen = Sub(newfiles, stolen)
if not_stolen:
ui.status("# Add files to CL. To undo:\n")
for f in not_stolen:
ui.status("# hg change -d %s %s\n" % (cl.name, f))
if newfiles:
cl.files += newfiles
dirty[cl] = True
if opts["delete"]:
oldfiles = Intersect(files, cl.files)
if oldfiles:
ui.status("# Removing files from CL. To undo:\n")
for f in oldfiles:
ui.status("# hg change -a %s %s\n" % (cl.name, f))
cl.files = Sub(cl.files, oldfiles)
dirty[cl] = True
if not opts["add"] and not opts["delete"] and not opts["stdin"] and not opts["stdout"]:
if name == "new": if name == "new":
cl.files = files cl.files = files
err = EditCL(ui, repo, cl) err = EditCL(ui, repo, cl)
...@@ -579,33 +612,74 @@ def change(ui, repo, *pats, **opts): ...@@ -579,33 +612,74 @@ def change(ui, repo, *pats, **opts):
if ui.quiet: if ui.quiet:
ui.write(cl.name) ui.write(cl.name)
else: else:
ui.write("URL: " + cl.url) ui.write("CL created: " + cl.url + "\n")
return return
def pending(ui, repo, *pats, **opts): def codereview_login(ui, repo, **opts):
m = LoadAllCL(ui, repo, web=True) """log in to code review server
names = m.keys()
names.sort()
for name in names:
cl = m[name]
ui.write(cl.PendingText() + "\n")
files = DefaultFiles(ui, repo, [], opts) Logs in to the code review server, saving a cookie in
if len(files) > 0: a file in your home directory.
s = "Changed files not in any CL:\n" """
for f in files: MySend(None)
s += "\t" + f + "\n"
ui.write(s)
def upload(ui, repo, name, **opts): def file(ui, repo, clname, pat, *pats, **opts):
repo.ui.quiet = True """assign files to or remove files from a change list
cl, err = LoadCL(ui, repo, name, web=True)
if err != "": Assign files to or (with -d) remove files from a change list.
The -d option only removes files from the change list.
It does not edit them or remove them from the repository.
"""
pats = tuple([pat] + list(pats))
if not GoodCLName(clname):
return "invalid CL name " + clname
dirty = {}
cl, err = LoadCL(ui, repo, clname, web=False)
if err != '':
return err return err
if not cl.local: if not cl.local:
return "cannot upload non-local change" return "cannot change non-local CL " + clname
cl.Upload(ui, repo)
print "%s%s\n" % (server_url_base, cl.name) files = ChangedFiles(ui, repo, pats, opts)
if opts["delete"]:
oldfiles = Intersect(files, cl.files)
if oldfiles:
if not ui.quiet:
ui.status("# Removing files from CL. To undo:\n")
ui.status("# cd %s\n" % (repo.root))
for f in oldfiles:
ui.status("# hg file %s %s\n" % (cl.name, f))
cl.files = Sub(cl.files, oldfiles)
cl.Flush(ui, repo)
else:
ui.status("no such files in CL")
return
if not files:
return "no such modified files"
files = Sub(files, cl.files)
taken = Taken(ui, repo)
warned = False
for f in files:
if f in taken:
if not warned and not ui.quiet:
ui.status("# Taking files from other CLs. To undo:\n")
ui.status("# cd %s\n" % (repo.root))
warned = True
ocl = taken[f]
if not ui.quiet:
ui.status("# hg file %s %s\n" % (ocl.name, f))
if ocl not in dirty:
ocl.files = Sub(ocl.files, files)
dirty[ocl] = True
cl.files = Add(cl.files, files)
dirty[cl] = True
for d, _ in dirty.items():
d.Flush(ui, repo)
return return
def mail(ui, repo, *pats, **opts): def mail(ui, repo, *pats, **opts):
...@@ -621,6 +695,30 @@ def mail(ui, repo, *pats, **opts): ...@@ -621,6 +695,30 @@ def mail(ui, repo, *pats, **opts):
subject = "code review %s: %s" % (cl.name, line1(cl.desc)) subject = "code review %s: %s" % (cl.name, line1(cl.desc))
PostMessage(cl.name, pmsg, send_mail="checked", subject=subject) PostMessage(cl.name, pmsg, send_mail="checked", subject=subject)
def nocommit(ui, repo, *pats, **opts):
return "The codereview extension is enabled; do not use commit."
def pending(ui, repo, *pats, **opts):
m = LoadAllCL(ui, repo, web=True)
names = m.keys()
names.sort()
for name in names:
cl = m[name]
ui.write(cl.PendingText() + "\n")
files = DefaultFiles(ui, repo, [], opts)
if len(files) > 0:
s = "Changed files not in any CL:\n"
for f in files:
s += "\t" + f + "\n"
ui.write(s)
def reposetup(ui, repo):
global original_match
original_match = cmdutil.match
cmdutil.match = ReplacementForCmdutilMatch
RietveldSetup(ui, repo)
def submit(ui, repo, *pats, **opts): def submit(ui, repo, *pats, **opts):
"""submit change to remote repository """submit change to remote repository
...@@ -660,12 +758,8 @@ def submit(ui, repo, *pats, **opts): ...@@ -660,12 +758,8 @@ def submit(ui, repo, *pats, **opts):
if date: if date:
opts['date'] = util.parsedate(date) opts['date'] = util.parsedate(date)
opts['message'] = cl.desc.rstrip() + "\n\n" + about opts['message'] = cl.desc.rstrip() + "\n\n" + about
try:
m = match.exact(repo.root, repo.getcwd(), cl.files) m = match.exact(repo.root, repo.getcwd(), cl.files)
node = repo.commit(opts['message'], opts.get('user'), opts.get('date'), m) node = repo.commit(opts['message'], opts.get('user'), opts.get('date'), m)
except Exception, e:
_, m, _ = util._matcher(repo.root, repo.getcwd(), cl.files, None, None, 'path', None)
node = repo.commit(text=opts['message'], user=opts.get('user'), date=opts.get('date'), match=m)
if not node: if not node:
return "nothing changed" return "nothing changed"
...@@ -683,10 +777,7 @@ def submit(ui, repo, *pats, **opts): ...@@ -683,10 +777,7 @@ def submit(ui, repo, *pats, **opts):
# if it works, we're committed. # if it works, we're committed.
# if not, roll back # if not, roll back
dest, _, _ = hg.parseurl(ui.expandpath("default"), None) dest, _, _ = hg.parseurl(ui.expandpath("default"), None)
try:
other = hg.repository(cmdutil.remoteui(repo, opts), dest) other = hg.repository(cmdutil.remoteui(repo, opts), dest)
except AttributeError, e:
other = hg.repository(ui, dest)
r = repo.push(other, False, None) r = repo.push(other, False, None)
if r == 0: if r == 0:
repo.rollback() repo.rollback()
...@@ -710,34 +801,39 @@ def sync(ui, repo, **opts): ...@@ -710,34 +801,39 @@ def sync(ui, repo, **opts):
Incorporates recent changes from the remote repository Incorporates recent changes from the remote repository
into the local repository. into the local repository.
Equivalent to the Mercurial command "hg pull -u".
""" """
repo.ui.quiet = True ui.status = sync_note
ui.note = sync_note
source, _, _ = hg.parseurl(ui.expandpath("default"), None) source, _, _ = hg.parseurl(ui.expandpath("default"), None)
try:
other = hg.repository(cmdutil.remoteui(repo, opts), source) other = hg.repository(cmdutil.remoteui(repo, opts), source)
except AttributeError, e:
other = hg.repository(ui, source)
modheads = repo.pull(other) modheads = repo.pull(other)
return commands.postincoming(ui, repo, modheads, True, "tip") err = commands.postincoming(ui, repo, modheads, True, "tip")
if err:
def dologin(ui, repo, **opts): return err
"""log in to code review server sync_changes(ui, repo)
Logs in to the code review server, saving a cookie in def sync_note(msg):
a file in your home directory. if msg == 'resolving manifests\n' or msg == 'searching for changes\n':
""" return
MySend("/") sys.stdout.write(msg)
def sync_changes(ui, repo):
pass
def uisetup(ui): def uisetup(ui):
if "^commit|ci" in commands.table: if "^commit|ci" in commands.table:
commands.table["^commit|ci"] = (nocommit, [], "") commands.table["^commit|ci"] = (nocommit, [], "")
RietveldSetup(ui)
def nocommit(ui, repo, *pats, **opts): def upload(ui, repo, name, **opts):
return "The codereview extension is enabled; do not use commit." repo.ui.quiet = True
cl, err = LoadCL(ui, repo, name, web=True)
if err != "":
return err
if not cl.local:
return "cannot upload non-local change"
cl.Upload(ui, repo)
print "%s%s\n" % (server_url_base, cl.name)
return
review_opts = [ review_opts = [
('r', 'reviewer', '', 'add reviewer'), ('r', 'reviewer', '', 'add reviewer'),
...@@ -749,39 +845,43 @@ review_opts = [ ...@@ -749,39 +845,43 @@ review_opts = [
cmdtable = { cmdtable = {
# The ^ means to show this command in the help text that # The ^ means to show this command in the help text that
# is printed when running hg with no arguments. # is printed when running hg with no arguments.
# TODO: Should change upload?
"^change": ( "^change": (
change, change,
[ [
('a', 'add', None, 'add files to change list'), ('d', 'delete', None, 'delete existing change list'),
('d', 'delete', None, 'remove files from change list'),
('o', 'stdout', None, 'print change list to standard output'),
('i', 'stdin', None, 'read change list from standard input'), ('i', 'stdin', None, 'read change list from standard input'),
('o', 'stdout', None, 'print change list to standard output'),
],
"[-i] [-o] change# or FILE ..."
),
"codereview-login": (
codereview_login,
[],
"",
),
"commit|ci": (
nocommit,
[],
"",
),
"^file": (
file,
[
('d', 'delete', None, 'delete files from change list (but not repository)'),
], ],
"[-a | -d | [-i] [-o]] [change#] [FILE ...]" "[-d] change# FILE ..."
), ),
"^pending|p": ( "^pending|p": (
pending, pending,
[], [],
"[FILE ...]" "[FILE ...]"
), ),
# TODO: cdiff - steal diff options and command line
"^upload": (
upload,
[],
"change#"
),
"^mail": ( "^mail": (
mail, mail,
review_opts + [ review_opts + [
] + commands.walkopts, ] + commands.walkopts,
"[-r reviewer] [--cc cc] [change# | file ...]" "[-r reviewer] [--cc cc] [change# | file ...]"
), ),
"^submit": ( "^submit": (
submit, submit,
review_opts + [ review_opts + [
...@@ -789,23 +889,15 @@ cmdtable = { ...@@ -789,23 +889,15 @@ cmdtable = {
] + commands.walkopts + commands.commitopts + commands.commitopts2, ] + commands.walkopts + commands.commitopts + commands.commitopts2,
"[-r reviewer] [--cc cc] [change# | file ...]" "[-r reviewer] [--cc cc] [change# | file ...]"
), ),
"^sync": ( "^sync": (
sync, sync,
[], [],
"", "",
), ),
"^upload": (
"commit|ci": ( upload,
nocommit,
[],
"",
),
"codereview-login": (
dologin,
[], [],
"", "change#"
), ),
} }
...@@ -890,6 +982,8 @@ def MySend(request_path, payload=None, ...@@ -890,6 +982,8 @@ def MySend(request_path, payload=None,
self = rpc self = rpc
if not self.authenticated: if not self.authenticated:
self._Authenticate() self._Authenticate()
if request_path is None:
return
old_timeout = socket.getdefaulttimeout() old_timeout = socket.getdefaulttimeout()
socket.setdefaulttimeout(timeout) socket.setdefaulttimeout(timeout)
...@@ -915,7 +1009,7 @@ def MySend(request_path, payload=None, ...@@ -915,7 +1009,7 @@ def MySend(request_path, payload=None,
self._Authenticate() self._Authenticate()
elif e.code == 302: elif e.code == 302:
loc = e.info()["location"] loc = e.info()["location"]
if not loc.startswith('https://www.google.com/accounts/ServiceLogin'): if not loc.startswith('https://www.google.com/a') or loc.find('/ServiceLogin') < 0:
return '' return ''
self._Authenticate() self._Authenticate()
else: else:
...@@ -933,8 +1027,7 @@ def GetForm(url): ...@@ -933,8 +1027,7 @@ def GetForm(url):
def GetSettings(issue): def GetSettings(issue):
f = GetForm("/" + issue + "/edit") f = GetForm("/" + issue + "/edit")
if not f: if not f or 'reviewers' not in f:
print "PUB"
f = GetForm("/" + issue + "/publish") f = GetForm("/" + issue + "/publish")
return f return f
...@@ -996,8 +1089,8 @@ def PostMessage(issue, message, reviewers=None, cc=None, send_mail=None, subject ...@@ -996,8 +1089,8 @@ def PostMessage(issue, message, reviewers=None, cc=None, send_mail=None, subject
class opt(object): class opt(object):
pass pass
def RietveldSetup(ui): def RietveldSetup(ui, repo):
global upload_options, rpc, server, server_url_base global upload_options, rpc, server, server_url_base, force_google_account, verbosity
# TODO(rsc): If the repository config has no codereview section, # TODO(rsc): If the repository config has no codereview section,
# do not enable the extension. This allows users to # do not enable the extension. This allows users to
...@@ -1007,6 +1100,9 @@ def RietveldSetup(ui): ...@@ -1007,6 +1100,9 @@ def RietveldSetup(ui):
# cmdtable = {} # cmdtable = {}
# return # return
if not ui.verbose:
verbosity = 0
# Config options. # Config options.
x = ui.config("codereview", "server") x = ui.config("codereview", "server")
if x is not None: if x is not None:
...@@ -1031,6 +1127,7 @@ def RietveldSetup(ui): ...@@ -1031,6 +1127,7 @@ def RietveldSetup(ui):
server_url_base += "/" server_url_base += "/"
testing = ui.config("codereview", "testing") testing = ui.config("codereview", "testing")
force_google_account = ui.configbool("codereview", "force_google_account", False)
upload_options = opt() upload_options = opt()
upload_options.email = email upload_options.email = email
...@@ -1272,7 +1369,7 @@ class AbstractRpcServer(object): ...@@ -1272,7 +1369,7 @@ class AbstractRpcServer(object):
The authentication token returned by ClientLogin. The authentication token returned by ClientLogin.
""" """
account_type = "GOOGLE" account_type = "GOOGLE"
if self.host.endswith(".google.com"): if self.host.endswith(".google.com") and not force_google_account:
# Needed for use inside Google. # Needed for use inside Google.
account_type = "HOSTED" account_type = "HOSTED"
req = self._CreateRequest( req = self._CreateRequest(
...@@ -1420,9 +1517,6 @@ class AbstractRpcServer(object): ...@@ -1420,9 +1517,6 @@ class AbstractRpcServer(object):
raise raise
elif e.code == 401 or e.code == 302: elif e.code == 401 or e.code == 302:
self._Authenticate() self._Authenticate()
## elif e.code >= 500 and e.code < 600:
## # Server Error - try again.
## continue
else: else:
raise raise
finally: finally:
...@@ -2561,9 +2655,9 @@ def RealMain(argv, data=None): ...@@ -2561,9 +2655,9 @@ def RealMain(argv, data=None):
msg = response_body msg = response_body
else: else:
msg = response_body msg = response_body
StatusUpdate(msg)
if not response_body.startswith("Issue created.") and \ if not response_body.startswith("Issue created.") and \
not response_body.startswith("Issue updated."): not response_body.startswith("Issue updated."):
print >>sys.stderr, msg
sys.exit(0) sys.exit(0)
issue = msg[msg.rfind("/")+1:] issue = msg[msg.rfind("/")+1:]
......
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