Skip to content

Commit

Permalink
Revert "Backport merged commit SHA instead of individual commits (#313)…
Browse files Browse the repository at this point in the history
…" (#320)

This reverts commit ed99ff6.

Co-authored-by: github-actions <[email protected]>
  • Loading branch information
rosstimothy and github-actions authored Feb 4, 2025
1 parent ed99ff6 commit dbfe3ae
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 27 deletions.
21 changes: 11 additions & 10 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.GetPullRequest(ctx,
pull, err := b.c.GitHub.GetPullRequestWithCommits(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.GetPullRequest(ctx,
pull, err := b.c.GitHub.GetPullRequestWithCommits(ctx,
b.c.Environment.Organization,
b.c.Environment.Repository,
b.c.Environment.Number)
Expand Down Expand Up @@ -236,15 +236,16 @@ func (b *Bot) createBackportBranch(ctx context.Context, organization string, rep
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)
// 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)
}
return trace.Wrap(err)
}

// Push the backport branch to Github.
Expand Down
3 changes: 3 additions & 0 deletions bot/internal/bot/bot.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ 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: 33 additions & 17 deletions bot/internal/github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,14 +196,11 @@ type PullRequest struct {
UnsafeLabels []string
// Fork determines if the pull request is from a fork.
Fork bool
// 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:
// Commits is a list of commit SHAs for the pull request.
//
// 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
// It is only populated if the pull request was fetched using
// GetPullRequestWithCommits method.
Commits []string
}

// Branch is a git Branch.
Expand Down Expand Up @@ -283,6 +280,27 @@ 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 @@ -311,11 +329,10 @@ 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(),
MergeCommitSHA: pull.GetMergeCommitSHA(),
UnsafeTitle: pull.GetTitle(),
UnsafeBody: pull.GetBody(),
UnsafeLabels: labels,
Fork: pull.GetHead().GetRepo().GetFork(),
}, nil
}

Expand Down Expand Up @@ -358,11 +375,10 @@ 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(),
MergeCommitSHA: pull.GetMergeCommitSHA(),
UnsafeTitle: pull.GetTitle(),
UnsafeBody: pull.GetBody(),
UnsafeLabels: labels,
Fork: pull.GetHead().GetRepo().GetFork(),
})
}
if resp.NextPage == 0 {
Expand Down

0 comments on commit dbfe3ae

Please sign in to comment.