Skip to content

.github/workflows/nix-parse-v2: also check parseability with Nix 2.3#398122

Closed
sternenseemann wants to merge 1 commit intoNixOS:masterfrom
sternenseemann:actions-parse-2.3
Closed

.github/workflows/nix-parse-v2: also check parseability with Nix 2.3#398122
sternenseemann wants to merge 1 commit intoNixOS:masterfrom
sternenseemann:actions-parse-2.3

Conversation

@sternenseemann
Copy link
Copy Markdown
Member

90% of eval issues affecting Nix < 2.4 (which is supported as per lib/minver.nix) are actually already detected at parse time since they involve use of path interpolation / path antiquotations added in Nix 2.4 (https://nix.dev/manual/nix/2.28/release-notes/rl-2.4).

This means that we can cheaply (compared to a full eval) prevent a lot of eval issues with the minimum supported version of Nix from ever reaching channels.

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: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions 6.topic: policy discussion Discuss policies to work in and around Nixpkgs backport release-24.11 labels Apr 12, 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 Apr 12, 2025
@sternenseemann sternenseemann force-pushed the actions-parse-2.3 branch 2 times, most recently from d7104d9 to 3132149 Compare April 12, 2025 12:11
90% of eval issues affecting Nix < 2.4 (which is supported as per
lib/minver.nix) are actually already detected at parse time since they
involve use of path interpolation / path antiquotations added in Nix
2.4 (https://nix.dev/manual/nix/2.28/release-notes/rl-2.4).

This means that we can cheaply (compared to a full eval) prevent a lot
of eval issues with the minimum supported version of Nix from ever
reaching channels.
@sternenseemann sternenseemann force-pushed the actions-parse-2.3 branch 2 times, most recently from d93e2a9 to 1b89848 Compare April 12, 2025 12:19
@sternenseemann
Copy link
Copy Markdown
Member Author

@sternenseemann sternenseemann marked this pull request as ready for review April 12, 2025 12:20
Copy link
Copy Markdown
Contributor

@wolfgangwalther wolfgangwalther left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See https://github.com/sternenseemann/nixpkgs/actions/runs/14419515028?pr=2 for an example run.

Since this only runs on changed files, but the test PR didn't have any, I think this only tested the install.

Are we sure that currently all files on master parse with nix 2.3, so that CI doesn't start failing for unrelated changes?

@sternenseemann
Copy link
Copy Markdown
Member Author

Are we sure that currently all files on master parse with nix 2.3, so that CI doesn't start failing for unrelated changes?

since #398119 yes.

@sternenseemann
Copy link
Copy Markdown
Member Author

Since this only runs on changed files, but the test PR didn't have any, I think this only tested the install.

It had one changed file at the time of the run.

@SuperSandro2000
Copy link
Copy Markdown
Member

Can we easily give feedback to contributers why eval is failing in the CI but not for them locally?

@sternenseemann
Copy link
Copy Markdown
Member Author

Can we easily give feedback to contributers why eval is failing in the CI but not for them locally?

I think it's good enough, i.e. it'll tell users about the Nix version that is failing. In any case a slightly confusing error message is better than silently breaking channels. It's unnecessary imo to block this on investing a ton of time into making this a little clearer by parsing the error output or whatever. (As a side note, if someone is looking for a “fun” project, they could try adding github actions annotations so that eval errors from CI show up in the diff viewer in the PR view.)

@Mic92
Copy link
Copy Markdown
Member

Mic92 commented Apr 13, 2025

Feels a bit uneasy to have a Nix version that not longer receives security updates running in

on:
  pull_request_target:

@wolfgangwalther
Copy link
Copy Markdown
Contributor

Feels a bit uneasy to have a Nix version that not longer receives security updates running in

Agreed.

We could maybe rewrite the job in a way that uses latest nix on the outside, and then runs nix 2.3 in the nix sandbox itself. This would also be a tiny step towards a future, where I could just do something like nix-build ci locally and have all the checks run there.

@wolfgangwalther
Copy link
Copy Markdown
Contributor

Superseded by #404466, where I implemented the parse check inside the nix sandbox, thus allowing to run nix 2.3 (and lix) securely.

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 Discuss policies to work in and around Nixpkgs 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.

4 participants