| 28 Jan 2023 |
@elvishjerricco:matrix.org | should be pretty easy really | 23:48:58 |
@lily:lily.flowers | In reply to @elvishjerricco:matrix.org I'm basically talking about making the cmdline stuff use literally the existing code path as the fstab stuff, but then move the cmdline specific stuff into if (ininitrd() && path_equals(p, "/sysroot")) Oh you know, that would actually kinda work wouldn't it. And not be horrible to do | 23:49:06 |
@lily:lily.flowers | In reply to @lily:lily.flowers I did mean invasive just within that file, but it is admittedly pretty simple compared to the entirety of systemd. I just couldn't imagine refactoring anything in systemd myself, since I have much too low confidence to make significant changes to something so critical (I was scared to even adjust the test runner to support fstab tests in https://github.com/systemd/systemd/pull/26038) | 23:50:05 |
@lily:lily.flowers | In reply to @elvishjerricco:matrix.org I'm basically talking about making the cmdline stuff use literally the existing code path as the fstab stuff, but then move the cmdline specific stuff into if (ininitrd() && path_equals(p, "/sysroot")) * Oh you know, that would actually kinda work, wouldn't it. And not be horrible to do | 23:50:36 |
@elvishjerricco:matrix.org | Does systemd even have testing for initrd stuff? I actually don't know lol | 23:51:07 |
@lily:lily.flowers | Yes | 23:51:11 |
@lily:lily.flowers | Well so they have both unit tests, which is what I adjusted in the above PR, but they also have the mkosi test where they build and boot a whole system. But that's really the best they can do for that I suppose | 23:51:54 |
@lily:lily.flowers | (and they use SYSTEMD_IN_INITRD to override detection and control behavior in unit tests) | 23:52:27 |
@lily:lily.flowers | * (and they use SYSTEMD_IN_INITRD env var to override detection and control behavior in unit tests) | 23:52:34 |
@lily:lily.flowers | (I gave up trying to run said tests on NixOS. They made way too many assumptions. I just have a Fedora VM I use) | 23:53:12 |
@elvishjerricco:matrix.org | All my testing when working on systemd is with NixOS tests :P | 23:53:35 |
@elvishjerricco:matrix.org | If they ask me to write real tests I will | 23:53:47 |
@lily:lily.flowers | I have been making nixos tests too. But I mean for running their unit test suite | 23:53:50 |
@lily:lily.flowers | (since I really didn't want to end up with a regression in that when I touched it and added a test) | 23:54:08 |
@elvishjerricco:matrix.org | yea I just... don't wanna :P | 23:54:06 |
@elvishjerricco:matrix.org | that's fair | 23:54:20 |
| 29 Jan 2023 |
@elvishjerricco:matrix.org | In reply to @elvishjerricco:matrix.org I'm basically talking about making the cmdline stuff use literally the existing code path as the fstab stuff, but then move the cmdline specific stuff into if (ininitrd() && path_equals(p, "/sysroot")) Lily Foster: So would your PR be subsumed by this? Aside from the nice work you did on the test stuff of course | 00:05:10 |
@lily:lily.flowers | In reply to @elvishjerricco:matrix.org Lily Foster: So would your PR be subsumed by this? Aside from the nice work you did on the test stuff of course Yes | 00:15:03 |
@lily:lily.flowers | And I'm 100% happy to let it be subsumed | 00:15:17 |
@lily:lily.flowers | That was mostly a quick hack anyway since I wanted it to work and regardless if I wasn't the one to make a whole refactoring PR, that one would ostensibly be stuck in review much longer | 00:16:07 |
@lily:lily.flowers | * That was mostly a quick hack anyway since I wanted it to work and regardless if I wasn't the one to make a whole refactoring PR, such a PR would ostensibly be stuck in review much longer | 00:16:22 |
@elvishjerricco:matrix.org | As for the hibernate issue, how to do we short-term-fix this in the meantime? | 00:16:32 |
@elvishjerricco:matrix.org | I guess we can add an override on systemd-hibernate-resume@.service that says Before=sysroot.mount? | 00:16:56 |
@elvishjerricco:matrix.org | oh but yea I meant to say I'll probably try making those two systemd PRs tonight/tomorrow. 1) cmdline <-> fstab, 2) local-fs-pre ordering | 00:17:50 |
@lily:lily.flowers | In reply to @elvishjerricco:matrix.org I guess we can add an override on systemd-hibernate-resume@.service that says Before=sysroot.mount? It needs to be before all mounts though, and I'm not sure that would be guaranteed in all cases with Before=sysroot.mount. If you'll have a PR ready with the local-fs-pre fix, I'd be more inclined to fetchpatch it (assuming it's either a simple enough diff or has received positive feedback from the systemd folks) since that's much simpler and also easy to remember to remove later | 02:43:49 |
@elvishjerricco:matrix.org | Lily Foster: well the issue with a patch is that it has to go through staging. It's not exactly hard to remove, and there are no mounts that come before sysroot other than those manually configured by people who know what they're doing | 02:45:33 |
@elvishjerricco:matrix.org | * Lily Foster: well the issue with a patch is that it has to go through staging. The Before ordering is not exactly hard to remove, and there are no mounts that come before sysroot other than those manually configured by people who know what they're doing | 02:46:07 |
@lily:lily.flowers | Ah, true I didn't think about staging. I'd say that override is fine enough then if it solves that one person's problem | 02:46:39 |
@lily:lily.flowers | It's better than the current status of things I suppose | 02:46:55 |
@lily:lily.flowers | I already do similar overrides in my personal config to fix some ordering issues anyway, so it wouldn't cause any problem to add that afaik https://github.com/lilyinstarlight/foosteros/blob/da1db4765c6326537ba8f6aefc30870ff3bda15c/config/base.nix#L112-L126 | 02:48:42 |