Only cancel in progress workflow runs for pull requests

[mcbarton]

Description

Please include a summary of changes, motivation and context for this PR.

Fixes # (issue)

Fixes #597

Type of change

Please tick all options which are relevant.

• Bug fix
• New feature
• Requires documentation updates

Testing

Please describe the test(s) that you added and ran to verify your changes.

Checklist

• I have read the contribution guide recently

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.69%. Comparing base (130dd30) to head (a060092).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #634   +/-   ##
=======================================
  Coverage   78.69%   78.69%           
=======================================
  Files           9        9           
  Lines        3835     3835           
=======================================
  Hits         3018     3018           
  Misses        817      817           

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[vgvassilev]

LGTM!

[Vipul-Cariappa]

Context:

@mcbarton said:

Once I take this in #634 , I highly advised that no PR be merged into CppInterOp, without squashing the commits into one. Failure to do so, will likely cause all other work across the Github organisation to grind to a complete halt. This is because each commit will need all workflow runs to complete on main. To complete all jobs for one commit takes quite a while for CppInterOp, so multiple commits will use up all available Github runners for several hours. Also if the commits are dependent on each other to build CppInterOp, or pass the workflow, then the workflow runs for specific commits could fail on main, wasting Github minutes (although we effectively have infinite minutes for Github runners as an open source project, eventually they start throttling your usage). I will merge it tomorrow to give people time to read this message.

@Vipul-Cariappa said:

Is this temporary?
If yes, can we fix the PR to not run the CI for each commit, then merge it?
If not, WE SHOULD NOT MERGE THIS.

I mostly use squash and merge, but there are valid situations where we want to present the commit history.

[Vipul-Cariappa]

Why do we want this? I don't know the pros of this PR. But the cons are severe.

[mcbarton]

@Vipul-Cariappa You will need to make your case to @vgvassilev if you do not have this in. He opened up it up as a bug (see #597). I provided a fix, but gave a warning about the consequences.

It is not possible to merge all commits of a PR, but only run the workflow for the most recent one. Currently if you merge 2 PR into main, before the workflow has finished running for the first one, then the workflow run will get cancelled for the first PR. That is all I will say on the matter.

[Vipul-Cariappa]

Ok, I understand the purpose.
I will let @vgvassilev take the call.

Points to note: (let me know if I am wrong)

• We can still perform a rebase and merge, but it will end up running the CI for all the commits.
• We always merge after the CI is green. CI merges the source branch against main before running the tests. If there are merge conflicts, CI is not run until they are resolved. So, do we still want this change?

[vgvassilev]

The way I understand this feature is that it allows us to cancel the current pr build when a new push to the same pr was done but it won’t cancel builds after something was merged. I did something similar in clad.

[vgvassilev]

It probably needs to be done like this:

https://github.com/vgvassilev/clad/blob/c15f37a873b1e21b341fb164d804782dc7996e84/.github/workflows/ci.yml#L13

[mcbarton]

@vgvassilev The way the feature works, is that if two workflows share the same group, when cancel-in-progress is set to true, then it will cancel the older workflow run.

The bug that this PR fixes exists in clad too. e.g.
The workflow run for this commit that was merged into main https://github.com/vgvassilev/clad/actions/runs/15684475337/job/44184036519 was cancelled because the commit was this workflow run was merged in https://github.com/vgvassilev/clad/actions/runs/15684717247/job/44184903750 .

[vgvassilev]

@vgvassilev The way the feature works, is that if two workflows share the same group, when cancel-in-progress is set to true, then it will cancel the older workflow run.

The bug that this PR fixes exists in clad too. e.g. The workflow run for this commit that was merged into main https://github.com/vgvassilev/clad/actions/runs/15684475337/job/44184036519 was cancelled because the commit was this workflow run was merged in https://github.com/vgvassilev/clad/actions/runs/15684717247/job/44184903750 .

What’s the fix for this. How does that work in root?

[mcbarton]

Looking at root, they don't seem to have the bug, and the workflow run is only for the latest commit of PR when its merged in. This is what they have defined for the group https://github.com/root-project/root/blob/2a6eaa17d8cdb82ea548d1c45deedf2759260b90/.github/workflows/root-ci.yml#L77 . Can't say I understand how that fixes the bug, but we can try it out. Before I make this change to the PR, I will take a closer look at roots ci, to check nothing else is at play to make this happen.