git-codereview: allow changing to a CL
Now 'git change NNNN/PP' can be used to checkout (detached) the git
commit corresponding to the patchset PP of the change NNNN. If the
patchset is omitted, the current (latest) will be used.
Branch names that are only numbers or numbers separated by a slash will
always be understood as refering to CL. This break workflows that were
using numbers as branch names, but this is expected to be uncommon. The
workaround in this case is to use checkout directly.
It gets the commit fetching the specific reference inside origin's
refs/changes/ and check it out. It uses the gerrit API only if
querying the current patchset is needed.
Given that it's easy to mistype the number, change to a CL will show the
subject of the commit it just checked out.
Fixes golang/go#9255.
Change-Id: I01b98f4672051a52b8b9110a41d93f3ffefebaf1
Reviewed-on: https://go-review.googlesource.com/20101
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Andrew Gerrand <adg@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
diff --git a/git-codereview/change.go b/git-codereview/change.go
index fac0f94..6e4fd54 100644
--- a/git-codereview/change.go
+++ b/git-codereview/change.go
@@ -8,6 +8,7 @@
"fmt"
"os"
"regexp"
+ "strconv"
"strings"
)
@@ -109,6 +110,19 @@
}
func checkoutOrCreate(target string) {
+ // If it's a valid Gerrit number, checkout the CL.
+ cl, ps, isCL := parseCL(target)
+ if isCL {
+ if !haveGerrit() {
+ dief("cannot change to a CL without gerrit")
+ }
+ if HasStagedChanges() || HasUnstagedChanges() {
+ dief("cannot change to a CL with uncommitted work")
+ }
+ checkoutCL(cl, ps)
+ return
+ }
+
if strings.ToUpper(target) == "HEAD" {
// Git gets very upset and confused if you 'git change head'
// on systems with case-insensitive file names: the branch
@@ -159,6 +173,55 @@
printf("created branch %v tracking %s.", target, origin)
}
+// Checkout the patch set of the given CL. When patch set is empty, use the latest.
+func checkoutCL(cl, ps string) {
+ if ps == "" {
+ change, err := readGerritChange(cl + "?o=CURRENT_REVISION")
+ if err != nil {
+ dief("cannot change to CL %s: %v", cl, err)
+ }
+ rev, ok := change.Revisions[change.CurrentRevision]
+ if !ok {
+ dief("cannot change to CL %s: invalid current revision from gerrit", cl)
+ }
+ ps = strconv.Itoa(rev.Number)
+ }
+
+ var group string
+ if len(cl) > 1 {
+ group = cl[len(cl)-2:]
+ } else {
+ group = "0" + cl
+ }
+ ref := fmt.Sprintf("refs/changes/%s/%s/%s", group, cl, ps)
+
+ err := runErr("git", "fetch", "-q", "origin", ref)
+ if err != nil {
+ dief("cannot change to CL %s/%s: %v", cl, ps, err)
+ }
+ err = runErr("git", "checkout", "-q", "FETCH_HEAD")
+ if err != nil {
+ dief("cannot change to CL %s/%s: %v", cl, ps, err)
+ }
+ subject, err := trimErr(cmdOutputErr("git", "log", "--format=%s", "-1"))
+ if err != nil {
+ printf("changed to CL %s/%s.", cl, ps)
+ dief("cannot read change subject from git: %v", err)
+ }
+ printf("changed to CL %s/%s.\n\t%s", cl, ps, subject)
+}
+
+var parseCLRE = regexp.MustCompile(`^([0-9]+)(?:/([0-9]+))?$`)
+
+// parseCL validates and splits the CL number and patch set (if present).
+func parseCL(arg string) (cl, patchset string, ok bool) {
+ m := parseCLRE.FindStringSubmatch(arg)
+ if len(m) == 0 {
+ return "", "", false
+ }
+ return m[1], m[2], true
+}
+
var messageRE = regexp.MustCompile(`^(\[[a-zA-Z0-9.-]+\] )?[a-zA-Z0-9-/,. ]+: `)
func commitMessageOK() bool {
diff --git a/git-codereview/change_test.go b/git-codereview/change_test.go
index 05e5c82..531f90e 100644
--- a/git-codereview/change_test.go
+++ b/git-codereview/change_test.go
@@ -4,7 +4,11 @@
package main
-import "testing"
+import (
+ "fmt"
+ "strings"
+ "testing"
+)
func TestChange(t *testing.T) {
gt := newGitTest(t)
@@ -102,3 +106,51 @@
testMainDied(t, "change")
testPrintedStderr(t, "multiple changes pending")
}
+
+func TestChangeCL(t *testing.T) {
+ gt := newGitTest(t)
+ defer gt.done()
+
+ srv := newGerritServer(t)
+ defer srv.done()
+
+ // Ensure that 'change' with a CL accepts we have gerrit. Test address is injected by newGerritServer.
+ write(t, gt.server+"/codereview.cfg", "gerrit: on")
+ trun(t, gt.server, "git", "add", "codereview.cfg")
+ trun(t, gt.server, "git", "commit", "-m", "codereview.cfg on master")
+ trun(t, gt.client, "git", "pull")
+ defer srv.done()
+
+ hash1 := trim(trun(t, gt.server, "git", "rev-parse", "dev.branch"))
+ hash2 := trim(trun(t, gt.server, "git", "rev-parse", "release.branch"))
+ trun(t, gt.server, "git", "update-ref", "refs/changes/00/100/1", hash1)
+ trun(t, gt.server, "git", "update-ref", "refs/changes/00/100/2", hash2)
+ trun(t, gt.server, "git", "update-ref", "refs/changes/00/100/3", hash1)
+ srv.setReply("/a/changes/100", gerritReply{f: func() gerritReply {
+ changeJSON := `{
+ "current_revision": "HASH",
+ "revisions": {
+ "HASH": {
+ "_number": 3
+ }
+ }
+ }`
+ changeJSON = strings.Replace(changeJSON, "HASH", hash1, -1)
+ return gerritReply{body: ")]}'\n" + changeJSON}
+ }})
+
+ checkChangeCL := func(arg, ref, hash string) {
+ testMain(t, "change", "master")
+ testMain(t, "change", arg)
+ testRan(t,
+ fmt.Sprintf("git fetch -q origin %s", ref),
+ "git checkout -q FETCH_HEAD")
+ if hash != trim(trun(t, gt.client, "git", "rev-parse", "HEAD")) {
+ t.Fatalf("hash do not match for CL %s", arg)
+ }
+ }
+
+ checkChangeCL("100/1", "refs/changes/00/100/1", hash1)
+ checkChangeCL("100/2", "refs/changes/00/100/2", hash2)
+ checkChangeCL("100", "refs/changes/00/100/3", hash1)
+}
diff --git a/git-codereview/review.go b/git-codereview/review.go
index c83a95b..4d2ebe6 100644
--- a/git-codereview/review.go
+++ b/git-codereview/review.go
@@ -63,6 +63,11 @@
If -a is specified, automatically add any unstaged changes in
tracked files during commit.
+ change NNNN[/PP]
+ Checkout the commit corresponding to CL number NNNN and
+ patch set PP from Gerrit.
+ If the patch set is omitted, use the current patch set.
+
gofmt [-l]
Run gofmt on all tracked files in the staging area and the
working tree.
