From 9bee0d8840b557f8e4696aeeb6ec0feb061d7b0c Mon Sep 17 00:00:00 2001 From: Frederick Zhang Date: Fri, 14 Jun 2024 14:43:38 +1000 Subject: [PATCH] Find PRs using @{push} When using a triangular workflow with push.default = current and upstream being set to the target branch [1], or a similar but simpler central workflow [2], we should use @{push} instead to locate the remote branch. In fact, @{push} covers most cases in push.default = upstream too. The branch..merge is probably only needed when using RemoteURL and different remote / local branch names. [1] https://github.com/cli/cli/issues/575#issuecomment-706853545 [2] https://github.com/tpope/vim-fugitive/issues/1172#issuecomment-522301607 --- git/client.go | 3 +++ git/objects.go | 1 + pkg/cmd/pr/shared/finder.go | 4 +++- pkg/cmd/pr/shared/finder_test.go | 10 ++++++++++ pkg/cmd/pr/status/status.go | 4 +++- 5 files changed, 20 insertions(+), 2 deletions(-) diff --git a/git/client.go b/git/client.go index b0194affc43..b68ca22373a 100644 --- a/git/client.go +++ b/git/client.go @@ -355,6 +355,9 @@ func (c *Client) ReadBranchConfig(ctx context.Context, branch string) (cfg Branc cfg.MergeRef = parts[1] } } + if out, err = c.revParse(ctx, "--verify", "--quiet", "--abbrev-ref", branch+"@{push}"); err == nil { + cfg.Push = strings.TrimSuffix(string(out), "\n") + } return } diff --git a/git/objects.go b/git/objects.go index c33d92b7cb2..138584fccf0 100644 --- a/git/objects.go +++ b/git/objects.go @@ -74,4 +74,5 @@ type BranchConfig struct { RemoteName string RemoteURL *url.URL MergeRef string + Push string } diff --git a/pkg/cmd/pr/shared/finder.go b/pkg/cmd/pr/shared/finder.go index 4bb79ac8b64..f28cc2aaaf2 100644 --- a/pkg/cmd/pr/shared/finder.go +++ b/pkg/cmd/pr/shared/finder.go @@ -261,7 +261,9 @@ func (f *finder) parseCurrentBranch() (string, int, error) { } if branchOwner != "" { - if strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") { + if branchConfig.Push != "" { + prHeadRef = strings.TrimPrefix(branchConfig.Push, branchConfig.RemoteName+"/") + } else if strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") { prHeadRef = strings.TrimPrefix(branchConfig.MergeRef, "refs/heads/") } // prepend `OWNER:` if this branch is pushed to a fork diff --git a/pkg/cmd/pr/shared/finder_test.go b/pkg/cmd/pr/shared/finder_test.go index 6df8d300048..f776f2ee53c 100644 --- a/pkg/cmd/pr/shared/finder_test.go +++ b/pkg/cmd/pr/shared/finder_test.go @@ -232,8 +232,17 @@ func TestFind(t *testing.T) { return "blueberries", nil }, branchConfig: func(branch string) (c git.BranchConfig) { + c.MergeRef = "refs/heads/blueberries" + c.RemoteName = "origin" + c.Push = "origin/blueberries" return }, + remotesFn: func() (context.Remotes, error) { + return context.Remotes{{ + Remote: &git.Remote{Name: "origin"}, + Repo: ghrepo.New("OWNER", "REPO"), + }}, nil + }, }, httpStub: func(r *httpmock.Registry) { r.Register( @@ -318,6 +327,7 @@ func TestFind(t *testing.T) { branchConfig: func(branch string) (c git.BranchConfig) { c.MergeRef = "refs/heads/blue-upstream-berries" c.RemoteName = "origin" + c.Push = "origin/blue-upstream-berries" return }, remotesFn: func() (context.Remotes, error) { diff --git a/pkg/cmd/pr/status/status.go b/pkg/cmd/pr/status/status.go index 2cd28f2edc6..b5da040af30 100644 --- a/pkg/cmd/pr/status/status.go +++ b/pkg/cmd/pr/status/status.go @@ -209,7 +209,9 @@ func prSelectorForCurrentBranch(gitClient *git.Client, baseRepo ghrepo.Interface } if branchOwner != "" { - if strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") { + if branchConfig.Push != "" { + selector = strings.TrimPrefix(branchConfig.Push, branchConfig.RemoteName+"/") + } else if strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") { selector = strings.TrimPrefix(branchConfig.MergeRef, "refs/heads/") } // prepend `OWNER:` if this branch is pushed to a fork