Skip to content
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

Jump to bottom

Misc fixes #371

Merged
merged 7 commits into from
Dec 6, 2023
Merged

Misc fixes #371

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
216f1af
do not label skipped module builds as 'Error!'
anbe42 Nov 24, 2023
99b4585
only enumerate kernels that potentially have headers installed
anbe42 Nov 24, 2023
de0a979
clean up after depmod if modules.dep is empty
anbe42 Nov 24, 2023
a4a1852
run_tests.sh: add more dkms status calls
anbe42 Dec 5, 2023
55f911a
add test module with AUTOINSTALL=""
anbe42 Dec 5, 2023
7b1b4fe
run tests on the AUTOINSTALL="" module
anbe42 Dec 5, 2023
918baa4
dkms_autoinstaller: improve console output
anbe42 Dec 6, 2023
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
23 changes: 21 additions & 2 deletions dkms.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,16 @@ die() {
[[ $die_is_fatal = yes ]] && exit $ret || return $ret
}

# Print a warning message and die with the passed error code.
diewarn() {
# $1 = error code to return with
# rest = strings to print before we exit.
ret=$1
shift
warn "$@"
[[ $die_is_fatal = yes ]] && exit $ret || return $ret
}

mktemp_or_die() {
local t
t=$(mktemp "$@") && echo "$t" && return
Expand Down Expand Up @@ -324,6 +334,13 @@ do_depmod()
else
depmod -a "$1"
fi
if [ -f /lib/modules/$1/modules.dep ] && [ ! -s /lib/modules/$1/modules.dep ]; then
# if modules.dep is empty, we just removed the last kernel module from
# no longer installed kernel $1, so do not leave stale depmod files around
rm -fv /lib/modules/$1/modules.{alias,dep,devname,softdep,symbols,*.bin}
rmdir --ignore-fail-on-non-empty /lib/modules/$1
test -d /lib/modules/$1 || echo $"Removed /lib/modules/$1"
Copy link
Collaborator

@evelikov evelikov Dec 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The rest of the series looks great, although this looks like a hack/workaround.

Namely: distros should kick off dkms remove/unbuild... before the actual kernel (modules) get removed. This is what Arch does at least - I didn't check others. If they use different order, then depmod shows some scary warnings:

depmod: WARNING: could not open modules.order at /some/path: No such file or directory
depmod: WARNING: could not open modules.builtin at /some/path: No such file or directory
depmod: WARNING: could not open modules.builtin.modinfo at /some/path: No such file or directory

If people manually remove modules after the kernel is gone, they will see the above warnings, which should be a flag that (minor) manual intervention is needed. That's perfectly fine IMHO.

In addition the empty /lib/modules/$1 can be owned by another package and removing it will cause package integrity checks to fail.

In a way I'm thinking that this should be handled at the depmod level - assuming you agree, can you open a proposal at https://github.com/kmod-project/kmod/ and I would have a look as time permits.

Few questions come to mind:

  • Toggle short/long name?
  • Omit output files when one or all input files are missing?
  • Opt-in or out of the new functionality?
  • Keep as-is or remove pre-existing output files?

Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If only linux-headers-$KVER are installed (maybe linux-image-$KVER is already gone) dkms builds and installs modules and calls depmod creating these files. There is noone left responsible for cleaning up these bits ...

Ideally depmod should be deferred to a trigger from linux-image-$KVER ... but it's probably a long way to get there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ack, can I ask you to open an issue (here dkms and/or in Debian) with a rough outline and link it in the code?

Thanks 🙇

fi
}

# Grab distro information from os-release.
Expand Down Expand Up @@ -1014,8 +1031,10 @@ prepare_build()
read_conf_or_die "$kernelver" "$arch"

# Error out if build_exclude is set
[[ $build_exclude ]] && die 77 \
$"The $base_dir/dkms.conf for module $module includes a BUILD_EXCLUSIVE directive which does not match this kernel/arch/config."\
[[ $build_exclude ]] && diewarn 77 \
$"The $base_dir/dkms.conf"\
$"for module $module includes a BUILD_EXCLUSIVE directive"\
$"which does not match this kernel/arch/config."\
$"This indicates that it should not be built."

# Error out if source_tree is basically empty (binary-only dkms tarball w/ --force check)
Expand Down
2 changes: 1 addition & 1 deletion dkms_common.postinst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ if [ -r /etc/dkms/framework.conf ]; then
. /etc/dkms/framework.conf
fi

