diff --git a/git/git.go b/git/git.go index 7a3a81437..31fea6233 100644 --- a/git/git.go +++ b/git/git.go @@ -307,6 +307,15 @@ func CheckoutBranch(branch string) error { return run.PrepareCmd(configCmd).Run() } +func CheckoutNewBranch(remoteName, branch string) error { + track := fmt.Sprintf("%s/%s", remoteName, branch) + configCmd, err := GitCommand("checkout", "-b", branch, "--track", track) + if err != nil { + return err + } + return run.PrepareCmd(configCmd).Run() +} + // pull changes from remote branch without version history func Pull(remote, branch string) error { pullCmd, err := GitCommand("pull", "--ff-only", remote, branch) diff --git a/pkg/cmd/pr/merge/merge.go b/pkg/cmd/pr/merge/merge.go index e13d08b7a..f6d5fb37b 100644 --- a/pkg/cmd/pr/merge/merge.go +++ b/pkg/cmd/pr/merge/merge.go @@ -11,7 +11,6 @@ import ( "github.com/cli/cli/v2/context" "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/config" - "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmd/pr/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" @@ -322,18 +321,35 @@ func mergeRun(opts *MergeOptions) error { var branchToSwitchTo string if currentBranch == pr.HeadRefName { - branchToSwitchTo, err = api.RepoDefaultBranch(apiClient, baseRepo) + branchToSwitchTo = pr.BaseRefName + if branchToSwitchTo == "" { + branchToSwitchTo, err = api.RepoDefaultBranch(apiClient, baseRepo) + if err != nil { + return err + } + } + + remotes, err := opts.Remotes() if err != nil { return err } - err = git.CheckoutBranch(branchToSwitchTo) + baseRemote, err := remotes.FindByRepo(baseRepo.RepoOwner(), baseRepo.RepoName()) if err != nil { return err } - err := pullLatestChanges(opts, baseRepo, branchToSwitchTo) - if err != nil { + if git.HasLocalBranch(branchToSwitchTo) { + if err := git.CheckoutBranch(branchToSwitchTo); err != nil { + return err + } + } else { + if err := git.CheckoutNewBranch(baseRemote.Name, branchToSwitchTo); err != nil { + return err + } + } + + if err := git.Pull(baseRemote.Name, branchToSwitchTo); err != nil { fmt.Fprintf(opts.IO.ErrOut, "%s warning: not possible to fast-forward to: %q\n", cs.WarningIcon(), branchToSwitchTo) } } @@ -364,25 +380,6 @@ func mergeRun(opts *MergeOptions) error { return nil } -func pullLatestChanges(opts *MergeOptions, repo ghrepo.Interface, branch string) error { - remotes, err := opts.Remotes() - if err != nil { - return err - } - - baseRemote, err := remotes.FindByRepo(repo.RepoOwner(), repo.RepoName()) - if err != nil { - return err - } - - err = git.Pull(baseRemote.Name, branch) - if err != nil { - return err - } - - return nil -} - func mergeMethodSurvey(baseRepo *api.Repository) (PullRequestMergeMethod, error) { type mergeOption struct { title string diff --git a/pkg/cmd/pr/merge/merge_test.go b/pkg/cmd/pr/merge/merge_test.go index d692b0a08..c7f30f7c4 100644 --- a/pkg/cmd/pr/merge/merge_test.go +++ b/pkg/cmd/pr/merge/merge_test.go @@ -480,6 +480,7 @@ func TestPrMerge_deleteBranch(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) + cs.Register(`git rev-parse --verify refs/heads/master`, 0, "") cs.Register(`git checkout master`, 0, "") cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "") cs.Register(`git branch -D blueberries`, 0, "") @@ -497,6 +498,106 @@ func TestPrMerge_deleteBranch(t *testing.T) { `), output.Stderr()) } +func TestPrMerge_deleteBranch_nonDefault(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + + shared.RunCommandFinder( + "", + &api.PullRequest{ + ID: "PR_10", + Number: 10, + State: "OPEN", + Title: "Blueberries are a good fruit", + HeadRefName: "blueberries", + MergeStateStatus: "CLEAN", + BaseRefName: "fruit", + }, + baseRepo("OWNER", "REPO", "master"), + ) + + http.Register( + httpmock.GraphQL(`mutation PullRequestMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "PR_10", input["pullRequestId"].(string)) + assert.Equal(t, "MERGE", input["mergeMethod"].(string)) + assert.NotContains(t, input, "commitHeadline") + })) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) + + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git rev-parse --verify refs/heads/fruit`, 0, "") + cs.Register(`git checkout fruit`, 0, "") + cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "") + cs.Register(`git branch -D blueberries`, 0, "") + cs.Register(`git pull --ff-only`, 0, "") + + output, err := runCommand(http, "blueberries", true, `pr merge --merge --delete-branch`) + if err != nil { + t.Fatalf("Got unexpected error running `pr merge` %s", err) + } + + assert.Equal(t, "", output.String()) + assert.Equal(t, heredoc.Doc(` + ✓ Merged pull request #10 (Blueberries are a good fruit) + ✓ Deleted branch blueberries and switched to branch fruit + `), output.Stderr()) +} + +func TestPrMerge_deleteBranch_checkoutNewBranch(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + + shared.RunCommandFinder( + "", + &api.PullRequest{ + ID: "PR_10", + Number: 10, + State: "OPEN", + Title: "Blueberries are a good fruit", + HeadRefName: "blueberries", + MergeStateStatus: "CLEAN", + BaseRefName: "fruit", + }, + baseRepo("OWNER", "REPO", "master"), + ) + + http.Register( + httpmock.GraphQL(`mutation PullRequestMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "PR_10", input["pullRequestId"].(string)) + assert.Equal(t, "MERGE", input["mergeMethod"].(string)) + assert.NotContains(t, input, "commitHeadline") + })) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) + + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git rev-parse --verify refs/heads/fruit`, 1, "") + cs.Register(`git checkout -b fruit --track origin/fruit`, 0, "") + cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "") + cs.Register(`git branch -D blueberries`, 0, "") + cs.Register(`git pull --ff-only`, 0, "") + + output, err := runCommand(http, "blueberries", true, `pr merge --merge --delete-branch`) + if err != nil { + t.Fatalf("Got unexpected error running `pr merge` %s", err) + } + + assert.Equal(t, "", output.String()) + assert.Equal(t, heredoc.Doc(` + ✓ Merged pull request #10 (Blueberries are a good fruit) + ✓ Deleted branch blueberries and switched to branch fruit + `), output.Stderr()) +} + func TestPrMerge_deleteNonCurrentBranch(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) @@ -764,6 +865,7 @@ func TestPrMerge_alreadyMerged(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) + cs.Register(`git rev-parse --verify refs/heads/master`, 0, "") cs.Register(`git checkout master`, 0, "") cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "") cs.Register(`git branch -D blueberries`, 0, "") @@ -906,6 +1008,7 @@ func TestPRMerge_interactiveWithDeleteBranch(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) + cs.Register(`git rev-parse --verify refs/heads/master`, 0, "") cs.Register(`git checkout master`, 0, "") cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "") cs.Register(`git branch -D blueberries`, 0, "")