git-codereview: new sync-branch and related fixes
This CL adds a new command, "git codereview sync-branch",
which does the appropriate git merge for the current branch.
This CL also fixes a bug in "git codereview branchpoint",
and therefore also commands like "git codereview pending",
which was getting the branchpoint wrong for merges,
with the effect that a merge showed too many pending CLs.
This CL also fixes a bug in "git codereview change", which was
formerly willing to run "git checkout" with a pending merge,
which had the effect of flattening the merge mysteriously.
Now it detects the merge and refuses to run.
All of this should make merges easier and less error-prone
as we use dev branches more often.
With the earlier CL in this stack that allows working directly
on local branches, this is now a great way to run a merge
updating dev.regabi:
git change dev.regabi
git sync-branch
(with appropriate aliases to avoid typing "codereview").
Fixes golang/go#26201.
Change-Id: Ic24603123ca5135a72004309f5bb208ff149c9eb
Reviewed-on: https://go-review.googlesource.com/c/review/+/279772
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
diff --git a/git-codereview/branch.go b/git-codereview/branch.go
index 7c9b282..787a80d 100644
--- a/git-codereview/branch.go
+++ b/git-codereview/branch.go
@@ -7,9 +7,11 @@
import (
"bytes"
"fmt"
+ "io/ioutil"
"net/url"
"os"
"os/exec"
+ "path/filepath"
"regexp"
"runtime"
"strings"
@@ -17,12 +19,16 @@
// Branch describes a Git branch.
type Branch struct {
- Name string // branch name
+ Name string // branch name
+ Current bool // branch is currently checked out
+
loadedPending bool // following fields are valid
originBranch string // upstream origin branch
commitsAhead int // number of commits ahead of origin branch
branchpoint string // latest commit hash shared with origin branch
pending []*Commit // pending commits, newest first (children before parents)
+
+ config map[string]string // cached config; use Config instead
}
// A Commit describes a single pending commit on a Git branch.
@@ -41,10 +47,34 @@
committed []string // list of files in this commit
}
+// Config returns the configuration for the branch.
+func (b *Branch) Config() map[string]string {
+ if b.config != nil {
+ return b.config
+ }
+ var cfgText string
+ if b.Current {
+ data, _ := ioutil.ReadFile(filepath.Join(repoRoot(), "codereview.cfg"))
+ cfgText = string(data)
+ } else {
+ out, err := cmdOutputDirErr(repoRoot(), "git", "show", b.Name+":codereview.cfg")
+ if err == nil {
+ cfgText = out
+ }
+ }
+ cfg, err := parseConfig(cfgText)
+ if err != nil {
+ verbosef("failed to load config for branch %v: %v", b.Name, err)
+ cfg = make(map[string]string)
+ }
+ b.config = cfg
+ return b.config
+}
+
// CurrentBranch returns the current branch.
func CurrentBranch() *Branch {
name := strings.TrimPrefix(trim(cmdOutput("git", "rev-parse", "--abbrev-ref", "HEAD")), "heads/")
- return &Branch{Name: name}
+ return &Branch{Name: name, Current: true}
}
// DetachedHead reports whether branch b corresponds to a detached HEAD
@@ -68,6 +98,41 @@
if b.originBranch != "" {
return b.originBranch
}
+
+ cfg := b.Config()["branch"]
+ upstream := ""
+ if cfg != "" {
+ upstream = "origin/" + cfg
+ }
+ gitUpstream := b.gitOriginBranch()
+ if upstream == "" {
+ upstream = gitUpstream
+ }
+ if upstream == "" {
+ // Assume branch was created before we set upstream correctly.
+ // See if origin/main exists; if so, use it.
+ // Otherwise, fall back to origin/master.
+ argv := []string{"git", "rev-parse", "--abbrev-ref", "origin/main"}
+ cmd := exec.Command(argv[0], argv[1:]...)
+ setEnglishLocale(cmd)
+ if err := cmd.Run(); err == nil {
+ upstream = "origin/main"
+ } else {
+ upstream = "origin/master"
+ }
+ }
+
+ if gitUpstream != upstream && b.Current {
+ // Best effort attempt to correct setting for next time,
+ // and for "git status".
+ exec.Command("git", "branch", "-u", upstream).Run()
+ }
+
+ b.originBranch = upstream
+ return b.originBranch
+}
+
+func (b *Branch) gitOriginBranch() string {
argv := []string{"git", "rev-parse", "--abbrev-ref", b.Name + "@{u}"}
cmd := exec.Command(argv[0], argv[1:]...)
if runtime.GOOS == "windows" {
@@ -82,28 +147,14 @@
out, err := cmd.CombinedOutput()
if err == nil && len(out) > 0 {
- b.originBranch = string(bytes.TrimSpace(out))
- return b.originBranch
+ return string(bytes.TrimSpace(out))
}
// Have seen both "No upstream configured" and "no upstream configured".
if strings.Contains(string(out), "upstream configured") {
- // Assume branch was created before we set upstream correctly.
- // See if origin/main exists; if so, use it.
- // Otherwise, fall back to origin/master.
- argv := []string{"git", "rev-parse", "--abbrev-ref", "origin/main"}
- cmd := exec.Command(argv[0], argv[1:]...)
- setEnglishLocale(cmd)
- if out, err := cmd.CombinedOutput(); err == nil {
- b.originBranch = string(bytes.TrimSpace(out))
- // Best effort attempt to correct setting for next time,
- // and for "git status".
- exec.Command("git", "branch", "-u", "origin/main").Run()
- return b.originBranch
- }
- b.originBranch = "origin/master"
- return b.originBranch
+ return ""
}
+
fmt.Fprintf(stderr(), "%v\n%s\n", commandString(argv[0], argv[1:]), out)
dief("%v", err)
panic("not reached")
@@ -135,6 +186,20 @@
return b.branchpoint
}
+func gitHash(expr string) string {
+ out, err := cmdOutputErr("git", "rev-parse", expr)
+ if err != nil {
+ dief("cannot resolve %s: %v\n%s", expr, err, out)
+ }
+ if strings.HasPrefix(expr, "-") {
+ // Git has a bug where it will just echo these back with no error.
+ // (Try "git rev-parse -qwerty".)
+ // Reject them ourselves instead.
+ dief("cannot resolve %s: invalid reference", expr)
+ }
+ return trim(out)
+}
+
func (b *Branch) loadPending() {
if b.loadedPending {
return
@@ -142,7 +207,13 @@
b.loadedPending = true
// In case of early return.
- b.branchpoint = trim(cmdOutput("git", "rev-parse", "HEAD"))
+ // But avoid the git exec unless really needed.
+ b.branchpoint = ""
+ defer func() {
+ if b.branchpoint == "" {
+ b.branchpoint = gitHash("HEAD")
+ }
+ }()
if b.DetachedHead() {
return
@@ -165,7 +236,6 @@
for i, field := range fields {
fields[i] = strings.TrimLeft(field, "\r\n")
}
- foundMergeBranchpoint := false
for i := 0; i+numField <= len(fields); i += numField {
c := &Commit{
Hash: fields[i],
@@ -184,13 +254,15 @@
// even if we later see additional commits on a different branch leading down to
// a lower location on the same origin branch.
// Check c.Merge (the second parent) too, so we don't depend on the parent order.
- if strings.Contains(cmdOutput("git", "branch", "-a", "--contains", c.Parent), " "+origin+"\n") {
- foundMergeBranchpoint = true
+ if strings.Contains(cmdOutput("git", "branch", "-a", "--contains", c.Parent), " remotes/"+origin+"\n") {
+ b.pending = append(b.pending, c)
b.branchpoint = c.Parent
+ break
}
- if strings.Contains(cmdOutput("git", "branch", "-a", "--contains", c.Merge), " "+origin+"\n") {
- foundMergeBranchpoint = true
+ if strings.Contains(cmdOutput("git", "branch", "-a", "--contains", c.Merge), " remotes/"+origin+"\n") {
+ b.pending = append(b.pending, c)
b.branchpoint = c.Merge
+ break
}
}
for _, line := range lines(c.Message) {
@@ -203,11 +275,8 @@
c.ChangeID = line[len("Change-Id: "):]
}
}
-
b.pending = append(b.pending, c)
- if !foundMergeBranchpoint {
- b.branchpoint = c.Parent
- }
+ b.branchpoint = c.Parent
}
b.commitsAhead = len(b.pending)
}
@@ -418,6 +487,9 @@
// ListFiles returns the list of files in a given commit.
func ListFiles(c *Commit) []string {
+ if c.Parent == "" {
+ return nil
+ }
return nonBlankLines(cmdOutput("git", "diff", "--name-only", c.Parent, c.Hash, "--"))
}
diff --git a/git-codereview/branch_test.go b/git-codereview/branch_test.go
index e4530eb..9e999b7 100644
--- a/git-codereview/branch_test.go
+++ b/git-codereview/branch_test.go
@@ -166,9 +166,11 @@
hash := strings.TrimSpace(trun(t, gt.client, "git", "rev-parse", "HEAD"))
- // merge dev.branch
- testMain(t, "change", "work")
+ // Merge dev.branch but delete the codereview.cfg that comes in,
+ // or else we'll think we are on the wrong branch.
trun(t, gt.client, "git", "merge", "-m", "merge", "origin/dev.branch")
+ trun(t, gt.client, "git", "rm", "codereview.cfg")
+ trun(t, gt.client, "git", "commit", "-m", "rm codereview.cfg")
// check branchpoint is old head (despite this commit having two parents)
bp := CurrentBranch().Branchpoint()
diff --git a/git-codereview/change.go b/git-codereview/change.go
index 5320d9d..24cbb71 100644
--- a/git-codereview/change.go
+++ b/git-codereview/change.go
@@ -26,6 +26,13 @@
exit(2)
}
+ if _, err := cmdOutputErr("git", "rev-parse", "--abbrev-ref", "MERGE_HEAD"); err == nil {
+ diePendingMerge("change")
+ }
+ if _, err := cmdOutputErr("git", "rev-parse", "--abbrev-ref", "REBASE_HEAD"); err == nil {
+ dief("cannot change: found pending rebase or sync")
+ }
+
// Checkout or create branch, if specified.
target := flags.Arg(0)
if target != "" {
diff --git a/git-codereview/doc.go b/git-codereview/doc.go
index a72b6db..b9eb230 100644
--- a/git-codereview/doc.go
+++ b/git-codereview/doc.go
@@ -344,5 +344,27 @@
lines such as “Fixes #123” in a commit message will be rewritten to
“Fixes golang/go#123”.
+The “branch” key specifies the name of the branch on the origin server
+corresponding to the current checkout. If this setting is missing, git-codereview
+uses the name of the remote branch that the current checkout is tracking.
+If that setting is missing, git-codereview uses “main”.
+
+The “parent-branch” key specifies the name of the parent branch on
+the origin server. The parent branch is the branch from which the current
+pulls regular updates. For example the parent branch of “dev.feature”
+would typically be “main”, in which case it would have this codereview.cfg:
+
+ branch: dev.feature
+ parent-branch: main
+
+In a more complex configuration, one feature branch might depend upon
+another, like “dev.feature2” containing follow-on work for “dev.feature”,
+neither of which has merged yet. In this case, the dev.feature2 branch
+would have this codereview.cfg:
+
+ branch: dev.feature2
+ parent-branch: dev.feature
+
+The parent branch setting is used by the sync-branch command.
*/
package main
diff --git a/git-codereview/mail.go b/git-codereview/mail.go
index 55ab8ed..3b60eb3 100644
--- a/git-codereview/mail.go
+++ b/git-codereview/mail.go
@@ -59,7 +59,7 @@
return
}
- if len(ListFiles(c)) == 0 {
+ if len(ListFiles(c)) == 0 && c.Merge == "" {
dief("cannot mail: commit %s is empty", c.ShortHash)
}
diff --git a/git-codereview/pending.go b/git-codereview/pending.go
index c1594b5..f3e3369 100644
--- a/git-codereview/pending.go
+++ b/git-codereview/pending.go
@@ -293,6 +293,9 @@
// commit and its Gerrit state.
func formatCommit(w io.Writer, c *Commit, short bool) {
g := c.g
+ if g == nil {
+ g = new(GerritChange)
+ }
msg := strings.TrimRight(c.Message, "\r\n")
fmt.Fprintf(w, "%s", c.ShortHash)
var tags []string
@@ -318,6 +321,9 @@
case "ABANDONED":
tags = append(tags, "abandoned")
}
+ if c.Merge != "" {
+ tags = append(tags, "merge="+c.Merge[:7])
+ }
if len(tags) > 0 {
fmt.Fprintf(w, " (%s)", strings.Join(tags, ", "))
}
diff --git a/git-codereview/review.go b/git-codereview/review.go
index 0deb039..5c7f16d 100644
--- a/git-codereview/review.go
+++ b/git-codereview/review.go
@@ -67,6 +67,7 @@
rebase-work
submit [-i | commit...]
sync
+ sync-branch [-continue]
See https://pkg.go.dev/golang.org/x/review/git-codereview
for the full details of each command.
@@ -112,6 +113,8 @@
cmd = cmdSubmit
case "sync":
cmd = cmdSync
+ case "sync-branch":
+ cmd = cmdSyncBranch
case "test-loadAuth": // for testing only.
cmd = func([]string) { loadAuth() }
}
diff --git a/git-codereview/sync.go b/git-codereview/sync.go
index 9ea3e67..0334341 100644
--- a/git-codereview/sync.go
+++ b/git-codereview/sync.go
@@ -4,7 +4,15 @@
package main
-import "strings"
+import (
+ "encoding/json"
+ "flag"
+ "fmt"
+ "io/ioutil"
+ "os"
+ "path/filepath"
+ "strings"
+)
func cmdSync(args []string) {
expectZeroArgs(args, "sync")
@@ -78,3 +86,234 @@
"\trun 'git add' and 'git-codereview change' to commit staged changes", cmd)
}
}
+
+type syncBranchStatus struct {
+ Local string
+ Parent string
+ Branch string
+ ParentHash string
+ BranchHash string
+ Conflicts []string
+}
+
+func syncBranchStatusFile() string {
+ return filepath.Join(repoRoot(), ".git/codereview-sync-branch-status")
+}
+
+func readSyncBranchStatus() *syncBranchStatus {
+ data, err := ioutil.ReadFile(syncBranchStatusFile())
+ if err != nil {
+ dief("cannot sync-branch: reading status: %v", err)
+ }
+ status := new(syncBranchStatus)
+ err = json.Unmarshal(data, status)
+ if err != nil {
+ dief("cannot sync-branch: reading status: %v", err)
+ }
+ return status
+}
+
+func writeSyncBranchStatus(status *syncBranchStatus) {
+ js, err := json.MarshalIndent(status, "", "\t")
+ if err != nil {
+ dief("cannot sync-branch: writing status: %v", err)
+ }
+ if err := ioutil.WriteFile(syncBranchStatusFile(), js, 0666); err != nil {
+ dief("cannot sync-branch: writing status: %v", err)
+ }
+}
+
+func cmdSyncBranch(args []string) {
+ os.Setenv("GIT_EDITOR", ":") // do not bring up editor during merge, commit
+
+ var cont bool
+ flags.BoolVar(&cont, "continue", false, "continue after merge conflicts")
+ flags.Parse(args)
+ if len(flag.Args()) > 0 {
+ fmt.Fprintf(stderr(), "Usage: %s sync-branch %s [-continue]\n", progName, globalFlags)
+ exit(2)
+ }
+
+ parent := config()["parent-branch"]
+ if parent == "" {
+ dief("cannot sync-branch: codereview.cfg does not list parent-branch")
+ }
+
+ branch := config()["branch"]
+ if parent == "" {
+ dief("cannot sync-branch: codereview.cfg does not list branch")
+ }
+
+ b := CurrentBranch()
+ if b.DetachedHead() {
+ dief("cannot sync-branch: on detached head")
+ }
+ if len(b.Pending()) > 0 {
+ dief("cannot sync-branch: pending changes exist\n" +
+ "\trun 'git codereview pending' to see them")
+ }
+
+ if cont {
+ if _, err := os.Stat(syncBranchStatusFile()); err != nil {
+ dief("cannot sync-branch -continue: no pending sync-branch status file found")
+ }
+ syncBranchContinue(" -continue", b, readSyncBranchStatus())
+ return
+ }
+
+ if _, err := cmdOutputErr("git", "rev-parse", "--abbrev-ref", "MERGE_HEAD"); err == nil {
+ diePendingMerge("sync-branch")
+ }
+
+ // Don't sync with staged or unstaged changes.
+ // rebase is going to complain if we don't, and we can give a nicer error.
+ checkStaged("sync")
+ checkUnstaged("sync")
+
+ // Make sure client is up-to-date on current branch.
+ // Note that this does a remote fetch of b.OriginBranch() (aka branch).
+ cmdSync(nil)
+
+ // Pull down parent commits too.
+ quiet := "-q"
+ if *verbose > 0 {
+ quiet = "-v"
+ }
+ run("git", "fetch", quiet, "origin", "refs/heads/"+parent+":refs/remotes/origin/"+parent)
+
+ // Write the status file to make sure we can, before starting a merge.
+ status := &syncBranchStatus{
+ Local: b.Name,
+ Parent: parent,
+ ParentHash: gitHash("origin/" + parent),
+ Branch: branch,
+ BranchHash: gitHash("origin/" + branch),
+ }
+ writeSyncBranchStatus(status)
+
+ // Start the merge.
+ _, err := cmdOutputErr("git", "merge", "origin/"+parent)
+
+ // Resolve codereview.cfg the right way (never take it from the merge).
+ cmdOutputDir(repoRoot(), "git", "checkout", "HEAD", "--", "codereview.cfg")
+
+ if err != nil {
+ // Check whether the only listed file is codereview.cfg and try again if so.
+ // Build list of unmerged files.
+ for _, s := range nonBlankLines(cmdOutputDir(repoRoot(), "git", "status", "-b", "--porcelain")) {
+ // Unmerged status is anything with a U and also AA and DD.
+ if len(s) >= 4 && s[2] == ' ' && (s[0] == 'U' || s[1] == 'U' || s[0:2] == "AA" || s[0:2] == "DD") {
+ status.Conflicts = append(status.Conflicts, s[3:])
+ }
+ }
+ if len(status.Conflicts) == 0 {
+ // Must have been codereview.cfg that was the problem.
+ // Try continuing the merge.
+ // Note that as of Git 2.12, git merge --continue is a synonym for git commit,
+ // but older Gits do not have merge --continue.
+ var out string
+ out, err = cmdOutputErr("git", "commit", "-m", "TEMPORARY MERGE MESSAGE")
+ if err != nil {
+ printf("git commit failed with no apparent unmerged files:\n%s\n", out)
+ }
+ } else {
+ writeSyncBranchStatus(status)
+ }
+ }
+
+ if err != nil {
+ if len(status.Conflicts) == 0 {
+ dief("cannot sync-branch: git merge failed but no conflicts found\n" +
+ "(unexpected error, please ask for help!)")
+ }
+ dief("sync-branch: merge conflicts in:\n\t- %s\n\n"+
+ "Please fix them (use 'git status' to see the list again),\n"+
+ "then 'git add' or 'git rm' to resolve them,\n"+
+ "and then 'git sync-branch -continue' to continue.\n"+
+ "Or run 'git merge --abort' to give up on this sync-branch.\n",
+ strings.Join(status.Conflicts, "\n\t- "))
+ }
+
+ syncBranchContinue("", b, status)
+}
+
+func diePendingMerge(cmd string) {
+ dief("cannot %s: found pending merge\n"+
+ "Run 'git codereview sync-branch -continue' if you fixed\n"+
+ "merge conflicts after a previous sync-branch operation.\n"+
+ "Or run 'git merge --abort' to give up on the sync-branch.\n",
+ cmd)
+}
+
+func syncBranchContinue(flag string, b *Branch, status *syncBranchStatus) {
+ if h := gitHash("origin/" + status.Parent); h != status.ParentHash {
+ dief("cannot sync-branch%s: parent hash changed: %.7s -> %.7s", flag, status.ParentHash, h)
+ }
+ if h := gitHash("origin/" + status.Branch); h != status.BranchHash {
+ dief("cannot sync-branch%s: branch hash changed: %.7s -> %.7s", flag, status.BranchHash, h)
+ }
+ if b.Name != status.Local {
+ dief("cannot sync-branch%s: branch changed underfoot: %s -> %s", flag, status.Local, b.Name)
+ }
+
+ branch := status.Branch
+ parent := status.Parent
+ branchHash := status.BranchHash
+ parentHash := status.ParentHash
+
+ prefix := ""
+ if strings.HasPrefix(branch, "dev.") || strings.HasPrefix(branch, "release-branch.") {
+ prefix = "[" + branch + "] "
+ }
+ msg := fmt.Sprintf("%sall: merge %s (%.7s) into %s", prefix, parent, parentHash, branch)
+
+ if flag != "" {
+ // Need to commit the merge.
+
+ // Check that the state of the client is the way we left it before any merge conflicts.
+ mergeHead, err := cmdOutputErr("git", "rev-parse", "MERGE_HEAD")
+ if err != nil {
+ dief("cannot sync-branch%s: no pending merge\n"+
+ "If you accidentally ran 'git merge --continue',\n"+
+ "then use 'git reset --hard HEAD^' to undo.\n", flag)
+ }
+ mergeHead = trim(mergeHead)
+ if mergeHead != parentHash {
+ dief("cannot sync-branch%s: MERGE_HEAD is %.7s, but origin/%s is %.7s", flag, mergeHead, parent, parentHash)
+ }
+ head := gitHash("HEAD")
+ if head != branchHash {
+ dief("cannot sync-branch%s: HEAD is %.7s, but origin/%s is %.7s", flag, head, branch, branchHash)
+ }
+
+ if HasUnstagedChanges() {
+ dief("cannot sync-branch%s: unstaged changes (unresolved conflicts)\n"+
+ "\tUse 'git status' to see them, 'git add' or 'git rm' to resolve them,\n"+
+ "\tand then run 'git sync-branch -continue' again.\n", flag)
+ }
+
+ run("git", "commit", "-m", msg)
+ }
+
+ // Amend the merge message, which may be auto-generated by git
+ // or may have been written by us during the post-conflict commit above,
+ // to use our standard format and list the incorporated CLs.
+
+ // Merge must never sync codereview.cfg,
+ // because it contains the parent and branch config.
+ // Force the on-branch copy back while amending the commit.
+ cmdOutputDir(repoRoot(), "git", "checkout", "origin/"+branch, "--", "codereview.cfg")
+
+ conflictMsg := ""
+ if len(status.Conflicts) > 0 {
+ conflictMsg = "Conflicts:\n\n- " + strings.Join(status.Conflicts, "\n- ") + "\n\n"
+ }
+ msg = fmt.Sprintf("%s\n\n%sMerge List:\n\n%s", msg, conflictMsg,
+ cmdOutput("git", "log", "--format=format:+ %cd %h %s", "--date=short", "HEAD^1..HEAD^2"))
+ run("git", "commit", "--amend", "-m", msg)
+
+ fmt.Fprintf(stderr(), "\n")
+
+ cmdPending([]string{"-c", "-l"})
+ fmt.Fprintf(stderr(), "\n* Merge commit created.\nRun 'git codereview mail' to send for review.\n")
+}
diff --git a/git-codereview/sync_test.go b/git-codereview/sync_test.go
index 72cbdf8..1498098 100644
--- a/git-codereview/sync_test.go
+++ b/git-codereview/sync_test.go
@@ -4,7 +4,10 @@
package main
-import "testing"
+import (
+ "strings"
+ "testing"
+)
func TestSync(t *testing.T) {
gt := newGitTest(t)
@@ -123,3 +126,158 @@
testNoStdout(t)
testNoStderr(t)
}
+
+func TestBranchConfig(t *testing.T) {
+ gt := newGitTest(t)
+ defer gt.done()
+ gt.work(t) // do the main-branch work setup now to avoid unwanted change below
+
+ trun(t, gt.client, "git", "checkout", "dev.branch")
+ testMain(t, "pending", "-c", "-l")
+ // The !+ means reject any output with a +, which introduces a pending commit.
+ // There should be no pending commits.
+ testPrintedStdout(t, "dev.branch (current branch, tracking dev.branch)", "!+")
+
+ // If we make a branch with raw git,
+ // the codereview.cfg should help us see the tracking info
+ // even though git doesn't know the right upstream.
+ trun(t, gt.client, "git", "checkout", "-b", "mywork", "HEAD^0")
+ if out, err := cmdOutputDirErr(gt.client, "git", "rev-parse", "--abbrev-ref", "@{u}"); err == nil {
+ t.Fatalf("git knows @{u} but should not:\n%s", out)
+ }
+ testMain(t, "pending", "-c", "-l")
+ testPrintedStdout(t, "mywork (current branch, tracking dev.branch)", "!+")
+ // Pending should have set @{u} correctly for us.
+ if out, err := cmdOutputDirErr(gt.client, "git", "rev-parse", "--abbrev-ref", "@{u}"); err != nil {
+ t.Fatalf("git does not know @{u} but should: %v\n%s", err, out)
+ } else if out = strings.TrimSpace(out); out != "origin/dev.branch" {
+ t.Fatalf("git @{u} = %q, want %q", out, "origin/dev.branch")
+ }
+
+ // Even if we add a pending commit, we should see the right tracking info.
+ // The !codereview.cfg makes sure we do not see the codereview.cfg-changing
+ // commit from the server in the output. (That would happen if we were printing
+ // new commits relative to main instead of relative to dev.branch.)
+ gt.work(t)
+ testMain(t, "pending", "-c", "-l")
+ testHideRevHashes(t)
+ testPrintedStdout(t, "mywork REVHASH..REVHASH (current branch, tracking dev.branch)", "!codereview.cfg")
+
+ // If we make a new branch using the old work HEAD
+ // then we should be back to something tracking main.
+ trun(t, gt.client, "git", "checkout", "-b", "mywork2", "work^0")
+ gt.work(t)
+ testMain(t, "pending", "-c", "-l")
+ testHideRevHashes(t)
+ testPrintedStdout(t, "mywork2 REVHASH..REVHASH (current branch)", "!codereview.cfg")
+
+ // Now look at all branches, which should use the appropriate configs
+ // from the commits on each branch.
+ testMain(t, "pending", "-l")
+ testHideRevHashes(t)
+ testPrintedStdout(t, "mywork2 REVHASH..REVHASH (current branch)",
+ "mywork REVHASH..REVHASH (tracking dev.branch)",
+ "work REVHASH..REVHASH\n") // the \n checks for not having a (tracking main)
+}
+
+func TestSyncBranch(t *testing.T) {
+ gt := newGitTest(t)
+ defer gt.done()
+
+ gt.serverWork(t)
+ gt.serverWork(t)
+ trun(t, gt.server, "git", "checkout", "dev.branch")
+ gt.serverWorkUnrelated(t)
+ gt.serverWorkUnrelated(t)
+ gt.serverWorkUnrelated(t)
+ trun(t, gt.server, "git", "checkout", "main")
+
+ testMain(t, "change", "dev.branch")
+ testMain(t, "sync-branch")
+ testHideRevHashes(t)
+ testPrintedStdout(t, "[dev.branch] all: merge main (REVHASH) into dev.branch",
+ "Merge List:",
+ "+ DATE REVHASH msg #2",
+ "+ DATE REVHASH",
+ )
+ testPrintedStderr(t, "* Merge commit created.",
+ "Run 'git codereview mail' to send for review.")
+}
+
+func TestSyncBranchConflict(t *testing.T) {
+ gt := newGitTest(t)
+ defer gt.done()
+
+ gt.serverWork(t)
+ gt.serverWork(t)
+ trun(t, gt.server, "git", "checkout", "dev.branch")
+ gt.serverWork(t)
+ trun(t, gt.server, "git", "checkout", "main")
+
+ testMain(t, "change", "dev.branch")
+
+ testMainDied(t, "sync-branch")
+ testNoStdout(t)
+ testPrintedStderr(t,
+ "git-codereview: sync-branch: merge conflicts in:",
+ " - file",
+ "Please fix them (use 'git status' to see the list again),",
+ "then 'git add' or 'git rm' to resolve them,",
+ "and then 'git sync-branch -continue' to continue.",
+ "Or run 'git merge --abort' to give up on this sync-branch.",
+ )
+
+ // Other client-changing commands should fail now.
+ testDisallowed := func(cmd ...string) {
+ t.Helper()
+ testMainDied(t, cmd...)
+ testNoStdout(t)
+ testPrintedStderr(t,
+ "git-codereview: cannot "+cmd[0]+": found pending merge",
+ "Run 'git codereview sync-branch -continue' if you fixed",
+ "merge conflicts after a previous sync-branch operation.",
+ "Or run 'git merge --abort' to give up on the sync-branch.",
+ )
+ }
+ testDisallowed("change", "main")
+ testDisallowed("sync-branch")
+
+ // throw away server changes to resolve merge
+ trun(t, gt.client, "git", "checkout", "HEAD", "file")
+
+ // Still cannot change branches even with conflicts resolved.
+ testDisallowed("change", "main")
+ testDisallowed("sync-branch")
+
+ testMain(t, "sync-branch", "-continue")
+ testHideRevHashes(t)
+ testPrintedStdout(t,
+ "[dev.branch] all: merge main (REVHASH) into dev.branch",
+ "+ REVHASH (merge=REVHASH)",
+ "Conflicts:",
+ "- file",
+ "Merge List:",
+ "+ DATE REVHASH msg #2",
+ "+ DATE REVHASH",
+ )
+ testPrintedStderr(t,
+ "* Merge commit created.",
+ "Run 'git codereview mail' to send for review.",
+ )
+
+ // Check that pending only shows the merge, not more commits.
+ testMain(t, "pending", "-c", "-l", "-s")
+ n := strings.Count(testStdout.String(), "+")
+ if n != 1 {
+ t.Fatalf("git pending shows %d commits, want 1:\n%s", n, testStdout.String())
+ }
+ testNoStderr(t)
+
+ // Check that mail sends the merge to the right place!
+ testMain(t, "mail", "-n")
+ testNoStdout(t)
+ testPrintedStderr(t,
+ "git push -q origin HEAD:refs/for/dev.branch",
+ "git tag -f dev.branch.mailed",
+ )
+}
diff --git a/git-codereview/util_test.go b/git-codereview/util_test.go
index b1b5534..a4f0b51 100644
--- a/git-codereview/util_test.go
+++ b/git-codereview/util_test.go
@@ -15,6 +15,8 @@
"os/exec"
"path/filepath"
"reflect"
+ "regexp"
+ "runtime/debug"
"strings"
"sync"
"testing"
@@ -167,7 +169,12 @@
trun(t, server, "git", "checkout", "main")
trun(t, server, "git", "checkout", "-b", name)
write(t, server+"/file."+name, "this is "+name, 0644)
- trun(t, server, "git", "add", "file."+name)
+ cfg := "branch: " + name + "\n"
+ if name == "dev.branch" {
+ cfg += "parent-branch: main\n"
+ }
+ write(t, server+"/codereview.cfg", cfg, 0644)
+ trun(t, server, "git", "add", "file."+name, "codereview.cfg")
trun(t, server, "git", "commit", "-m", "on "+name)
}
trun(t, server, "git", "checkout", "main")
@@ -330,7 +337,7 @@
if died {
msg = "died"
} else {
- msg = fmt.Sprintf("panic: %v", err)
+ msg = fmt.Sprintf("panic: %v\n%s", err, debug.Stack())
}
t.Fatalf("%s\nstdout:\n%sstderr:\n%s", msg, testStdout, testStderr)
}
@@ -380,6 +387,16 @@
}
}
+func testHideRevHashes(t *testing.T) {
+ for _, b := range []*bytes.Buffer{testStdout, testStderr} {
+ out := b.Bytes()
+ out = regexp.MustCompile(`\b[0-9a-f]{7}\b`).ReplaceAllLiteral(out, []byte("REVHASH"))
+ out = regexp.MustCompile(`\b\d{4}-\d{2}-\d{2}\b`).ReplaceAllLiteral(out, []byte("DATE"))
+ b.Reset()
+ b.Write(out)
+ }
+}
+
func testPrintedStdout(t *testing.T, messages ...string) {
t.Helper()
testPrinted(t, testStdout, "stdout", messages...)
