Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nixosImages: add to pkgs #153551

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

nixosImages: add to pkgs #153551

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
97a79a4
nixosImages: add to pkgs
jonringer Jan 5, 2022
fa50987
nixosImages: conditionally add to pkgs
jonringer Jan 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions pkgs/top-level/all-packages.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,16 @@ with pkgs;

nix-update-script = callPackage ../common-updater/nix-update.nix { };

### Push NixOS images into the fixed point
Copy link
Member

@roberth roberth Jan 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

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 or nixpkgs.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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can also conditionally add this if hostPlatform.system == targetPlatform.system

Copy link
Member

Choose a reason for hiding this comment

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

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.


nixosImages = let
system = stdenv.hostPlatform.system;
release-packages = (import ../../nixos/release.nix) {
supportedSystems = [ stdenv.hostPlatform.system ];
};
in lib.mapAttrs (name: value: value.${system})
{ inherit (release-packages) iso_minimal iso_gnome iso_plasma5 amazonImage; };
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.


### Push NixOS tests inside the fixed point

nixosTests = import ../../nixos/tests/all-tests.nix {
Expand Down