Skip to content

[CI] Enable action-based workflow filter #14537

Open
roigcarlo wants to merge 8 commits into
masterfrom
ci/workflow-filtering-v2
Open

[CI] Enable action-based workflow filter #14537
roigcarlo wants to merge 8 commits into
masterfrom
ci/workflow-filtering-v2

Conversation

@roigcarlo

Copy link
Copy Markdown
Member

📝 Description

This PR tries to enable our workflow filter back.

We formerly had this feature enabled with a ci-dummy.yml which triggered based on file changed rules. As for now both mechanism that allowed that solution no longer work ( 1- acceptance rules based on workflow with same names from different sources and paths-ignore as a filter on pull-request trigger)

Key changes

Added "job-filters" job with a action based filter with files excluded for the builds (can be extended with more filters on demand)
Added conditional trigger for the builds for all jobs based on the exclusion file list.

Validation

I am afraid in order to validate this we will have to fork the repo, merge, and try to trigger different builds based on different conditions.

@philbucher I will take the opportunity to ask: Do we still need the old changed-files job? I see that is used on some tests but it has no real usage insde Kratos. I if I remember correctly this was meant to be the foundation to allow application exclusions from the builds. Chan we achieve the same with those filters maybe? Do you use it for something on your side?

@roigcarlo
roigcarlo requested review from a team, avdg81 and philbucher July 1, 2026 09:04

@avdg81 avdg81 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @roigcarlo,
Thanks a lot for preparing this PR. I'm not very familiar with GitHub Actions, but as far as I can see, your changes seem to make sense. I have one question about a potentially missing line, but again, it could be my lack of understanding GitHub Actions.
I'm also wondering about the best way to test this. One way could be to change a Markdown file on this branch, push it and then check the workflow run to see which steps are actually being carried out. In the same way, we can modify a file that should trigger a build and test job (e.g. modifying a C++ or Python file), and see what happens on GitHub. If you are aware of better ways of testing this, please do that. Thanks.

Comment thread .github/workflows/ci.yml
Comment thread .github/workflows/ci.yml
Comment on lines +44 to +45
with:
filters: |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The example that I've found and mentioned above includes one more line:

Suggested change
with:
filters: |
with:
predicate-quantifier: 'every'
filters: |

Do you think we also need it here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect it does the same as the '**' token, let me test it out.

@avdg81 avdg81 Jul 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that it works like an "and" operator. Only files that match all predicates will be considered. My understanding is based on the following snippet from the documentation:

    # The filters below will match markdown files despite the exclusion syntax UNLESS
    # you specify 'every' as the predicate-quantifier parameter. When you do that,
    # it will only match the .ts files in the subdirectory as expected.
    #
    # backend:
    #  - 'pkg/a/b/c/**'
    #  - '!**/*.jpeg'
    #  - '!**/*.md'
    predicate-quantifier: 'some'

Note that the default value of predicate-quantifier is some, not every.

@philbucher

Copy link
Copy Markdown
Member

@roigcarlo I am not sure whats the purpose of this, could explain it a bit more?

What I proposed but never finished is #11925: Compile only apps that have changes. E.g. if only files of the GeoMech App are changed, then why does the CI need to build and run the tests e.g. in the Fluid or MPM-App?

I kinda forgot about this, but if interested I could give it another go

Comment thread .github/workflows/ci.yml
Co-authored-by: Carlos Roig <roigcarlo@users.noreply.github.com>
@roigcarlo

Copy link
Copy Markdown
Member Author

@avdg81 Github actions are just hooks executed by the CI which perform different actions related with the PR info. For example the checkout one downloads the files from the last commit.

The one I added as you have guessed it just a glorified list with the files that contain diff to which you can apply different operations (filter, expand, etc...).

Testing this PR's changes directly here will be specially difficult because of how github action work. Our repo configuration only enforces running our workflows if the target branch is master. We could make a different PR only modifying a *.md and try to merge it here so it sees the newer version of the ci.yml, but that would not trigger the workflow.

My guess is that the best thing we can do it is to create an empty toy repo, force a dummy workflow and see if the overall idea works. If its the case we can merge this and tweak it afterwards if something is missing. If you agree I can make one after lunch and we can play on it.

@philbucher The purpose of the PR is to avoid compiling Kratos every time someone wants to modify documentation files. This used to be possible because we had a ci-dummy which triggered a workflow that did not include building the source, but at some point that solution broke because of the reasons I say in the description (in short this is an alternative to the old on: pull-request: path:). Following a meeting with @avdg81 we agreed on looking onto alternatives and seems that this is the most widely accepted one (at least the most widely spreader one that I could find)

@roigcarlo

Copy link
Copy Markdown
Member Author

@philbucher as for #11925 as you wish, right now compilations times are "tamed" and there are only a couple of tests (cof cof usual suspects) that take long time to run.

I recently added the possibility to compile the whole code in stages ( currently deployed in the ci) so maybe its easier than ever to finish.

Personally, every time I tried to calculate a propagation tree from a change I failed miserably. But I don't say its impossible 🤷

@avdg81

avdg81 commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

@avdg81 Github actions are just hooks executed by the CI which perform different actions related with the PR info. For example the checkout one downloads the files from the last commit.

The one I added as you have guessed it just a glorified list with the files that contain diff to which you can apply different operations (filter, expand, etc...).

Testing this PR's changes directly here will be specially difficult because of how github action work. Our repo configuration only enforces running our workflows if the target branch is master. We could make a different PR only modifying a *.md and try to merge it here so it sees the newer version of the ci.yml, but that would not trigger the workflow.

My guess is that the best thing we can do it is to create an empty toy repo, force a dummy workflow and see if the overall idea works. If its the case we can merge this and tweak it afterwards if something is missing. If you agree I can make one after lunch and we can play on it.

@philbucher The purpose of the PR is to avoid compiling Kratos every time someone wants to modify documentation files. This used to be possible because we had a ci-dummy which triggered a workflow that did not include building the source, but at some point that solution broke because of the reasons I say in the description (in short this is an alternative to the old on: pull-request: path:). Following a meeting with @avdg81 we agreed on looking onto alternatives and seems that this is the most widely accepted one (at least the most widely spreader one that I could find)

I agree with testing the suggested approach by using a toy repo. 👍

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

Successfully merging this pull request may close these issues.

3 participants