Skip to content

makeModulesClosure: handle builtin modules better #92081

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

Merged
lheckemann merged 1 commit into from
Aug 13, 2020
Merged

makeModulesClosure: handle builtin modules better #92081

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
67 changes: 43 additions & 24 deletions pkgs/build-support/kernel/modules-closure.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

@lheckemann lheckemann Aug 11, 2020

Choose a reason for hiding this comment

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

Using an array could be nicer here, since it avoids word-splitting "fun":

closure=()
[…]
closure+=("$module")
[…]
for module in "${closure[@]}"; do […]

Copy link
Contributor Author

@CrystalGamma CrystalGamma Aug 17, 2020

Choose a reason for hiding this comment

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

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. :)

Copy link
Member

@lheckemann lheckemann Aug 17, 2020

Choose a reason for hiding this comment

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

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 :)

for module in $rootModules; do
echo "root module: $module"
deps=$(modprobe --config no-config -d $kernel --set-version "$version" --show-depends "$module" \
| sed 's/^insmod //') \
|| if test -z "$allowMissing"; then exit 1; fi
if [[ "$deps" != builtin* ]]; then
closure="$closure $deps"
modprobe --config no-config -d $kernel --set-version "$version" --show-depends "$module" \
| while read cmd module args; do
case "$cmd" in
builtin)
touch found
echo "$module" >>closure
echo " builtin dependency: $module";;
insmod)
touch found
if ! test -e "$module"; then
echo " dependency not found: $module"
exit 1
fi
target=$(echo "$module" | sed "s^$NIX_STORE.*/lib/modules/^$out/lib/modules/^")
if test -e "$target"; then
echo " dependency already copied: $module"
continue
fi
echo "$module" >>closure
echo " copying dependency: $module"
mkdir -p $(dirname $target)
cp "$module" "$target"
# If the kernel is compiled with coverage instrumentation, it
# contains the paths of the *.gcda coverage data output files
# (which it doesn't actually use...). Get rid of them to prevent
# the whole kernel from being included in the initrd.
nuke-refs "$target"
echo "$target" >> $out/insmod-list;;
*)
echo " unexpected modprobe output: $cmd $module"
exit 1;;
esac
done || test -n "$allowMissing"
if ! test -e found; then
echo " not found"
if test -z "$allowMissing"; then
exit 1
fi
else
rm found
fi
done

echo "closure:"
mkdir -p $out/lib/modules/"$version"
for module in $closure; do
target=$(echo $module | sed "s^$NIX_STORE.*/lib/modules/^$out/lib/modules/^")
if test -e "$target"; then continue; fi
if test \! -e "$module"; then continue; fi # XXX: to avoid error with "cp builtin builtin"
mkdir -p $(dirname $target)
echo $module
cp $module $target
# If the kernel is compiled with coverage instrumentation, it
# contains the paths of the *.gcda coverage data output files
# (which it doesn't actually use...). Get rid of them to prevent
# the whole kernel from being included in the initrd.
nuke-refs $target
echo $target >> $out/insmod-list
done

mkdir -p $out/lib/firmware
for module in $closure; do
for module in $(cat closure); do
for i in $(modinfo -F firmware $module); do
mkdir -p "$out/lib/firmware/$(dirname "$i")"
echo "firmware for $module: $i"
Expand Down