makeModulesClosure: handle builtin modules better #92081
Conversation
|
While at it, can we also handle module arguments properly (in the spirit of https://github.com/NixOS/nixpkgs/pull/66235/files)? |
|
@xaverdh should be just another dummy variable in the |
Well if you set a module parameter in the kernel command line (e.g. via update: I can confirm that with your pr and an additional parameter to |
The previous code discarded entire dependency trees if the first entry in the dependency list compiled by `modprobe --show-depends` is a builtin and otherwise handled its output in a rather hackish way.
CrystalGamma
force-pushed
the
modules-closure
branch
from
89620df
to
b155b1d
Compare
|
Thanks! Works on my system = ) |
|
@ofborg test boot |
| @@ -19,36 +19,55 @@ version=$(cd $kernel/lib/modules && ls -d *) | |||
| echo "kernel version is $version" | |||
|
|
|||
| # Determine the dependencies of each root module. | |||
| closure= | |||
| mkdir -p $out/lib/modules/"$version" | |||
| touch closure | |||
There was a problem hiding this comment.
Using an array could be nicer here, since it avoids word-splitting "fun":
closure=()
[…]
closure+=("$module")
[…]
for module in "${closure[@]}"; do […]
There was a problem hiding this comment.
The reason I used a file here was because the while read loop seems to run in a subshell, so code afterwards can't use any variables that are set in the loop. I personally don't have experience with arrays in Bourne-based shells (I use fish where I can) but I would assume the same holds for them. Anyway, thanks for merging. :)
There was a problem hiding this comment.
Aah, I see. Yeah, you can work around that by making what's piped into while run in the subshell instead, as in…
while read cmd module args ; do
[…]
done < <(modprobe --config no-config -d $kernel --set-version "$version" --show-depends "$module")
but that does make reading it a little more confusing. Alternatively:
exec 5< <(modprobe --config no-config -d $kernel --set-version "$version" --show-depends "$module")
while read -u 5 cmd module args ; do
[…]
done
but yeah I'd say leave it as is now :)
|
Thanks for finally making some progress on this! |
Rewrite the closure computation and copying.
Motivation for this change
The previous code discarded entire dependency trees if the first entry in the dependency list compiled by
modprobe --show-dependsis a builtin and otherwise handled its output in a rather hackish way.Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)