Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

workflows/eval: run when base branch changed #372475

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

wolfgangwalther
Copy link
Contributor

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:
  • 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.

@github-actions github-actions bot added 6.topic: policy discussion 6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions labels Jan 9, 2025
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Jan 9, 2025
We intend to use the edited event to react to base branch changes - but
before this change, we also ran those jobs on simple edits like title or
description.

While this works for some of the quicker jobs, it will not be
sustainable for all evaluation-related jobs. But evaluation needs to be
re-triggered on a base branch change as well, thus this change.
@wolfgangwalther wolfgangwalther force-pushed the ci-pull-request-target-edited branch from b256c34 to 6c24f43 Compare January 11, 2025 11:31
When the base branch changes, we need to run full eval again, because
the changed output paths depend on the base branch.
@wolfgangwalther wolfgangwalther force-pushed the ci-pull-request-target-edited branch from 6c24f43 to 941a690 Compare January 11, 2025 21:01
@wolfgangwalther
Copy link
Contributor Author

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:
    20250111-215242-061778333
    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:
    20250111-215609-456517845
    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:
20250111-215947-822711236

All of that happened in wolfgangwalther#632.

@wolfgangwalther
Copy link
Contributor Author

@infinisil WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions 6.topic: policy discussion 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant