Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Checkout does not fetch the latest target branch when workflow re-run #1036

Closed
ddongminstar opened this issue Dec 9, 2022 · 4 comments
Closed

Comments

@ddongminstar
Copy link

In case of on pull request event,

With default settings,

use: actions/checkout@v3

As input option reference is empty, reference and SHA will be filled up with github context.

In case of workflow re-run, github context is same with initial workflow run context - https://docs.github.com/en/actions/managing-workflow-runs/re-running-workflows-and-jobs

If commit id is not empty we use commit-id to make refSpec and fetch the specific merge reference's commit. - https://github.com/actions/checkout/blob/main/src/ref-helper.ts#L93

So workflow checkout act as deterministic and does not match with target branch's latest commit if changed.

But if we made any new commit to PR, it will work as expected. (As commit id changed, fetch the merged version of latest target branch)

It works like checkout workflow cache the result internally as #696 and #461 mentioned.


We use github actions to CI and many tester and verifier.

In case of default branch's test code failed, we made a new pull request and merge to fix test failure.
But We have to tell all other PR's author to made a any new commit or rebase and force push to pass the test.

So we made little trick like

use: actions/checkout@v3
with:
  ref: ${{ github.ref }}

just as same with actual reference

Checkout workflow run as non-deterministic but good for deal with target-branch's test failure.


So I think it will be great if there will be some document like checkout will not follow the target branch's latest commit in case of workflow re-run and you can make it follow by putting same reference as option.

Or to make it more understandable, how about provide a new option to choose whether use commit or use reference when fetching

I'm not sure this is a big issue, but please consider some document or options. Thank you in advance.

@marc-hb
Copy link

marc-hb commented Dec 9, 2022

I think it's better like it is now. Sometimes you want to re-test the very same merge commit, for instance when there was a network issue. Other times you want to re test against the latest target branch. Right now you can do both:

  • To re-test the very same merge commit use the "re-run" button
  • To re-test with the newest target branch do a simple git commit --amend -C HEAD and force push.

If the "re-run" button does the latter then how do you do the former?

In any case it should be better documented, maybe here:
https://github.com/actions/checkout#Checkout-pull-request-HEAD-commit-instead-of-merge-commit

@vbuberen
Copy link

Just found out this issue as well while was trying to understand why changes are not checked out by a workflow.
In case logs are needed here it is where it is clearly visible that action defaults to main branch after re-run: https://github.com/fluttercommunity/plus_plugins/actions/runs/3670853327/jobs/6205663810

Agree with the reporter that documenting such case better would be beneficial.

@vanZeben
Copy link
Contributor

To start, I want to say thanks a lot for providing some verbose feedback here. It's really good to see the community engage with us and be very clear with their problems. So thank you :) Secondly, I want to clarify on this a bit, as I think there is some contextual information about Pull requests that is missing and causing the confusion here.


In case of on pull request event,

With default settings,

use: actions/checkout@v3

...

From what I gather with the initial issue, you're looking for a way to keep Pull Requests up to date with the inclusion of more recent commits from the target branch's history using the pull_request event. Would that be correct?

If so, this isn't necessarily something that's intended directly by actions/checkout rather it's intended by the nature of the PR's themselves and how workflows are triggered off those PR's with the pull_request event. At a high level, your branch (which is associated to the PR) is forked off of a specific commit from the main branch and works in isolation from that point forward. When we run workflows with the pull_request event, they are contextualized to the Pull request itself, and so any updates to the target, aren't visible by the workflow until the Pull request is updated to match and as @marc-hb noted, there are ways to update PR's to test workflows with latest changes and we definitely want to preserve the deterministic nature of workflows with their context.

