checkout action performing merge commit from incorrect base SHA about checkout HOT 16 CLOSED

We're observing actions/checkout creating merge commits based on the repo's latest master SHA, rather than github.event.pull_request.base.sha from the event that initiated the action.

and:

Is it correct that refs/remotes/pull/15/merge is a merge commit from the latest master, not from master when the PR event triggered the job?

Yes, this is the intended default behavior. The goal is to validate that the pull request will build and test against what it would be merged into. Continuous integration builds need to take into account what they'll be merge into, not the state of the repository when they were created. This prevents you from merging a commit that breaks master but "worked on my machine".

If your goal is not to validate the CI but to do some fixups (ie, automatic updates from linting) then I agree that you would not want to check out the merge commit but to actually

If you really want to validate the pull request as it was actually sent, and not what would be produced by a merge into master (ie, in isolation of the master branch), you can specify the ref to checkout:

steps:
- uses: actions/checkout@v1
  with:
    ref: ${{ github.head_ref }} 

However, I strongly encourage you not to make this a separate workflow - one workflow that lints and updates the PR if (and only if) it made some changes, and then a second workflow that does a build and test on the merge produced into master for verification.

from checkout.

@ethomson Appreciate the detailed reply. Testing a PR merged into master is in fact what we want, not github.head_ref.

Our core issue is that our workflow that executes actions/checkout across multiple jobs, and if master changes between those jobs, actions/checkout yields different copies of the repo.

This is problematic because, for example, a job in our workflow builds docker images versioned as docker-image:git-sha-foo, and then a subsequent job tries to deploy docker-image:git-sha-bar.

The result is every time we merge master, most currently-running CI workflows fail. Here's an example, note actions/checkout yielding different repo SHAs within a single CI workflow:
https://github.com/linkerd/linkerd2/runs/238047516#step:2:1142
https://github.com/linkerd/linkerd2/runs/238049216#step:2:1142

Is there any plan to open source this action? We'd love to just submit a PR to better illustrate the issue. Failing that, we may write our own checkout action, but we're concerned about potential rate limit issues.

Any guidance is much appreciated, thanks.

from checkout.

Our core issue is that our workflow that executes actions/checkout across multiple jobs, and if master changes between those jobs, actions/checkout yields different copies of the repo.

Thanks, @siggy, for the clarification. We'll give this some thought.

from checkout.

We're also running into this issue. -- Basically under the hood, Github Actions/Checkout is checking out this phantom Merge Commit, when I look at other environment variables like GITHUB_SHA they actually point to this fake commit -- and the commit claims to be from me! (but it's not signed/verified like my other commits are).

from checkout.

@siggy actions/checkout@v2 fixes the race condition. For PRs, the individual SHA is now fetched.

from checkout.

@siggy This is kind of worrying behavior. Does using command line git avoid this issue?

from checkout.

I think the second job should failed if your PR branch get updated, since we can't find the same SHA to build anymore.

from checkout.

@chingc @TingluoHuang Thanks for looking into this. I have set up a simpler, more contrived example:

https://github.com/siggy/linkerd2/pull/15/checks?check_run_id=220501197
https://github.com/siggy/linkerd2/blob/a2583cac37a6c958dfc9bf5ae2075e6af7c1cf0b/.github/workflows/workflow.yml

I am not reliably reproducing the issue in the above example, but I do I see commands from actions/checkout that I think may be causing this:

