diff --git a/bot/internal/bot/backport.go b/bot/internal/bot/backport.go index d015542c..caad629a 100644 --- a/bot/internal/bot/backport.go +++ b/bot/internal/bot/backport.go @@ -46,7 +46,7 @@ func (b *Bot) Backport(ctx context.Context) error { return trace.BadParameter("automatic backports are only supported for internal contributors") } - pull, err := b.c.GitHub.GetPullRequestWithCommits(ctx, + pull, err := b.c.GitHub.GetPullRequest(ctx, b.c.Environment.Organization, b.c.Environment.Repository, b.c.Environment.Number) @@ -155,7 +155,7 @@ func (b *Bot) Backport(ctx context.Context) error { // BackportLocal executes dry run backport workflow locally. No git commands // are actually executed, just printed in the console. func (b *Bot) BackportLocal(ctx context.Context, branch string) error { - pull, err := b.c.GitHub.GetPullRequestWithCommits(ctx, + pull, err := b.c.GitHub.GetPullRequest(ctx, b.c.Environment.Organization, b.c.Environment.Repository, b.c.Environment.Number) @@ -236,16 +236,15 @@ func (b *Bot) createBackportBranch(ctx context.Context, organization string, rep return trace.Wrap(err) } - // Cherry-pick all commits from the PR to the backport branch. - for _, commit := range pull.Commits { - if err := git("cherry-pick", commit); err != nil { - // If cherry-pick fails with conflict, abort it, otherwise we - // won't be able to switch branch for the next backport. - if errAbrt := git("cherry-pick", "--abort"); errAbrt != nil { - return trace.NewAggregate(err, errAbrt) - } - return trace.Wrap(err) + // Cherry-pick the _merged_ commit on the base branch instead of + // the individual commits from the PR to the backport branch. + if err := git("cherry-pick", pull.MergeCommitSHA); err != nil { + // If cherry-pick fails with conflict, abort it, otherwise we + // won't be able to switch branch for the next backport. + if errAbrt := git("cherry-pick", "--abort"); errAbrt != nil { + return trace.NewAggregate(err, errAbrt) } + return trace.Wrap(err) } // Push the backport branch to Github. diff --git a/bot/internal/bot/bot.go b/bot/internal/bot/bot.go index 8fe2c398..c22e2056 100644 --- a/bot/internal/bot/bot.go +++ b/bot/internal/bot/bot.go @@ -50,9 +50,6 @@ type Client interface { // GetPullRequest returns a specific Pull Request. GetPullRequest(ctx context.Context, organization string, repository string, number int) (github.PullRequest, error) - // GetPullRequestWithCommits returns a specific Pull Request with commits. - GetPullRequestWithCommits(ctx context.Context, organization string, repository string, number int) (github.PullRequest, error) - // CreatePullRequest will create a Pull Request. CreatePullRequest(ctx context.Context, organization string, repository string, title string, head string, base string, body string, draft bool) (int, error) diff --git a/bot/internal/github/github.go b/bot/internal/github/github.go index 7cd1e737..501a65d4 100644 --- a/bot/internal/github/github.go +++ b/bot/internal/github/github.go @@ -196,11 +196,14 @@ type PullRequest struct { UnsafeLabels []string // Fork determines if the pull request is from a fork. Fork bool - // Commits is a list of commit SHAs for the pull request. + // MergeCommitSHA changes depending on the state of the pull request. Before merging a pull request, + // the MergeCommitSHA holds the SHA of the test merge commit. After merging a pull request, + // the MergeCommitSHA changes depending on how you merged the pull request: // - // It is only populated if the pull request was fetched using - // GetPullRequestWithCommits method. - Commits []string + // If merged as a merge commit, MergeCommitSHA represents the SHA of the merge commit. + // If merged via a squash, MergeCommitSHA represents the SHA of the squashed commit on the base branch. + // If rebased, MergeCommitSHA represents the commit that the base branch was updated to. + MergeCommitSHA string } // Branch is a git Branch. @@ -280,27 +283,6 @@ func (f PullRequestFiles) SourceFiles() (files PullRequestFiles) { return files } -// GetPullRequestWithCommits returns the specified pull request with commits. -func (c *Client) GetPullRequestWithCommits(ctx context.Context, organization string, repository string, number int) (PullRequest, error) { - pull, err := c.GetPullRequest(ctx, organization, repository, number) - if err != nil { - return PullRequest{}, trace.Wrap(err) - } - - commits, _, err := c.client.PullRequests.ListCommits(ctx, organization, repository, number, &go_github.ListOptions{}) - if err != nil { - return PullRequest{}, trace.Wrap(err) - } - - for _, commit := range commits { - if len(commit.Parents) <= 1 { // Skip merge commits. - pull.Commits = append(pull.Commits, *commit.SHA) - } - } - - return pull, nil -} - // GetPullRequest returns a specific Pull Request. func (c *Client) GetPullRequest(ctx context.Context, organization string, repository string, number int) (PullRequest, error) { pull, _, err := c.client.PullRequests.Get(ctx, @@ -329,10 +311,11 @@ func (c *Client) GetPullRequest(ctx context.Context, organization string, reposi Ref: pull.GetHead().GetRef(), SHA: pull.GetHead().GetSHA(), }, - UnsafeTitle: pull.GetTitle(), - UnsafeBody: pull.GetBody(), - UnsafeLabels: labels, - Fork: pull.GetHead().GetRepo().GetFork(), + UnsafeTitle: pull.GetTitle(), + UnsafeBody: pull.GetBody(), + UnsafeLabels: labels, + Fork: pull.GetHead().GetRepo().GetFork(), + MergeCommitSHA: pull.GetMergeCommitSHA(), }, nil } @@ -375,10 +358,11 @@ func (c *Client) ListPullRequests(ctx context.Context, organization string, repo Ref: pull.GetHead().GetRef(), SHA: pull.GetHead().GetSHA(), }, - UnsafeTitle: pull.GetTitle(), - UnsafeBody: pull.GetBody(), - UnsafeLabels: labels, - Fork: pull.GetHead().GetRepo().GetFork(), + UnsafeTitle: pull.GetTitle(), + UnsafeBody: pull.GetBody(), + UnsafeLabels: labels, + Fork: pull.GetHead().GetRepo().GetFork(), + MergeCommitSHA: pull.GetMergeCommitSHA(), }) } if resp.NextPage == 0 {