Skip to content

ci/eval: allow local comparison with rebuilds#437934

Merged
philiptaron merged 2 commits intoNixOS:masterfrom
wolfgangwalther:ci-local-eval
Aug 28, 2025
Merged

ci/eval: allow local comparison with rebuilds#437934
philiptaron merged 2 commits intoNixOS:masterfrom
wolfgangwalther:ci-local-eval

Conversation

@wolfgangwalther
Copy link
Contributor

@wolfgangwalther wolfgangwalther commented Aug 28, 2025

This allows running a full comparison between two commits locally.

What was previously eval.full is now called eval.all. The new eval.full takes a baseline argument for the comparison.

Fixes #437697 (comment)

@philiptaron could you give this a test-run? I only did some basic testing with "baseline == HEAD", but I think this should work with your example as well.

Things done


Add a 👍 reaction to pull requests you find important.

@nixpkgs-ci nixpkgs-ci 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. 6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions backport release-25.05 labels Aug 28, 2025
@wolfgangwalther
Copy link
Contributor Author

Ah, mh, something is not working properly, yet. the before/ directory in the combined-eval result is still empty.

@wolfgangwalther
Copy link
Contributor Author

Ah, it's that readlink stuff again. So currently, you'd need to do this:

nix-build ci -A eval.full --arg baseline $(readlink ./baseline)

on the second step. Maybe we should fix this once and for all - but I have no idea where the ./baseline symlink is not followed properly...

@wolfgangwalther
Copy link
Contributor Author

The problem is that, ./baseline is a symlink. And when we pass that into eval, it will be transformed into a path via builtins.path, which copies the symlink to the store. But a symlink in the store will only help when the target of that symlink is also made available to the builder that tries to access it. But that's not the case, thus this will fail.

When using readlink, the link will be resolved before calling nix, nix will identify the store path passed and correctly make that store path available inside the builder.

I don't think we can properly solve this here :/

However, I changed the README slightly. It now has these two commands:

BASELINE=$(nix-build ci -A eval.all --no-out-link)

nix-build ci -A eval.full --arg baseline $BASELINE

This works because it doesn't use another symlink.

@philiptaron
Copy link
Contributor

This works because it doesn't use another symlink.

That's precisely the fix I was going to suggest. Thanks Wolfgang.

Copy link
Contributor

@philiptaron philiptaron left a comment

Choose a reason for hiding this comment

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

Approved with some comments and gripes.

Copy link
Contributor

Choose a reason for hiding this comment

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

There are some non-README.md docs in this ci/eval/compare/default.nix file that are also relevant.

@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Aug 28, 2025
We had set a default of 5000 for local evaluation earlier for
`singleSystem`, it makes sense to also use that for `full`.

The README is also a bit outdated, because Nix 2.30 significantly
changed the memory requirements. Rewriting the README to also show the
ability to directly evaluate the current system only.
This allows running a full comparison between two commits locally.

What was previously `eval.full` is now called `eval.all`. The new
`eval.full` takes a `baseline` argument for the comparison.
@philiptaron philiptaron merged commit e393b89 into NixOS:master Aug 28, 2025
27 of 31 checks passed
@nixpkgs-ci
Copy link
Contributor

nixpkgs-ci bot commented Aug 28, 2025

Successfully created backport PR for release-25.05:

@github-actions github-actions bot added the 8.has: port to stable This PR already has a backport to the stable release. label Aug 28, 2025
@wolfgangwalther wolfgangwalther deleted the ci-local-eval branch August 28, 2025 17:11
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 8.has: port to stable This PR already has a backport to the stable release. 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. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants