[llvm] Reapply [workflows] Split pr-code-format into two parts to make it more secure (#78215) (PR #80495)

James Y Knight via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 6 13:12:50 PST 2024


jyknight wrote:

OK, I looked into this in a bit more detail:
Firstly -- a pull_request run is _always_ run in the context of the "merge" branch, not the PR head. The "merge" branch is auto-updated (if there are no file conflicts) to the latest revision of the target branch. So, a PR will generally automatically see a new .github/workflow file after it's committed -- and by default, we'd get all the _rest_ of the new files too!

However, the issue is that we're explicitly instructing it to checkout the SHA of the "head" commit, not the "merge" commit.

In fact, I think this _whole process_ gets a lot easier if we make use the auto-generated merge commit, because then we:
1. Checkout the merge commit (which is the default behavior of "checkout" if we weren't passing `with: ref: ${{ github.event.pull_request.head.sha }}`) with fetch_depth=2. We don't need to deepen, because the 2 parents of the merge-commit are the exact two things we need to compare!
2. For the diff between those two commit hashes: get the list of changed files, run the formatter on the diff, etc...we don't care about any intermediate commits -- just those two endpoints is all we need.

Another advantage: if we fix an issue in the formatting jobs, most PRs

That said, I think it's fine to defer these fixes to a separate PR: it becomes a lot easier to test this when we're no longer using pull_request_target!

https://github.com/llvm/llvm-project/pull/80495


More information about the llvm-commits mailing list