[BUG] Using the fork point to detect file changes.

[grzegorzkrukowski]

Is there an existing issue for this?

• I have searched the existing issues

Does this issue exist in the latest version?

• I'm using the latest release

Describe the bug?

We are experience a weird behaviour of changed files being returned on Pull Requests.

You can find my workflow runs here:
https://github.com/raycast/extensions/runs/5148147932?check_suite_focus=true#step:3:107
https://github.com/raycast/extensions/runs/5149182650?check_suite_focus=true#step:3:107

Both of them runs on the same PR and against same main - the only different is that on main there were 2 commits added meanwhile I was testing it, you can see it here:

Goal of our PRs is to validate files added by PR - I want to get files that were modified in PRs - only those files.
But from time to time we are getting weird combination of files coming from changes done on main branch like you can see in first workflow run above.
Expected behaviour is to get just the list of changed files in given PR - so the correct one is the second run.

Our workflow is doing just this:

- name: Checkout
        uses: actions/checkout@v2
        with:
          fetch-depth: 0
 - name: Get changed files
      id: changed-files
      uses: tj-actions/changed-files@v14.3

We have been playing with both:

base_sha: ${{ github.event.pull_request.base.sha }}
sha: ${{ github.event.pull_request.head.sha }}

and for some runs it helped, but it broke other PRs :/ it's a bit randomly behaving.

To Reproduce

1. Use:

- name: Checkout
        uses: actions/checkout@v2
        with:
          fetch-depth: 0
 - name: Get changed files
      id: changed-files
      uses: tj-actions/changed-files@v14.3

2. Create several PRs coming from forked repositories, meanwhile adding few commits in main repository
3. Observe how randomly sometimes commits from main are taken into account. Some commits on main can fix the problem or break it when you re-run existing checks on PRs.

What OS are you seeing the problem on?

all

Expected behavior?

I would expect consistency.

Goal is to always get just changed files in PR - ignoring any changes that happened on main.

Relevant log output

Wrong:
https://github.com/raycast/extensions/runs/5148147932?check_suite_focus=true#step:3:107

Right:
https://github.com/raycast/extensions/runs/5149182650?check_suite_focus=true#step:3:107

Just re-runed after 2 commits were added.

Anything else?

All repositories and workflows are public.
We have more runs like this happening:
https://github.com/raycast/extensions/runs/5143485235?check_suite_focus=true

Not sure if that's happening because of forking or what's the problem - if we are doing something wrong in our workflows, please advice.

Code of Conduct

• I agree to follow this project's Code of Conduct

[jackton1]

This is expected based on actions/checkout#27, I'll recommend taking a look at actions/checkout#27 (comment) if you want to prevent files from showing up that were recently merged into the main branch

You can reopen an issue on https://github.com/actions/checkout if you facing other issues related to the above. Thanks

[grzegorzkrukowski]

I am afraid that doesn't work:

Here is failing run for Forked PR - it doesn't see this SHA:
https://github.com/raycast/extensions/actions/runs/1833497844

I think it may be actually a bug in this action as this SHA is pre-generated by this action:
c38c02d0258d88fedf6c56184149bbb9fc41ee26

I am not passing it to changed-files - using all defaults

[jackton1]

@grzegorzkrukowski After testing this I found the source of the error and should be resolved in #358

[grzegorzkrukowski]

Thanks @jackton1 - so now should I use my original version or one with ‘ref’ ?

[jackton1]

@grzegorzkrukowski It should be available in the latest release v14.6.

[grzegorzkrukowski]

Ok still - which version should be working now ? :) the one where I checkout merged commit or one with “ref” ?

[jackton1]

@grzegorzkrukowski You'll need to upgrade the action which fixes usage of merge commits

   - name: Checkout
      uses: actions/checkout@v2
      with:
        fetch-depth: 0
   - name: Get changed files
      id: changed-files
      uses: tj-actions/changed-files@v14.6

[grzegorzkrukowski]

Hey @jackton1 - I'm afraid it's still now working as it should:

Here is a run using code from your last message:
https://github.com/raycast/extensions/runs/5185457484?check_suite_focus=true

And here is a run when I tried to run with ref and repository to get PR repo:

- name: Checkout
      uses: actions/checkout@v2
      with:
        ref: ${{ github.head_ref }}
        repository: ${{ github.event.pull_request.head.repo.full_name }}
        fetch-depth: 0

