]> Cypherpunks repositories - gostls13.git/commitdiff
code review fixes
authorRuss Cox <rsc@golang.org>
Sun, 1 Nov 2009 13:49:35 +0000 (05:49 -0800)
committerRuss Cox <rsc@golang.org>
Sun, 1 Nov 2009 13:49:35 +0000 (05:49 -0800)
* 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

lib/codereview/codereview.py

index 8ee0a6b77afe29d7c2d8d5150aa0c505a1ed8b45..7bb7e6b5005d6b22397d3374d41495cdab6cbc58 100644 (file)
@@ -23,27 +23,32 @@ your repository's .hg/hgrc file.
     codereview = path/to/codereview.py
 
     [codereview]
-    project = project-url        # optional
+       server = codereview.appspot.com
 
-If the project URL is specified, codereview will fetch
-default the reviewer and cc list from that URL each time
-it runs an "upload" command.
+The server should be running Rietveld; see http://code.google.com/p/rietveld/.
 '''
 
+# 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.node import nullrev, hex, nullid, short
 import os, re
 import stat
+import threading
 from HTMLParser import HTMLParser
 
 try:
        hgversion = util.version()
-except Exception, e:
+except:
        from mercurial.version import version as v
        hgversion = v.get_version()
 
 
-# To experiment with Mercurial in the python interpreter: 
+# To experiment with Mercurial in the python interpreter:
 #    >>> repo = hg.repository(ui.ui(), path = ".")
 
 #######################################################################
@@ -108,7 +113,7 @@ class CL(object):
                        s += "\t" + f + "\n"
                s += "\n"
                return s
-       
+
        def PendingText(self):
                cl = self
                s = cl.name + ":" + "\n"
@@ -120,7 +125,7 @@ class CL(object):
                for f in cl.files:
                        s += "\t\t" + f + "\n"
                return s
-       
+
        def Flush(self, ui, repo):
                if self.name == "new":
                        self.Upload(ui, repo)
@@ -133,7 +138,7 @@ class CL(object):
                if self.web:
                        EditDesc(self.name, subject=line1(self.desc), desc=self.desc,
                                reviewers=JoinComma(self.reviewer), cc=JoinComma(self.cc))
-       
+
        def Delete(self, ui, repo):
                dir = CodeReviewDir(ui, repo)
                os.unlink(dir + "/cl." + self.name)
@@ -150,7 +155,7 @@ class CL(object):
                ]
 
                # 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":
                        form_fields.append(("issue", self.name))
                vcs = GuessVCS(upload_options)
@@ -170,12 +175,14 @@ class CL(object):
                        msg = lines[0]
                        patchset = lines[1].strip()
                        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."):
                        print response_body
                        raise "failed to update issue"
                issue = msg[msg.rfind("/")+1:]
                self.name = issue
+               if not self.url:
+                       self.url = server_url_base + self.name
                if not uploaded_diff_file:
                        patches = UploadSeparatePatches(issue, rpc, patchset, data, upload_options)
                vcs.UploadBaseFiles(issue, rpc, patches, patchset, upload_options, files)
@@ -186,7 +193,7 @@ class CL(object):
                return
 
 def GoodCLName(name):
-       return re.match("^[0-9]+$", name)       
+       return re.match("^[0-9]+$", name)
 
 def ParseCL(text, name):
        sname = None
@@ -244,27 +251,40 @@ def SplitCommaSpace(s):
 def JoinComma(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.
 def LoadCL(ui, repo, name, web=True):
        if not GoodCLName(name):
                return None, "invalid CL name"
        dir = CodeReviewDir(ui, repo)
        path = dir + "cl." + name
-       try:
+       if os.access(path, 0):
                ff = open(path)
                text = ff.read()
                ff.close()
                cl, lineno, err = ParseCL(text, name)
                if err != "":
-                       return None, "malformed CL data"
+                       return None, "malformed CL data: "+err
                cl.local = True
-       except Exception, e:
+       else:
                cl = CL(name)
        if web:
                try:
                        f = GetSettings(name)
-               except Exception, e:
-                       return None, "cannot load CL data from code review server"
+               except:
+                       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.cc = SplitCommaSpace(f['cc'])
                cl.desc = f['description']
@@ -272,17 +292,40 @@ def LoadCL(ui, repo, name, web=True):
                cl.web = True
        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.
 def LoadAllCL(ui, repo, web=True):
        dir = CodeReviewDir(ui, repo)
        m = {}
-       for f in os.listdir(dir):
-               if f.startswith('cl.'):
-                       cl, err = LoadCL(ui, repo, f[3:], web=web)
-                       if err != '':
-                               ui.warn("loading "+dir+f+": " + err + "\n")
-                               continue
-                       m[cl.name] = cl
+       files = [f for f in os.listdir(dir) if f.startswith('cl.')]
+       if not files:
+               return m
+       if web:
+               # Authenticate now, so we can use threads below
+               MySend(None)
+       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
 
 # Find repository root.  On error, ui.warn and return None
@@ -305,8 +348,8 @@ def CodeReviewDir(ui, repo):
        if not os.path.isdir(dir):
                try:
                        os.mkdir(dir, 0700)
-               except Exception, e:
-                       ui.warn('cannot mkdir %s: %s\n' % (dir, e))
+               except:
+                       ui.warn('cannot mkdir %s: %s\n' % (dir, ExceptionDetail()))
                        return None
        return dir
 
@@ -364,21 +407,18 @@ _change_prolog = """# Change list.
 # Return list of changed files in repository that match pats.
 def ChangedFiles(ui, repo, pats, opts):
        # 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)
-               node1, node2 = cmdutil.revpair(repo, None)
-               modified, added, removed = repo.status(node1, node2, matcher)[:3]
-       except AttributeError, e:
-               # Probably in earlier Mercurial, say 1.0.2.
-               _, matcher, _ = cmdutil.matchpats(repo, pats, opts)
-               node1, node2 = cmdutil.revpair(repo, None)
-               modified, added, removed = repo.status(node1, node2, match=matcher)[:3]
-       return modified + added + removed
+       matcher = cmdutil.match(repo, pats, opts)
+       node1, node2 = cmdutil.revpair(repo, None)
+       modified, added, removed = repo.status(node1, node2, matcher)[:3]
+       l = modified + added + removed
+       l.sort()
+       return l
 
 # Return list of files claimed by existing CLs
 def TakenFiles(ui, repo):
+       return Taken(ui, repo).keys()
+
+def Taken(ui, repo):
        all = LoadAllCL(ui, repo, web=False)
        taken = {}
        for _, cl in all.items():
@@ -394,19 +434,17 @@ def Sub(l1, l2):
        return [l for l in l1 if l not in l2]
 
 def Add(l1, l2):
-       return l1 + Sub(l2, l1)
+       l = l1 + Sub(l2, l1)
+       l.sort()
+       return l
 
 def Intersect(l1, l2):
        return [l for l in l1 if l in l2]
 
 def Incoming(ui, repo, opts, op):
        source, _, _ = hg.parseurl(ui.expandpath("default"), None)
-       try:
-               other = hg.repository(cmdutil.remoteui(repo, opts), source)
-               _, incoming, _ = repo.findcommonincoming(other)
-       except AttributeError, e:
-               other = hg.repository(ui, source)
-               incoming = repo.findincoming(other)
+       other = hg.repository(cmdutil.remoteui(repo, opts), source)
+       _, incoming, _ = repo.findcommonincoming(other)
        return incoming
 
 def EditCL(ui, repo, cl):
@@ -415,7 +453,6 @@ def EditCL(ui, repo, cl):
                s = ui.edit(s, ui.username())
                clx, line, err = ParseCL(s, cl.name)
                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":
                                return "change list not modified"
                        continue
@@ -458,6 +495,26 @@ def CommandLineCL(ui, repo, pats, opts):
                                return None, err
        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
 
@@ -473,48 +530,52 @@ server_url_base = None
 # Other parameters are taken in order from items on the command line that
 # don't start with a dash.  If no default value is given in the parameter list,
 # they are required.
-# 
+#
 
-# Change command.
 def change(ui, repo, *pats, **opts):
        """create or edit a change list
-       
+
        Create or edit a change list.
        A change list is a group of files to be reviewed and submitted together,
        plus a textual description of the change.
        Change lists are referred to by simple alphanumeric names.
 
        Changes must be reviewed before they can be submitted.
-       
+
        In the absence of options, the change command opens the
-       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 = {}
        if len(pats) > 0 and GoodCLName(pats[0]):
                name = pats[0]
+               if len(pats) != 1:
+                       return "cannot specify CL name and file patterns"
                pats = pats[1:]
                cl, err = LoadCL(ui, repo, name, web=True)
                if 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
        else:
-               if opts["add"] or opts["delete"]:
-                       return "cannot use -a/-d when creating CL"
                name = "new"
                cl = CL("new")
                dirty[cl] = True
+               files = ChangedFiles(ui, repo, pats, opts)
+               taken = TakenFiles(ui, repo)
+               files = Sub(files, taken)
        
-       files = ChangedFiles(ui, repo, pats, opts)
-       taken = TakenFiles(ui, repo)
-       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"]:
                s = sys.stdin.read()
@@ -534,35 +595,7 @@ def change(ui, repo, *pats, **opts):
                        cl.files = clx.files
                        dirty[cl] = True
 
-       if opts["add"]:
-               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 not opts["stdin"] and not opts["stdout"]:
                if name == "new":
                        cl.files = files
                err = EditCL(ui, repo, cl)
@@ -572,42 +605,83 @@ def change(ui, repo, *pats, **opts):
 
        for d, _ in dirty.items():
                d.Flush(ui, repo)
-       
+
        if opts["stdout"]:
                ui.write(cl.EditorText())
        elif name == "new":
                if ui.quiet:
                        ui.write(cl.name)
                else:
-                       ui.write("URL: " + cl.url)
+                       ui.write("CL created: " + cl.url + "\n")
        return
 
-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")
+def codereview_login(ui, repo, **opts):
+       """log in to code review server
 
-       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)
+       Logs in to the code review server, saving a cookie in
+       a file in your home directory.
+       """
+       MySend(None)
 
-def upload(ui, repo, name, **opts):
-       repo.ui.quiet = True
-       cl, err = LoadCL(ui, repo, name, web=True)
-       if err != "":
+def file(ui, repo, clname, pat, *pats, **opts):
+       """assign files to or remove files from a change list
+       
+       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
        if not cl.local:
-               return "cannot upload non-local change"
-       cl.Upload(ui, repo)
-       print "%s%s\n" % (server_url_base, cl.name)
-       return
+               return "cannot change non-local CL " + clname
+
+       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
+       
 def mail(ui, repo, *pats, **opts):
        cl, err = CommandLineCL(ui, repo, pats, opts)
        if err != "":
@@ -620,10 +694,34 @@ def mail(ui, repo, *pats, **opts):
        pmsg += "I'd like you to review the following change.\n"
        subject = "code review %s: %s" % (cl.name, line1(cl.desc))
        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):
        """submit change to remote repository
-       
+
        Submits change to remote repository.
        Bails out if the local repository is not in sync with the remote one.
        """
@@ -634,7 +732,7 @@ def submit(ui, repo, *pats, **opts):
        cl, err = CommandLineCL(ui, repo, pats, opts)
        if err != "":
                return err
-       
+
        about = ""
        if cl.reviewer:
                about += "R=" + JoinComma(cl.reviewer) + "\n"
@@ -660,12 +758,8 @@ def submit(ui, repo, *pats, **opts):
        if date:
                opts['date'] = util.parsedate(date)
        opts['message'] = cl.desc.rstrip() + "\n\n" + about
-       try:
-               m = match.exact(repo.root, repo.getcwd(), cl.files)
-               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)
+       m = match.exact(repo.root, repo.getcwd(), cl.files)
+       node = repo.commit(opts['message'], opts.get('user'), opts.get('date'), m)
        if not node:
                return "nothing changed"
 
@@ -683,10 +777,7 @@ def submit(ui, repo, *pats, **opts):
        # if it works, we're committed.
        # if not, roll back
        dest, _, _ = hg.parseurl(ui.expandpath("default"), None)
-       try:
-               other = hg.repository(cmdutil.remoteui(repo, opts), dest)
-       except AttributeError, e:
-               other = hg.repository(ui, dest)
+       other = hg.repository(cmdutil.remoteui(repo, opts), dest)
        r = repo.push(other, False, None)
        if r == 0:
                repo.rollback()
@@ -707,37 +798,42 @@ def submit(ui, repo, *pats, **opts):
 
 def sync(ui, repo, **opts):
        """synchronize with remote repository
-       
+
        Incorporates recent changes from the remote 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)
-       try:
-               other = hg.repository(cmdutil.remoteui(repo, opts), source)
-       except AttributeError, e:
-               other = hg.repository(ui, source)
+       other = hg.repository(cmdutil.remoteui(repo, opts), source)
        modheads = repo.pull(other)
-       return commands.postincoming(ui, repo, modheads, True, "tip")
+       err = commands.postincoming(ui, repo, modheads, True, "tip")
+       if err:
+               return err
+       sync_changes(ui, repo)
 
-def dologin(ui, repo, **opts):
-       """log in to code review server
-       
-       Logs in to the code review server, saving a cookie in
-       a file in your home directory.
-       """
-       MySend("/")
+def sync_note(msg):
+       if msg == 'resolving manifests\n' or msg == 'searching for changes\n':
+               return
+       sys.stdout.write(msg)
 
+def sync_changes(ui, repo):
+       pass
 
 def uisetup(ui):
        if "^commit|ci" in commands.table:
                commands.table["^commit|ci"] = (nocommit, [], "")
-       RietveldSetup(ui)
 
-def nocommit(ui, repo, *pats, **opts):
-       return "The codereview extension is enabled; do not use commit."
+def upload(ui, repo, name, **opts):
+       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 = [
        ('r', 'reviewer', '', 'add reviewer'),
@@ -749,39 +845,43 @@ review_opts = [
 cmdtable = {
        # The ^ means to show this command in the help text that
        # is printed when running hg with no arguments.
-
-       # TODO: Should change upload?
        "^change": (
                change,
                [
-                       ('a', 'add', None, 'add files to change list'),
-                       ('d', 'delete', None, 'remove files from change list'),
-                       ('o', 'stdout', None, 'print change list to standard output'),
+                       ('d', 'delete', None, 'delete existing change list'),
                        ('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,
                [],
                "[FILE ...]"
        ),
-
-       # TODO: cdiff - steal diff options and command line
-
-       "^upload": (
-               upload,
-               [],
-               "change#"
-       ),
-       
        "^mail": (
                mail,
                review_opts + [
                ] + commands.walkopts,
                "[-r reviewer] [--cc cc] [change# | file ...]"
        ),
-
        "^submit": (
                submit,
                review_opts + [
@@ -789,23 +889,15 @@ cmdtable = {
                ] + commands.walkopts + commands.commitopts + commands.commitopts2,
                "[-r reviewer] [--cc cc] [change# | file ...]"
        ),
-       
        "^sync": (
                sync,
                [],
                "",
        ),
-       
-       "commit|ci": (
-               nocommit,
-               [],
-               "",
-       ),
-       
-       "codereview-login": (
-               dologin,
+       "^upload": (
+               upload,
                [],
-               "",
+               "change#"
        ),
 }
 
@@ -862,7 +954,7 @@ class FormParser(HTMLParser):
                if self.curdata is not None:
                        self.curdata += data
 
-# Like upload.py Send but only authenticates when the 
+# Like upload.py Send but only authenticates when the
 # redirect is to www.google.com/accounts.  This keeps
 # unnecessary redirects from happening during testing.
 def MySend(request_path, payload=None,
@@ -890,6 +982,8 @@ def MySend(request_path, payload=None,
     self = rpc
     if not self.authenticated:
       self._Authenticate()
+    if request_path is None:
+      return
 
     old_timeout = socket.getdefaulttimeout()
     socket.setdefaulttimeout(timeout)
@@ -915,7 +1009,7 @@ def MySend(request_path, payload=None,
             self._Authenticate()
           elif e.code == 302:
             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 ''
             self._Authenticate()
           else:
@@ -933,8 +1027,7 @@ def GetForm(url):
 
 def GetSettings(issue):
        f = GetForm("/" + issue + "/edit")
-       if not f:
-               print "PUB"
+       if not f or 'reviewers' not in f:
                f = GetForm("/" + issue + "/publish")
        return f
 
@@ -996,8 +1089,8 @@ def PostMessage(issue, message, reviewers=None, cc=None, send_mail=None, subject
 class opt(object):
        pass
 
-def RietveldSetup(ui):
-       global upload_options, rpc, server, server_url_base
+def RietveldSetup(ui, repo):
+       global upload_options, rpc, server, server_url_base, force_google_account, verbosity
 
        # TODO(rsc): If the repository config has no codereview section,
        # do not enable the extension.  This allows users to
@@ -1006,12 +1099,15 @@ def RietveldSetup(ui):
        # if not ui.has_section("codereview"):
        #       cmdtable = {}
        #       return
+       
+       if not ui.verbose:
+               verbosity = 0
 
        # Config options.
        x = ui.config("codereview", "server")
        if x is not None:
                server = x
-       
+
        # TODO(rsc): Take from ui.username?
        email = None
        x = ui.config("codereview", "email")
@@ -1022,7 +1118,7 @@ def RietveldSetup(ui):
        x = ui.config("codereview", "cc")
        if x is not None:
                cc = x
-       
+
        server_url_base = "http://" + server + "/"
        x = ui.config("codereview", "server_url_base")
        if x is not None:
@@ -1031,6 +1127,7 @@ def RietveldSetup(ui):
                server_url_base += "/"
 
        testing = ui.config("codereview", "testing")
+       force_google_account = ui.configbool("codereview", "force_google_account", False)
 
        upload_options = opt()
        upload_options.email = email
@@ -1048,7 +1145,7 @@ def RietveldSetup(ui):
        upload_options.vcs = None
        upload_options.server = server
        upload_options.save_cookies = True
-       
+
        if testing:
                upload_options.save_cookies = False
                upload_options.email = "test@example.com"
@@ -1272,7 +1369,7 @@ class AbstractRpcServer(object):
       The authentication token returned by ClientLogin.
     """
     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.
       account_type = "HOSTED"
     req = self._CreateRequest(
@@ -1420,9 +1517,6 @@ class AbstractRpcServer(object):
             raise
           elif e.code == 401 or e.code == 302:
             self._Authenticate()
-##           elif e.code >= 500 and e.code < 600:
-##             # Server Error - try again.
-##             continue
           else:
             raise
     finally:
@@ -2561,9 +2655,9 @@ def RealMain(argv, data=None):
       msg = response_body
   else:
     msg = response_body
-  StatusUpdate(msg)
   if not response_body.startswith("Issue created.") and \
   not response_body.startswith("Issue updated."):
+    print >>sys.stderr, msg
     sys.exit(0)
   issue = msg[msg.rfind("/")+1:]