Skip to content

Commit

Permalink
Backport merged commit SHA instead of individual commits
Browse files Browse the repository at this point in the history
Attempts to simplify backport operations by cherry-picking the
merged commit of a PR on the base branch instead of cherry-picking
each individual commit from the PR.
  • Loading branch information
rosstimothy committed Feb 3, 2025
1 parent e28b432 commit 4d35219
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 47 deletions.
21 changes: 10 additions & 11 deletions bot/internal/bot/backport.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down
3 changes: 0 additions & 3 deletions bot/internal/bot/bot.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
50 changes: 17 additions & 33 deletions bot/internal/github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 merge_commit_sha attribute holds the SHA of the test merge commit. After merging a pull request,
// the merge_commit_sha attribute 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, merge_commit_sha represents the SHA of the merge commit.
// If merged via a squash, merge_commit_sha represents the SHA of the squashed commit on the base branch.
// If rebased, merge_commit_sha represents the commit that the base branch was updated to.
MergeCommitSHA string
}

// Branch is a git Branch.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 4d35219

Please sign in to comment.