https://github.com/raycast/extensions/runs/5185420161?check_suite_focus=true

[jackton1]

@grzegorzkrukowski The second job looks like a force push we nukes the commit hash and possibly leads to errors fetching the commit.

I'll reopen this issue, as the test currently don't handle pull requests from forks and try to replicate the issue.

[grzegorzkrukowski]

Yes I am also looking into this problem.

The weird thing I don't understand full is that:

git checkout --progress --force refs/remotes/pull/534/merge
HEAD is now at d64793a0 Merge 4706f01c76ef11c4220e9f81e99e13cc58e80642 into cd113f35aec390114be714ad88e88104dc88054f

I would expect it to be merged with bfe4cbbdf985254855b1ab36228187e9ce719a89 - head of main repo.

[grzegorzkrukowski]

I found something that maybe related to this issue:
actions/checkout#299

It seems like git checkout --progress --force refs/remotes/pull/534/merge is really doing a merge with last commit that was available time-wise on origin/main:

Which doesn't make any sense to me - I also send a support ticket to GitHub Support to get an answer for this one.
If that's desired behaviour from git - I think there's no way to use to get changed files.
In that case, it may be better to just go with:
gh pr view 534 --json files -q '.files[].path'

[jackton1]

@grzegorzkrukowski Just to confirm using ad-m/github-push-action#20 (comment) also doesn't solve the issue for you ??

[jackton1]

@grzegorzkrukowski Can you outline the issue you faced using ?

   - name: Checkout
      uses: actions/checkout@v2
      with:
        fetch-depth: 0
        ref: ${{ github.head_ref }}
   - name: Get changed files
      id: changed-files
      uses: tj-actions/changed-files@v14.6
      with:
         base_sha: ${{ github.event.pull_request.head.sha }}
         sha: ${{ github.event.sha }}

[jackton1]

@grzegorzkrukowski I stumbled upon a possible solution that might work but I’ll need your help in replicating the issue.

Based on: #355 (comment)

• I've already made a pull request from a fork Update diff-sha.sh #379
• I've merged a couple of PR's into the main branch
  • Update README.md #381
  • Update README.md #380

But I'm unable to replicate more files showing up in the diff

Possible resolution:

• https://stackoverflow.com/a/29813554/4584672

[grzegorzkrukowski]

Hey @jackton1 thanks for looking into this - let me see if I can reproduce it with your example

[grzegorzkrukowski]

@jackton1 I see you have been doing a lot of merges from main into your branch:

When you look at our example - we don't really have merges - branch is never merged with main or rebased after its created.

[jackton1]

@grzegorzkrukowski This should be resolved using the use_fork_point input set to true which is available in the latest release.

See: https://github.com/tj-actions/changed-files/runs/5228672940?check_suite_focus=true

[jackton1]

@grzegorzkrukowski If you haven't already don't forget to star this project to help us reach a wider audience. Thanks

[grzegorzkrukowski]

Will do !! Thanks for your support ! Its one of the best axtions out there, couldnt live without it.

[grzegorzkrukowski]

Hey @jackton1 I am afraid it doesn't work for some reason in our repository:
https://github.com/raycast/extensions/runs/6129212969?check_suite_focus=true

It says about fetch-depth: 0 - but it's there and still it doesn't find properly the fork point :(
We are still constantly struggling with a proper solution there...

[jackton1]

I’m guessing this is the link to the PR
raycast/extensions#1266

I’ll take a look at it this weekend.

Something that stands out is the previous sha is unset which seems odd.

This also occurs on a fork that is based on a different branch than main/master

[grzegorzkrukowski]

Thanks for looking into this so quickly!

Yes, exactly that's the related PR.

We haven't been using the use_fork_point - I wanted to test it out just today and it didn't work - so I cannot really confirm it was working fully before in our setup.

[grzegorzkrukowski]

Hey - any news on this one? I still couldn't figure out a proper way to make it work.

[jackton1]

@grzegorzkrukowski The bug has been fixed in the latest release v20

[grzegorzkrukowski]

I am afraid I still cannot make it work...

Here is the log from a failed job for one of my tries when using it:
https://github.com/raycast/extensions/actions/runs/2420559225/attempts/2

It says:

Warning: Unable to locate the previous sha: 

and I am fetching with fetch-depth: 0 :(