KERNELS=$(ls -v /lib/modules/ 2>/dev/null || true)
KERNELS=$(ls -dv /lib/modules/*/build 2>/dev/null | cut -d/ -f4 || true)
CURRENT_KERNEL=$(uname -r)

#We never want to keep an older version side by side to prevent conflicts
Expand Down
24 changes: 18 additions & 6 deletions run_test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1366,7 +1366,9 @@ fi
# Should this really fail?
echo '(Not) building the build-exclusive test module'
run_with_expected_error 77 dkms build -k "${KERNEL_VER}" -m dkms_build_exclusive_test -v 1.0 << EOF
Error! The /var/lib/dkms/dkms_build_exclusive_test/1.0/${KERNEL_VER}/${KERNEL_ARCH}/dkms.conf for module dkms_build_exclusive_test includes a BUILD_EXCLUSIVE directive which does not match this kernel/arch/config.
Warning: The /var/lib/dkms/dkms_build_exclusive_test/1.0/${KERNEL_VER}/${KERNEL_ARCH}/dkms.conf
for module dkms_build_exclusive_test includes a BUILD_EXCLUSIVE directive
which does not match this kernel/arch/config.
This indicates that it should not be built.
EOF
run_status_with_expected_output 'dkms_build_exclusive_test' << EOF
Expand All @@ -1375,7 +1377,9 @@ EOF

echo "Running dkms autoinstall (1 x skip)"
run_with_expected_output dkms autoinstall -k "${KERNEL_VER}" << EOF
Error! The /var/lib/dkms/dkms_build_exclusive_test/1.0/${KERNEL_VER}/${KERNEL_ARCH}/dkms.conf for module dkms_build_exclusive_test includes a BUILD_EXCLUSIVE directive which does not match this kernel/arch/config.
Warning: The /var/lib/dkms/dkms_build_exclusive_test/1.0/${KERNEL_VER}/${KERNEL_ARCH}/dkms.conf
for module dkms_build_exclusive_test includes a BUILD_EXCLUSIVE directive
which does not match this kernel/arch/config.
This indicates that it should not be built.
dkms autoinstall on ${KERNEL_VER}/${KERNEL_ARCH} was skipped for dkms_build_exclusive_test
EOF
Expand All @@ -1393,7 +1397,9 @@ EOF

echo "Running dkms autoinstall (1 x skip, 1 x pass)"
run_with_expected_output dkms autoinstall -k "${KERNEL_VER}" << EOF
Error! The /var/lib/dkms/dkms_build_exclusive_test/1.0/${KERNEL_VER}/${KERNEL_ARCH}/dkms.conf for module dkms_build_exclusive_test includes a BUILD_EXCLUSIVE directive which does not match this kernel/arch/config.
Warning: The /var/lib/dkms/dkms_build_exclusive_test/1.0/${KERNEL_VER}/${KERNEL_ARCH}/dkms.conf
for module dkms_build_exclusive_test includes a BUILD_EXCLUSIVE directive
which does not match this kernel/arch/config.
This indicates that it should not be built.

Building module:
Expand Down Expand Up @@ -1441,7 +1447,9 @@ EOF

echo "Running dkms autoinstall (1 x skip, 1 x fail, 1 x pass) (expected error)"
run_with_expected_error 11 dkms autoinstall -k "${KERNEL_VER}" << EOF
Error! The /var/lib/dkms/dkms_build_exclusive_test/1.0/${KERNEL_VER}/${KERNEL_ARCH}/dkms.conf for module dkms_build_exclusive_test includes a BUILD_EXCLUSIVE directive which does not match this kernel/arch/config.
Warning: The /var/lib/dkms/dkms_build_exclusive_test/1.0/${KERNEL_VER}/${KERNEL_ARCH}/dkms.conf
for module dkms_build_exclusive_test includes a BUILD_EXCLUSIVE directive
which does not match this kernel/arch/config.
This indicates that it should not be built.

Building module:
Expand Down Expand Up @@ -1510,9 +1518,13 @@ fi

echo "Running dkms autoinstall (2 x skip, with dependency)"
run_with_expected_output dkms autoinstall -k "${KERNEL_VER}" << EOF
Error! The /var/lib/dkms/dkms_build_exclusive_test/1.0/${KERNEL_VER}/${KERNEL_ARCH}/dkms.conf for module dkms_build_exclusive_test includes a BUILD_EXCLUSIVE directive which does not match this kernel/arch/config.
Warning: The /var/lib/dkms/dkms_build_exclusive_test/1.0/${KERNEL_VER}/${KERNEL_ARCH}/dkms.conf
for module dkms_build_exclusive_test includes a BUILD_EXCLUSIVE directive
which does not match this kernel/arch/config.
This indicates that it should not be built.
Error! The /var/lib/dkms/dkms_build_exclusive_dependencies_test/1.0/${KERNEL_VER}/${KERNEL_ARCH}/dkms.conf for module dkms_build_exclusive_dependencies_test includes a BUILD_EXCLUSIVE directive which does not match this kernel/arch/config.
Warning: The /var/lib/dkms/dkms_build_exclusive_dependencies_test/1.0/${KERNEL_VER}/${KERNEL_ARCH}/dkms.conf
for module dkms_build_exclusive_dependencies_test includes a BUILD_EXCLUSIVE directive
which does not match this kernel/arch/config.
This indicates that it should not be built.
dkms autoinstall on ${KERNEL_VER}/${KERNEL_ARCH} was skipped for dkms_build_exclusive_test dkms_build_exclusive_dependencies_test
EOF
Expand Down