mirror of
https://github.com/golang/go
synced 2024-11-05 18:36:08 +00:00
codereview: new commands
* clpatch * download * submit, on behalf of clpatch stir hgpatch to fix a few bugs R=r CC=go-dev http://go/go-review/1016051
This commit is contained in:
parent
9786b3f1bd
commit
790c9b59d6
2 changed files with 249 additions and 46 deletions
|
@ -16,28 +16,34 @@
|
|||
|
||||
'''Mercurial interface to codereview.appspot.com.
|
||||
|
||||
To configure it, set the following options in
|
||||
To configure, set the following options in
|
||||
your repository's .hg/hgrc file.
|
||||
|
||||
[extensions]
|
||||
codereview = path/to/codereview.py
|
||||
[extensions]
|
||||
codereview = path/to/codereview.py
|
||||
|
||||
[codereview]
|
||||
[codereview]
|
||||
server = codereview.appspot.com
|
||||
|
||||
The server should be running Rietveld; see http://code.google.com/p/rietveld/.
|
||||
|
||||
In addition to the new commands, this extension introduces
|
||||
the file pattern syntax @nnnnnn, where nnnnnn is a change list
|
||||
number, to mean the files included in that change list, which
|
||||
must be associated with the current client.
|
||||
|
||||
For example, if change 123456 contains the files x.go and y.go,
|
||||
"hg diff @123456" is equivalent to"hg diff x.go y.go".
|
||||
'''
|
||||
|
||||
# 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 subprocess
|
||||
import threading
|
||||
from HTMLParser import HTMLParser
|
||||
from xml.etree import ElementTree as ET
|
||||
|
@ -83,10 +89,13 @@ class CL(object):
|
|||
self.url = ''
|
||||
self.local = False
|
||||
self.web = False
|
||||
self.original_author = None # None means current user
|
||||
|
||||
def DiskText(self):
|
||||
cl = self
|
||||
s = ""
|
||||
if cl.original_author:
|
||||
s += "Author: " + cl.original_author + "\n\n"
|
||||
s += "Description:\n"
|
||||
s += Indent(cl.desc, "\t")
|
||||
s += "Files:\n"
|
||||
|
@ -98,6 +107,8 @@ class CL(object):
|
|||
cl = self
|
||||
s = _change_prolog
|
||||
s += "\n"
|
||||
if cl.original_author:
|
||||
s += "Author: " + cl.original_author + "\n"
|
||||
if cl.url != '':
|
||||
s += 'URL: ' + cl.url + ' # cannot edit\n\n'
|
||||
s += "Reviewer: " + JoinComma(cl.reviewer) + "\n"
|
||||
|
@ -109,10 +120,11 @@ class CL(object):
|
|||
else:
|
||||
s += Indent(cl.desc, "\t")
|
||||
s += "\n"
|
||||
s += "Files:\n"
|
||||
for f in cl.files:
|
||||
s += "\t" + f + "\n"
|
||||
s += "\n"
|
||||
if cl.local or cl.name == "new":
|
||||
s += "Files:\n"
|
||||
for f in cl.files:
|
||||
s += "\t" + f + "\n"
|
||||
s += "\n"
|
||||
return s
|
||||
|
||||
def PendingText(self):
|
||||
|
@ -120,6 +132,8 @@ class CL(object):
|
|||
s = cl.name + ":" + "\n"
|
||||
s += Indent(cl.desc, "\t")
|
||||
s += "\n"
|
||||
if cl.original_author:
|
||||
s += "\tAuthor: " + cl.original_author + "\n"
|
||||
s += "\tReviewer: " + JoinComma(cl.reviewer) + "\n"
|
||||
s += "\tCC: " + JoinComma(cl.cc) + "\n"
|
||||
s += "\tFiles:\n"
|
||||
|
@ -136,7 +150,7 @@ class CL(object):
|
|||
f.write(self.DiskText())
|
||||
f.close()
|
||||
os.rename(path+'!', path)
|
||||
if self.web:
|
||||
if self.web and not self.original_author:
|
||||
EditDesc(self.name, desc=self.desc,
|
||||
reviewers=JoinComma(self.reviewer), cc=JoinComma(self.cc))
|
||||
|
||||
|
@ -211,6 +225,7 @@ def ParseCL(text, name):
|
|||
sname = None
|
||||
lineno = 0
|
||||
sections = {
|
||||
'Author': '',
|
||||
'Description': '',
|
||||
'Files': '',
|
||||
'URL': '',
|
||||
|
@ -242,6 +257,8 @@ def ParseCL(text, name):
|
|||
sections[k] = StripCommon(sections[k]).rstrip()
|
||||
|
||||
cl = CL(name)
|
||||
if sections['Author']:
|
||||
cl.original_author = sections['Author']
|
||||
cl.desc = sections['Description']
|
||||
for line in sections['Files'].split('\n'):
|
||||
i = line.find('#')
|
||||
|
@ -303,7 +320,7 @@ def LoadCL(ui, repo, name, web=True):
|
|||
try:
|
||||
f = GetSettings(name)
|
||||
except:
|
||||
return None, "cannot load CL data from code review server: "+ExceptionDetail()
|
||||
return None, "cannot load CL %s from code review server: %s" % (name, ExceptionDetail())
|
||||
if 'reviewers' not in f:
|
||||
return None, "malformed response loading CL data from code review server"
|
||||
cl.reviewer = SplitCommaSpace(f['reviewers'])
|
||||
|
@ -515,6 +532,8 @@ def CommandLineCL(ui, repo, pats, opts):
|
|||
if opts.get('message'):
|
||||
return None, "cannot use -m with existing CL"
|
||||
cl, err = LoadCL(ui, repo, pats[0], web=True)
|
||||
if err != "":
|
||||
return None, err
|
||||
else:
|
||||
cl = CL("new")
|
||||
cl.local = True
|
||||
|
@ -610,6 +629,14 @@ def change(ui, repo, *pats, **opts):
|
|||
|
||||
In the absence of options, the change command opens the
|
||||
change list for editing in the default editor.
|
||||
|
||||
Deleting a change with the -d or -D flag does not affect
|
||||
the contents of the files listed in that change. To revert
|
||||
the files listed in a change, use
|
||||
|
||||
hg revert @123456
|
||||
|
||||
before running hg change -d 123456.
|
||||
"""
|
||||
|
||||
dirty = {}
|
||||
|
@ -631,15 +658,23 @@ def change(ui, repo, *pats, **opts):
|
|||
taken = TakenFiles(ui, repo)
|
||||
files = Sub(files, taken)
|
||||
|
||||
if opts["delete"]:
|
||||
if opts["delete"] or opts["deletelocal"]:
|
||||
if opts["delete"] and opts["deletelocal"]:
|
||||
return "cannot use -d and -D together"
|
||||
flag = "-d"
|
||||
if opts["deletelocal"]:
|
||||
flag = "-D"
|
||||
if name == "new":
|
||||
return "cannot use -d with file patterns"
|
||||
return "cannot use "+flag+" with file patterns"
|
||||
if opts["stdin"] or opts["stdout"]:
|
||||
return "cannot use -d with -i or -o"
|
||||
return "cannot use "+flag+" 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")
|
||||
if opts["delete"]:
|
||||
if cl.original_author:
|
||||
return "original author must delete CL; hg change -D will remove locally"
|
||||
PostMessage(cl.name, "*** Abandoned ***", send_mail="checked")
|
||||
EditDesc(cl.name, closed="checked")
|
||||
cl.Delete(ui, repo)
|
||||
return
|
||||
|
||||
|
@ -689,6 +724,55 @@ def code_login(ui, repo, **opts):
|
|||
"""
|
||||
MySend(None)
|
||||
|
||||
def clpatch(ui, repo, clname, **opts):
|
||||
"""import a patch from the code review server
|
||||
|
||||
Imports a patch from the code review server into the local client.
|
||||
If the local client has already modified any of the files that the
|
||||
patch modifies, this command will refuse to apply the patch.
|
||||
|
||||
Submitting an imported patch will keep the original author's
|
||||
name as the Author: line but add your own name to a Committer: line.
|
||||
"""
|
||||
cl, patch, err = DownloadCL(ui, repo, clname)
|
||||
argv = ["hgpatch"]
|
||||
if opts["no_incoming"]:
|
||||
argv += ["--checksync=false"]
|
||||
if err != "":
|
||||
return err
|
||||
try:
|
||||
cmd = subprocess.Popen(argv, shell=False, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=None, close_fds=True)
|
||||
except:
|
||||
return "hgpatch: " + ExceptionDetail()
|
||||
if os.fork() == 0:
|
||||
cmd.stdin.write(patch)
|
||||
os._exit(0)
|
||||
cmd.stdin.close()
|
||||
out = cmd.stdout.read()
|
||||
if cmd.wait() != 0:
|
||||
return "hgpatch failed"
|
||||
cl.local = True
|
||||
cl.files = out.strip().split()
|
||||
files = ChangedFiles(ui, repo, [], opts)
|
||||
extra = Sub(cl.files, files)
|
||||
if extra:
|
||||
ui.warn("warning: these files were listed in the patch but not changed:\n\t" + "\n\t".join(extra) + "\n")
|
||||
cl.Flush(ui, repo)
|
||||
ui.write(cl.PendingText() + "\n")
|
||||
|
||||
def download(ui, repo, clname, **opts):
|
||||
"""download a change from the code review server
|
||||
|
||||
Download prints a description of the given change list
|
||||
followed by its diff, downloaded from the code review server.
|
||||
"""
|
||||
cl, patch, err = DownloadCL(ui, repo, clname)
|
||||
if err != "":
|
||||
return err
|
||||
ui.write(cl.EditorText() + "\n")
|
||||
ui.write(patch + "\n")
|
||||
return
|
||||
|
||||
def file(ui, repo, clname, pat, *pats, **opts):
|
||||
"""assign files to or remove files from a change list
|
||||
|
||||
|
@ -784,7 +868,10 @@ def mail(ui, repo, *pats, **opts):
|
|||
if not cl.reviewer:
|
||||
return "no reviewers listed in CL"
|
||||
cl.Upload(ui, repo)
|
||||
pmsg = "Hello " + JoinComma(cl.reviewer) + ",\n"
|
||||
pmsg = "Hello " + JoinComma(cl.reviewer)
|
||||
if cl.cc:
|
||||
pmsg += " (cc: %s)" % (', '.join(cl.cc),)
|
||||
pmsg += ",\n"
|
||||
pmsg += "\n"
|
||||
pmsg += "I'd like you to review the following change.\n"
|
||||
PostMessage(cl.name, pmsg, send_mail="checked", subject=cl.Subject())
|
||||
|
@ -819,18 +906,35 @@ def reposetup(ui, repo):
|
|||
cmdutil.match = ReplacementForCmdutilMatch
|
||||
RietveldSetup(ui, repo)
|
||||
|
||||
def CheckContributor(ui, repo):
|
||||
user = ui.config("ui", "username")
|
||||
def CheckContributor(ui, repo, user=None):
|
||||
if not user:
|
||||
raise util.Abort("[ui] username is not configured in .hgrc")
|
||||
user = ui.config("ui", "username")
|
||||
if not user:
|
||||
raise util.Abort("[ui] username is not configured in .hgrc")
|
||||
userline = FindContributor(ui, repo, user, warn=False)
|
||||
if not userline:
|
||||
raise util.Abort("cannot find %s in CONTRIBUTORS" % (user,))
|
||||
return userline
|
||||
|
||||
def FindContributor(ui, repo, user, warn=True):
|
||||
try:
|
||||
f = open(repo.root + '/CONTRIBUTORS', 'r')
|
||||
except:
|
||||
raise util.Abort("cannot open %s: %s" % (repo.root+'/CONTRIBUTORS', ExceptionDetail()))
|
||||
for line in f.readlines():
|
||||
if line.rstrip() == user.rstrip():
|
||||
return
|
||||
raise util.Abort("cannot find %s in CONTRIBUTORS" % (user,))
|
||||
line = line.rstrip()
|
||||
if line.startswith('#'):
|
||||
continue
|
||||
if line == user:
|
||||
return line
|
||||
match = re.match(r"(.*) <(.*)>", line)
|
||||
if not match:
|
||||
continue
|
||||
if match.group(2) == user:
|
||||
return line
|
||||
if warn:
|
||||
ui.warn("warning: cannot find %s in CONTRIBUTORS\n" % (user,))
|
||||
return None
|
||||
|
||||
def submit(ui, repo, *pats, **opts):
|
||||
"""submit change to remote repository
|
||||
|
@ -838,7 +942,6 @@ def submit(ui, repo, *pats, **opts):
|
|||
Submits change to remote repository.
|
||||
Bails out if the local repository is not in sync with the remote one.
|
||||
"""
|
||||
CheckContributor(ui, repo)
|
||||
repo.ui.quiet = True
|
||||
if not opts["no_incoming"] and Incoming(ui, repo, opts):
|
||||
return "local repository out of date; must sync before submit"
|
||||
|
@ -847,6 +950,11 @@ def submit(ui, repo, *pats, **opts):
|
|||
if err != "":
|
||||
return err
|
||||
|
||||
user = None
|
||||
if cl.original_author:
|
||||
user = cl.original_author
|
||||
userline = CheckContributor(ui, repo, user)
|
||||
|
||||
about = ""
|
||||
if cl.reviewer:
|
||||
about += "R=" + JoinComma([CutDomain(s) for s in cl.reviewer]) + "\n"
|
||||
|
@ -864,16 +972,30 @@ def submit(ui, repo, *pats, **opts):
|
|||
return "cannot submit non-local CL"
|
||||
|
||||
# upload, to sync current patch and also get change number if CL is new.
|
||||
cl.Upload(ui, repo)
|
||||
if not cl.original_author:
|
||||
cl.Upload(ui, repo)
|
||||
about += "%s%s\n" % (server_url_base, cl.name)
|
||||
|
||||
if cl.original_author:
|
||||
about += "\nCommitter: " + CheckContributor(ui, repo, None) + "\n"
|
||||
|
||||
# submit changes locally
|
||||
date = opts.get('date')
|
||||
if date:
|
||||
opts['date'] = util.parsedate(date)
|
||||
opts['message'] = cl.desc.rstrip() + "\n\n" + about
|
||||
|
||||
if opts['dryrun']:
|
||||
print "NOT SUBMITTING:"
|
||||
print "User: ", userline
|
||||
print "Message:"
|
||||
print Indent(opts['message'], "\t")
|
||||
print "Files:"
|
||||
print Indent('\n'.join(cl.files), "\t")
|
||||
return "dry run; not submitted"
|
||||
|
||||
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'], userline, opts.get('date'), m)
|
||||
if not node:
|
||||
return "nothing changed"
|
||||
|
||||
|
@ -906,7 +1028,8 @@ def submit(ui, repo, *pats, **opts):
|
|||
print >>sys.stderr, "URL: ", url
|
||||
pmsg = "*** Submitted as " + changeURL + " ***\n\n" + opts['message']
|
||||
PostMessage(cl.name, pmsg, send_mail="checked")
|
||||
EditDesc(cl.name, closed="checked")
|
||||
if not cl.original_author:
|
||||
EditDesc(cl.name, closed="checked")
|
||||
cl.Delete(ui, repo)
|
||||
|
||||
def sync(ui, repo, **opts):
|
||||
|
@ -1002,10 +1125,18 @@ cmdtable = {
|
|||
change,
|
||||
[
|
||||
('d', 'delete', None, 'delete existing change list'),
|
||||
('D', 'deletelocal', None, 'delete locally, but do not change CL on server'),
|
||||
('i', 'stdin', None, 'read change list from standard input'),
|
||||
('o', 'stdout', None, 'print change list to standard output'),
|
||||
],
|
||||
"[-i] [-o] change# or FILE ..."
|
||||
"[-d | -D] [-i] [-o] change# or FILE ..."
|
||||
),
|
||||
"^clpatch": (
|
||||
clpatch,
|
||||
[
|
||||
('', 'no_incoming', None, 'disable check for incoming changes'),
|
||||
],
|
||||
"change#"
|
||||
),
|
||||
# Would prefer to call this codereview-login, but then
|
||||
# hg help codereview prints the help for this command
|
||||
|
@ -1020,6 +1151,11 @@ cmdtable = {
|
|||
[],
|
||||
"",
|
||||
),
|
||||
"^download": (
|
||||
download,
|
||||
[],
|
||||
"change#"
|
||||
),
|
||||
"^file": (
|
||||
file,
|
||||
[
|
||||
|
@ -1049,6 +1185,7 @@ cmdtable = {
|
|||
submit,
|
||||
review_opts + [
|
||||
('', 'no_incoming', None, 'disable initial incoming check (for testing)'),
|
||||
('n', 'dryrun', None, 'make change only locally (for testing)'),
|
||||
] + commands.walkopts + commands.commitopts + commands.commitopts2,
|
||||
"[-r reviewer] [--cc cc] [change# | file ...]"
|
||||
),
|
||||
|
@ -1139,6 +1276,57 @@ def IsRietveldSubmitted(ui, clname, hex):
|
|||
return True
|
||||
return False
|
||||
|
||||
def DownloadCL(ui, repo, clname):
|
||||
cl, err = LoadCL(ui, repo, clname)
|
||||
if err != "":
|
||||
return None, None, "error loading CL %s: %s" % (clname, ExceptionDetail())
|
||||
|
||||
# Grab RSS feed to learn about CL
|
||||
feed = XMLGet(ui, "/rss/issue/" + clname)
|
||||
if feed is None:
|
||||
return None, None, "cannot download CL"
|
||||
|
||||
# Find most recent diff
|
||||
diff = None
|
||||
prefix = 'http://' + server + '/'
|
||||
for link in feed.findall("{http://www.w3.org/2005/Atom}entry/{http://www.w3.org/2005/Atom}link"):
|
||||
if link.get('rel') != 'alternate':
|
||||
continue
|
||||
text = link.get('href')
|
||||
if not text.startswith(prefix) or not text.endswith('.diff'):
|
||||
continue
|
||||
diff = text[len(prefix)-1:]
|
||||
if diff is None:
|
||||
return None, None, "CL has no diff"
|
||||
diffdata = MySend(diff, force_auth=False)
|
||||
|
||||
# Find author - first entry will be author who created CL.
|
||||
nick = None
|
||||
for author in feed.findall("{http://www.w3.org/2005/Atom}entry/{http://www.w3.org/2005/Atom}author/{http://www.w3.org/2005/Atom}name"):
|
||||
nick = author.findtext("", None).strip()
|
||||
break
|
||||
if not nick:
|
||||
return None, None, "CL has no author"
|
||||
|
||||
# The author is just a nickname: get the real email address.
|
||||
try:
|
||||
data = MySend("/user_popup/" + nick, force_auth=False)
|
||||
except:
|
||||
return None, None, "error looking up %s: %s" % (nick, ExceptionDetail())
|
||||
match = re.match(r"<b>(.*) \((.*)\)</b>", data)
|
||||
if not match or match.group(2) != nick:
|
||||
return None, None, "error looking up %s: cannot parse result" % (nick,)
|
||||
email = match.group(1)
|
||||
|
||||
# Temporary hack until we move to the public code review server.
|
||||
email = re.sub("@google.com$", "@golang.org", email)
|
||||
|
||||
# Print warning if email is not in CONTRIBUTORS file.
|
||||
FindContributor(ui, repo, email)
|
||||
cl.original_author = email
|
||||
|
||||
return cl, diffdata, ""
|
||||
|
||||
# Like upload.py Send but only authenticates when the
|
||||
# redirect is to www.google.com/accounts. This keeps
|
||||
# unnecessary redirects from happening during testing.
|
||||
|
@ -1210,10 +1398,22 @@ def GetForm(url):
|
|||
f.map[k] = v.replace("\r\n", "\n");
|
||||
return f.map
|
||||
|
||||
# Fetch the settings for the CL, like reviewer and CC list, by
|
||||
# scraping the Rietveld editing forms.
|
||||
def GetSettings(issue):
|
||||
f = GetForm("/" + issue + "/edit")
|
||||
# The /issue/edit page has everything but only the
|
||||
# CL owner is allowed to fetch it (and submit it).
|
||||
f = None
|
||||
try:
|
||||
f = GetForm("/" + issue + "/edit")
|
||||
except:
|
||||
pass
|
||||
if not f or 'reviewers' not in f:
|
||||
# Maybe we're not the CL owner. Fall back to the
|
||||
# /publish page, which has the reviewer and CC lists,
|
||||
# and then fetch the description separately.
|
||||
f = GetForm("/" + issue + "/publish")
|
||||
f['description'] = MySend("/"+issue+"/description", force_auth=False)
|
||||
return f
|
||||
|
||||
def CreateIssue(subject, desc):
|
||||
|
|
|
@ -18,8 +18,11 @@ import (
|
|||
"strings";
|
||||
)
|
||||
|
||||
var checkSync = flag.Bool("checksync", true, "check whether repository is out of sync")
|
||||
|
||||
func usage() {
|
||||
fmt.Fprintf(os.Stderr, "usage: hgpatch [patchfile]\n");
|
||||
fmt.Fprintf(os.Stderr, "usage: hgpatch [options] [patchfile]\n");
|
||||
flag.PrintDefaults();
|
||||
os.Exit(2);
|
||||
}
|
||||
|
||||
|
@ -49,11 +52,8 @@ func main() {
|
|||
chk(err);
|
||||
chk(os.Chdir(root));
|
||||
|
||||
op, err := pset.Apply(io.ReadFile);
|
||||
chk(err);
|
||||
|
||||
// Make sure there are no pending changes on the server.
|
||||
if hgIncoming() {
|
||||
if *checkSync && hgIncoming() {
|
||||
fmt.Fprintf(os.Stderr, "incoming changes waiting; run hg sync first\n");
|
||||
os.Exit(2);
|
||||
}
|
||||
|
@ -66,16 +66,15 @@ func main() {
|
|||
dirty[f] = 1;
|
||||
}
|
||||
conflict := make(map[string]int);
|
||||
for i := range op {
|
||||
o := &op[i];
|
||||
if o.Verb == patch.Delete || o.Verb == patch.Rename {
|
||||
if _, ok := dirty[o.Src]; ok {
|
||||
conflict[o.Src] = 1;
|
||||
for _, f := range pset.File {
|
||||
if f.Verb == patch.Delete || f.Verb == patch.Rename {
|
||||
if _, ok := dirty[f.Src]; ok {
|
||||
conflict[f.Src] = 1;
|
||||
}
|
||||
}
|
||||
if o.Verb != patch.Delete {
|
||||
if _, ok := dirty[o.Dst]; ok {
|
||||
conflict[o.Dst] = 1;
|
||||
if f.Verb != patch.Delete {
|
||||
if _, ok := dirty[f.Dst]; ok {
|
||||
conflict[f.Dst] = 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -87,7 +86,11 @@ func main() {
|
|||
os.Exit(2);
|
||||
}
|
||||
|
||||
// Apply to local copy: order of commands matters.
|
||||
// Apply changes in memory.
|
||||
op, err := pset.Apply(io.ReadFile);
|
||||
chk(err);
|
||||
|
||||
// Write changes to disk copy: order of commands matters.
|
||||
// Accumulate undo log as we go, in case there is an error.
|
||||
// Also accumulate list of modified files to print at end.
|
||||
changed := make(map[string]int);
|
||||
|
@ -343,7 +346,7 @@ func run(argv []string, input []byte) (out string, err os.Error) {
|
|||
}
|
||||
lookPathCache[argv[0]] = prog;
|
||||
}
|
||||
fmt.Fprintf(os.Stderr, "%v\n", argv);
|
||||
// fmt.Fprintf(os.Stderr, "%v\n", argv);
|
||||
var cmd *exec.Cmd;
|
||||
if len(input) == 0 {
|
||||
cmd, err = exec.Run(prog, argv, os.Environ(), exec.DevNull, exec.Pipe, exec.MergeWithStdout);
|
||||
|
|
Loading…
Reference in a new issue