ci: fix changelog checks for fork pull requests - #10161
Closed
adonesky1 wants to merge 1 commit into
Closed
Conversation
Both changelog checks resolved the pull request head as a branch in this repository, which does not exist for pull requests opened from a fork, so they failed before running any validation. Resolve the head commit through refs/pull/<number>/head instead, which GitHub populates for every pull request.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation
Summary
Resolve the pull request head commit through
refs/pull/<number>/headin both changelog checks, so they work for pull requests opened from a fork.Problem
Both changelog checks assume the pull request head branch exists in
MetaMask/core. That is false for a pull request opened from a fork, so both jobs fail before running any changelog validation. This currently blocks #9875, which comes fromwzrdk3lly/core.Check Changelogpasseshead-ref: ${{ github.head_ref }}(a bare branch name) together withrepo: ${{ github.repository }}toMetaMask/github-tools/.github/actions/check-changelog@v1. That action checks outinputs.repoatinputs.head-ref, soactions/checkoutlooks for a branch or tag of that name in this repository and fails:Lint, build, and test / Validate changelog diffsresolves the head viagh api .../pulls/<n> --jq=.head.refand then references it asorigin/$PR_BRANCH. That remote-tracking ref does not exist either:Solution
GitHub populates
refs/pull/<number>/headin the base repository for every pull request, including forks, so it is a single ref that both checks can resolve without knowing where the head branch lives.changelog-check.yml: passhead-ref: refs/pull/${{ github.event.pull_request.number }}/head.actions/checkoutspecial-casesrefs/pull/refs and fetches them intorefs/remotes/pull/*instead of trying to resolve a branch or tag name (ref-helper.ts).repostays asgithub.repositoryso thatoriginremains this repository and the action's owngit fetch origin <base-branch>plusgit diff origin/<base-branch>...HEADstill resolve.check-merge-queue-changelogs: fetchrefs/pull/<number>/headinto a localrefs/remotes/pr/<number>/headref and use it in place oforigin/$PR_BRANCH. This replaces thegh apilookup, since the pull request number the action already derives is enough to name the ref.Both checks keep working for same-repository pull requests and for
merge_groupevents, becauserefs/pull/<number>/headis populated for those pull requests too.Risk
git fetchruns against a checkout created withpersist-credentials: false.MetaMask/coreis public, so the anonymous fetch ofrefs/pull/*succeeds. If this repository ever becomes private, that fetch would need credentials.MetaMask/metamask-mobilewirescheck-changelogup the same way and has the same latent problem, but it is out of scope here.Verification
Replayed the
check-merge-queue-changelogsscript locally against #9875 (a fork pull request). Before the change it fails atgit merge-base; after the change the ref resolves and the script proceeds to real changelog validation:References
head-ref: https://github.com/MetaMask/github-tools/blob/v1/.github/actions/check-changelog/action.ymlrefs/pull/handling inactions/checkout: https://github.com/actions/checkout/blob/v6/src/ref-helper.tsChecklist