GitHub Community
github.event.pull_request.base.sha should be updated each time the pull request is updated #59677
Replies: 3 comments 10 replies
-
UpvoteI want to upvote this. I see the problem from a different angle, but the issue remains the inconsistency between the documented principle that GitHub uses three-dot diffs for pull requests, and the fact that Github Actions' I'll pose it a different way if that's not clear:
I understand that, as a CI service, Github Actions is primarily concerned with "Continuous Integration," and that, as this user says here:
However, it just leads to so much confusion (ex1, ex2 ...) that this is the default behavior. It feels like there should, at least, be a clearer way to specify what you want from a Our use case:I work for a service in the CI toolchain that tracks test coverage for a repo's successive commits, potentially for its full history. Where this test coverage is most useful is for The problem, for us, is that to determine changes in coverage between two commits, we need to diff their coverage reports, which are predicated on a build for each commit. For a given pull request, we've typically already received a coverage report for the However, since the But that's the coverage change the user expects to see. They want to see how coverage has changed for their pull request—to wit, for "what [their] pull request introduces"—so they understand what changes they can make to their pull request, to improve coverage. |
Beta Was this translation helpful? Give feedback.
-
|
As of September 2024, Hence, this means that:
will always yield the correct fork point of the PR branch off the base branch. This in turn means that one can list all commits in a PR with:
or its equivalent:
|
Beta Was this translation helpful? Give feedback.
-
|
@carlescufi this doesn't fix the issue. The ask is that the value of Merge branch:
|
Beta Was this translation helpful? Give feedback.
-
|
Right, but when you write:
what do you mean? you choose what ref to check out when you actually check out the code in the workflow. |
Beta Was this translation helpful? Give feedback.
-
|
@carlescufi apologies if you know this already. I find myself explaining it a lot to new users of our service, so for anyone who may be reading this who's not super familiar with git and how
That's because And then, once the PR branch is merged into the target branch, git creates a new, historical That's how I think of it, anyway. I'm sure there's a more technically accurate definition.
Not really. One of the HEAD commits comes from your PR branch, which is why you may see the PR branch mentioned. It's the context of the |
Beta Was this translation helpful? Give feedback.
-
|
@afinetooth thanks for the additional context
Yep, I do understand that, though please note that you can also "merge" the PR by rebasing the commits in the PR branch, which then creates no merge commits at all, and allows you to have a pure linear history.
But you choose what code to check out in order to run tests on your Pull Request. As I wrote here, I always check out |
Beta Was this translation helpful? Give feedback.
-
I choose what ref to check out, but I don't choose what version of the workflow that runs. That version is always github.sha. Choosing any ref other than that leads to version skew. |
Beta Was this translation helpful? Give feedback.
-
If you refer to the revision of the workflow file itself that is being used to execute the action, that depends on whether you use |
Beta Was this translation helpful? Give feedback.
-
Select Topic Area
Product Feedback
Body
This has been discussed a bit in https://github.com/orgs/community/discussions/39880 and actions/runner#1689, but the discussions there have gone a bit on a tangent. I wanted to make a separate thread because I recently spent a lot of time debugging this behavior in my CI pipeline and I strongly believe the current behavior, while technically consistent with documentation, results in an subpar developer experience and deserves to be changed. Let me make the case:
Current Behavior
The value of
${{ github.event.pull_request.base.sha }}is not updated on every commit to the PR. It is the SHA of the base branch at the time the PR was created, which becomes out of date as the PR gets updated.Desired Behavior
The value of
${{ github.event.pull_request.base.sha }}should be updated on every commit to the value of the base branch SHA used to construct the PR's merge branch.Why
A core use case when writing GitHub Actions workflows on pull requests is to find the changes introduced in the PR. This requires finding 2 commits that will produce a correct diff output, which can then be passed to
git diffor various other third-party apps and integrations.If we choose the PR branch as the head, (i.e.
${{ github.event.pull_request.head.sha }}), then the correct base SHA is$(git merge-base ${{ github.event.pull_request.base.sha }} ${{ github.event.pull_request.head.sha }}). This produces the equivalent 3-dot-diff behavior that the GitHub UI uses.If we choose the PR's merge branch as the head (i.e.
${{ github.sha }}), then the correct base SHA is the value of the base branch used to construct the merge branch. This is currently NOT${{ github.event.pull_request.base.sha }}, because that value isn't updated every time the PR is updated. The only way I can currently get this base SHA is to parse the auto-generated commit message in the merge branch. (They currently have a message of "Merge <head sha> into <base sha>"). The ask is to put <base sha> as the value of${{ github.event.pull_request.base.sha }}so I can use the latter instead.In general, I think there should be a first-class way of getting the base SHA where the head is the PR's merge branch. This is because actions on PRs run on the PR's merge branch, not the PR branch. Because PR actions run on the merge branch, it's easiest for the workflow to also check out the merge branch and run checks on it. Checking out any other branch results in version skew between the workflow version and the repo, leading to a subpar developer experience.
Beta Was this translation helpful? Give feedback.
All reactions