aboutsummaryrefslogtreecommitdiff
path: root/git-codereview
diff options
context:
space:
mode:
Diffstat (limited to 'git-codereview')
-rw-r--r--git-codereview/sync.go140
-rw-r--r--git-codereview/sync_test.go95
-rw-r--r--git-codereview/util_test.go19
3 files changed, 217 insertions, 37 deletions
diff --git a/git-codereview/sync.go b/git-codereview/sync.go
index c38215c..b079c84 100644
--- a/git-codereview/sync.go
+++ b/git-codereview/sync.go
@@ -127,8 +127,9 @@ func writeSyncBranchStatus(status *syncBranchStatus) {
func cmdSyncBranch(args []string) {
os.Setenv("GIT_EDITOR", ":") // do not bring up editor during merge, commit
- var cont bool
+ var cont, mergeBackToParent bool
flags.BoolVar(&cont, "continue", false, "continue after merge conflicts")
+ flags.BoolVar(&mergeBackToParent, "merge-back-to-parent", false, "for shutting down the dev branch")
flags.Parse(args)
if len(flag.Args()) > 0 {
fmt.Fprintf(stderr(), "Usage: %s sync-branch %s [-continue]\n", progName, globalFlags)
@@ -155,10 +156,17 @@ func cmdSyncBranch(args []string) {
}
if cont {
+ // Note: There is no -merge-back-to-parent -continue
+ // because -merge-back-to-parent never has merge conflicts.
+ // (It requires that the parent be fully merged into the
+ // dev branch or it won't even attempt the reverse merge.)
+ if mergeBackToParent {
+ dief("cannot use -continue with -merge-back-to-parent")
+ }
if _, err := os.Stat(syncBranchStatusFile()); err != nil {
dief("cannot sync-branch -continue: no pending sync-branch status file found")
}
- syncBranchContinue(" -continue", b, readSyncBranchStatus())
+ syncBranchContinue(syncBranchContinueFlag, b, readSyncBranchStatus())
return
}
@@ -192,11 +200,59 @@ func cmdSyncBranch(args []string) {
}
writeSyncBranchStatus(status)
+ parentHash, err := cmdOutputErr("git", "rev-parse", "origin/"+parent)
+ if err != nil {
+ dief("cannot sync-branch: cannot resolve origin/%s: %v\n%s", parent, err, parentHash)
+ }
+ branchHash, err := cmdOutputErr("git", "rev-parse", "origin/"+branch)
+ if err != nil {
+ dief("cannot sync-branch: cannot resolve origin/%s: %v\n%s", branch, err, branchHash)
+ }
+ parentHash = trim(parentHash)
+ branchHash = trim(branchHash)
+
+ // Only --merge-back-to-parent when there's nothing waiting
+ // to be merged in from parent. If a non-trivial merge needs
+ // to be done, it should be done first on the dev branch,
+ // not the parent branch.
+ if mergeBackToParent {
+ other := cmdOutput("git", "log", "--format=format:+ %cd %h %s", "--date=short", "origin/"+branch+"..origin/"+parent)
+ if other != "" {
+ dief("cannot sync-branch --merge-back-to-parent: parent has new commits.\n"+
+ "\trun 'git sync-branch' to bring them into this branch first:\n%s",
+ other)
+ }
+ }
+
// Start the merge.
- _, err := cmdOutputErr("git", "merge", "origin/"+parent)
+ if mergeBackToParent {
+ // Change HEAD back to "parent" and merge "branch" into it,
+ // even though we could instead merge "parent" into "branch".
+ // This way the parent-branch lineage ends up the first parent
+ // of the merge, the same as it would when we are doing it by hand
+ // with a plain "git merge". This may help the display of the
+ // merge graph in some tools more closely reflect what we did.
+ run("git", "reset", "--hard", "origin/"+parent)
+ _, err = cmdOutputErr("git", "merge", "--no-ff", "origin/"+branch)
+ } else {
+ _, 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")
+ // Resolve codereview.cfg the right way - never take it from the merge.
+ // For a regular sync-branch we keep the branch's.
+ // For a merge-back-to-parent we take the parent's.
+ // The codereview.cfg contains the branch config and we don't want
+ // it to change.
+ what := branchHash
+ if mergeBackToParent {
+ what = parentHash
+ }
+ cmdOutputDir(repoRoot(), "git", "checkout", what, "--", "codereview.cfg")
+
+ if mergeBackToParent {
+ syncBranchContinue(syncBranchMergeBackFlag, b, status)
+ return
+ }
if err != nil {
// Check whether the only listed file is codereview.cfg and try again if so.
@@ -224,8 +280,10 @@ func cmdSyncBranch(args []string) {
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("cannot sync-branch: git merge failed but no conflicts found\n"+
+ "(unexpected error, please ask for help!)\n\ngit status:\n%s\ngit status -b --porcelain:\n%s",
+ cmdOutputDir(repoRoot(), "git", "status"),
+ cmdOutputDir(repoRoot(), "git", "status", "-b", "--porcelain"))
}
dief("sync-branch: merge conflicts in:\n\t- %s\n\n"+
"Please fix them (use 'git status' to see the list again),\n"+
@@ -246,6 +304,18 @@ func diePendingMerge(cmd string) {
cmd)
}
+func prefixFor(branch string) string {
+ if strings.HasPrefix(branch, "dev.") || strings.HasPrefix(branch, "release-branch.") {
+ return "[" + branch + "] "
+ }
+ return ""
+}
+
+const (
+ syncBranchContinueFlag = " -continue"
+ syncBranchMergeBackFlag = " -merge-back-to-parent"
+)
+
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)
@@ -257,34 +327,43 @@ func syncBranchContinue(flag string, b *Branch, status *syncBranchStatus) {
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
+ var (
+ dst = status.Branch
+ dstHash = status.BranchHash
+ src = status.Parent
+ srcHash = status.ParentHash
+ )
+ if flag == syncBranchMergeBackFlag {
+ // This is a reverse merge: commits are flowing
+ // in the opposite direction from normal.
+ dst, src = src, dst
+ dstHash, srcHash = srcHash, dstHash
+ }
- prefix := ""
- if strings.HasPrefix(branch, "dev.") || strings.HasPrefix(branch, "release-branch.") {
- prefix = "[" + branch + "] "
+ prefix := prefixFor(dst)
+ op := "merge"
+ if flag == syncBranchMergeBackFlag {
+ op = "REVERSE MERGE"
}
- msg := fmt.Sprintf("%sall: merge %s (%.7s) into %s", prefix, parent, parentHash, branch)
+ msg := fmt.Sprintf("%sall: %s %s (%.7s) into %s", prefix, op, src, srcHash, dst)
- if flag != "" {
+ if flag == syncBranchContinueFlag {
// 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"+
+ "If you accidentally ran 'git merge --continue' or 'git commit',\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)
+ if mergeHead != srcHash {
+ dief("cannot sync-branch%s: MERGE_HEAD is %.7s, but origin/%s is %.7s", flag, mergeHead, src, srcHash)
}
head := gitHash("HEAD")
- if head != branchHash {
- dief("cannot sync-branch%s: HEAD is %.7s, but origin/%s is %.7s", flag, head, branch, branchHash)
+ if head != dstHash {
+ dief("cannot sync-branch%s: HEAD is %.7s, but origin/%s is %.7s", flag, head, dst, dstHash)
}
if HasUnstagedChanges() {
@@ -301,15 +380,24 @@ func syncBranchContinue(flag string, b *Branch, status *syncBranchStatus) {
// 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")
+ // because it contains the src and dst config.
+ // Force the on-dst copy back while amending the commit.
+ cmdOutputDir(repoRoot(), "git", "checkout", "origin/"+dst, "--", "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,
+
+ if flag == syncBranchMergeBackFlag {
+ msg += fmt.Sprintf("\n\n"+
+ "This commit is a REVERSE MERGE.\n"+
+ "It merges %s back into its parent branch, %s.\n"+
+ "This marks the end of development on %s.\n",
+ status.Branch, status.Parent, status.Branch)
+ }
+
+ msg += fmt.Sprintf("\n\n%sMerge List:\n\n%s", conflictMsg,
cmdOutput("git", "log", "--format=format:+ %cd %h %s", "--date=short", "HEAD^1..HEAD^2"))
run("git", "commit", "--amend", "-m", msg)
@@ -317,4 +405,6 @@ func syncBranchContinue(flag string, b *Branch, status *syncBranchStatus) {
cmdPending([]string{"-c", "-l"})
fmt.Fprintf(stderr(), "\n* Merge commit created.\nRun 'git codereview mail' to send for review.\n")
+
+ os.Remove(syncBranchStatusFile())
}
diff --git a/git-codereview/sync_test.go b/git-codereview/sync_test.go
index d8733c8..0d3978d 100644
--- a/git-codereview/sync_test.go
+++ b/git-codereview/sync_test.go
@@ -5,6 +5,9 @@
package main
import (
+ "bytes"
+ "io/ioutil"
+ "path/filepath"
"strings"
"testing"
)
@@ -79,7 +82,7 @@ func TestSyncRebase(t *testing.T) {
// submit first two CLs - gt.serverWork does same thing gt.work does, but on server
gt.serverWork(t)
- gt.serverWorkUnrelated(t) // wedge in unrelated work to get different hashes
+ gt.serverWorkUnrelated(t, "") // wedge in unrelated work to get different hashes
gt.serverWork(t)
testMain(t, "sync")
@@ -215,9 +218,9 @@ func TestSyncBranch(t *testing.T) {
gt.serverWork(t)
gt.serverWork(t)
trun(t, gt.server, "git", "checkout", "dev.branch")
- gt.serverWorkUnrelated(t)
- gt.serverWorkUnrelated(t)
- gt.serverWorkUnrelated(t)
+ gt.serverWorkUnrelated(t, "")
+ gt.serverWorkUnrelated(t, "")
+ gt.serverWorkUnrelated(t, "")
trun(t, gt.server, "git", "checkout", "main")
testMain(t, "change", "dev.branch")
@@ -232,6 +235,90 @@ func TestSyncBranch(t *testing.T) {
"Run 'git codereview mail' to send for review.")
}
+func TestSyncBranchMergeBack(t *testing.T) {
+ gt := newGitTest(t)
+ defer gt.done()
+
+ // server does not default to having a codereview.cfg on main,
+ // but sync-branch requires one.
+ var mainCfg = []byte("branch: main\n")
+ ioutil.WriteFile(filepath.Join(gt.server, "codereview.cfg"), mainCfg, 0666)
+ trun(t, gt.server, "git", "add", "codereview.cfg")
+ trun(t, gt.server, "git", "commit", "-m", "config for main", "codereview.cfg")
+
+ gt.serverWork(t)
+ gt.serverWork(t)
+ trun(t, gt.server, "git", "checkout", "dev.branch")
+ gt.serverWorkUnrelated(t, "work on dev.branch#1")
+ gt.serverWorkUnrelated(t, "work on dev.branch#2")
+ gt.serverWorkUnrelated(t, "work on dev.branch#3")
+ trun(t, gt.server, "git", "checkout", "main")
+ testMain(t, "change", "dev.branch")
+
+ // Merge back should fail because there are
+ // commits in the parent that have not been merged here yet.
+ testMainDied(t, "sync-branch", "--merge-back-to-parent")
+ testNoStdout(t)
+ testPrintedStderr(t, "parent has new commits")
+
+ // Bring them in, creating a merge commit.
+ testMain(t, "sync-branch")
+
+ // Merge back should still fail - merge commit not submitted yet.
+ testMainDied(t, "sync-branch", "--merge-back-to-parent")
+ testNoStdout(t)
+ testPrintedStderr(t, "pending changes exist")
+
+ // Push the changes back to the server.
+ // (In a real system this would happen through Gerrit.)
+ trun(t, gt.client, "git", "push", "origin")
+
+ devHash := trim(trun(t, gt.client, "git", "rev-parse", "HEAD"))
+
+ // Nothing should be pending.
+ testMain(t, "pending", "-c")
+ testPrintedStdout(t, "!+")
+
+ // This time should work.
+ testMain(t, "sync-branch", "--merge-back-to-parent")
+ testPrintedStdout(t,
+ "\n\tall: REVERSE MERGE dev.branch ("+devHash[:7]+") into main",
+ "This commit is a REVERSE MERGE.",
+ "It merges dev.branch back into its parent branch, main.",
+ "This marks the end of development on dev.branch.",
+ "Merge List:",
+ "msg #2",
+ "!config for main",
+ )
+ testPrintedStderr(t, "Merge commit created.")
+
+ data, err := ioutil.ReadFile(filepath.Join(gt.client, "codereview.cfg"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ if !bytes.Equal(data, mainCfg) {
+ t.Fatalf("codereview.cfg not restored:\n%s", data)
+ }
+
+ // Check pending knows what branch it is on.
+ testMain(t, "pending", "-c")
+ testHideRevHashes(t)
+ testPrintedStdout(t,
+ "dev.branch REVHASH..REVHASH (current branch)", // no "tracking dev.branch" anymore
+ "REVHASH (merge=REVHASH)",
+ "Merge List:",
+ "!config for main",
+ )
+
+ // 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/main",
+ "git tag -f dev.branch.mailed",
+ )
+}
+
func TestSyncBranchConflict(t *testing.T) {
gt := newGitTest(t)
defer gt.done()
diff --git a/git-codereview/util_test.go b/git-codereview/util_test.go
index a4f0b51..775b244 100644
--- a/git-codereview/util_test.go
+++ b/git-codereview/util_test.go
@@ -69,7 +69,7 @@ func (gt *gitTest) done() {
}
// doWork simulates commit 'n' touching 'file' in 'dir'
-func doWork(t *testing.T, n int, dir, file, changeid string) {
+func doWork(t *testing.T, n int, dir, file, changeid string, msg string) {
t.Helper()
write(t, dir+"/"+file, fmt.Sprintf("new content %d", n), 0644)
trun(t, dir, "git", "add", file)
@@ -77,8 +77,11 @@ func doWork(t *testing.T, n int, dir, file, changeid string) {
if n > 1 {
suffix = fmt.Sprintf(" #%d", n)
}
- msg := fmt.Sprintf("msg%s\n\nChange-Id: I%d%s\n", suffix, n, changeid)
- trun(t, dir, "git", "commit", "-m", msg)
+ if msg != "" {
+ msg += "\n\n"
+ }
+ cmsg := fmt.Sprintf("%smsg%s\n\nChange-Id: I%d%s\n", msg, suffix, n, changeid)
+ trun(t, dir, "git", "commit", "-m", cmsg)
}
func (gt *gitTest) work(t *testing.T) {
@@ -91,14 +94,14 @@ func (gt *gitTest) work(t *testing.T) {
// make local change on client
gt.nwork++
- doWork(t, gt.nwork, gt.client, "file", "23456789")
+ doWork(t, gt.nwork, gt.client, "file", "23456789", "")
}
func (gt *gitTest) workFile(t *testing.T, file string) {
t.Helper()
// make local change on client in the specific file
gt.nwork++
- doWork(t, gt.nwork, gt.client, file, "23456789")
+ doWork(t, gt.nwork, gt.client, file, "23456789", "")
}
func (gt *gitTest) serverWork(t *testing.T) {
@@ -108,15 +111,15 @@ func (gt *gitTest) serverWork(t *testing.T) {
// having gone through Gerrit and submitted with possibly
// different commit hashes but the same content.
gt.nworkServer++
- doWork(t, gt.nworkServer, gt.server, "file", "23456789")
+ doWork(t, gt.nworkServer, gt.server, "file", "23456789", "")
}
-func (gt *gitTest) serverWorkUnrelated(t *testing.T) {
+func (gt *gitTest) serverWorkUnrelated(t *testing.T, msg string) {
t.Helper()
// make unrelated change on server
// this makes history different on client and server
gt.nworkOther++
- doWork(t, gt.nworkOther, gt.server, "otherfile", "9999")
+ doWork(t, gt.nworkOther, gt.server, "otherfile", "9999", msg)
}
func newGitTest(t *testing.T) (gt *gitTest) {
generated by cgit v1.3 (git 2.53.0) at 2026-04-15 13:22:53 +0000