@lheckemann do you think this is a sane approach generally? (not asking you to review the code just yet, bash can cause quite a headache)

Side remark: I would have much preferred to write this is a proper language (vs bash). It would be quite nice to settle on a language for nontrivial builders in nixpkgs..

👍 on the idea.

Regarding a different language: I'm not sure a bash-free build would sell well since you lose stdenv, but I'm all for writing a different-language script to do the actual logic and using it from the bash builder (like for configuring the kernel, where the logic happens in perl).

there are no cycles in the module dependency graph

That should be a safe assumption: modules can't be loaded if their dependencies aren't loaded yet.

Regarding a different language: I'm not sure a bash-free build would sell well since you lose stdenv, but I'm all for writing a different-language script to do the actual logic and using it from the bash builder (like for configuring the kernel, where the logic happens in perl).

Actually an interpreted language is probably not a good idea here, since this code will run locally on almost every NixOS machine, so the interpreter would effectively slip into the runtime closure. I would be quite happy to write this external program in, say haskell, but I am not quite sure if people are willing to have ghc in the build time closure of (a minimal) NixOS (even if it doesn't need to run on every machine).

there are no cycles in the module dependency graph

That should be a safe assumption: modules can't be loaded if their dependencies aren't loaded yet.

Thats good to know! I noticed that it won't help as much as I initially thought though, because two modules still can depend on a common one, so the undirected graph may (and likely will) still have cycles. So we still need to either track seen modules, or accept that we visit nodes several times, which is bad performance wise.

Actually an interpreted language is probably not a good idea here, since this code will run locally on almost every NixOS machine, so the interpreter would effectively slip into the runtime closure.

We already require perl in the runtime closure since a number of core nixos utilities (e.g. the /etc/passwd generator script) are written in perl, and python is in the closure of any system using grub (dependency via libzfs) or systemd-boot (boot menu generator script is written in python). That said, I'm not sure I understand how it would get into the runtime closure? After all, it's only used within the derivation, and the derivation's output (modules closure) certainly shouldn't have any references to the interpreter.

Hard agree on not pulling ghc in though. :)

which is bad performance wise.

I think this is premature optimisation, module closure generation (a) doesn't happen that often, and (b) is still going to be orders of magnitude faster than other steps that usually happen at the same time, e.g. evaluating the system config. Though I don't think tracking seen modules would be too hard?

That said, I'm not sure I understand how it would get into the runtime closure? After all, it's only used within the derivation, and the derivation's output (modules closure) certainly shouldn't have any references to the interpreter.

Well formally speaking its not in the runtime closure, but since most people build their initrd on the system itself, they will need the interpreter.

which is bad performance wise.

I think this is premature optimisation, module closure generation (a) doesn't happen that often, and (b) is still going to be orders of magnitude faster than other steps that usually happen at the same time, e.g. evaluating the system config. Though I don't think tracking seen modules would be too hard?

Its already implemented actually, I was thinking about dropping it to simplify the code.

Ok lets stick with the bash implementation for now.

cc @iblech (as an expert on bash scripting)

😊 That's very kind of you. Let's say that I went enough times through quoting hell that I now know my path through that hell. :-) Also I never worked with arrays in bash. In line 124, is there a particular reason why you don't use -n or -z? Generally, this seems like the safer path (though in this case it doesn't matter). On line 73, in the comment, you have a double "if".

Thank you for your work on #60416!

blush That's very kind of you. Let's say that I went enough times through quoting hell that I now know my path through that hell. :-) Also I never worked with arrays in bash. In line 124, is there a particular reason why you don't use -n or -z? Generally, this seems like the safer path (though in this case it doesn't matter). On line 73, in the comment, you have a double "if".

Thank you for your work on #60416!

The motivation for the +x pattern in line 124 is that it also works if the arrays elements are empty strings (i.e. it actually checks for the presence of the entry in the array).
The double ifis fixed, thanks!

@ofborg test boot zfs

and again

@ofborg test boot zfs

That might or might not work currently due to #78430, I will rebase this once #109030 is merged.

also I gave rust a shot in the pr linked above

the other PR is merged now.

I marked this as stale due to inactivity. → More info