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>

10 files changed, 562 insertions, 36 deletions

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 @@ package main
 import (
 	"bytes"
 	"fmt"
+	"io/ioutil"
 	"net/url"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"regexp"
 	"runtime"
 	"strings"
@@ -17,12 +19,16 @@ import (
 
 // 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 @@ type Commit struct {
 	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 @@ func (b *Branch) OriginBranch() string {
 	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 @@ func (b *Branch) OriginBranch() string {
 
 	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 @@ func (b *Branch) Branchpoint() string {
 	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 @@ func (b *Branch) loadPending() {
 	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 @@ func (b *Branch) loadPending() {
 	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 @@ func (b *Branch) loadPending() {
 			// 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 @@ func (b *Branch) loadPending() {
 				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 @@ func (b *Branch) DefaultCommit(action, extra string) *Commit {
 
 // 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 @@ func TestBranchpointMerge(t *testing.T) {
 
 	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 @@ func cmdChange(args []string) {
 		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 @@ if different from the source repository. If set to “golang/go”, for example,
 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 @@ func cmdMail(args []string) {
 		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 @@ func cmdPending(args []string) {
 // 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 @@ func formatCommit(w io.Writer, c *Commit, short bool) {
 	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 @@ Available commands:
 	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 @@ func main() {
 		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 @@ func checkUnstaged(cmd string) {
 			"\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 @@ func TestSyncRebase(t *testing.T) {
 	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 @@ import (
 	"os/exec"
 	"path/filepath"
 	"reflect"
+	"regexp"
+	"runtime/debug"
 	"strings"
 	"sync"
 	"testing"
@@ -167,7 +169,12 @@ func newGitTest(t *testing.T) (gt *gitTest) {
 		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 @@ func testMain(t *testing.T, args ...string) {
 			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 testPrinted(t *testing.T, buf *bytes.Buffer, name string, messages ...strin
 	}
 }
 
+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...)