workflows/eval: evaluate ci/pinned.json changes at their commit by dyegoaurelio · Pull Request #480395 · NixOS/nixpkgs

dyegoaurelio · 2026-01-15T15:25:23Z

When testVersions is enabled, the version matrix check now evaluates the specific commit that modified ci/pinned.json, rather than the PR head. This allows formatting changes and other modifications to be in the same PR without affecting the hash consistency ratchet check.

The workflow now:

Finds the single commit that touched ci/pinned.json
Errors if multiple commits modify it
Checks out that commit for versioned evaluations
Reports which commit was evaluated in the summary

Things done

Add a 👍 reaction to pull requests you find important.

MattSturgeon

SGTM overall. Minor thoughts:

philiptaron · 2026-01-15T18:03:56Z

Can you give us a sense of the performance differential, @dyegoaurelio and @MattSturgeon?

dyegoaurelio · 2026-01-15T18:08:16Z

edit:
https://github.com/NixOS/nixpkgs/actions/runs/21041282212/job/60503570981?pr=480436

I didn't realize how slow Check out the PR with full history step would be 😅

@philiptaron I don't expect much performance changes.

The goal of this PR is to simplify ci/pinned.json bumps, such as #480141

MattSturgeon · 2026-01-15T18:10:47Z

Can you give us a sense of the performance differential, @dyegoaurelio and @MattSturgeon?

The versions job only runs on PRs that touch ci/pinned.json, so performance isn't too critical. Normal PRs are not affected at all.

I'm not sure what metrics you're interested in. You can see it running "for real" in #480436

