diff options
| author | Russ Cox <rsc@golang.org> | 2021-01-07 21:14:22 -0500 |
|---|---|---|
| committer | Russ Cox <rsc@golang.org> | 2021-01-13 09:36:16 -0500 |
| commit | 6244401143db649bfa09234e4b0e4db1b40b8ec3 (patch) | |
| tree | c196f7ba683ef1920535bbc5bb2842266179cb61 /git-codereview/sync.go | |
| parent | bc5a6b70306f5512d03ea7ddb2da996887092555 (diff) | |
| download | go-x-review-1.0.0.tar.xz | |
git-codereview: add sync-branch -merge-back-to-parentv1.0.0
Dev branches come to an end.
Making sync-branch help that process instead of forcing
people to follow a playbook will help avoid mistakes.
The flag name was chosen to be very unlikely to be used
accidentally, and the commit subject and message both
are distinct to make clear to reviewers what they are being
asked to +2.
The Merge List is also included in full and is likely to be
quite large, yet another signal for everyone involved about
the magnitude and weight of the change.
Change-Id: I91cdda2b85cd3811711a339f4f3290fee109022e
Diffstat (limited to 'git-codereview/sync.go')
| -rw-r--r-- | git-codereview/sync.go | 140 |
1 files changed, 115 insertions, 25 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()) } |
