git-codereview: clean up detached HEAD mode

Long ago I decided to return origin/HEAD from b.OriginBranch
in detached HEAD mode, thinking it would cause obvious failures.
But the joke was on me - origin/HEAD is a real thing in git,
and HEAD tracking origin/HEAD is not the right answer on dev branches.

Now that each branch's codereview.cfg typically has the branch info
we need, we can use that in detached HEAD mode to be able to provide
useful displays in commands like "git pending". And we can be careful
not to do that when we don't know the actual branch.

This commit cleans all that up.

Change-Id: I0e59bcb6f9b61e0cdce7a27299b7f29fef8e7048
Reviewed-on: https://go-review.googlesource.com/c/review/+/282616
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>

1 files changed, 47 insertions, 31 deletions

diff --git a/git-codereview/branch.go b/git-codereview/branch.go
index 5d277c8..d25c4c9 100644
--- a/git-codereview/branch.go
+++ b/git-codereview/branch.go
@@ -98,18 +98,26 @@ func (b *Branch) DetachedHead() bool {
 	return b.Name == "HEAD"
 }
 
+// NeedOriginBranch exits with an error message
+// if the origin branch is unknown.
+// The cmd is the user command reported in the failure message.
+func (b *Branch) NeedOriginBranch(cmd string) {
+	if b.OriginBranch() == "" {
+		why := ""
+		if b.DetachedHead() {
+			why = " (in detached HEAD mode)"
+		}
+		if cmd == "<internal branchpoint>" && exitTrap != nil {
+			panic("NeedOriginBranch")
+		}
+		dief("cannot %s: no origin branch%s", cmd, why)
+	}
+}
+
 // OriginBranch returns the name of the origin branch that branch b tracks.
 // The returned name is like "origin/master" or "origin/dev.garbage" or
 // "origin/release-branch.go1.4".
 func (b *Branch) OriginBranch() string {
-	if b.DetachedHead() {
-		// Detached head mode.
-		// "origin/HEAD" is clearly false, but it should be easy to find when it
-		// appears in other commands. Really any caller of OriginBranch
-		// should check for detached head mode.
-		return "origin/HEAD"
-	}
-
 	if b.originBranch != "" {
 		return b.originBranch
 	}
@@ -119,28 +127,33 @@ func (b *Branch) OriginBranch() string {
 	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()
+	// Consult and possibly update git,
+	// but only if we are actually on a real branch,
+	// not in detached HEAD mode.
+	if !b.DetachedHead() {
+		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
@@ -197,6 +210,7 @@ func (b *Branch) Pending() []*Commit {
 // Branchpoint returns an identifier for the latest revision
 // common to both this branch and its upstream branch.
 func (b *Branch) Branchpoint() string {
+	b.NeedOriginBranch("<internal branchpoint>")
 	b.loadPending()
 	return b.branchpoint
 }
@@ -515,8 +529,10 @@ func ListFiles(c *Commit) []string {
 }
 
 func cmdBranchpoint(args []string) {
-	expectZeroArgs(args, "sync")
-	fmt.Fprintf(stdout(), "%s\n", CurrentBranch().Branchpoint())
+	expectZeroArgs(args, "branchpoint")
+	b := CurrentBranch()
+	b.NeedOriginBranch("branchpoint")
+	fmt.Fprintf(stdout(), "%s\n", b.Branchpoint())
 }
 
 func cmdRebaseWork(args []string) {