NixOS / ofborg

@ofborg tooling automation https://monitoring.ofborg.org/dashboard/db/ofborg
https://ofborg.org
MIT License
246 stars 164 forks source link

Evaluate with aliases enabled additionally #693

Open sternenseemann opened 3 weeks ago

sternenseemann commented 3 weeks ago

https://github.com/NixOS/nixpkgs/pull/349431/ https://github.com/NixOS/nixpkgs/pull/349783 show that changes to aliases can break evaluation of nixpkgs with aliases enabled while not affecting it with aliases disabled. Almost all downstream users will probably have aliases enabled, so this is a situation we should ideally check for. Saving grace is probably that most Hydra jobsets have aliases enabled.

emilazy commented 3 weeks ago

If anyone implements this please at least give consideration to doing it in a way that lets changes like https://github.com/NixOS/nixpkgs/pull/342112 be able to produce warnings without breaking CI 🥲

SuperSandro2000 commented 3 weeks ago

Are there any other breakages known other than the unmaintained node2nix project?

lilyinstarlight commented 3 weeks ago

Are there any other breakages known other than the unmaintained node2nix project?

Well given ofborg does already evaluate with aliases enabled as an additional step, no

The ofborg code could be made to have one fewer gotcha (that may have very very indirectly caught this, but I'd have to test and it would have significantly increased eval check time) by forcing outpath eval, but nodePackages being dontRecurseIntoAttrs and therefore expected to have busted eval in general is a nixpkgs expectation/problem

SuperSandro2000 commented 3 weeks ago

it would have significantly increased eval check time

That is my main concern.

but nodePackages being dontRecurseIntoAttrs and therefore expected to have busted eval in general is a nixpkgs expectation/problem

That's because it is to big and has to many irrelevant things because well, it is auto generated. By moving to buildNpmPackage and friends we dodge that issue finally.