diff --git a/acceptance/testdata/pr/pr-create-from-issue-develop-base.txtar b/acceptance/testdata/pr/pr-create-from-issue-develop-base.txtar new file mode 100644 index 000000000..f0619940e --- /dev/null +++ b/acceptance/testdata/pr/pr-create-from-issue-develop-base.txtar @@ -0,0 +1,37 @@ +# Set up env vars +env REPO=${SCRIPT_NAME}-${RANDOM_STRING} + +# Use gh as a credential helper +exec gh auth setup-git + +# Create a repository with a file so it has a default branch +exec gh repo create ${ORG}/${REPO} --add-readme --private + +# Defer repo cleanup +defer gh repo delete --yes ${ORG}/${REPO} + +# Clone the repo +exec gh repo clone ${ORG}/${REPO} + +# Create a branch to act as the merge base branch +cd ${REPO} +exec git checkout -b long-lived-feature-branch +exec git push -u origin long-lived-feature-branch + +# Create an issue to develop against +exec gh issue create --title 'Feature Request' --body 'Request Body' +stdout2env ISSUE_URL + +# Create a new branch using issue develop with the long lived branch as the base +exec gh issue develop --name 'feature-branch' --base 'long-lived-feature-branch' --checkout ${ISSUE_URL} + +# Prepare a PR on the develop branch +exec git commit --allow-empty -m 'Empty Commit' +exec git push -u origin feature-branch + +# Create the PR +exec gh pr create --title 'Feature Title' --body 'Feature Body' + +# Check the PR is created against the base branch we specified +exec gh pr view --json 'baseRefName' --jq '.baseRefName' +stdout 'long-lived-feature-branch' diff --git a/acceptance/testdata/pr/pr-create-from-manual-merge-base.txtar b/acceptance/testdata/pr/pr-create-from-manual-merge-base.txtar new file mode 100644 index 000000000..97ae168f5 --- /dev/null +++ b/acceptance/testdata/pr/pr-create-from-manual-merge-base.txtar @@ -0,0 +1,34 @@ +# Set up env vars +env REPO=${SCRIPT_NAME}-${RANDOM_STRING} + +# Use gh as a credential helper +exec gh auth setup-git + +# Create a repository with a file so it has a default branch +exec gh repo create ${ORG}/${REPO} --add-readme --private + +# Defer repo cleanup +defer gh repo delete --yes ${ORG}/${REPO} + +# Clone the repo +exec gh repo clone ${ORG}/${REPO} + +# Create a branch to act as the merge base branch +cd ${REPO} +exec git checkout -b long-lived-feature-branch +exec git push -u origin long-lived-feature-branch + +# Prepare a branch from the merge base to PR +exec git checkout -b feature-branch +exec git commit --allow-empty -m 'Empty Commit' +exec git push -u origin feature-branch + +# Set the merge-base branch config +exec git config 'branch.feature-branch.gh-merge-base' 'long-lived-feature-branch' + +# Create the PR +exec gh pr create --title 'Feature Title' --body 'Feature Body' + +# Check the PR is created against the merge base branch +exec gh pr view --json 'baseRefName' --jq '.baseRefName' +stdout 'long-lived-feature-branch' diff --git a/git/client.go b/git/client.go index 1dea7a6d6..35b9cf16c 100644 --- a/git/client.go +++ b/git/client.go @@ -20,6 +20,9 @@ import ( "github.com/cli/safeexec" ) +// MergeBaseConfig is the configuration setting to keep track of the PR target branch. +const MergeBaseConfig = "gh-merge-base" + var remoteRE = regexp.MustCompile(`(.+)\s+(.+)\s+\((push|fetch)\)`) // This regexp exists to match lines of the following form: @@ -373,10 +376,10 @@ func (c *Client) lookupCommit(ctx context.Context, sha, format string) ([]byte, return out, nil } -// ReadBranchConfig parses the `branch.BRANCH.(remote|merge)` part of git config. +// ReadBranchConfig parses the `branch.BRANCH.(remote|merge|gh-merge-base)` part of git config. func (c *Client) ReadBranchConfig(ctx context.Context, branch string) (cfg BranchConfig) { prefix := regexp.QuoteMeta(fmt.Sprintf("branch.%s.", branch)) - args := []string{"config", "--get-regexp", fmt.Sprintf("^%s(remote|merge)$", prefix)} + args := []string{"config", "--get-regexp", fmt.Sprintf("^%s(remote|merge|%s)$", prefix, MergeBaseConfig)} cmd, err := c.Command(ctx, args...) if err != nil { return @@ -385,6 +388,8 @@ func (c *Client) ReadBranchConfig(ctx context.Context, branch string) (cfg Branc if err != nil { return } + + cfg.LocalName = branch for _, line := range outputLines(out) { parts := strings.SplitN(line, " ", 2) if len(parts) < 2 { @@ -404,11 +409,26 @@ func (c *Client) ReadBranchConfig(ctx context.Context, branch string) (cfg Branc } case "merge": cfg.MergeRef = parts[1] + case MergeBaseConfig: + cfg.MergeBase = parts[1] } } return } +// SetBranchConfig sets the named value on the given branch. +func (c *Client) SetBranchConfig(ctx context.Context, branch, name, value string) error { + name = fmt.Sprintf("branch.%s.%s", branch, name) + args := []string{"config", name, value} + cmd, err := c.Command(ctx, args...) + if err != nil { + return err + } + // No output expected but check for any printed git error. + _, err = cmd.Output() + return err +} + func (c *Client) DeleteLocalTag(ctx context.Context, tag string) error { args := []string{"tag", "-d", tag} cmd, err := c.Command(ctx, args...) diff --git a/git/client_test.go b/git/client_test.go index 0fb7953bc..0ec4f7334 100644 --- a/git/client_test.go +++ b/git/client_test.go @@ -735,9 +735,9 @@ func TestClientReadBranchConfig(t *testing.T) { }{ { name: "read branch config", - cmdStdout: "branch.trunk.remote origin\nbranch.trunk.merge refs/heads/trunk", - wantCmdArgs: `path/to/git config --get-regexp ^branch\.trunk\.(remote|merge)$`, - wantBranchConfig: BranchConfig{RemoteName: "origin", MergeRef: "refs/heads/trunk"}, + cmdStdout: "branch.trunk.remote origin\nbranch.trunk.merge refs/heads/trunk\nbranch.trunk.gh-merge-base trunk", + wantCmdArgs: `path/to/git config --get-regexp ^branch\.trunk\.(remote|merge|gh-merge-base)$`, + wantBranchConfig: BranchConfig{LocalName: "trunk", RemoteName: "origin", MergeRef: "refs/heads/trunk", MergeBase: "trunk"}, }, } for _, tt := range tests { diff --git a/git/objects.go b/git/objects.go index c33d92b7c..f371429dd 100644 --- a/git/objects.go +++ b/git/objects.go @@ -71,7 +71,11 @@ type Commit struct { } type BranchConfig struct { + // LocalName of the branch. + LocalName string RemoteName string RemoteURL *url.URL - MergeRef string + // MergeBase is the optional base branch to target in a new PR if `--base` is not specified. + MergeBase string + MergeRef string } diff --git a/pkg/cmd/issue/develop/develop.go b/pkg/cmd/issue/develop/develop.go index fa01a3dac..1536800f0 100644 --- a/pkg/cmd/issue/develop/develop.go +++ b/pkg/cmd/issue/develop/develop.go @@ -44,6 +44,13 @@ func NewCmdDevelop(f *cmdutil.Factory, runF func(*DevelopOptions) error) *cobra. cmd := &cobra.Command{ Use: "develop { | }", Short: "Manage linked branches for an issue", + Long: heredoc.Docf(` + Manage linked branches for an issue. + + When using the %[1]s--base%[1]s flag, the new development branch will be created from the specified + remote branch. The new branch will be configured as the base branch for pull requests created using + %[1]sgh pr create%[1]s. + `, "`"), Example: heredoc.Doc(` # List branches for issue 123 $ gh issue develop --list 123 @@ -171,6 +178,14 @@ func developRunCreate(opts *DevelopOptions, apiClient *api.Client, issueRepo ghr return err } + // Remember which branch to target when creating a PR. + if opts.BaseBranch != "" { + err = opts.GitClient.SetBranchConfig(ctx.Background(), branchName, git.MergeBaseConfig, opts.BaseBranch) + if err != nil { + return err + } + } + fmt.Fprintf(opts.IO.Out, "%s/%s/tree/%s\n", branchRepo.RepoHost(), ghrepo.FullName(branchRepo), branchName) return checkoutBranch(opts, branchRepo, branchName) diff --git a/pkg/cmd/issue/develop/develop_test.go b/pkg/cmd/issue/develop/develop_test.go index abdebf0c8..831f03fc3 100644 --- a/pkg/cmd/issue/develop/develop_test.go +++ b/pkg/cmd/issue/develop/develop_test.go @@ -399,6 +399,7 @@ func TestDevelopRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git fetch origin \+refs/heads/my-branch:refs/remotes/origin/my-branch`, 0, "") + cs.Register(`git config branch\.my-branch\.gh-merge-base main`, 0, "") }, expectedOut: "github.com/OWNER/REPO/tree/my-branch\n", }, diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index 06f4e1e89..f3bd12870 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -119,6 +119,10 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co alongside %[1]s--fill%[1]s, the values specified by %[1]s--title%[1]s and/or %[1]s--body%[1]s will take precedence and overwrite any autofilled content. + The base branch for the created PR can be specified using the %[1]s--base%[1]s flag. If not provided, + the value of %[1]sgh-merge-base%[1]s git branch config will be used. If not configured, the repository's + default branch will be used. + Link an issue to the pull request by referencing the issue in the body of the pull request. If the body text mentions %[1]sFixes #123%[1]s or %[1]sCloses #123%[1]s, the referenced issue will automatically get closed when the pull request gets merged. @@ -513,11 +517,10 @@ func initDefaultTitleBody(ctx CreateContext, state *shared.IssueMetadataState, u return nil } -func determineTrackingBranch(gitClient *git.Client, remotes ghContext.Remotes, headBranch string) *git.TrackingRef { +func determineTrackingBranch(gitClient *git.Client, remotes ghContext.Remotes, headBranchConfig *git.BranchConfig) *git.TrackingRef { refsForLookup := []string{"HEAD"} var trackingRefs []git.TrackingRef - headBranchConfig := gitClient.ReadBranchConfig(context.Background(), headBranch) if headBranchConfig.RemoteName != "" { tr := git.TrackingRef{ RemoteName: headBranchConfig.RemoteName, @@ -530,7 +533,7 @@ func determineTrackingBranch(gitClient *git.Client, remotes ghContext.Remotes, h for _, remote := range remotes { tr := git.TrackingRef{ RemoteName: remote.Name, - BranchName: headBranch, + BranchName: headBranchConfig.LocalName, } trackingRefs = append(trackingRefs, tr) refsForLookup = append(refsForLookup, tr.String()) @@ -640,9 +643,10 @@ func NewCreateContext(opts *CreateOptions) (*CreateContext, error) { var headRepo ghrepo.Interface var headRemote *ghContext.Remote + headBranchConfig := gitClient.ReadBranchConfig(context.Background(), headBranch) if isPushEnabled { // determine whether the head branch is already pushed to a remote - if pushedTo := determineTrackingBranch(gitClient, remotes, headBranch); pushedTo != nil { + if pushedTo := determineTrackingBranch(gitClient, remotes, &headBranchConfig); pushedTo != nil { isPushEnabled = false if r, err := remotes.FindByName(pushedTo.RemoteName); err == nil { headRepo = r @@ -715,6 +719,9 @@ func NewCreateContext(opts *CreateOptions) (*CreateContext, error) { } baseBranch := opts.BaseBranch + if baseBranch == "" { + baseBranch = headBranchConfig.MergeBase + } if baseBranch == "" { baseBranch = baseRepo.DefaultBranchRef.Name } diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index d31174999..27220d052 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -1,6 +1,7 @@ package create import ( + ctx "context" "encoding/json" "errors" "fmt" @@ -261,6 +262,15 @@ func TestNewCmdCreate(t *testing.T) { cli: "--editor", wantsErr: true, }, + { + name: "fill and base", + cli: "--fill --base trunk", + wantsOpts: CreateOptions{ + Autofill: true, + BaseBranch: "trunk", + MaintainerCanModify: true, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -323,17 +333,18 @@ func TestNewCmdCreate(t *testing.T) { func Test_createRun(t *testing.T) { tests := []struct { - name string - setup func(*CreateOptions, *testing.T) func() - cmdStubs func(*run.CommandStubber) - promptStubs func(*prompter.PrompterMock) - httpStubs func(*httpmock.Registry, *testing.T) - expectedOutputs []string - expectedOut string - expectedErrOut string - expectedBrowse string - wantErr string - tty bool + name string + setup func(*CreateOptions, *testing.T) func() + cmdStubs func(*run.CommandStubber) + promptStubs func(*prompter.PrompterMock) + httpStubs func(*httpmock.Registry, *testing.T) + expectedOutputs []string + expectedOut string + expectedErrOut string + expectedBrowse string + wantErr string + tty bool + customBranchConfig bool }{ { name: "nontty web", @@ -626,7 +637,6 @@ func Test_createRun(t *testing.T) { })) }, cmdStubs: func(cs *run.CommandStubber) { - cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "") cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature`, 0, "") cs.Register(`git push --set-upstream origin HEAD:refs/heads/feature`, 0, "") }, @@ -690,7 +700,6 @@ func Test_createRun(t *testing.T) { })) }, cmdStubs: func(cs *run.CommandStubber) { - cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "") cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature`, 0, "") cs.Register(`git push --set-upstream origin HEAD:refs/heads/feature`, 0, "") }, @@ -737,7 +746,6 @@ func Test_createRun(t *testing.T) { })) }, cmdStubs: func(cs *run.CommandStubber) { - cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "") cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature`, 0, "") cs.Register(`git push --set-upstream origin HEAD:refs/heads/feature`, 0, "") }, @@ -787,7 +795,6 @@ func Test_createRun(t *testing.T) { })) }, cmdStubs: func(cs *run.CommandStubber) { - cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "") cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature`, 0, "") cs.Register("git remote rename origin upstream", 0, "") cs.Register(`git remote add origin https://github.com/monalisa/REPO.git`, 0, "") @@ -846,7 +853,6 @@ func Test_createRun(t *testing.T) { })) }, cmdStubs: func(cs *run.CommandStubber) { - cs.Register(`git config --get-regexp \^branch\\\.feature\\\.`, 1, "") // determineTrackingBranch cs.Register("git show-ref --verify", 0, heredoc.Doc(` deadbeef HEAD deadb00f refs/remotes/upstream/feature @@ -878,6 +884,7 @@ func Test_createRun(t *testing.T) { assert.Equal(t, "my-feat2", input["headRefName"].(string)) })) }, + customBranchConfig: true, cmdStubs: func(cs *run.CommandStubber) { cs.Register(`git config --get-regexp \^branch\\\.feature\\\.`, 0, heredoc.Doc(` branch.feature.remote origin @@ -1066,7 +1073,6 @@ func Test_createRun(t *testing.T) { httpmock.StringResponse(`{"data": {"viewer": {"login": "OWNER"} } }`)) }, cmdStubs: func(cs *run.CommandStubber) { - cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "") cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature`, 0, "") cs.Register(`git( .+)? log( .+)? origin/master\.\.\.feature`, 0, "") cs.Register(`git push --set-upstream origin HEAD:refs/heads/feature`, 0, "") @@ -1099,7 +1105,6 @@ func Test_createRun(t *testing.T) { mockRetrieveProjects(t, reg) }, cmdStubs: func(cs *run.CommandStubber) { - cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "") cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature`, 0, "") cs.Register(`git( .+)? log( .+)? origin/master\.\.\.feature`, 0, "") cs.Register(`git push --set-upstream origin HEAD:refs/heads/feature`, 0, "") @@ -1464,6 +1469,65 @@ func Test_createRun(t *testing.T) { }, expectedOut: "https://github.com/OWNER/REPO/pull/12\n", }, + { + name: "gh-merge-base", + tty: true, + setup: func(opts *CreateOptions, t *testing.T) func() { + opts.TitleProvided = true + opts.BodyProvided = true + opts.Title = "my title" + opts.Body = "my body" + opts.Branch = func() (string, error) { + return "task1", nil + } + opts.Remotes = func() (context.Remotes, error) { + return context.Remotes{ + { + Remote: &git.Remote{ + Name: "upstream", + Resolved: "base", + }, + Repo: ghrepo.New("OWNER", "REPO"), + }, + { + Remote: &git.Remote{ + Name: "origin", + }, + Repo: ghrepo.New("monalisa", "REPO"), + }, + }, nil + } + return func() {} + }, + httpStubs: func(reg *httpmock.Registry, t *testing.T) { + reg.Register( + httpmock.GraphQL(`mutation PullRequestCreate\b`), + httpmock.GraphQLMutation(` + { "data": { "createPullRequest": { "pullRequest": { + "URL": "https://github.com/OWNER/REPO/pull/12" + } } } } + `, func(input map[string]interface{}) { + assert.Equal(t, "REPOID", input["repositoryId"].(string)) + assert.Equal(t, "my title", input["title"].(string)) + assert.Equal(t, "my body", input["body"].(string)) + assert.Equal(t, "feature/feat2", input["baseRefName"].(string)) + assert.Equal(t, "monalisa:task1", input["headRefName"].(string)) + })) + }, + customBranchConfig: true, + cmdStubs: func(cs *run.CommandStubber) { + cs.Register(`git config --get-regexp \^branch\\\.task1\\\.\(remote\|merge\|gh-merge-base\)\$`, 0, heredoc.Doc(` + branch.task1.remote origin + branch.task1.merge refs/heads/task1 + branch.task1.gh-merge-base feature/feat2`)) // ReadBranchConfig + cs.Register(`git show-ref --verify`, 0, heredoc.Doc(` + deadbeef HEAD + deadb00f refs/remotes/upstream/feature/feat2 + deadbeef refs/remotes/origin/task1`)) // determineTrackingBranch + }, + expectedOut: "https://github.com/OWNER/REPO/pull/12\n", + expectedErrOut: "\nCreating pull request for monalisa:task1 into feature/feat2 in OWNER/REPO\n\n", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -1485,6 +1549,9 @@ func Test_createRun(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) cs.Register(`git status --porcelain`, 0, "") + if !tt.customBranchConfig { + cs.Register(`git config --get-regexp \^branch\\\..+\\\.\(remote\|merge\|gh-merge-base\)\$`, 0, "") + } if tt.cmdStubs != nil { tt.cmdStubs(cs) @@ -1651,7 +1718,8 @@ func Test_determineTrackingBranch(t *testing.T) { GhPath: "some/path/gh", GitPath: "some/path/git", } - ref := determineTrackingBranch(gitClient, tt.remotes, "feature") + headBranchConfig := gitClient.ReadBranchConfig(ctx.Background(), "feature") + ref := determineTrackingBranch(gitClient, tt.remotes, &headBranchConfig) tt.assert(ref, t) }) }