Skip to content

Commit

Permalink
tools/psltool: support checking an arbitrary past commit hash
Browse files Browse the repository at this point in the history
This robustly finds the two diff commits for open PRs, PRs closed
by squash-and-merge, and PRs closed by traditional merge commits
with 2 parents.
  • Loading branch information
danderson committed Sep 18, 2024
1 parent e5a1782 commit 61e72fc
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 37 deletions.
67 changes: 32 additions & 35 deletions tools/internal/github/pr.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,52 +60,49 @@ func (c *Client) PSLForPullRequest(ctx context.Context, prNum int) (withoutPR, w
return nil, nil, err
}

if state := pr.GetState(); state != "open" {
return nil, nil, fmt.Errorf("cannot get PSL for PR %d with status %q", prNum, state)
mergeCommit := pr.GetMergeCommitSHA()
if mergeCommit == "" {
return nil, nil, fmt.Errorf("no merge commit available for PR %d", prNum)
}
if !pr.GetMergeable() {
return nil, nil, fmt.Errorf("cannot get PSL for PR %d, needs rebase", prNum)
}
trialMergeCommit := pr.GetMergeCommitSHA()
if trialMergeCommit == "" {
return nil, nil, fmt.Errorf("no trial merge commit available for PR %d", prNum)
}

prHeadCommit := pr.GetHead().GetSHA()
if prHeadCommit == "" {
return nil, nil, fmt.Errorf("no commit SHA available for head of PR %d", prNum)
}

// We want to return the trial merge commit's PSL as withPR, and
// the non-PR parent of that merge as withoutPR. Github only
// provides information about the trial merge commit and the PR
// head commit in the PR API. It also provides a "base" ref, but
// empirical evidence shows this points at some random commit
// somewhere and updates based on unclear triggers. IOW, it is
// _not_ "master without the PR applied".
//
// Instead, we have to ask the git API for information about the
// trial merge commit, and find the correct withoutPR SHA from
// that.
commitInfo, _, err := c.apiClient().Git.GetCommit(ctx, c.owner(), c.repo(), trialMergeCommit)
commitInfo, _, err := c.apiClient().Git.GetCommit(ctx, c.owner(), c.repo(), mergeCommit)
if err != nil {
return nil, nil, fmt.Errorf("getting info for trial merge SHA %q: %w", trialMergeCommit, err)
return nil, nil, fmt.Errorf("getting info for trial merge SHA %q: %w", mergeCommit, err)
}

var beforeMergeCommit string
if numParents := len(commitInfo.Parents); numParents != 2 {
return nil, nil, fmt.Errorf("unexpected parent count %d for trial merge commit on PR %d, expected 2 parents", numParents, prNum)
}
if commitInfo.Parents[0].GetSHA() == prHeadCommit {
beforeMergeCommit = commitInfo.Parents[1].GetSHA()
} else {
if pr.GetMerged() && len(commitInfo.Parents) == 1 {
// PR was merged, PSL policy is to use squash-and-merge, so
// the pre-PR commit is simply the parent of the merge commit.
beforeMergeCommit = commitInfo.Parents[0].GetSHA()
} else if !pr.GetMergeable() {
// PR isn't merged, and there's a merge conflict that prevents
// us from knowing what the pre- and post-merge states are.
return nil, nil, fmt.Errorf("cannot get PSL for PR %d, needs rebase", prNum)
} else {
// PR is open, which means the merge commit is a "trial merge"
// that shows what would happen if you merged the PR. The
// trial merge commit has 2 parents, one is the PR head commit
// and the other is master without the PR's changes.
if numParents := len(commitInfo.Parents); numParents != 2 {
return nil, nil, fmt.Errorf("unexpected parent count %d for trial merge commit on PR %d, expected 2 parents", numParents, prNum)
}

prHeadCommit := pr.GetHead().GetSHA()
if prHeadCommit == "" {
return nil, nil, fmt.Errorf("no commit SHA available for head of PR %d", prNum)
}
if commitInfo.Parents[0].GetSHA() == prHeadCommit {
beforeMergeCommit = commitInfo.Parents[1].GetSHA()
} else {
beforeMergeCommit = commitInfo.Parents[0].GetSHA()
}
}

withoutPR, err = c.PSLForHash(ctx, beforeMergeCommit)
if err != nil {
return nil, nil, err
}
withPR, err = c.PSLForHash(ctx, trialMergeCommit)
withPR, err = c.PSLForHash(ctx, mergeCommit)
if err != nil {
return nil, nil, err
}
Expand Down
12 changes: 10 additions & 2 deletions tools/psltool/psltool.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,19 @@ func runFmt(env *command.Env, path string) error {
}

var validateArgs struct {
Online bool `flag:"online-checks,Run validations that require querying third-party servers"`
Commit string `flag:"commit,SHA hash of the commit to check"`
Online bool `flag:"online-checks,Run validations that require querying third-party servers"`
}

func runValidate(env *command.Env, path string) error {
bs, err := os.ReadFile(path)
var bs []byte
var err error
if validateArgs.Commit != "" {
client := github.Client{}
bs, err = client.PSLForHash(context.Background(), validateArgs.Commit)
} else {
bs, err = os.ReadFile(path)
}
if err != nil {
return fmt.Errorf("Failed to read PSL file: %w", err)
}
Expand Down

0 comments on commit 61e72fc

Please sign in to comment.