Something that might be helpful to you is that we do also have some documentation about keeping PR's in-sync. What this will do is help to alert authors of PR's automatically when there are changes to the target branch and prevent merging on their PR's if they don't have the latest changes (specifically take a look at this section in our docs if you want to make PR's more strict in this sense).


In regard to @vbuberen with;

Just found out this issue as well while was trying to understand why changes are not checked out by a workflow. In case logs are needed here it is where it is clearly visible that action defaults to main branch after re-run: https://github.com/fluttercommunity/plus_plugins/actions/runs/3670853327/jobs/6205663810

Agree with the reporter that documenting such case better would be beneficial.

I took a look into your case and I think it differs from the above, and I wouldn't classify this as the same issue originally noted, although contextually it has the same core misconceptions about the purpose of actions/checkout. Your workflow uses the event trigger pull_request_target, which to quote directly from the docs on the usage of this (which you can find here)

This event runs in the context of the base of the pull request, rather than in the context of the merge commit, as the pull_request event does..
{...}
Avoid using this event if you need to build or run code from the pull request.

What that means in the context of this actions and workflows using this action, you are the target branch and the the changes in the pull request that triggered the event are not included, which is why you need to use a workaround "re-center" the ref of the PR.

I'm not sure the specifics of your workflows, however unless you need to write permission to the target repository, or access to the target's secrets, I would recommend avoiding using pull_request_target except for cases where you need that write permission. We have a pretty lengthy security post about the dangers of pull_request_target that might be helpful to read, which (if you look at the code snippet for insecure pull_request_target cases in that document) actually includes retargeting the actions/checkout with the HEAD ref SHA in the same same manner as you did.


Given that both these comments aren't directly related to actions/checkout and from my perspective more are about how pull requests work or specific workflow event's work and we have other documentation directly talking about issues, Im going to close this.

If I've wildly misunderstood your problems, please reopen this issue and tag me directly and I would be happy to address your concerns :)

@vanZeben vanZeben closed this as not planned Won't fix, can't repro, duplicate, stale Dec 16, 2022
@marc-hb
Copy link

marc-hb commented Dec 16, 2022

https://en.wikipedia.org/wiki/A_picture_is_worth_a_thousand_words

Git graphs made this truer than ever.

I generally avoid branching discussions that use only words, they're at best very time consuming and at worst a waste of time because people talk across each other.

Unlike "plain git", many github projects tend to have a linear history. This gives users the wrong impression that they can discuss git issues without any graphical representation. Big mistake.

None of these pages mentioned above has any git graph on them:

I haven't read every single docs.github.com page (obviously not) but I've read many over the years and I don't remember seeing a git graph in any of them. This is a huge documentation mistake. Searching the internet for something like "git merge strategy" shows thousands of pictures of git graphs - and rightly so.

craig-reahl added a commit to craig-reahl/reahl that referenced this issue Nov 7, 2023
ahoppen added a commit to ahoppen/github-workflows that referenced this issue Nov 5, 2024
When a GitHub action is re-run, it uses the same context as the original run. This means that it runs on the exact same merge commit and does not pick up changes done to the base branch since the last run.

In case the base branch is was failing when a PR was opened, this means that we can’t get CI to pass by fixing the base branch without pushing a new commit to the PR branch.

Adding `ref: ${{ github.ref }}` to the `actions/checkout` action works around this by checking out the PR merge ref (ie. `refs/pull/<pr_number>/merge`) instead of a fixed commit and the PR merge ref is updated, as described in actions/checkout#1036.
ahoppen added a commit to ahoppen/github-workflows that referenced this issue Nov 5, 2024
When a GitHub action is re-run, it uses the same context as the original run. This means that it runs on the exact same merge commit and does not pick up changes done to the base branch since the last run.

In case the base branch is was failing when a PR was opened, this means that we can’t get CI to pass by fixing the base branch without pushing a new commit to the PR branch.

Adding `ref: ${{ github.ref }}` to the `actions/checkout` action works around this by checking out the PR merge ref (ie. `refs/pull/<pr_number>/merge`) instead of a fixed commit and the PR merge ref is updated, as described in actions/checkout#1036.
ahoppen added a commit to ahoppen/github-workflows that referenced this issue Nov 5, 2024
When a GitHub action is re-run, it uses the same context as the original run. This means that it runs on the exact same merge commit and does not pick up changes done to the base branch since the last run.

In case the base branch is was failing when a PR was opened, this means that we can’t get CI to pass by fixing the base branch without pushing a new commit to the PR branch.

Adding `ref: ${{ github.ref }}` to the `actions/checkout` action works around this by checking out the PR merge ref (ie. `refs/pull/<pr_number>/merge`) instead of a fixed commit and the PR merge ref is updated, as described in actions/checkout#1036.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants