refactor merge command

This commit is contained in:
Ariel Deitcher 2022-04-25 08:38:21 -07:00
parent c445529719
commit c50786ed39
3 changed files with 345 additions and 202 deletions

View file

@ -19,6 +19,16 @@ const (
PullRequestMergeMethodSquash
)
const (
MergeStateStatusBehind = "BEHIND"
MergeStateStatusBlocked = "BLOCKED"
MergeStateStatusClean = "CLEAN"
MergeStateStatusDirty = "DIRTY"
MergeStateStatusHasHooks = "HAS_HOOKS"
MergeStateStatusMerged = "MERGED"
MergeStateStatusUnstable = "UNSTABLE"
)
type mergePayload struct {
repo ghrepo.Interface
pullRequestID string

View file

@ -168,224 +168,326 @@ func NewCmdMerge(f *cmdutil.Factory, runF func(*MergeOptions) error) *cobra.Comm
return cmd
}
func mergeRun(opts *MergeOptions) error {
cs := opts.IO.ColorScheme()
// mergeContext contains state and dependencies to merge a pull request.
type mergeContext struct {
pr *api.PullRequest
baseRepo ghrepo.Interface
apiClient *api.Client
httpClient *http.Client
opts *MergeOptions
cs *iostreams.ColorScheme
isTerminal bool
merged bool
localBranchExists bool
autoMerge bool
crossRepoPR bool
deleteBranch bool
currentBranch string
}
// Attempt to disable auto merge on the pull request.
func (m *mergeContext) disableAutoMerge() error {
err := disableAutoMerge(m.httpClient, m.baseRepo, m.pr.ID)
if err != nil {
return err
} else if m.isTerminal {
m.log(fmt.Sprintf("%s Auto-merge disabled for pull request #%d\n", m.cs.SuccessIconWithColor(m.cs.Green), m.pr.Number))
}
return nil
}
// Warn if the pull request and the remote branch have diverged.
func (m *mergeContext) warnIfDiverged() {
if m.opts.SelectorArg == "" && len(m.pr.Commits.Nodes) > 0 {
if localBranchLastCommit, err := git.LastCommit(); err == nil {
if localBranchLastCommit.Sha != m.pr.Commits.Nodes[len(m.pr.Commits.Nodes)-1].Commit.OID {
m.log(fmt.Sprintf("%s Pull request #%d (%s) has diverged from local branch\n", m.cs.Yellow("!"), m.pr.Number, m.pr.Title))
}
}
}
}
// Check if the current state of the pull request allows for merging
func (m *mergeContext) canMerge() error {
reason := blockedReason(m.pr.MergeStateStatus, m.opts.UseAdmin)
if reason == "" || m.autoMerge || m.merged {
return nil
}
m.log(fmt.Sprintf("%s Pull request #%d is not mergeable: %s.\n", m.cs.FailureIcon(), m.pr.Number, reason))
m.log("To have the pull request merged after all the requirements have been met, add the `--auto` flag.\n")
if remote := remoteForMergeConflictResolution(m.baseRepo, m.pr, m.opts); remote != nil {
mergeOrRebase := "merge"
if m.opts.MergeMethod == PullRequestMergeMethodRebase {
mergeOrRebase = "rebase"
}
fetchBranch := fmt.Sprintf("%s %s", remote.Name, m.pr.BaseRefName)
mergeBranch := fmt.Sprintf("%s %s/%s", mergeOrRebase, remote.Name, m.pr.BaseRefName)
cmd := fmt.Sprintf("gh pr checkout %d && git fetch %s && git %s", m.pr.Number, fetchBranch, mergeBranch)
m.log(fmt.Sprintf("Run the following to resolve the merge conflicts locally:\n %s\n", m.cs.Bold(cmd)))
}
if !m.opts.UseAdmin && allowsAdminOverride(m.pr.MergeStateStatus) {
// TODO: show this flag only to repo admins
m.log("To use administrator privileges to immediately merge the pull request, add the `--admin` flag.\n")
}
return cmdutil.SilentError
}
// Merge the pull request. May prompt the user for input parameters for the merge.
func (m *mergeContext) merge() error {
if m.merged {
return nil
}
payload := mergePayload{
repo: m.baseRepo,
pullRequestID: m.pr.ID,
method: m.opts.MergeMethod,
auto: m.autoMerge,
commitSubject: m.opts.Subject,
commitBody: m.opts.Body,
setCommitBody: m.opts.BodySet,
}
// get user input if not already given
if m.opts.InteractiveMode {
r, err := api.GitHubRepo(m.apiClient, m.baseRepo)
if err != nil {
return err
}
payload.method, err = mergeMethodSurvey(r)
if err != nil {
return err
}
m.deleteBranch, err = deleteBranchSurvey(m.opts, m.crossRepoPR, m.localBranchExists)
if err != nil {
return err
}
allowEditMsg := payload.method != PullRequestMergeMethodRebase
for {
action, err := confirmSurvey(allowEditMsg)
if err != nil {
return fmt.Errorf("unable to confirm: %w", err)
}
submit, err := confirmSubmission(m.httpClient, m.opts, action, &payload)
if err != nil {
return err
}
if submit {
break
}
}
}
err := mergePullRequest(m.httpClient, payload)
if err != nil {
return err
}
// log successful merge
if m.isTerminal {
if payload.auto {
method := ""
switch payload.method {
case PullRequestMergeMethodRebase:
method = " via rebase"
case PullRequestMergeMethodSquash:
method = " via squash"
}
m.log(fmt.Sprintf("%s Pull request #%d will be automatically merged%s when all requirements are met\n", m.cs.SuccessIconWithColor(m.cs.Green), m.pr.Number, method))
} else {
action := "Merged"
switch payload.method {
case PullRequestMergeMethodRebase:
action = "Rebased and merged"
case PullRequestMergeMethodSquash:
action = "Squashed and merged"
}
m.log(fmt.Sprintf("%s %s pull request #%d (%s)\n", m.cs.SuccessIconWithColor(m.cs.Magenta), action, m.pr.Number, m.pr.Title))
}
}
return nil
}
// Delete local branch if requested and if allowed.
func (m *mergeContext) deleteLocalBranch() error {
if m.crossRepoPR || m.autoMerge {
return nil
}
if m.merged {
// prompt for delete
if m.opts.InteractiveMode && !m.opts.IsDeleteBranchIndicated {
err := prompt.SurveyAskOne(&survey.Confirm{
Message: fmt.Sprintf("Pull request #%d was already merged. Delete the branch locally?", m.pr.Number),
Default: false,
}, &m.deleteBranch)
if err != nil {
return fmt.Errorf("could not prompt: %w", err)
}
} else {
m.log(fmt.Sprintf("%s Pull request #%d was already merged\n", m.cs.WarningIcon(), m.pr.Number))
}
}
// the user does not want to delete the branch
if !m.deleteBranch {
return nil
}
if m.opts.CanDeleteLocalBranch && m.localBranchExists {
currentBranch, err := m.opts.Branch()
if err != nil {
return err
}
// branch the command was run on is the same as the pull request branch
if currentBranch == m.pr.HeadRefName {
// if the target branch of the PR is not known, set the current branch to the
// default branch of the repository
m.currentBranch = m.pr.BaseRefName
if m.currentBranch == "" {
m.currentBranch, err = api.RepoDefaultBranch(m.apiClient, m.baseRepo)
if err != nil {
return err
}
}
remotes, err := m.opts.Remotes()
if err != nil {
return err
}
baseRemote, err := remotes.FindByRepo(m.baseRepo.RepoOwner(), m.baseRepo.RepoName())
if err != nil {
return err
}
if git.HasLocalBranch(m.currentBranch) {
if err := git.CheckoutBranch(m.currentBranch); err != nil {
return err
}
} else {
if err := git.CheckoutNewBranch(baseRemote.Name, m.currentBranch); err != nil {
return err
}
}
if err := git.Pull(baseRemote.Name, m.currentBranch); err != nil {
m.log(fmt.Sprintf("%s warning: not possible to fast-forward to: %q\n", m.cs.WarningIcon(), m.currentBranch))
}
}
if err := git.DeleteLocalBranch(m.pr.HeadRefName); err != nil {
err = fmt.Errorf("failed to delete local branch %s: %w", m.cs.Cyan(m.pr.HeadRefName), err)
return err
}
}
return nil
}
// Delete the remote branch if requested and if allowed.
func (m *mergeContext) deleteRemoteBranch() error {
// the user was already asked if they want to delete the branch if they didn't provide the flag
if !m.deleteBranch || m.crossRepoPR || m.autoMerge {
return nil
}
if !m.merged {
err := api.BranchDeleteRemote(m.apiClient, m.baseRepo, m.pr.HeadRefName)
var httpErr api.HTTPError
// The ref might have already been deleted by GitHub
if err != nil && (!errors.As(err, &httpErr) || httpErr.StatusCode != 422) {
return fmt.Errorf("failed to delete remote branch %s: %w", m.cs.Cyan(m.pr.HeadRefName), err)
}
}
if m.isTerminal {
branch := ""
if m.currentBranch != "" {
branch = fmt.Sprintf(" and switched to branch %s", m.cs.Cyan(m.currentBranch))
}
m.log(fmt.Sprintf("%s Deleted branch %s%s\n", m.cs.SuccessIconWithColor(m.cs.Red), m.cs.Cyan(m.pr.HeadRefName), branch))
}
return nil
}
// Log operations and results to the error stream.
func (m *mergeContext) log(s string) {
fmt.Fprint(m.opts.IO.ErrOut, s)
}
// Creates a new MergeConext from MergeOptions.
func NewMergeContext(opts *MergeOptions) (*mergeContext, error) {
findOptions := shared.FindOptions{
Selector: opts.SelectorArg,
Fields: []string{"id", "number", "state", "title", "lastCommit", "mergeStateStatus", "headRepositoryOwner", "headRefName"},
}
pr, baseRepo, err := opts.Finder.Find(findOptions)
if err != nil {
return err
return nil, err
}
isTerminal := opts.IO.IsStdoutTTY()
httpClient, err := opts.HttpClient()
if err != nil {
return err
return nil, err
}
apiClient := api.NewClientFromHTTP(httpClient)
return &mergeContext{
opts: opts,
pr: pr,
cs: opts.IO.ColorScheme(),
baseRepo: baseRepo,
isTerminal: isTerminal,
apiClient: apiClient,
httpClient: httpClient,
merged: pr.State == MergeStateStatusMerged,
deleteBranch: opts.DeleteBranch,
crossRepoPR: pr.HeadRepositoryOwner.Login != baseRepo.RepoOwner(),
autoMerge: opts.AutoMergeEnable && !isImmediatelyMergeable(pr.MergeStateStatus),
localBranchExists: opts.CanDeleteLocalBranch && git.HasLocalBranch(pr.HeadRefName),
}, nil
}
// Run the merge command.
func mergeRun(opts *MergeOptions) error {
ctx, err := NewMergeContext(opts)
if err != nil {
return err
}
// no further action is possible when disabling auto merge
if opts.AutoMergeDisable {
err := disableAutoMerge(httpClient, baseRepo, pr.ID)
if err != nil {
return err
}
if isTerminal {
fmt.Fprintf(opts.IO.ErrOut, "%s Auto-merge disabled for pull request #%d\n", cs.SuccessIconWithColor(cs.Green), pr.Number)
}
return nil
return ctx.disableAutoMerge()
}
if opts.SelectorArg == "" && len(pr.Commits.Nodes) > 0 {
if localBranchLastCommit, err := git.LastCommit(); err == nil {
if localBranchLastCommit.Sha != pr.Commits.Nodes[len(pr.Commits.Nodes)-1].Commit.OID {
fmt.Fprintf(opts.IO.ErrOut,
"%s Pull request #%d (%s) has diverged from local branch\n", cs.Yellow("!"), pr.Number, pr.Title)
}
}
ctx.warnIfDiverged()
if err := ctx.canMerge(); err != nil {
return err
}
isPRAlreadyMerged := pr.State == "MERGED"
if reason := blockedReason(pr.MergeStateStatus, opts.UseAdmin); !opts.AutoMergeEnable && !isPRAlreadyMerged && reason != "" {
fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d is not mergeable: %s.\n", cs.FailureIcon(), pr.Number, reason)
fmt.Fprintf(opts.IO.ErrOut, "To have the pull request merged after all the requirements have been met, add the `--auto` flag.\n")
if remote := remoteForMergeConflictResolution(baseRepo, pr, opts); remote != nil {
mergeOrRebase := "merge"
if opts.MergeMethod == PullRequestMergeMethodRebase {
mergeOrRebase = "rebase"
}
fetchBranch := fmt.Sprintf("%s %s", remote.Name, pr.BaseRefName)
mergeBranch := fmt.Sprintf("%s %s/%s", mergeOrRebase, remote.Name, pr.BaseRefName)
cmd := fmt.Sprintf("gh pr checkout %d && git fetch %s && git %s", pr.Number, fetchBranch, mergeBranch)
fmt.Fprintf(opts.IO.ErrOut, "Run the following to resolve the merge conflicts locally:\n %s\n", cs.Bold(cmd))
}
if !opts.UseAdmin && allowsAdminOverride(pr.MergeStateStatus) {
// TODO: show this flag only to repo admins
fmt.Fprintf(opts.IO.ErrOut, "To use administrator privileges to immediately merge the pull request, add the `--admin` flag.\n")
}
return cmdutil.SilentError
if err := ctx.merge(); err != nil {
return err
}
deleteBranch := opts.DeleteBranch
crossRepoPR := pr.HeadRepositoryOwner.Login != baseRepo.RepoOwner()
autoMerge := opts.AutoMergeEnable && !isImmediatelyMergeable(pr.MergeStateStatus)
localBranchExists := false
if opts.CanDeleteLocalBranch {
localBranchExists = git.HasLocalBranch(pr.HeadRefName)
if err := ctx.deleteLocalBranch(); err != nil {
return err
}
if !isPRAlreadyMerged {
payload := mergePayload{
repo: baseRepo,
pullRequestID: pr.ID,
method: opts.MergeMethod,
auto: autoMerge,
commitSubject: opts.Subject,
commitBody: opts.Body,
setCommitBody: opts.BodySet,
}
if opts.InteractiveMode {
r, err := api.GitHubRepo(apiClient, baseRepo)
if err != nil {
return err
}
payload.method, err = mergeMethodSurvey(r)
if err != nil {
return err
}
deleteBranch, err = deleteBranchSurvey(opts, crossRepoPR, localBranchExists)
if err != nil {
return err
}
allowEditMsg := payload.method != PullRequestMergeMethodRebase
for {
action, err := confirmSurvey(allowEditMsg)
if err != nil {
return fmt.Errorf("unable to confirm: %w", err)
}
submit, err := confirmSubmission(httpClient, opts, action, &payload)
if err != nil {
return err
}
if submit {
break
}
}
}
err = mergePullRequest(httpClient, payload)
if err != nil {
return err
}
if isTerminal {
if payload.auto {
method := ""
switch payload.method {
case PullRequestMergeMethodRebase:
method = " via rebase"
case PullRequestMergeMethodSquash:
method = " via squash"
}
fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d will be automatically merged%s when all requirements are met\n", cs.SuccessIconWithColor(cs.Green), pr.Number, method)
} else {
action := "Merged"
switch payload.method {
case PullRequestMergeMethodRebase:
action = "Rebased and merged"
case PullRequestMergeMethodSquash:
action = "Squashed and merged"
}
fmt.Fprintf(opts.IO.ErrOut, "%s %s pull request #%d (%s)\n", cs.SuccessIconWithColor(cs.Magenta), action, pr.Number, pr.Title)
}
}
} else if !opts.IsDeleteBranchIndicated && opts.InteractiveMode && !crossRepoPR && !opts.AutoMergeEnable {
err := prompt.SurveyAskOne(&survey.Confirm{
Message: fmt.Sprintf("Pull request #%d was already merged. Delete the branch locally?", pr.Number),
Default: false,
}, &deleteBranch)
if err != nil {
return fmt.Errorf("could not prompt: %w", err)
}
} else {
fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d was already merged\n", cs.WarningIcon(), pr.Number)
}
if !deleteBranch || crossRepoPR || autoMerge {
return nil
}
branchSwitchString := ""
if opts.CanDeleteLocalBranch && localBranchExists {
currentBranch, err := opts.Branch()
if err != nil {
return err
}
var branchToSwitchTo string
if currentBranch == pr.HeadRefName {
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
}
baseRemote, err := remotes.FindByRepo(baseRepo.RepoOwner(), baseRepo.RepoName())
if err != nil {
return err
}
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)
}
}
if err := git.DeleteLocalBranch(pr.HeadRefName); err != nil {
err = fmt.Errorf("failed to delete local branch %s: %w", cs.Cyan(pr.HeadRefName), err)
return err
}
if branchToSwitchTo != "" {
branchSwitchString = fmt.Sprintf(" and switched to branch %s", cs.Cyan(branchToSwitchTo))
}
}
if !isPRAlreadyMerged {
err = api.BranchDeleteRemote(apiClient, baseRepo, pr.HeadRefName)
var httpErr api.HTTPError
// The ref might have already been deleted by GitHub
if err != nil && (!errors.As(err, &httpErr) || httpErr.StatusCode != 422) {
err = fmt.Errorf("failed to delete remote branch %s: %w", cs.Cyan(pr.HeadRefName), err)
return err
}
}
if isTerminal {
fmt.Fprintf(opts.IO.ErrOut, "%s Deleted branch %s%s\n", cs.SuccessIconWithColor(cs.Red), cs.Cyan(pr.HeadRefName), branchSwitchString)
if err := ctx.deleteRemoteBranch(); err != nil {
return err
}
return nil
@ -547,17 +649,17 @@ func (e *userEditor) Edit(filename, startingText string) (string, error) {
// blockedReason translates various MergeStateStatus GraphQL values into human-readable reason
func blockedReason(status string, useAdmin bool) string {
switch status {
case "BLOCKED":
case MergeStateStatusBlocked:
if useAdmin {
return ""
}
return "the base branch policy prohibits the merge"
case "BEHIND":
case MergeStateStatusBehind:
if useAdmin {
return ""
}
return "the head branch is not up to date with the base branch"
case "DIRTY":
case MergeStateStatusDirty:
return "the merge commit cannot be cleanly created"
default:
return ""
@ -566,7 +668,7 @@ func blockedReason(status string, useAdmin bool) string {
func allowsAdminOverride(status string) bool {
switch status {
case "BLOCKED", "BEHIND":
case MergeStateStatusBlocked, MergeStateStatusBehind:
return true
default:
return false
@ -589,12 +691,12 @@ func remoteForMergeConflictResolution(baseRepo ghrepo.Interface, pr *api.PullReq
}
func mergeConflictStatus(status string) bool {
return status == "DIRTY"
return status == MergeStateStatusDirty
}
func isImmediatelyMergeable(status string) bool {
switch status {
case "CLEAN", "HAS_HOOKS", "UNSTABLE":
case MergeStateStatusClean, MergeStateStatusHasHooks, MergeStateStatusUnstable:
return true
default:
return false

View file

@ -319,8 +319,9 @@ func TestPrMerge_blocked(t *testing.T) {
baseRepo("OWNER", "REPO", "master"),
)
_, cmdTeardown := run.Stub()
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git rev-parse --verify refs/heads/`, 0, "")
output, err := runCommand(http, "master", true, "pr merge 1 --merge")
assert.EqualError(t, err, "SilentError")
@ -351,8 +352,9 @@ func TestPrMerge_dirty(t *testing.T) {
baseRepo("OWNER", "REPO", "master"),
)
_, cmdTeardown := run.Stub()
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git rev-parse --verify refs/heads/`, 0, "")
output, err := runCommand(http, "master", true, "pr merge 1 --merge")
assert.EqualError(t, err, "SilentError")
@ -926,7 +928,7 @@ func TestPrMerge_alreadyMerged_nonInteractive(t *testing.T) {
ID: "THE-ID",
Number: 4,
State: "MERGED",
HeadRepositoryOwner: api.Owner{Login: "monalisa"},
HeadRepositoryOwner: api.Owner{Login: "OWNER"},
MergeStateStatus: "CLEAN",
},
baseRepo("OWNER", "REPO", "master"),
@ -946,6 +948,35 @@ func TestPrMerge_alreadyMerged_nonInteractive(t *testing.T) {
assert.Equal(t, "! Pull request #4 was already merged\n", output.Stderr())
}
func TestPrMerge_alreadyMerged_nonInteractive_crossRepo(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
"4",
&api.PullRequest{
ID: "THE-ID",
Number: 4,
State: "MERGED",
HeadRepositoryOwner: api.Owner{Login: "monalisa"},
MergeStateStatus: "CLEAN",
},
baseRepo("OWNER", "REPO", "master"),
)
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git rev-parse --verify refs/heads/`, 0, "")
output, err := runCommand(http, "blueberries", true, "pr merge 4 --merge")
if err != nil {
t.Fatalf("Got unexpected error running `pr merge` %s", err)
}
assert.Equal(t, "", output.String())
assert.Equal(t, "", output.Stderr())
}
func TestPRMerge_interactive(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)