nixosImages: add to pkgs #153551
nixosImages: add to pkgs #153551
Conversation
ofborg
bot
added
10.rebuild-darwin: 0
pkgs/top-level/all-packages.nix
Outdated
| @@ -111,6 +111,16 @@ with pkgs; | |||
|
|
|||
| nix-update-script = callPackage ../common-updater/nix-update.nix { }; | |||
|
|
|||
| ### Push NixOS images into the fixed point | |||
There was a problem hiding this comment.
Are they really inside the fixpoint or dangling from it?
They don't use any other value from it than system. pkgsStatic.nixosImages is wrong; so are the cross sets.
Part of the problem of course is the nixpkgs.nix module, which is somewhat mandatory because of its existence in module-list.nix; mkForcing _module.args.pkgs or settings nixpkgs.pkgs is counter to traditional NixOS expectations, but probably ok if you use assertions to make sure no nixpkgs.overlays ornixpkgs.config are set.
If we insert pkgs into the images, which is not a technical problem, we do get a little eval performance improvement, as we evaluate pkgs once for all images instead of over and over for each.
There was a problem hiding this comment.
I can also conditionally add this if hostPlatform.system == targetPlatform.system
There was a problem hiding this comment.
I don't think restricting is necessary, although it does reduce the changes to release.nix to make this work. The conditional attr does make the fixpoint attrset strict in stdenv, so that could slow down exclusive use of pkgsStatic for example (if it isn't already slowed down by that), but that can be fixed by supporting cross invocations later.
First, what you can do is define a module here that you pass as a new optional argument to release.nix, which in turn adds it directly in the list of modules that makeModules returns, and to the other modules = params.
The module then only has to set nixpkgs.pkgs and make sure no nixpkgs.config is set. Overlays are actually already supported by the nixpkgs.nix module.
pkgs/top-level/all-packages.nix
Outdated
| supportedSystems = [ stdenv.hostPlatform.system ]; | ||
| }; | ||
| in lib.mapAttrs (name: value: value.${system}) | ||
| { inherit (release-packages) iso_minimal iso_gnome iso_plasma5 amazonImage; }; |
There was a problem hiding this comment.
These underscores are counter to Nixpkgs conventions.
Does it make sense to use . here? If we use the same config to generate multiple images, it'd make sense to have minimal.iso for example. Otherwise, isoMinimal is fine.
There was a problem hiding this comment.
I think something like isos.minimal would be better. But that should be done in another PR. This is trying to tackle, how to test images from packages.
pkgs/top-level/all-packages.nix
Outdated
| @@ -111,6 +111,16 @@ with pkgs; | |||
|
|
|||
| nix-update-script = callPackage ../common-updater/nix-update.nix { }; | |||
|
|
|||
| ### Push NixOS images into the fixed point | |||
There was a problem hiding this comment.
I can also conditionally add this if hostPlatform.system == targetPlatform.system
| # Conditional logic must use super, as using pkgs will | ||
| # cause infinite recursion when trying to determine a fixed point | ||
|
|
||
| } // lib.optionalAttrs (super.stdenv.isLinux && (super.hostPlatform.system == super.targetPlatform.system)) { |
There was a problem hiding this comment.
| } // lib.optionalAttrs (super.stdenv.isLinux && (super.hostPlatform.system == super.targetPlatform.system)) { | |
| } // lib.optionalAttrs (super.stdenv.isLinux && super.hostPlatform.system == super.targetPlatform.system) { |
Technically unnecessary, just liked how they read.
stale
bot
added
the
2.status: stale
wegank
added
the
2.status: merge conflict
stale
bot
removed
the
2.status: stale
wegank
added
the
2.status: stale
Motivation for this change
Useful for testing purposes, to help avoid future #152505 scenarios
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