darwin.builder: Fix gratuitous rebuilds#207902
Conversation
See the discussion starting here: NixOS#206951 (comment) The `darwin.builder` derivation had a gratuitous dependency on the current Nixpkgs revision due to `config.system.nixos.revision`. Setting the revision explicitly to null fixes this problem and prevents the derivation from being rebuilt on every change to Nixpkgs.
github-actions
bot
added
6.topic: nixos
ofborg
bot
added
10.rebuild-darwin: 1-10
| system.stateVersion = "22.05"; | ||
| system = { | ||
| # To prevent gratuitous rebuilds on each change to Nixpkgs | ||
| nixos.revision = null; |
There was a problem hiding this comment.
Does it matter if this is null instead of a string?
nixpkgs/pkgs/top-level/release.nix
Line 11 in 70c476b
There was a problem hiding this comment.
Looks like it's totally valid for it to be null, but not sure if we'd prefer that or a dummy string:
nixpkgs/nixos/modules/misc/version.nix
Lines 76 to 81 in 8bb14f1
There was a problem hiding this comment.
According to nix-diff, the only thing that this option affects is the output of the nixos-version --json command. If you set it to null then the output is:
{"nixosVersion":"23.05pre-git"}… whereas if you were to set it to a dummy revision then the output would be:
{"nixosVersion":"23.05pre-git","nixpkgsRevision":"0000000000000000000000000000000000000000"}I personally feel that setting it to null is the cleaner solution rather than generating a sentinel value that is invalid
There was a problem hiding this comment.
null sgtm. It seems to be the better solution, considering the intent of the type. If it breaks something, we'd learn what to fix, which is great.
| # To prevent gratuitous rebuilds on each change to Nixpkgs | ||
| nixos.revision = null; | ||
|
|
||
| stateVersion = "22.05"; |
There was a problem hiding this comment.
This is how the nixos tests set stateVersion, can we do the same here?
There was a problem hiding this comment.
(IMO, this should be a separate commit.)
There was a problem hiding this comment.
This config should be independent of stateVersion. It's within reach
I'll merge the uncontroversial bit, so that it can be made to work with disabledModules instead of the two more or less breaking changes in that PR.
There was a problem hiding this comment.
Here's how to avoid stateVersion
There was a problem hiding this comment.
I can fix the stateVersion, but I feel like that should be a separate PR, which I'm happy to do afterwards
There was a problem hiding this comment.
Also, setting the stateVersion to match the current release is not necessarily the correct behavior here. If you were to do that then any necessary migrations would never get applied if the user updates their nixpkgs pin in order to upgrade the builder. The stateVersion is not supposed to change after the machine is deployed
There was a problem hiding this comment.
I agree, and I don't think a bare NixOS should depend on stateVersion for any such migrations, which is why I've opened #207958 as an alternative.
|
Successfully created backport PR #207979 for |
5 participants
See the discussion starting here:
#206951 (comment)
The
darwin.builderderivation had a gratuitous dependency on the current Nixpkgs revision due toconfig.system.nixos.revision. Setting the revision explicitly to null fixes this problem and prevents the derivation from being rebuilt on every change to Nixpkgs.I verified that this change works by testing that
nix-instantiate --attr darwin.builderproducesthe exact same derivation before and after a trivial commit based on this branch.
Description of changes
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes