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

Picking up extra files during workflow dispatch. #71

Closed
sauln opened this issue Feb 24, 2021 · 5 comments · Fixed by #74
Closed

Picking up extra files during workflow dispatch. #71

sauln opened this issue Feb 24, 2021 · 5 comments · Fixed by #74
Labels
bug Something isn't working

Comments

@sauln
Copy link

sauln commented Feb 24, 2021

When I merge a PR, the filters work correctly, but when I run the same thing from a workflow_dispatch trigger, this action views all files as added. From the logs, I see No merge base found - all files will be listed as added when the base is correctly resolved as main. This happens even when the number of commits in the PR is small, so I don't think increasing depth would fix this.

Here's the relevant snippet from my action:

     - uses: dorny/paths-filter@v2
        id: changes
        with:
          filters: |
            settings:
              - 'es_settings/**'

     - name: "Trigger bootstrap"
        if: steps.changes.outputs.settings == 'true'
        id: bootstrap
        uses: ./.github/actions/bootstrap

When run as workflow_dispatch, all files in es_settings directory are flagged as added even though none of these files changed.

Matching files:
  es_settings/<filename>.json [added]
  ...
  es_settings/<other filename>.json [added]

Here's the run logs during workflow_dispatch:

Changes will be detected against the branch main
Checking if commit for main is locally available
  /usr/bin/git cat-file -e main^{commit}
  fatal: Not a valid object name main^{commit}
Fetching main
  /usr/bin/git fetch --depth=10 --no-tags origin main:main
  From https://github.com/<repo name>
   * [new branch]      main       -> main
   * [new branch]      main       -> origin/main
Searching for merge-base with main
  /usr/bin/git merge-base main HEAD
  /usr/bin/git rev-list --count HEAD
  1
  /usr/bin/git rev-list --count main
  46
  /usr/bin/git fetch --deepen=10 --no-tags
  From https://github.com/<repo name>
   * [new branch]      <branch name> -> origin/<branch name>
  /usr/bin/git rev-list --count HEAD
  25
  /usr/bin/git rev-list --count main
  14
  No merge base found - all files will be listed as added

and here's the run logs when running from a merge:

Fetching list of changed files for PR#58 from Github API

Am I using this wrong during workflow_dispatch? It looks like the base of main branch is inferred correctly, but no merge base is being correctly set?

@sauln
Copy link
Author

sauln commented Feb 25, 2021

So I played with a few parameters and got it working. With the extra params base and initial-fetch-depth set, it seems fine. I don't really understand why this is, as the defaults for both of these should be sufficient. The default for base should be set to main correctly, and the initial-fetch-depth: 20 should be plenty for the PRs I was playing with earlier today.

Any explanation why this fixed it? Possibly, I don't correctly understand what these parameters are doing, or the defaults are not set correctly?

      - uses: dorny/paths-filter@v2
        id: changes
        with:
          filters: |
            settings:
              - 'es_settings/**'
          base: main
          initial-fetch-depth: 50

      - name: "Trigger bootstrap"
        if: steps.changes.outputs.settings == 'true'
        id: bootstrap
        uses: ./.github/actions/bootstrap

Here are the logs:

Changes will be detected against the branch main
Checking if commit for main is locally available
  /usr/bin/git cat-file -e main^{commit}
  fatal: Not a valid object name main^{commit}
Fetching main
  /usr/bin/git fetch --depth=10 --no-tags origin main:main
  From https://github.com/<repo>
   * [new branch]      main       -> main
   * [new branch]      main       -> origin/main
Searching for merge-base with main
  /usr/bin/git merge-base main HEAD
  /usr/bin/git rev-list --count HEAD
  1
  /usr/bin/git rev-list --count main
  48
  /usr/bin/git fetch --deepen=10 --no-tags
  From https://github.com/<repo>
   * [new branch]      <branch name> ->  origin/<branch name>
   * [new branch]      <other branch name> -> origin/<other branch name>
  /usr/bin/git rev-list --count HEAD
  111
  /usr/bin/git rev-list --count main
  109
  /usr/bin/git merge-base main HEAD
 <hash>
Change detection main...HEAD
  /usr/bin/git diff --no-renames --name-status -z main...HEAD
  M<filename>M<filename>M<filename>M<filename>M<filename>M<filename>M<filename>
Results:
Filter settings = false
  Matching files: none

@sauln sauln closed this as completed Feb 25, 2021
@dorny dorny reopened this Feb 25, 2021
@dorny dorny added the bug Something isn't working label Feb 25, 2021
@dorny
Copy link
Owner

dorny commented Feb 25, 2021

Hello,
Thanks for reporting the issue. It's definitely a bug in the paths-filter action.

I will try to explain what happened:
If workflow is triggered by PR, GitHub makes available which files were changed via API and this action is using it.
For all other triggers (e.g. workflow_dispatch) we have to figure it out on our own from git history.
Therefore you have to use action/checkout before paths-filter.
Checkout action by default uses shallow clone - it will fetch only single commit of single branch.
Now we need to fetch the base branch - we do it using --depth=${initial-fetch-depth} argument to avoid fetching whole history. Fetching whole history could take unnecessary long for really big repositories.
We still cannot compare HEAD to top of the base directly - diff would contain all changes between those commits while we are interested on changes introduced only on the feature branch. We need to find the merge base between those two.
if we already have merge base, we can get what changed. Otherwise paths-filter will start executing git fetch --deepen=10 --no-tags in a loop until the merge base is found or there is no more history to fetch. It's possible to create disconnected branches so it's an edge case which might happen. This is detected by counting number of commits in history. If it's not higher than it was before the fetch, it means we reached the end and there is no merge base.

That's where the bug is. After initial fetch there was 46 commits on main. Then after git fetch --deepen it reports 14.
When you increased the initial-fetch-depth this loop did not start because it already found merge base after initial fetch.
I have to find out why git rev-list --count reported lower number than before. Wild guess is that it followed wrong path after some previous merge. Anyway I will also add fallback solution to fetch whole history.

@sauln
Copy link
Author

sauln commented Feb 25, 2021

Thank you for mastering the dark arts of git and building this action! I'm glad you've uncovered what the issue is. 🙇

@dorny
Copy link
Owner

dorny commented Feb 26, 2021

mastering the dark arts of git

Made my day :)

Unfortunately I have to keep this open for a week as I didn't manage to fix it before my vacation.
So please, for now use the workaround with increasing initial-fetch-depth in paths-filter or using fetch-depth: 0 in actions/checkout.

@sauln
Copy link
Author

sauln commented Feb 26, 2021

Enjoy your vacation 🏖️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants