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

iso: avoid systemd service startup #202

Merged
merged 5 commits into from
Mar 22, 2021
Merged

iso: avoid systemd service startup #202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
db716d1
lib: ensure path type in mkProfileAttrs
Mar 19, 2021
15cf15b
iso: filter out al profiles (except core)
Mar 18, 2021
4358478
style / organization
Mar 18, 2021
0074476
iso: add back complete host profile to the nix store
Mar 18, 2021
49b1ad8
iso: ensure tools of deactivated profiles are still available
Mar 19, 2021
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
23 changes: 19 additions & 4 deletions lib/devos/devosSystem.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,42 @@ lib.nixosSystem (args // {
let
moduleList = builtins.attrValues modules;
modpath = "nixos/modules";
cd = "installer/cd-dvd/installation-cd-minimal-new-kernel.nix";

fullHostConfig = (lib.nixosSystem (args // { modules = moduleList; })).config;

isoConfig = (lib.nixosSystem
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would really love to see the bonus goal of your PR implemented. Is it possible that we could create an isoConfig' that is everything that the current isoConfig defines with storeContents abstracted out into a separate variable, while the new isoConfig would just be something like:

{
  isoConfig = isoConfig' // {  isoImage.storeContents = storeContents ++ [ isoConfig' ];
}

I'm trying to think of a way to do this without triggering infinite recursion, although perhaps I haven't thought this through enough and the above does cause and infinite loop. I'll have more time to test tomorrow and see if I can work it out.

Copy link
Contributor Author

@blaggacao blaggacao Mar 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Within the devshell of a live iso, it is ultimately upstream nixos-install at this line that is hindering us, specifically: nix flake metadata evaluates very greedily. Couldn't we obtain that .url — which seemed to be a store path — more cheaply?

On the other hand, the build itself is already fully cached (auto?trusted=1) courtesy of the fullHostConfig's toplevel. That extends to the system profile, which is loaded locally from the build's output

After that there is no potential network call left. So the culprit must be indeed nix flake metadata...

Copy link
Collaborator

@nrdxp nrdxp Mar 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

If it is only nixos-install that's blocking us (and it has other problems with flakes), perhaps we could just do what's mentioned in that comment, or something similar to work around it. I personally had to do a legacy nix-build via compat dir to install onto my laptop. Something about my configuration triggered the error mentioned in that thread.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We just moved to a new appartment and, for a varying operational reasons, I probably will be unavailable to follow trough on this bonus item any time soon. (No inet, no setup, obly cell-phone, life getting in the way, ...)

Can we carve this out into an issue while moving on with this PR in its current state?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure. I'd like to have a crack at it first though, if you don't mind, as I think it would be pretty straight-forward. If I get it working, I'll pushing a PR to your branch.

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 suspect the --offline option would get us quite some mileage (combined with a small patch to nixos-install / flakes-first reconception of it), unfortunately I havn't been able to test this in time.

Copy link
Collaborator

Choose a reason for hiding this comment

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

seems I don't have as much time as I'd hoped. I'll go ahead and merge this and we will try to work it out later.
bors r+

(args // {
modules = moduleList ++ [
"${nixos}/${modpath}/${cd}"
({ config, ... }: {

"${nixos}/${modpath}/installer/cd-dvd/installation-cd-minimal-new-kernel.nix"

({ config, suites, ... }: {

# avoid unwanted systemd service startups
disabledModules = lib.remove modules.core suites.allProfiles;
blaggacao marked this conversation as resolved.
Show resolved Hide resolved

nix.registry = lib.mapAttrs (n: v: { flake = v; }) inputs;

isoImage.isoBaseName = "nixos-" + config.networking.hostName;
isoImage.contents = [{
source = self;
target = "/devos/";
}];
nix.registry = lib.mapAttrs (n: v: { flake = v; }) inputs;
isoImage.storeContents = [
self.devShell.${config.nixpkgs.system}
# include also closures that are "switched off" by the
# above profile filter on the local config attribute
fullHostConfig.system.build.toplevel
];
# still pull in tools of deactivated profiles
environment.systemPackages = fullHostConfig.environment.systemPackages;

# confilcts with networking.wireless which might be slightly
# more useful on a stick
networking.networkmanager.enable = lib.mkForce false;
# confilcts with networking.wireless
networking.wireless.iwd.enable = lib.mkForce false;

# Set up a link-local boostrap network
# See also: https://github.com/NixOS/nixpkgs/issues/75515#issuecomment-571661659
networking.usePredictableInterfaceNames = lib.mkForce true; # so prefix matching works
Expand Down
2 changes: 1 addition & 1 deletion lib/devos/mkProfileAttrs.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ let mkProfileAttrs =
f = n: _:
lib.optionalAttrs
(lib.pathExists "${dir}/${n}/default.nix")
{ default = "${dir}/${n}"; }
{ default = /. + "${dir}/${n}"; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the motivation here?

Copy link
Member

Choose a reason for hiding this comment

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

for the expression in devosSystem lib.remove modules.core suite.allProfiles. For the matching to work all items in the list must be the same type. And in general its better to pass around modules as paths rather than strings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But also.most importantly for disabledModules to filter.on module.key proper instead of a rleative to.modulePath string.

// mkProfileAttrs "${dir}/${n}";
in
lib.mapAttrs f imports;
Expand Down