chore: auto cancel previous branch or PR runs

[acolombier]

This changes automatically cancel the previous CI run on branch and PR. This is useful when quickly pushes after a previous wrong or incomplete one,

[jclsn]

I'm all for this

[ronso0]

Here's a Gemini Pro review, does this make sense?

A Minor Optimization to Consider (The github.ref Gotcha)

The code you provided uses this group logic:

concurrency:
  group: ${{ github.workflow }}-${{ github.ref }}
  cancel-in-progress: true

This will work fine, but there is a slight quirk with how GitHub Actions handles github.ref:

• When a workflow is triggered by a push, github.ref evaluates to something like refs/heads/feature-branch.
• When triggered by a pull_request, github.ref evaluates to a system ref like refs/pull/123/merge.

Because these refs are different, if a contributor pushes a new commit to a branch that currently has a PR open, GitHub Actions actually fires two workflows in two different concurrency groups.

GitHub's widely recommended best practice to group these together seamlessly is to use github.head_ref || github.ref:

concurrency:
  group: ${{ github.workflow }}-${{ github.head_ref || github.ref }}
  cancel-in-progress: true

Why this is better:
github.head_ref is only available during pull_request events and contains the actual branch name (e.g., feature-branch). If it's a standard push event, it falls back to github.ref. This ensures that PR updates and direct branch pushes are grouped together much more cleanly, reducing the chance of edge-case race conditions where a PR build and a push build try to run at the same time.

Combined with your stop-build script, either method will successfully save you CI time, but the github.head_ref addition is the standard way to make it bulletproof.

[acolombier]

We have previously decided that push trigger and pull request trigger are separated and should run independently.
By default, we have already made it so if one has a PR open for a given branch, the CI will not run. This allows contributor that matures the work in branches before submitting a PR to still get the automation to work.

Now, Gemini suggestion wouldn't work anyway because the concurrency group is specific to the repo. In other term, the PR one would be as mixxxdj/mixxx:${{ github.workflow }}-${{ github.ref }}, and the branch push would be ronso0/mixxx:${{ github.workflow }}-${{ github.ref }}, so both wouldn't cancel each other

[ronso0]

Okay, thanks for the explanation (though must admit that I'm not into it to fully understand everything).

[ronso0]

I did a test run in https://github.com/ronso0/mixxx/tree/ci-auto-cancel-test and it works as expected.
Anyone else wants to take a look @mixxxdj/developers ?

If no one objects with 2 days I'll merge this.

[JoergAtGithub]

To my understanding this would also cancel builds at normal pushes and not only for force-pushes.

As it is required that we ensure that all our commits build to allow bisecting later, it is a normal use case to push commit by commit and build all of them. With auto-canceling after each push, devs would have wait for a whole build time cycle, before they can push the next commit.

To not brake this use case, please restrict the auto-cancel to force-push operations.

[acolombier]

To not brake this use case, please restrict the auto-cancel to force-push operations.

I cannot do this, but I could restrict this to pull_request event, so normal push will continue to get atomic build, that is one for each push, as it is today already.

As it is required that we ensure that all our commits build to allow bisecting later

Just for full transparency, note that this is isn't the case today anyway. You only get a build per push, not per commit. This means that contributors that makes multiple commits and then eventually pushes will only get a single build of their N commits, not validating that their branch with N-X commit will still build.

In any case, building every single commit on release branch makes sense (for bisect, as you mentioned), not sure why it makes sense on somebody local dev branch, since arguably, if they pushed more commits, it means previous commits, were flawed/incomplete/broken/...

[JoergAtGithub]

I cannot do this, but I could restrict this to pull_request event, so normal push will continue to get atomic build, that is one for each push, as it is today already.

That would work for me too!

[acolombier]

Pushed a change, let me know if you are happy with the new behaviour.

[jclsn]

To my understanding this would also cancel builds at normal pushes and not only for force-pushes.

As it is required that we ensure that all our commits build to allow bisecting later, it is a normal use case to push commit by commit and build all of them. With auto-canceling after each push, devs would have wait for a whole build time cycle, before they can push the next commit.

To not brake this use case, please restrict the auto-cancel to force-push operations.

Wouldn't this require the user to push only single commits every time? I mean it's hard to guarantee every commit gets built. Ultimately this is the response of the developer. You can only approximate this with automation.

[acolombier]

Wouldn't this require the user to push only single commits every time?

Yep, this is what I meant there with multiple commit push

[acolombier]

Friendly ping.
Could we consider trialling this, and revert if it cause more problems than it solves?

[JoergAtGithub]

It seems that there is now the problem that in case of an open PR both builds are disabled, the push build by "Check if build should be stopped" and the pull-request build by this code. How do I achieve step by step CI builds once I've openend a pull request?

[acolombier]

Just to make sure I go the the problem right, let me reformulate. It looks like there's now an issue where, once a pull request is open, both builds are being skipped:

• The push build is stopped by the "Check if build should be stopped" step.
• The pull request build is skipped.

Do you have a link to the test build you did? You can see the last push didn't seem to suffer from this problem so wondering what went wrong on your test.

[acolombier]

@mixxxdj/developers can we please prioritise this issue? This is really leading to a lot of waste