aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorCaio Marcelo de Oliveira Filho <caio.oliveira@intel.com>2016-03-01 15:04:52 -0300
committerAndrew Gerrand <adg@golang.org>2016-09-15 00:16:11 +0000
commit50528ca764718386baa44c0daec70913cf14addc (patch)
treef821b87070e4fffd4ec535eb1d90daa7c085fb07
parente143ba9c054637a068c21480cee8249caa0d8f9f (diff)
downloadgo-x-review-50528ca764718386baa44c0daec70913cf14addc.tar.xz
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>
-rw-r--r--git-codereview/change.go63
-rw-r--r--git-codereview/change_test.go54
-rw-r--r--git-codereview/review.go5
3 files changed, 121 insertions, 1 deletions
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 @@ import (
"fmt"
"os"
"regexp"
+ "strconv"
"strings"
)
@@ -109,6 +110,19 @@ func commitChanges(amend bool) {
}
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 @@ func checkoutOrCreate(target string) {
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 @@ func TestChangeFailAmendWithMultiplePending(t *testing.T) {
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 @@ Available commands:
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.