workflows/eval: run when base branch changed

[wolfgangwalther]

This is based on #371216, so the biggest chunk of commits is from that PR and should be reviewed / commented on there.

Only the last two commits are relevant for this PR. Here, we first change the way we react to the "edited" event for pull_request_target. This avoids running jobs when only title or description of a PR change. This then allows us to run eval on base branch changes as well, without triggering those on small edits all the time.

Tested the concepts, but not the final PR in that constellation, yet.

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[wolfgangwalther]

Tested the concepts, but not the final PR in that constellation, yet.

Now tested. Forgot to pass through permission for the tag job, fixed.

It works. Here's how:

• When changing the title of a PR, we get a single skipped job:
  
  Because this is triggered via a different workflow file, the successful jobs from before are still visible and are not removed.
• When changing the base branch of the PR, we get additional jobs. Example:
  
  All the jobs that are new for the base branch change are prefixed with "Edited / Base /". In this example we can see that the old job from before are also not replaced, thus the eval from before is still running. We can't prevent them from still showing up, but we can eventually skip the jobs that are still running with something like ci/eval: cancel previous run if new code is pushed #357990 (comment).

Not visible on the second screenshot, yet: We are also running eval again after the base branch change, which is the point of all of this:

All of that happened in wolfgangwalther#632.

[wolfgangwalther]

@infinisil WDYT?

[infinisil]

I reviewed it for now. Clever! Though also rather messy 😅, so I'm a bit undecided whether we should have this.

Advantages:

• Doesn't run CI when PR title/body is changed.
  • Less resource usage
  • No more waiting for CI in such cases.

Disadvantages:

• Temporarily messy and potentially confusing CI results in the rare case of changing the base branch
• Duplicated parts in the workflows
• More complexity

Based on that, and unless I'm missing anything, I don't feel inclined to have this. Instead it would be best for GitHub to address https://github.com/orgs/community/discussions/64119 so that we don't need such a workaround, I've made a comment about that in NixOS/foundation#25 (comment).

[infinisil]

These lines should have some generic comment indicating that it's used via edited-base.yml

[wolfgangwalther]

I reviewed it for now. Clever! Though also rather messy 😅, so I'm a bit undecided whether we should have this.

It's the least messy, at least by output, that I could make it :D, but yes I agree.

• Doesn't run CI when PR title/body is changed.

  • Less resource usage
  • No more waiting for CI in such cases.

This happens quite frequently to me, when I'm merging backport PRs. Building and testing stuff, then going through a final check of the changelog.. yeah all, good set the checkmark "acceptable for release".. ah, damn CI is running again.

I merge in those cases and don't wait for CI, but I'll always get error notifications, because those jobs can't finish when the PR's branch is suddenly gone.

• Duplicated parts in the workflows

• More complexity

My idea was to come back with more of the same. In this case it's technically a "correctness" issue, so I did that first.

But I also think the following is quite annoying:

I have a ton of checks.. and those visible at the top are always a list of "get-merge-commit", because we run this so often.

With the same approach as in edited-base.yml, i.e. calling those other workflows via workflow_call, we could run the merge-commit job once and pass the result to all other workflows. This would give us much less cluttered output.

One more step would then be to collect most of the small "check" jobs into a single job, so it only appears once. I really don't need to see maintainers-sorted, nix-format, nixf-tidy, editorconfig etc. etc. all in separate jobs in the list. They are almost never failing and I could easily look it up which step failed.

My goal: Make the "check list" usable without scrolling. Not everything, but at least have useful information at the top already.

We could probably make this a bit easier to understand in the workflows folder, if we went a step further, made all actual jobs be called by workflow_call only. Then put them into a jobs folder or so, and have only those "trigger workflows" in the workflows folder.

Doing it this way would also give us a nice overview of: One file has all the jobs that will run on a branch push, one file has all the jobs that will run on a PR, ... Potentially easier to reach consistent results with that.

Based on that, and unless I'm missing anything, I don't feel inclined to have this.

Hopefully my vision above changes your mind ;)

Or.. maybe you have other ideas about the issues mentioned?

[philiptaron]

made all actual jobs be called by workflow_call only

I really like this idea. The selection logic being under workflow control seems like the only way to avoid fatigue over there being yet another check that I need to run locally prior to proposing a PR... and is directionally aligned with running CI locally, outside of GitHub, so we can replicate locally what it would do.

[wolfgangwalther]

Marked as draft for now. I will come back to it, but will do other things first.