ci: replaced dorny/path-filter to tj-actions/changed-files

[JagjeevanAK]

Hey @stephanegigandet and @alexgarel, we finally made it! 🎉

I've tested this solution in my local repo, so there won't be any further excuses. I just want to clarify that the issue last time wasn't with the merge file solution — that was correct. The real problem was with the dorny/path-filter action, which turned out to be buggy.

I've now switched to tj-actions/changed-files, which works perfectly and is well-maintained. ✅

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 49.88%. Comparing base (e8deee5) to head (3d045cd).
Report is 6 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #11630   +/-   ##
=======================================
  Coverage   49.88%   49.88%           
=======================================
  Files          83       83           
  Lines       22984    22984           
  Branches     5508     5508           
=======================================
  Hits        11466    11466           
  Misses      10121    10121           
  Partials     1397     1397           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[stephanegigandet]

Thanks

[stephanegigandet]

Seems to work well :)

I refreshed #11621 after merging the PR, and the tests were skipped:

[JagjeevanAK]

Hey @stephanegigandet, I tested everything on my personal repo yesterday, and it should work in all aspects. If there's an extreme case where it fails for that, I'm here to resolve it.

[jayaddison]

@JagjeevanAK @alexgarel @stephanegigandet if I understand correctly, the reason for this is that it is not possible to use the GitHub Actions workflows paths and paths-ignored feature in combination with GitHub Actions CI Required Status (in other words: require success) checks, as described in the comment here:

openfoodfacts-server/.github/workflows/pull_request.yml

Lines 5 to 11 in db8daa9

	# we can't do that, because status are required
	# see https://stackoverflow.com/questions/66751567/return-passing-status-on-github-workflow-when-using-paths-ignore
	# paths-ignore:
	# - "**.md"
	# - ".github/CODEOWNERS"
	# - ".github/PULL_REQUEST_TEMPLATE.md"
	# - ".editorconfig"

However: previously there was a GitHub-documented workaround for that -- the advice has disappeared from GitHub's documentation some time since Y2023 (now the page redirects to one that does not mention it).

The Y2023 copy (that the poster in the linked discussion was referring to when they posted) can today be found at: https://web.archive.org/web/20230321195500/https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/troubleshooting-required-status-checks

The workaround was to create one paths-ignore: x entry, and then an inverted separate workflow -- with a job with the same name -- that has paths: x that always() succeeds.

Is that workaround not possible any more?

[jayaddison]

However: previously there was a GitHub-documented workaround for that -- the advice has disappeared from GitHub's documentation some time since Y2023 (now the page redirects to one that does not mention it).
...
The workaround was to create one paths-ignore: x entry, and then an inverted separate workflow -- with a job with the same name -- that has paths: x that always() succeeds.

Is that workaround not possible any more?

I have begun testing this in openculinary#1. So far, if my experiments are correct, it seems that the workaround remains feasible.

[JagjeevanAK]

I've never heard of a workaround, but what I mean is that the actions I've used are widely adopted and actively maintained. If you check their repositories, you'll see their high level of activity.

Additionally, we're currently using tj-actions/changed-files to skip unnecessary Docker builds and optimize our existing CI operations. So in some way they are helping more than a one thing.

[jayaddison]

The workaround - previously a documented solution according to GitHub - would allow us to enable paths and paths-ignore, meaning that we do not need to use a third-party action. The tj-actions/changed-files action experienced a security incident recently, so it may be worth being cautious about it.

[jayaddison]

As a followup to my previous comments: it seems the built-in paths feature from GitHub Actions itself is not sufficient here; we want the ability to filter individual job steps.

I'm trying to work out what the blockers for usage of dorny/paths-filter were; I've learned from discussion in #11644 that it is due to something about that action not behaving as documented. My first guess is that it is this comment:

https://github.com/dorny/paths-filter/blob/9b7572ffb2cddcbb0397d8a240c253352a845140/README.md?plain=1#L205

    steps:
    # For pull requests it's not necessary to checkout the code
    - uses: dorny/paths-filter@v2

Although that may have been true at the time it was written, I don't think it remains true today. I believe that GHA workflows activated by the pull_request event do need to perform a checkout for subsequent steps to be able to use the content of the repo.

I will begin experimenting with using dorny/paths-filter in a fork, using pull requests, and testing what the behaviour is with/without an actions/checkout step beforehand.

[jayaddison]

I will begin experimenting with using dorny/paths-filter in a fork, using pull requests, and testing what the behaviour is with/without an actions/checkout step beforehand.

The logic expressed in #11555 seems to neatly express what we want, and it uses the dorny/paths-filter action; that's approximately the baseline/GHA workflow definition that I will commence testing from.

[JagjeevanAK]

Glob characters are not working as they are intended to work as given in doc's.

[JagjeevanAK]

Mate #11555 has some logic error's so we reverted it back and then I have solved that problem by creating another PR. you can take look at it.

[jayaddison]

Ok, cool, I've looked at those PRs. Was it the ! (not) globs that were the problem? Files appearing modified even with those added?

[jayaddison]

Again: I should answer my own question! I'll use a fork to check.

[jayaddison]

Ok, I think I have confirmed: the predicate-quantifier setting in dorny/paths-filter does not appear to function as documented. Even after enabling that, a ! blob to ignore some files did not result in the filter having a false value when those files were modified.

For anyone curious, my testing for this was two comparative pull requests at:

• ci: Testing: GitHub Actions / paths-filter action testing with negated blob regexes openculinary/openfoodfacts-server#4
• ci: Testing: GitHub Actions / paths-filter action testing with negated blob regexes openculinary/openfoodfacts-server#5

[jayaddison]

Hold on! We were using dorny/paths-filter@v2!

The predicate-quantifier feature only arrived for dorny/paths-filter@v3: dorny/paths-filter@cf89abd

[jayaddison]

@JagjeevanAK if the problem in #11555 was that the ignore blobs (!...) did not work, then I think that two small fixups could resolve that:

• Upgrade from dorny/paths-action version v2 to v3.
• Configure predicate-quantifier: 'every' on the filter rule -- so that all of the blobs in it have to match.

I've been testing that using the following fork PR, and it was successful:

• ci: Testing: make a trivial docker config documentation-related edit openculinary/openfoodfacts-server#6

However, if the problem was something else -- let me know and I can investigate.