git -c http.extraheader="AUTHORIZATION: basic ***" fetch --tags --prune --progress --no-recurse-submodules origin +refs/heads/*:refs/remotes/origin/* +refs/pull/15/merge:refs/remotes/pull/15/merge
git checkout --progress --force refs/remotes/pull/15/merge

Is it correct that refs/remotes/pull/15/merge is a merge commit from the latest master, not from master when the PR event triggered the job?

from checkout.

This is particularly problematic, because we have some automatic commits like Prettier Formatting, and generating some documentation when certain code files change, and these automatic commits are effectively doing a merge-master-into-branch every time they trigger!

Each of those Merge SHA into SHA commits are generated by Github and end up being the context under which the action is Running.

This is also a problem as we have an external CI, that we trigger from Github Actions (based on labels), and we try to prevent duplicate builds for a given Commit, but since this Phantom Commit is the GITHUB_SHA we work with, it never finds a build for that commit, so we're wasting CI cycles building the app more than a few times.

from checkout.

Oh wow! That’s super interesting. And very much not what I was guessing or expecting.

What happens if there are merge conflicts?
What happens if master has changes to a dependency that we rely on? It’s common in the react* to use snapshot tests, and these currently would fail our PR every time we push a commit when the PR is not up to date with master. It seems like this expects a continuous rebasing against master? Even then… ouch?

from checkout.

steps:
- uses: actions/checkout@v1
  with:
    ref: ${{ github.head_ref }} 

This is exactly what I've had to resort to when running some ci checks that report status based on branch. Otherwise the reported branch is incorrect.

from checkout.

Hi @fbartho -

What happens if there are merge conflicts?

The first check that happens on GitHub when you open a pull request is whether it's mergeable or not.

What happens if master has changes to a dependency that we rely on? It’s common in the react* to use snapshot tests, and these currently would fail our PR every time we push a commit when the PR is not up to date with master. It seems like this expects a continuous rebasing against master? Even then… ouch?

I'd like to understand more about the scenario you're describing, but generally this is an advantage. You usually want to know if there was a change to a dependency in master that you depend on in your pull request. Consider the case where the dependency was changed in master in an incompatible way and that PR used it. If the CI build didn't do the merge into master, then that would just be a successful build, but as soon as you merged the pull request, that would break.

By building the merge commit, you're able to have high confidence that the integration of your pull request will be successful.

from checkout.

@ethomson FWIW, we have worked around this issue by only using actions/checkout in the first job of our workflow, and then saving that copy of the repo as an artifact for all subsequent jobs: linkerd/linkerd2#3602

from checkout.

@ericsciple Thanks! Will check it out.

from checkout.

I've just run into what I believe is a related issue.
Yesterday, I open a PR, and CI runs.
Some time later that day, there's an unrelated commit pushed to master.
Today, I push a new commit to my PR.
In this PR run ${{github.event.pull_request.base.sha}} is set to to SHA1 of master yesterday when the PR was opened, whereas I expected it to be set to the SHA1 of master today.
The parent commits of refs/pull/1234/merge are the head of my branch and the SHA1 of master today.
I expected ${{github.event.pull_request.base.sha}} to be one of the parent commits of refs/pull/1234/merge, but it's not.
I noticed this because I use actions/checkout with fetch-depth: 2, which fetches the merge commit and its two parent commits: the head of the branch being tested, and the head of the base branch master. I expected ${{github.event.pull_request.base.sha}} to be in the list of revisions fetched by actions/checkout, but it's not.
That causes this failure:

$ git diff --name-only ${{github.event.pull_request.base.sha}}...
fatal: Invalid symmetric difference expression ddf331629b3147875282f18f52fc6f3483d75dff...

This repo is private. For GitHub staff who may investigate:
Yesterday's CI run (which succeeded) is 624420073
Today's CI run (which failed) is 627096250

The workaround of instead using git diff --name-only 'HEAD~1' looks like it'll work.

from checkout.

Bumping, since it seems like folks (myself included) are still having issues with this. I see a lot of people are switching to ref: ${{ github.event.pull_request.head.sha }} as noted in the readme, but this doesn't sound like the best fix here.

From what I understand, creating a new merge commit between the PR's HEAD and the base branch, and running the CI against that, is telling us if our CI would pass after the PR is merged. So if any code conflicts are resolved cleanly during the merge, or if behavior has changed elsewhere that could cause test failures, this CI build will catch that.

However, it looks like we're still running into issues with the SHA of the checked out code being incorrect, or originating from an incorrect base.

Edit: found this issue when poking around, which asks why a merge commit is the default behavior. I think my description here answers that, but please correct me if I'm wrong!

from checkout.