The checkout will be slower as it fetches history (I assume there's no way to fetch only history since the base commit?).

IIUC, the actual eval report should have the same perf cost as before; it does one eval per system per nix version, in addition to the usual "compare" eval done on all PRs.

philiptaron · 2026-01-15T18:16:43Z

OK, my "run" of the GHA in my head thought it changed every one. Thanks for the clarification.

MattSturgeon · 2026-01-15T18:42:34Z

(I assume there's no way to fetch only history since the base commit?).

Looking at https://github.com/NixOS/nixpkgs/actions/runs/21041282212/job/60503570981?pr=480436#step:3:416 (which timed out while fetching all of Nixpkgs)¹, this may be worth doing.

Caveat: GitHub seems to be struggling a bit atm anyway. ↩

MattSturgeon

Diff LGTM overall. A couple questions and minor nits, but I think we've got this to a good functional state over the last few iterations!

Testing in https://github.com/NixOS/nixpkgs/actions/runs/21102528097?pr=480436#summary-60689633319 seems to show everything working correctly.

@philiptaron it'd be great to have your thoughts on the diff too, especially as I'm a co-author and don't want to sign this off entirely on my own.

MattSturgeon · 2026-01-18T22:01:56Z

+            console.log('Applying untrusted ci/pinned.json bump:', pin_bump_sha)
+            try {
+              await run('git', '-C', join('nixpkgs', 'untrusted'), 'cherry-pick', '--no-commit', pin_bump_sha)
+            } catch {


Note: For simplicity I chose to catch any error. If we know the exit code for "cannot apply" we could check it if preferred.

MattSturgeon · 2026-01-18T22:05:16Z

+            // Compare the two commits to get the list of commits in between
+            const comparison = await github.rest.repos.compareCommitsWithBasehead({
+              ...context.repo,
+              basehead: `${targetSha}...${headSha}`,


I'm a little rusty on my .. vs ... and also unsure whether this API supports ^ syntax (or if that even works for the 2nd parent of mergedSha)

I'm wondering if we can avoid needing to know headSha by using something like target..merged or target...merged^?

philiptaron

Security review complete. This PR properly isolates ci/pinned.json bump commits for versioned evaluations through cherry-picking onto the target branch.

Security aspects reviewed:

Trust boundary maintained: cherry-pick only modifies nixpkgs/untrusted, preserving the trusted/untrusted separation
Commit isolation prevents unrelated PR changes from affecting version matrix evaluation
Multi-commit detection fails fast if >1 commit touches ci/pinned.json
Cherry-pick failure handling calls core.setFailed() with clear error messaging
No command injection, path traversal, or secret exposure risks
Proper permissions: {} restrictions maintained

Minor notes:

Typo on line 85 of action.yml: "Fetching out" → "Fetching"
Rate limit logging before/after the commit API calls wasn't implemented per the review discussion, but the 50-commit cap provides reasonable protection

LGTM from a security perspective.

nixos-discourse · 2026-01-20T20:16:03Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/formatting-team-meeting-2026-01-20/74520/1

When testVersions is enabled, the version matrix check now identifies the specific commit that modified ci/pinned.json, rather than evaluating the full PR. This allows formatting changes and other modifications to coexist in the same PR without affecting the hash consistency check. The workflow now: - Adds headSha input to compare PR commits against target - Finds the single commit that touched ci/pinned.json - Errors if multiple commits modify it (to ensure clear attribution) - Passes the bump commit to the checkout action for isolated evaluation - Reports which commit was evaluated in the summary The actual cherry-pick and pinned checkout handling is delegated to the checkout action via the new `untrusted-pin-bump` input. Co-Authored-By: Matt Sturgeon <matt@sturgeon.me.uk>

nixpkgs-ci · 2026-01-20T21:46:48Z

Successfully created backport PR for release-25.11:

[Backport release-25.11] workflows/eval: evaluate ci/pinned.json changes at their commit #482087

dyegoaurelio requested a review from MattSturgeon January 15, 2026 15:25

dyegoaurelio mentioned this pull request Jan 15, 2026

ci/pinned:update #480141

Merged

13 tasks

nixpkgs-ci Bot requested review from Mic92, philiptaron and zowoq January 15, 2026 15:32

dyegoaurelio force-pushed the ci-eval-workflow branch from 406bf44 to 6fb2107 Compare January 15, 2026 15:36

MattSturgeon reviewed Jan 15, 2026

View reviewed changes

Comment thread .github/workflows/eval.yml Outdated

Comment thread .github/workflows/eval.yml Outdated

dyegoaurelio force-pushed the ci-eval-workflow branch from 6fb2107 to 5ad5407 Compare January 15, 2026 17:44

dyegoaurelio requested a review from MattSturgeon January 15, 2026 17:44

dyegoaurelio force-pushed the ci-eval-workflow branch from 5ad5407 to 0f119a9 Compare January 15, 2026 17:48

MattSturgeon mentioned this pull request Jan 15, 2026

[TEST] workflows/eval: evaluate ci/pinned.json changes at their commit #480436

Closed

dyegoaurelio force-pushed the ci-eval-workflow branch 2 times, most recently from 24c7917 to 3686fd6 Compare January 15, 2026 18:25

This comment was marked as resolved.

Sign in to view

dyegoaurelio force-pushed the ci-eval-workflow branch from 3686fd6 to 2cf3078 Compare January 15, 2026 18:45

MattSturgeon reviewed Jan 15, 2026

View reviewed changes

Comment thread .github/workflows/eval.yml

MattSturgeon reviewed Jan 15, 2026

View reviewed changes

Comment thread .github/workflows/eval.yml Outdated

MattSturgeon reviewed Jan 15, 2026

View reviewed changes

Comment thread .github/workflows/eval.yml Outdated

dyegoaurelio force-pushed the ci-eval-workflow branch from 2cf3078 to eb2db1e Compare January 15, 2026 20:11

MattSturgeon reviewed Jan 15, 2026

View reviewed changes

Comment thread .github/workflows/eval.yml

dyegoaurelio force-pushed the ci-eval-workflow branch 2 times, most recently from 66b34ba to 2ff0858 Compare January 18, 2026 13:58

MattSturgeon reviewed Jan 18, 2026

View reviewed changes

MattSturgeon requested a review from mdaniels5757 January 20, 2026 08:26

philiptaron approved these changes Jan 20, 2026

View reviewed changes

nixpkgs-ci Bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jan 20, 2026

dyegoaurelio force-pushed the ci-eval-workflow branch 3 times, most recently from ed5713a to db63d2f Compare January 20, 2026 16:57

dyegoaurelio requested a review from MattSturgeon January 20, 2026 17:00

MattSturgeon approved these changes Jan 20, 2026

View reviewed changes

dyegoaurelio force-pushed the ci-eval-workflow branch from db63d2f to 7533fff Compare January 20, 2026 19:28

nixpkgs-ci Bot added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Jan 20, 2026

dyegoaurelio force-pushed the ci-eval-workflow branch from 7533fff to 31ae7e6 Compare January 20, 2026 19:33

actions/checkout: handle ci/pinned.json bump commits

4787f35

dyegoaurelio force-pushed the ci-eval-workflow branch from 31ae7e6 to 2a754fe Compare January 20, 2026 20:14

dyegoaurelio force-pushed the ci-eval-workflow branch from 2a754fe to 8c4f3d3 Compare January 20, 2026 21:37

MattSturgeon enabled auto-merge January 20, 2026 21:40

MattSturgeon added this pull request to the merge queue Jan 20, 2026

Merged via the queue into NixOS:master with commit 2991e1e Jan 20, 2026
62 checks passed

nixpkgs-ci Bot mentioned this pull request Jan 20, 2026

[Backport release-25.11] workflows/eval: evaluate ci/pinned.json changes at their commit #482087

Merged

github-actions Bot added the 8.has: port to stable This PR already has a backport to the stable release. label Jan 20, 2026

dyegoaurelio deleted the ci-eval-workflow branch January 20, 2026 22:06

MattSturgeon mentioned this pull request Jan 21, 2026

actions/checkout: manually fetch ci/pinned.json patch #482470

Merged

2 tasks

Uh oh!

Uh oh!

Conversation

dyegoaurelio commented Jan 15, 2026

Things done

Uh oh!

MattSturgeon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

philiptaron commented Jan 15, 2026

Uh oh!

dyegoaurelio commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MattSturgeon commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

philiptaron commented Jan 15, 2026

Uh oh!

This comment was marked as resolved.

MattSturgeon commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Footnotes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MattSturgeon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

MattSturgeon Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

MattSturgeon Jan 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

philiptaron left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nixos-discourse commented Jan 20, 2026

Uh oh!

Uh oh!

nixpkgs-ci Bot commented Jan 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dyegoaurelio commented Jan 15, 2026 •

edited

Loading

MattSturgeon commented Jan 15, 2026 •

edited

Loading

MattSturgeon commented Jan 15, 2026 •

edited

Loading

MattSturgeon Jan 18, 2026 •

edited

Loading

philiptaron left a comment •

edited

Loading