| 20 Mar 2022 |
bobvanderlinden | In reply to @janne.hess:helsinki-systems.de (on top of your PR). I wonder if we should use cfg.globalEnvironment here. It sets LOCALE_ARCHIVE and TZDIR * I'd rather see a boot.inird.systemd.globalEnvironment instead. Just to avoid unintended changes to initrd. That said, I'm not sure if it's needed if we can set envvars to specific values boot.initrd.systemd.services.emergency.environment.SOME_VAR = "3";. Are things like LOCALE_ARCHIVE and TZDIR needed (or even usable) in initrd? | 17:00:58 |
@janne.hess:helsinki-systems.de | LOCALE_ARCHIVE and TZDIR are probably more optional. Adding a global option and not populating it by default would be a good way imo. For me, I needed the environment for boot.initrd.systemd.services.emergency.environment.SYSTEMD_SULOGIN_FORCE = "1"; | 17:02:00 |
@janne.hess:helsinki-systems.de | also, you mentioned something about /proc/sys/kernel/modprobe. How did you set it in your attempt? Because it looks like it's not set at all in the PR | 17:02:46 |
@elvishjerricco:matrix.org | In reply to @arianvp:matrix.org This might also be something we want to change. Have the first executable in stage-2 be systemd proper Yep. I mentioned this in the PR. We can either do activation in stage one with nixos-enter, or maybe do something clever with systemd generators | 17:05:26 |
@elvishjerricco:matrix.org | In reply to @bobvanderlinden_:matrix.org I do think it might help to have an executable like make-initramfs-ng/find-dependencies that doesn't copy everything first to a root dir and afterwards package those files up using cpio. A slight optimization, but I think that can be a separate PR with a separate discission. I won't mention it in the PR to avoid nitty-gritty discussions. I think that's a good point. I'd like to make that change but just didn't bother last night. I wouldn't be opposed to a patch to change that in this PR | 17:06:41 |
@elvishjerricco:matrix.org | Janne Heß: I did not realize it was possible to do the serialization thing without switch-root'ing to exactly the systemd binary. Cool, I'll take a look | 17:09:25 |
@elvishjerricco:matrix.org | Janne Heß: Yea we might want a globalEnvironment thing, I just didn't see a need, since initrd is really supposed to just launch and die as quickly as possible | 17:11:33 |
@janne.hess:helsinki-systems.de | yeah people will probably not get mad if it's missing. It's something that can be added later. A working systemd.services.<?>.environment is more useful though ;) | 17:13:02 |
@elvishjerricco:matrix.org | Janne Heß: Is there a reason boot.initrd.systemd.services.emergency.environtment doesn't work on its own though? | 17:13:27 |
@elvishjerricco:matrix.org | Also, for the modprobe thing, it's in the object that gets symlinked to /etc/sysctl.d/nixos.conf | 17:13:45 |
@janne.hess:helsinki-systems.de | In reply to @elvishjerricco:matrix.org Janne Heß: Is there a reason boot.initrd.systemd.services.emergency.environtment doesn't work on its own though? Yeah you are missing:
diff --git a/nixos/lib/systemd-lib.nix b/nixos/lib/systemd-lib.nix
index 3129fbe7bdb..716128a49ac 100644
--- a/nixos/lib/systemd-lib.nix
+++ b/nixos/lib/systemd-lib.nix
@@ -396,6 +396,13 @@ in rec {
text = commonUnitText def +
''
[Service]
+ ${let env = cfg.globalEnvironment // def.environment;
+ in concatMapStrings (n:
+ let s = optionalString (env.${n} != null)
+ "Environment=${builtins.toJSON "${n}=${env.${n}}"}\n";
+ # systemd max line length is now 1MiB
+ # https://github.com/systemd/systemd/commit/e6dde451a51dc5aaa7f4d98d39b8fe735f73d2af
+ in if stringLength s >= 1048576 then throw "The value of the environment variable ‘${n}’ in systemd service ‘${name}.service’ is too long." else s) (attrNames env)}
${attrsToSection def.serviceConfig}
'';
};
| 17:14:04 |
@janne.hess:helsinki-systems.de | (but without the globalEnvironment, just def.environment) | 17:14:12 |
@elvishjerricco:matrix.org | Ohhh, def.environment doesn't just get assigned to serviceConfig.Environment like e.g. path gets set to environment.PATH | 17:15:13 |
@elvishjerricco:matrix.org | Ok, yea that needs a fixing | 17:15:23 |
@janne.hess:helsinki-systems.de | yeah | 17:15:29 |
bobvanderlinden |
I think that's a good point. I'd like to make that change but just didn't bother last night. I wouldn't be opposed to a patch to change that in this PR
ElvishJerricco no worries. I was thinking these kinds of optimizations can also happen after the PR. I'd rather be in a state where the PR is merged and multiple people can create PRs to improve the various parts of initrd-systemd, rather than everyone commenting/attempting to make changes to the PR itself, leaving you to be the central person that needs to do things: a situation I'd like to avoid with the time it takes for some PRs
| 17:16:14 |
@elvishjerricco:matrix.org | bobvanderlinden: Yea, I agree that delaying the PR with endless discussion would be unfortunate. But delaying for actual completed patches to be included is reasonable | 17:17:09 |
@elvishjerricco:matrix.org | So long as they don't escape the scope of "core functionality" | 17:17:33 |
@elvishjerricco:matrix.org | And I think that would be in scope | 17:18:00 |
bobvanderlinden | I wish it was easier to make it clear that this section of nixos is experimental and people should not use it. That would make it easier to merge the PR in its current state. There are tons of ideas and suggestions to come up for the PR regarding its core functionality, but imo it doesn't really help to make you apply all of those suggestions rather than people making small PRs on top to improve things. Things like formatting, naming, file-placement are really popular to comment on, but do not contribute that much to moving forward imo. | 17:22:53 |
@janne.hess:helsinki-systems.de | Something that I really think would contribute to the initial PR would be a simple nixos test that just boots a simple VM without anything fancy. I'm currently working on that and struggling with ext4 atm :/ For some reason the kernel and mount hate me | 17:24:15 |
@elvishjerricco:matrix.org | I should probably hide these options from the nixos manual for the time being (I forget how to do that; something about visible or internal?) | 17:24:24 |
bobvanderlinden | Janne Heß: I'm working on a test ;D | 17:24:38 |
@janne.hess:helsinki-systems.de | In reply to @bobvanderlinden_:matrix.org Janne Heß: I'm working on a test ;D oh that's great. Have you made it load ext4? | 17:24:56 |
@elvishjerricco:matrix.org | Janne Heß: Well I think an issue there is the lack of auto formatting for the rootfs. Don't NixOS tests rely on stage 1 to format a disk image for / | 17:25:06 |
@elvishjerricco:matrix.org | * Janne Heß: Well I think an issue there is the lack of auto formatting for the rootfs. Don't NixOS tests rely on stage 1 to format a disk image for /? | 17:25:09 |
@elvishjerricco:matrix.org | ext4 works for me | 17:25:19 |
bobvanderlinden | lol, no that's what I'm struggling with. I think the test hasn't yet formatted /dev/vda, so that's why it is failing to mount with auto fsType | 17:25:30 |
@janne.hess:helsinki-systems.de | ohh so the tests don't work as I thought :// | 17:25:55 |
@elvishjerricco:matrix.org | Yea I've been testing with a pre-formatted nixos.qcow2. We need the x-systemd.makefs and x-systemd.growfs fstab options so systemd will just do the formatting for us | 17:26:25 |