[PR PATCH] dkms: run depmod if necessary in kernel post-install hook

[PR PATCH] dkms: run depmod if necessary in kernel post-install hook

[ahesford]

[-- Attachment #1: Type: text/plain, Size: 1882 bytes --]

There is a new pull request by ahesford against master on the void-packages repository

https://github.com/ahesford/void-packages dkms
https://github.com/void-linux/void-packages/pull/23392

dkms: run depmod if necessary in kernel post-install hook
Normally, `dkms install` will run depmod with each installed module to update module dependency lists. When force-reinstalling kernel packages, `dkms install` will not run because modules are already marked installed, but the kernel package will overwrite `modules.dep` and `modules.dep.bin` with packaged versions that do not include any modules installed by dkms. The kernel post-install hook now tracks whether depmod should be run and will do so if necessary.

To demonstrate the original problem:

1. Install `zfs` and enable the dracut zfs module with `add_dracutmodules+=" zfs "` in `dracut.conf` or a `.conf` file in `/etc/dracut.conf.d`.
2. Rerun `dracut`, e.g., with `xbps-reconfigure -f linuxX.Y` for an appropriate kernel package.
3. Confirm that `zfs.ko.gz` is in the newly generated initramfs.
4. Force-reinstall the kernel package: `xbps-install -f linuxX.Y`.
5. Confirm that `zfs.ko.gz` is *missing* from the newly generated initramfs.

With the changes in the hook, the force-installation of `linuxX.Y should display `Generating kernel module dependency lists... done.` after "building" all DKMS modules (note: the modules were already built for the reinstalled kernel, so the `dkms build` run by the hook is basically a no-op). The resulting initramfs will contain `zfs.ko.gz` as expected.

Also, I propagated some return codes from `dkms` through the hook, which I believe was the original intention. (Before the fix, `exit $?` after an `echo` would generally always exit 0 because `echo` is unlikely to fail.)

A patch file from https://github.com/void-linux/void-packages/pull/23392.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-dkms-23392.patch --]
[-- Type: text/x-diff, Size: 2707 bytes --]

From ffb8b4fb170686dc5027498e48c21fe9ac57a769 Mon Sep 17 00:00:00 2001
From: "Andrew J. Hesford" <ajh@sideband.org>
Date: Sat, 4 Jul 2020 16:33:31 -0400
Subject: [PATCH] dkms: run depmod if necessary in kernel post-install hook

Normally, `dkms install` will run depmod with each installed module to
update module dependency lists. When force-reinstalling kernel packages,
`dkms install` will not run because modules are already marked
installed, but the kernel package will overwrite `modules.dep` and
`modules.dep.bin` with packaged versions that do not include any modules
installed by dkms. The kernel post-install hook now tracks whether
depmod should be run and will do so if necessary.
---
 srcpkgs/dkms/files/kernel.d/dkms.postinst | 24 ++++++++++++++++++++---
 srcpkgs/dkms/template                     |  2 +-
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/srcpkgs/dkms/files/kernel.d/dkms.postinst b/srcpkgs/dkms/files/kernel.d/dkms.postinst
index bfeaf34fccf..1d05ec88e08 100644
--- a/srcpkgs/dkms/files/kernel.d/dkms.postinst
+++ b/srcpkgs/dkms/files/kernel.d/dkms.postinst
@@ -45,6 +45,9 @@ while [ $# -ne 0 ]; do
 	module="$1"
 	modulever="$2"
 
+	# If adding a module, depmod is necessary unless dkms runs it
+	do_depmod="yes"
+
 	status=$(dkms status -m ${module} -v ${modulever} -k ${VERSION})
 	if [ $(echo "$status"|grep -c ": built") -eq 0 ]; then
 		# Check if the module is still there.
@@ -65,7 +68,7 @@ while [ $# -ne 0 ]; do
 			echo "done."
 		else
 			echo "FAILED!"
-			exit $?
+			exit $rval
 		fi
 		status=$(dkms status -m ${module} -v ${modulever} -k ${VERSION})
 	fi
@@ -75,14 +78,29 @@ while [ $# -ne 0 ]; do
 	   [ $(echo "$status"|grep -c ": installed") -eq 0 ]; then
 		echo -n "Installing DKMS module: ${module}-${modulever}... "
 	        dkms install -q -m ${module} -v ${modulever} -k ${VERSION} -a ${ARCH}
-		if [ $? -eq 0 ]; then
+		rval=$?
+		if [ $rval -eq 0 ]; then
 			echo "done."
+			# dkms runs depmod as part of the installation
+			unset do_depmod
 		else
 			echo "FAILED!"
-			exit $?
+			exit $rval
 		fi
 	fi
 	shift; shift
 done
 
+if [ -n "$do_depmod" ]; then
+	echo -n "Generating kernel module dependency lists... "
+	depmod -a ${VERSION}
+	rval=$?
+	if [ $rval -eq 0 ]; then
+		echo "done."
+	else
+		echo "FAILED!"
+		exit $rval
+	fi
+fi
+
 exit 0
diff --git a/srcpkgs/dkms/template b/srcpkgs/dkms/template
index 521dfde1114..32345b9b421 100644
--- a/srcpkgs/dkms/template
+++ b/srcpkgs/dkms/template
@@ -2,7 +2,7 @@
 pkgname=dkms
 reverts="2.8.2_1"
 version=2.8.1
-revision=3
+revision=4
 conf_files="/etc/dkms/framework.conf"
 depends="bash kmod gcc make coreutils linux-headers"
 short_desc="Dynamic Kernel Modules System"

Re: [PR PATCH] [Updated] dkms: run depmod if necessary in kernel post-install hook

[ahesford]

[-- Attachment #1: Type: text/plain, Size: 1888 bytes --]

There is an updated pull request by ahesford against master on the void-packages repository

https://github.com/ahesford/void-packages dkms
https://github.com/void-linux/void-packages/pull/23392

dkms: run depmod if necessary in kernel post-install hook
Normally, `dkms install` will run depmod with each installed module to update module dependency lists. When force-reinstalling kernel packages, `dkms install` will not run because modules are already marked installed, but the kernel package will overwrite `modules.dep` and `modules.dep.bin` with packaged versions that do not include any modules installed by dkms. The kernel post-install hook now tracks whether depmod should be run and will do so if necessary.

To demonstrate the original problem:

1. Install `zfs` and enable the dracut zfs module with `add_dracutmodules+=" zfs "` in `dracut.conf` or a `.conf` file in `/etc/dracut.conf.d`.
2. Rerun `dracut`, e.g., with `xbps-reconfigure -f linuxX.Y` for an appropriate kernel package.
3. Confirm that `zfs.ko.gz` is in the newly generated initramfs.
4. Force-reinstall the kernel package: `xbps-install -f linuxX.Y`.
5. Confirm that `zfs.ko.gz` is *missing* from the newly generated initramfs.

With the changes in the hook, the force-installation of `linuxX.Y` should display `Generating kernel module dependency lists... done.` after "building" all DKMS modules (note: the modules were already built for the reinstalled kernel, so the `dkms build` run by the hook is basically a no-op). The resulting initramfs will contain `zfs.ko.gz` as expected.

Also, I propagated some return codes from `dkms` through the hook, which I believe was the original intention. (Before the fix, `exit $?` after an `echo` would generally always exit 0 because `echo` is unlikely to fail.)

A patch file from https://github.com/void-linux/void-packages/pull/23392.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-dkms-23392.patch --]
[-- Type: text/x-diff, Size: 4269 bytes --]

From 4e0fc9fd770d069fc5802b6aff687a430787d705 Mon Sep 17 00:00:00 2001
From: "Andrew J. Hesford" <ajh@sideband.org>
Date: Sat, 4 Jul 2020 16:33:31 -0400
Subject: [PATCH] dkms: improvements to kernel post-install hook

When force-reinstalling kernel packages, `dkms install` is not run in
the kernel post-install hook because the modules are already installed.
This prevents `dkms install` from running `depmod -a` as it normally
does, but the forced reinstallation clobbers `modules.dep{,.bin}` with
the packaged versions that do not include DKMS-installed modules. The
post-install hook now tracks whether `depmod -a` should be run and does
so when necessary.

Other changes, courtesy of @ericonr, address issue #23124 wherein a DKMS
failure will terminate the script, preventing building or installation
of subsequent modules. The fix follows logic similar to that in the DKMS
xbps-trigger.
---
 srcpkgs/dkms/files/kernel.d/dkms.postinst | 55 +++++++++++++++++------
 srcpkgs/dkms/template                     |  2 +-
 2 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/srcpkgs/dkms/files/kernel.d/dkms.postinst b/srcpkgs/dkms/files/kernel.d/dkms.postinst
index bfeaf34fccf..c8a8deae0d4 100644
--- a/srcpkgs/dkms/files/kernel.d/dkms.postinst
+++ b/srcpkgs/dkms/files/kernel.d/dkms.postinst
@@ -40,49 +40,78 @@ for _mod_ in /var/lib/dkms/*; do
 	done
 done
 
+# This sections builds and install the available modules.
+#
+# If either building or installing a module fails, a warning is
+# printed and it is skipped, but the script still tries to build
+# the other modules.
+#
+# The list of available modules is in the form
+# [module1, modulever1, module2, modulever2, ...]
+#
 set -- ${DKMS_MODULES}
 while [ $# -ne 0 ]; do
 	module="$1"
 	modulever="$2"
 
+	# If adding a module, depmod is necessary unless dkms runs it
+	do_depmod="yes"
+
 	status=$(dkms status -m ${module} -v ${modulever} -k ${VERSION})
 	if [ $(echo "$status"|grep -c ": built") -eq 0 ]; then
 		# Check if the module is still there.
 		if [ ! -f usr/src/${module}-${modulever}/dkms.conf ]; then
 			echo "Skipping unexistent DKMS module: ${module}-${modulever}."
-			shift 2
-			continue
+			shift 2; continue
 		fi
 		# Build the module
 		echo -n "Building DKMS module: ${module}-${modulever}... "
 		dkms build -q -m ${module} -v ${modulever} -k ${VERSION} -a ${ARCH}
 		rval=$?
-		if [ $rval -eq 9 ]; then
-			echo "skipped!"
-			shift; shift
-			continue
-		elif [ $rval -eq 0 ]; then
+		# If the module was skipped or failed, go to the next module.
+		if [ $rval -eq 0 ]; then
 			echo "done."
+		elif [ $rval -eq 9 ]; then
+			echo "skipped!"
+			shift 2; continue
 		else
 			echo "FAILED!"
-			exit $?
+			shift 2; continue
 		fi
 		status=$(dkms status -m ${module} -v ${modulever} -k ${VERSION})
 	fi
 
-	#if the module is built (either pre-built or just now), install it
+	# If the module is built (either pre-built or just now), install it
 	if [ $(echo "$status"|grep -c ": built") -eq 1 ] &&
 	   [ $(echo "$status"|grep -c ": installed") -eq 0 ]; then
 		echo -n "Installing DKMS module: ${module}-${modulever}... "
-	        dkms install -q -m ${module} -v ${modulever} -k ${VERSION} -a ${ARCH}
-		if [ $? -eq 0 ]; then
+		dkms install -q -m ${module} -v ${modulever} -k ${VERSION} -a ${ARCH}
+		rval=$?
+		# If the module failed installation, go to the next module.
+		if [ $rval -eq 0 ]; then
+			# dkms runs depmod as part of the installation
+			unset do_depmod
 			echo "done."
 		else
 			echo "FAILED!"
-			exit $?
+			shift 2; continue
 		fi
 	fi
-	shift; shift
+
+	# Go to the next module - all steps for this module were successful.
+	shift 2
 done
 
+if [ -n "$do_depmod" ]; then
+	echo -n "Generating kernel module dependency lists... "
+	depmod -a ${VERSION}
+	rval=$?
+	if [ $rval -eq 0 ]; then
+		echo "done."
+	else
+		echo "FAILED!"
+		exit $rval
+	fi
+fi
+
 exit 0
diff --git a/srcpkgs/dkms/template b/srcpkgs/dkms/template
index 521dfde1114..32345b9b421 100644
--- a/srcpkgs/dkms/template
+++ b/srcpkgs/dkms/template
@@ -2,7 +2,7 @@
 pkgname=dkms
 reverts="2.8.2_1"
 version=2.8.1
-revision=3
+revision=4
 conf_files="/etc/dkms/framework.conf"
 depends="bash kmod gcc make coreutils linux-headers"
 short_desc="Dynamic Kernel Modules System"

Re: dkms: run depmod if necessary in kernel post-install hook

[ahesford]

[-- Attachment #1: Type: text/plain, Size: 274 bytes --]

New comment by ahesford on void-packages repository

https://github.com/void-linux/void-packages/pull/23392#issuecomment-653829893

Comment:
At the request of @ericonr, subsequent changes have been made to merge the fixes for issue #23124 that were originally in PR #23214.

Re: [PR PATCH] [Updated] dkms: improvements to post-install hook: run depmod if necessary and better handle DKMS build/install failures

[ahesford]

[-- Attachment #1: Type: text/plain, Size: 1941 bytes --]

There is an updated pull request by ahesford against master on the void-packages repository

https://github.com/ahesford/void-packages dkms
https://github.com/void-linux/void-packages/pull/23392

dkms: improvements to post-install hook: run depmod if necessary and better handle DKMS build/install failures
Normally, `dkms install` will run depmod with each installed module to update module dependency lists. When force-reinstalling kernel packages, `dkms install` will not run because modules are already marked installed, but the kernel package will overwrite `modules.dep` and `modules.dep.bin` with packaged versions that do not include any modules installed by dkms. The kernel post-install hook now tracks whether depmod should be run and will do so if necessary.

To demonstrate the original problem:

1. Install `zfs` and enable the dracut zfs module with `add_dracutmodules+=" zfs "` in `dracut.conf` or a `.conf` file in `/etc/dracut.conf.d`.
2. Rerun `dracut`, e.g., with `xbps-reconfigure -f linuxX.Y` for an appropriate kernel package.
3. Confirm that `zfs.ko.gz` is in the newly generated initramfs.
4. Force-reinstall the kernel package: `xbps-install -f linuxX.Y`.
5. Confirm that `zfs.ko.gz` is *missing* from the newly generated initramfs.

With the changes in the hook, the force-installation of `linuxX.Y` should display `Generating kernel module dependency lists... done.` after "building" all DKMS modules (note: the modules were already built for the reinstalled kernel, so the `dkms build` run by the hook is basically a no-op). The resulting initramfs will contain `zfs.ko.gz` as expected.

Also, I propagated some return codes from `dkms` through the hook, which I believe was the original intention. (Before the fix, `exit $?` after an `echo` would generally always exit 0 because `echo` is unlikely to fail.)

A patch file from https://github.com/void-linux/void-packages/pull/23392.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-dkms-23392.patch --]
[-- Type: text/x-diff, Size: 5970 bytes --]

From b6494bf7fd815f7e640028afa973109c22744b31 Mon Sep 17 00:00:00 2001
From: "Andrew J. Hesford" <ajh@sideband.org>
Date: Sat, 4 Jul 2020 16:33:31 -0400
Subject: [PATCH 1/2] dkms: improvements to kernel post-install hook

When force-reinstalling kernel packages, `dkms install` is not run in
the kernel post-install hook because the modules are already installed.
This prevents `dkms install` from running `depmod -a` as it normally
does, but the forced reinstallation clobbers `modules.dep{,.bin}` with
the packaged versions that do not include DKMS-installed modules. The
post-install hook now tracks whether `depmod -a` should be run and does
so when necessary.

Other changes, courtesy of @ericonr, address issue #23124 wherein a DKMS
failure will terminate the script, preventing building or installation
of subsequent modules. The fix follows logic similar to that in the DKMS
xbps-trigger.
---
 srcpkgs/dkms/files/kernel.d/dkms.postinst | 55 +++++++++++++++++------
 srcpkgs/dkms/template                     |  2 +-
 2 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/srcpkgs/dkms/files/kernel.d/dkms.postinst b/srcpkgs/dkms/files/kernel.d/dkms.postinst
index bfeaf34fccf..c8a8deae0d4 100644
--- a/srcpkgs/dkms/files/kernel.d/dkms.postinst
+++ b/srcpkgs/dkms/files/kernel.d/dkms.postinst
@@ -40,49 +40,78 @@ for _mod_ in /var/lib/dkms/*; do
 	done
 done
 
+# This sections builds and install the available modules.
+#
+# If either building or installing a module fails, a warning is
+# printed and it is skipped, but the script still tries to build
+# the other modules.
+#
+# The list of available modules is in the form
+# [module1, modulever1, module2, modulever2, ...]
+#
 set -- ${DKMS_MODULES}
 while [ $# -ne 0 ]; do
 	module="$1"
 	modulever="$2"
 
+	# If adding a module, depmod is necessary unless dkms runs it
+	do_depmod="yes"
+
 	status=$(dkms status -m ${module} -v ${modulever} -k ${VERSION})
 	if [ $(echo "$status"|grep -c ": built") -eq 0 ]; then
 		# Check if the module is still there.
 		if [ ! -f usr/src/${module}-${modulever}/dkms.conf ]; then
 			echo "Skipping unexistent DKMS module: ${module}-${modulever}."
-			shift 2
-			continue
+			shift 2; continue
 		fi
 		# Build the module
 		echo -n "Building DKMS module: ${module}-${modulever}... "
 		dkms build -q -m ${module} -v ${modulever} -k ${VERSION} -a ${ARCH}
 		rval=$?
-		if [ $rval -eq 9 ]; then
-			echo "skipped!"
-			shift; shift
-			continue
-		elif [ $rval -eq 0 ]; then
+		# If the module was skipped or failed, go to the next module.
+		if [ $rval -eq 0 ]; then
 			echo "done."
+		elif [ $rval -eq 9 ]; then
+			echo "skipped!"
+			shift 2; continue
 		else
 			echo "FAILED!"
-			exit $?
+			shift 2; continue
 		fi
 		status=$(dkms status -m ${module} -v ${modulever} -k ${VERSION})
 	fi
 
-	#if the module is built (either pre-built or just now), install it
+	# If the module is built (either pre-built or just now), install it
 	if [ $(echo "$status"|grep -c ": built") -eq 1 ] &&
 	   [ $(echo "$status"|grep -c ": installed") -eq 0 ]; then
 		echo -n "Installing DKMS module: ${module}-${modulever}... "
-	        dkms install -q -m ${module} -v ${modulever} -k ${VERSION} -a ${ARCH}
-		if [ $? -eq 0 ]; then
+		dkms install -q -m ${module} -v ${modulever} -k ${VERSION} -a ${ARCH}
+		rval=$?
+		# If the module failed installation, go to the next module.
+		if [ $rval -eq 0 ]; then
+			# dkms runs depmod as part of the installation
+			unset do_depmod
 			echo "done."
 		else
 			echo "FAILED!"
-			exit $?
+			shift 2; continue
 		fi
 	fi
-	shift; shift
+
+	# Go to the next module - all steps for this module were successful.
+	shift 2
 done
 
+if [ -n "$do_depmod" ]; then
+	echo -n "Generating kernel module dependency lists... "
+	depmod -a ${VERSION}
+	rval=$?
+	if [ $rval -eq 0 ]; then
+		echo "done."
+	else
+		echo "FAILED!"
+		exit $rval
+	fi
+fi
+
 exit 0
diff --git a/srcpkgs/dkms/template b/srcpkgs/dkms/template
index 521dfde1114..32345b9b421 100644
--- a/srcpkgs/dkms/template
+++ b/srcpkgs/dkms/template
@@ -2,7 +2,7 @@
 pkgname=dkms
 reverts="2.8.2_1"
 version=2.8.1
-revision=3
+revision=4
 conf_files="/etc/dkms/framework.conf"
 depends="bash kmod gcc make coreutils linux-headers"
 short_desc="Dynamic Kernel Modules System"

From fbc70b2ef84e1705e62b2c004064de08ba083eb4 Mon Sep 17 00:00:00 2001
From: "Andrew J. Hesford" <ajh@sideband.org>
Date: Fri, 10 Jul 2020 21:34:34 -0400
Subject: [PATCH 2/2] Rework dkms post-install provided by ericonr

---
 srcpkgs/dkms/files/kernel.d/dkms.postinst | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/srcpkgs/dkms/files/kernel.d/dkms.postinst b/srcpkgs/dkms/files/kernel.d/dkms.postinst
index c8a8deae0d4..3bd4952c3cf 100644
--- a/srcpkgs/dkms/files/kernel.d/dkms.postinst
+++ b/srcpkgs/dkms/files/kernel.d/dkms.postinst
@@ -52,7 +52,11 @@ done
 set -- ${DKMS_MODULES}
 while [ $# -ne 0 ]; do
 	module="$1"
-	modulever="$2"
+	shift
+
+	[ $# -gt 0 ] || break
+	modulever="$1"
+	shift
 
 	# If adding a module, depmod is necessary unless dkms runs it
 	do_depmod="yes"
@@ -62,7 +66,7 @@ while [ $# -ne 0 ]; do
 		# Check if the module is still there.
 		if [ ! -f usr/src/${module}-${modulever}/dkms.conf ]; then
 			echo "Skipping unexistent DKMS module: ${module}-${modulever}."
-			shift 2; continue
+			continue
 		fi
 		# Build the module
 		echo -n "Building DKMS module: ${module}-${modulever}... "
@@ -73,10 +77,10 @@ while [ $# -ne 0 ]; do
 			echo "done."
 		elif [ $rval -eq 9 ]; then
 			echo "skipped!"
-			shift 2; continue
+			continue
 		else
 			echo "FAILED!"
-			shift 2; continue
+			continue
 		fi
 		status=$(dkms status -m ${module} -v ${modulever} -k ${VERSION})
 	fi
@@ -94,12 +98,9 @@ while [ $# -ne 0 ]; do
 			echo "done."
 		else
 			echo "FAILED!"
-			shift 2; continue
+			continue
 		fi
 	fi
-
-	# Go to the next module - all steps for this module were successful.
-	shift 2
 done
 
 if [ -n "$do_depmod" ]; then

Re: [PR REVIEW] dkms: improvements to post-install hook: run depmod if necessary and better handle DKMS build/install failures

[ericonr]

[-- Attachment #1: Type: text/plain, Size: 359 bytes --]

New review comment by ericonr on void-packages repository

https://github.com/void-linux/void-packages/pull/23392#discussion_r453139977

Comment:
I don't think we should reach this condition, right? Two shifts per iteration and an even number of arguments should mean that at one point the number of arguments will reach zero, and the while simply won't run.

Re: [PR PATCH] [Updated] dkms: improvements to post-install hook: run depmod if necessary and better handle DKMS build/install failures

[ahesford]

[-- Attachment #1: Type: text/plain, Size: 1941 bytes --]

There is an updated pull request by ahesford against master on the void-packages repository

https://github.com/ahesford/void-packages dkms
https://github.com/void-linux/void-packages/pull/23392

dkms: improvements to post-install hook: run depmod if necessary and better handle DKMS build/install failures
Normally, `dkms install` will run depmod with each installed module to update module dependency lists. When force-reinstalling kernel packages, `dkms install` will not run because modules are already marked installed, but the kernel package will overwrite `modules.dep` and `modules.dep.bin` with packaged versions that do not include any modules installed by dkms. The kernel post-install hook now tracks whether depmod should be run and will do so if necessary.

To demonstrate the original problem:

1. Install `zfs` and enable the dracut zfs module with `add_dracutmodules+=" zfs "` in `dracut.conf` or a `.conf` file in `/etc/dracut.conf.d`.
2. Rerun `dracut`, e.g., with `xbps-reconfigure -f linuxX.Y` for an appropriate kernel package.
3. Confirm that `zfs.ko.gz` is in the newly generated initramfs.
4. Force-reinstall the kernel package: `xbps-install -f linuxX.Y`.
5. Confirm that `zfs.ko.gz` is *missing* from the newly generated initramfs.

With the changes in the hook, the force-installation of `linuxX.Y` should display `Generating kernel module dependency lists... done.` after "building" all DKMS modules (note: the modules were already built for the reinstalled kernel, so the `dkms build` run by the hook is basically a no-op). The resulting initramfs will contain `zfs.ko.gz` as expected.

Also, I propagated some return codes from `dkms` through the hook, which I believe was the original intention. (Before the fix, `exit $?` after an `echo` would generally always exit 0 because `echo` is unlikely to fail.)

A patch file from https://github.com/void-linux/void-packages/pull/23392.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-dkms-23392.patch --]
[-- Type: text/x-diff, Size: 6022 bytes --]

From b6494bf7fd815f7e640028afa973109c22744b31 Mon Sep 17 00:00:00 2001
From: "Andrew J. Hesford" <ajh@sideband.org>
Date: Sat, 4 Jul 2020 16:33:31 -0400
Subject: [PATCH 1/2] dkms: improvements to kernel post-install hook

When force-reinstalling kernel packages, `dkms install` is not run in
the kernel post-install hook because the modules are already installed.
This prevents `dkms install` from running `depmod -a` as it normally
does, but the forced reinstallation clobbers `modules.dep{,.bin}` with
the packaged versions that do not include DKMS-installed modules. The
post-install hook now tracks whether `depmod -a` should be run and does
so when necessary.

Other changes, courtesy of @ericonr, address issue #23124 wherein a DKMS
failure will terminate the script, preventing building or installation
of subsequent modules. The fix follows logic similar to that in the DKMS
xbps-trigger.
---
 srcpkgs/dkms/files/kernel.d/dkms.postinst | 55 +++++++++++++++++------
 srcpkgs/dkms/template                     |  2 +-
 2 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/srcpkgs/dkms/files/kernel.d/dkms.postinst b/srcpkgs/dkms/files/kernel.d/dkms.postinst
index bfeaf34fccf..c8a8deae0d4 100644
--- a/srcpkgs/dkms/files/kernel.d/dkms.postinst
+++ b/srcpkgs/dkms/files/kernel.d/dkms.postinst
@@ -40,49 +40,78 @@ for _mod_ in /var/lib/dkms/*; do
 	done
 done
 
+# This sections builds and install the available modules.
+#
+# If either building or installing a module fails, a warning is
+# printed and it is skipped, but the script still tries to build
+# the other modules.
+#
+# The list of available modules is in the form
+# [module1, modulever1, module2, modulever2, ...]
+#
 set -- ${DKMS_MODULES}
 while [ $# -ne 0 ]; do
 	module="$1"
 	modulever="$2"
 
+	# If adding a module, depmod is necessary unless dkms runs it
+	do_depmod="yes"
+
 	status=$(dkms status -m ${module} -v ${modulever} -k ${VERSION})
 	if [ $(echo "$status"|grep -c ": built") -eq 0 ]; then
 		# Check if the module is still there.
 		if [ ! -f usr/src/${module}-${modulever}/dkms.conf ]; then
 			echo "Skipping unexistent DKMS module: ${module}-${modulever}."
-			shift 2
-			continue
+			shift 2; continue
 		fi
 		# Build the module
 		echo -n "Building DKMS module: ${module}-${modulever}... "
 		dkms build -q -m ${module} -v ${modulever} -k ${VERSION} -a ${ARCH}
 		rval=$?
-		if [ $rval -eq 9 ]; then
-			echo "skipped!"
-			shift; shift
-			continue
-		elif [ $rval -eq 0 ]; then
+		# If the module was skipped or failed, go to the next module.
+		if [ $rval -eq 0 ]; then
 			echo "done."
+		elif [ $rval -eq 9 ]; then
+			echo "skipped!"
+			shift 2; continue
 		else
 			echo "FAILED!"
-			exit $?
+			shift 2; continue
 		fi
 		status=$(dkms status -m ${module} -v ${modulever} -k ${VERSION})
 	fi
 
-	#if the module is built (either pre-built or just now), install it
+	# If the module is built (either pre-built or just now), install it
 	if [ $(echo "$status"|grep -c ": built") -eq 1 ] &&
 	   [ $(echo "$status"|grep -c ": installed") -eq 0 ]; then
 		echo -n "Installing DKMS module: ${module}-${modulever}... "
-	        dkms install -q -m ${module} -v ${modulever} -k ${VERSION} -a ${ARCH}
-		if [ $? -eq 0 ]; then
+		dkms install -q -m ${module} -v ${modulever} -k ${VERSION} -a ${ARCH}
+		rval=$?
+		# If the module failed installation, go to the next module.
+		if [ $rval -eq 0 ]; then
+			# dkms runs depmod as part of the installation
+			unset do_depmod
 			echo "done."
 		else
 			echo "FAILED!"
-			exit $?
+			shift 2; continue
 		fi
 	fi
-	shift; shift
+
+	# Go to the next module - all steps for this module were successful.
+	shift 2
 done
 
+if [ -n "$do_depmod" ]; then
+	echo -n "Generating kernel module dependency lists... "
+	depmod -a ${VERSION}
+	rval=$?
+	if [ $rval -eq 0 ]; then
+		echo "done."
+	else
+		echo "FAILED!"
+		exit $rval
+	fi
+fi
+
 exit 0
diff --git a/srcpkgs/dkms/template b/srcpkgs/dkms/template
index 521dfde1114..32345b9b421 100644
--- a/srcpkgs/dkms/template
+++ b/srcpkgs/dkms/template
@@ -2,7 +2,7 @@
 pkgname=dkms
 reverts="2.8.2_1"
 version=2.8.1
-revision=3
+revision=4
 conf_files="/etc/dkms/framework.conf"
 depends="bash kmod gcc make coreutils linux-headers"
 short_desc="Dynamic Kernel Modules System"

From 5ab81caca0916759e941d8f3c5ca5d7259aaed71 Mon Sep 17 00:00:00 2001
From: "Andrew J. Hesford" <ajh@sideband.org>
Date: Fri, 10 Jul 2020 21:34:34 -0400
Subject: [PATCH 2/2] Rework dkms post-install provided by ericonr

---
 srcpkgs/dkms/files/kernel.d/dkms.postinst | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/srcpkgs/dkms/files/kernel.d/dkms.postinst b/srcpkgs/dkms/files/kernel.d/dkms.postinst
index c8a8deae0d4..c5c28b59b27 100644
--- a/srcpkgs/dkms/files/kernel.d/dkms.postinst
+++ b/srcpkgs/dkms/files/kernel.d/dkms.postinst
@@ -50,9 +50,11 @@ done
 # [module1, modulever1, module2, modulever2, ...]
 #
 set -- ${DKMS_MODULES}
-while [ $# -ne 0 ]; do
+while [ $# -gt 1 ]; do
 	module="$1"
-	modulever="$2"
+	shift
+	modulever="$1"
+	shift
 
 	# If adding a module, depmod is necessary unless dkms runs it
 	do_depmod="yes"
@@ -62,7 +64,7 @@ while [ $# -ne 0 ]; do
 		# Check if the module is still there.
 		if [ ! -f usr/src/${module}-${modulever}/dkms.conf ]; then
 			echo "Skipping unexistent DKMS module: ${module}-${modulever}."
-			shift 2; continue
+			continue
 		fi
 		# Build the module
 		echo -n "Building DKMS module: ${module}-${modulever}... "
@@ -73,10 +75,10 @@ while [ $# -ne 0 ]; do
 			echo "done."
 		elif [ $rval -eq 9 ]; then
 			echo "skipped!"
-			shift 2; continue
+			continue
 		else
 			echo "FAILED!"
-			shift 2; continue
+			continue
 		fi
 		status=$(dkms status -m ${module} -v ${modulever} -k ${VERSION})
 	fi
@@ -94,12 +96,9 @@ while [ $# -ne 0 ]; do
 			echo "done."
 		else
 			echo "FAILED!"
-			shift 2; continue
+			continue
 		fi
 	fi
-
-	# Go to the next module - all steps for this module were successful.
-	shift 2
 done
 
 if [ -n "$do_depmod" ]; then

Re: [PR PATCH] [Updated] dkms: improvements to post-install hook: run depmod if necessary and better handle DKMS build/install failures

[ahesford]

[-- Attachment #1: Type: text/plain, Size: 1941 bytes --]

There is an updated pull request by ahesford against master on the void-packages repository

https://github.com/ahesford/void-packages dkms
https://github.com/void-linux/void-packages/pull/23392

dkms: improvements to post-install hook: run depmod if necessary and better handle DKMS build/install failures
Normally, `dkms install` will run depmod with each installed module to update module dependency lists. When force-reinstalling kernel packages, `dkms install` will not run because modules are already marked installed, but the kernel package will overwrite `modules.dep` and `modules.dep.bin` with packaged versions that do not include any modules installed by dkms. The kernel post-install hook now tracks whether depmod should be run and will do so if necessary.

To demonstrate the original problem:

1. Install `zfs` and enable the dracut zfs module with `add_dracutmodules+=" zfs "` in `dracut.conf` or a `.conf` file in `/etc/dracut.conf.d`.
2. Rerun `dracut`, e.g., with `xbps-reconfigure -f linuxX.Y` for an appropriate kernel package.
3. Confirm that `zfs.ko.gz` is in the newly generated initramfs.
4. Force-reinstall the kernel package: `xbps-install -f linuxX.Y`.
5. Confirm that `zfs.ko.gz` is *missing* from the newly generated initramfs.

With the changes in the hook, the force-installation of `linuxX.Y` should display `Generating kernel module dependency lists... done.` after "building" all DKMS modules (note: the modules were already built for the reinstalled kernel, so the `dkms build` run by the hook is basically a no-op). The resulting initramfs will contain `zfs.ko.gz` as expected.

Also, I propagated some return codes from `dkms` through the hook, which I believe was the original intention. (Before the fix, `exit $?` after an `echo` would generally always exit 0 because `echo` is unlikely to fail.)

A patch file from https://github.com/void-linux/void-packages/pull/23392.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-dkms-23392.patch --]
[-- Type: text/x-diff, Size: 4178 bytes --]

From c1fef7e8ac67c793c667f09b931782af25426eb9 Mon Sep 17 00:00:00 2001
From: "Andrew J. Hesford" <ajh@sideband.org>
Date: Sat, 4 Jul 2020 16:33:31 -0400
Subject: [PATCH] dkms: improvements to kernel post-install hook

When force-reinstalling kernel packages, `dkms install` is not run in
the kernel post-install hook because the modules are already installed.
This prevents `dkms install` from running `depmod -a` as it normally
does, but the forced reinstallation clobbers `modules.dep{,.bin}` with
the packaged versions that do not include DKMS-installed modules. The
post-install hook now tracks whether `depmod -a` should be run and does
so when necessary.

Other changes, courtesy of @ericonr, address issue #23124 wherein a DKMS
failure will terminate the script, preventing building or installation
of subsequent modules. The fix follows logic similar to that in the DKMS
xbps-trigger.
---
 srcpkgs/dkms/files/kernel.d/dkms.postinst | 54 +++++++++++++++++------
 srcpkgs/dkms/template                     |  2 +-
 2 files changed, 42 insertions(+), 14 deletions(-)

diff --git a/srcpkgs/dkms/files/kernel.d/dkms.postinst b/srcpkgs/dkms/files/kernel.d/dkms.postinst
index bfeaf34fccf..c5c28b59b27 100644
--- a/srcpkgs/dkms/files/kernel.d/dkms.postinst
+++ b/srcpkgs/dkms/files/kernel.d/dkms.postinst
@@ -40,49 +40,77 @@ for _mod_ in /var/lib/dkms/*; do
 	done
 done
 
+# This sections builds and install the available modules.
+#
+# If either building or installing a module fails, a warning is
+# printed and it is skipped, but the script still tries to build
+# the other modules.
+#
+# The list of available modules is in the form
+# [module1, modulever1, module2, modulever2, ...]
+#
 set -- ${DKMS_MODULES}
-while [ $# -ne 0 ]; do
+while [ $# -gt 1 ]; do
 	module="$1"
-	modulever="$2"
+	shift
+	modulever="$1"
+	shift
+
+	# If adding a module, depmod is necessary unless dkms runs it
+	do_depmod="yes"
 
 	status=$(dkms status -m ${module} -v ${modulever} -k ${VERSION})
 	if [ $(echo "$status"|grep -c ": built") -eq 0 ]; then
 		# Check if the module is still there.
 		if [ ! -f usr/src/${module}-${modulever}/dkms.conf ]; then
 			echo "Skipping unexistent DKMS module: ${module}-${modulever}."
-			shift 2
 			continue
 		fi
 		# Build the module
 		echo -n "Building DKMS module: ${module}-${modulever}... "
 		dkms build -q -m ${module} -v ${modulever} -k ${VERSION} -a ${ARCH}
 		rval=$?
-		if [ $rval -eq 9 ]; then
+		# If the module was skipped or failed, go to the next module.
+		if [ $rval -eq 0 ]; then
+			echo "done."
+		elif [ $rval -eq 9 ]; then
 			echo "skipped!"
-			shift; shift
 			continue
-		elif [ $rval -eq 0 ]; then
-			echo "done."
 		else
 			echo "FAILED!"
-			exit $?
+			continue
 		fi
 		status=$(dkms status -m ${module} -v ${modulever} -k ${VERSION})
 	fi
 
-	#if the module is built (either pre-built or just now), install it
+	# If the module is built (either pre-built or just now), install it
 	if [ $(echo "$status"|grep -c ": built") -eq 1 ] &&
 	   [ $(echo "$status"|grep -c ": installed") -eq 0 ]; then
 		echo -n "Installing DKMS module: ${module}-${modulever}... "
-	        dkms install -q -m ${module} -v ${modulever} -k ${VERSION} -a ${ARCH}
-		if [ $? -eq 0 ]; then
+		dkms install -q -m ${module} -v ${modulever} -k ${VERSION} -a ${ARCH}
+		rval=$?
+		# If the module failed installation, go to the next module.
+		if [ $rval -eq 0 ]; then
+			# dkms runs depmod as part of the installation
+			unset do_depmod
 			echo "done."
 		else
 			echo "FAILED!"
-			exit $?
+			continue
 		fi
 	fi
-	shift; shift
 done
 
+if [ -n "$do_depmod" ]; then
+	echo -n "Generating kernel module dependency lists... "
+	depmod -a ${VERSION}
+	rval=$?
+	if [ $rval -eq 0 ]; then
+		echo "done."
+	else
+		echo "FAILED!"
+		exit $rval
+	fi
+fi
+
 exit 0
diff --git a/srcpkgs/dkms/template b/srcpkgs/dkms/template
index 521dfde1114..32345b9b421 100644
--- a/srcpkgs/dkms/template
+++ b/srcpkgs/dkms/template
@@ -2,7 +2,7 @@
 pkgname=dkms
 reverts="2.8.2_1"
 version=2.8.1
-revision=3
+revision=4
 conf_files="/etc/dkms/framework.conf"
 depends="bash kmod gcc make coreutils linux-headers"
 short_desc="Dynamic Kernel Modules System"

Re: [PR REVIEW] dkms: improvements to post-install hook: run depmod if necessary and better handle DKMS build/install failures

[ahesford]

[-- Attachment #1: Type: text/plain, Size: 272 bytes --]

New review comment by ahesford on void-packages repository

https://github.com/void-linux/void-packages/pull/23392#discussion_r453141671

Comment:
Reworked to avoid the second test while still verifying that the number of arguments is large enough to avoid a shift error.

Re: [PR PATCH] [Updated] dkms: improvements to post-install hook: run depmod if necessary and better handle DKMS build/install failures

[ahesford]

[-- Attachment #1: Type: text/plain, Size: 1941 bytes --]

There is an updated pull request by ahesford against master on the void-packages repository

https://github.com/ahesford/void-packages dkms
https://github.com/void-linux/void-packages/pull/23392

dkms: improvements to post-install hook: run depmod if necessary and better handle DKMS build/install failures
Normally, `dkms install` will run depmod with each installed module to update module dependency lists. When force-reinstalling kernel packages, `dkms install` will not run because modules are already marked installed, but the kernel package will overwrite `modules.dep` and `modules.dep.bin` with packaged versions that do not include any modules installed by dkms. The kernel post-install hook now tracks whether depmod should be run and will do so if necessary.

To demonstrate the original problem:

1. Install `zfs` and enable the dracut zfs module with `add_dracutmodules+=" zfs "` in `dracut.conf` or a `.conf` file in `/etc/dracut.conf.d`.
2. Rerun `dracut`, e.g., with `xbps-reconfigure -f linuxX.Y` for an appropriate kernel package.
3. Confirm that `zfs.ko.gz` is in the newly generated initramfs.
4. Force-reinstall the kernel package: `xbps-install -f linuxX.Y`.
5. Confirm that `zfs.ko.gz` is *missing* from the newly generated initramfs.

With the changes in the hook, the force-installation of `linuxX.Y` should display `Generating kernel module dependency lists... done.` after "building" all DKMS modules (note: the modules were already built for the reinstalled kernel, so the `dkms build` run by the hook is basically a no-op). The resulting initramfs will contain `zfs.ko.gz` as expected.

Also, I propagated some return codes from `dkms` through the hook, which I believe was the original intention. (Before the fix, `exit $?` after an `echo` would generally always exit 0 because `echo` is unlikely to fail.)

A patch file from https://github.com/void-linux/void-packages/pull/23392.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-dkms-23392.patch --]
[-- Type: text/x-diff, Size: 4246 bytes --]

From 2bec57b6712f6587e94463b19b71048664a9e2ed Mon Sep 17 00:00:00 2001
From: "Andrew J. Hesford" <ajh@sideband.org>
Date: Sat, 4 Jul 2020 16:33:31 -0400
Subject: [PATCH] dkms: improvements to kernel post-install hook

When force-reinstalling kernel packages, `dkms install` is not run in
the kernel post-install hook because the modules are already installed.
This prevents `dkms install` from running `depmod -a` as it normally
does, but the forced reinstallation clobbers `modules.dep{,.bin}` with
the packaged versions that do not include DKMS-installed modules. The
post-install hook now tracks whether `depmod -a` should be run and does
so when necessary.

Other changes, courtesy of @ericonr, address issue #23124 wherein a DKMS
failure will terminate the script, preventing building or installation
of subsequent modules. The fix follows logic similar to that in the DKMS
xbps-trigger.
---
 srcpkgs/dkms/files/kernel.d/dkms.postinst | 56 +++++++++++++++++------
 srcpkgs/dkms/template                     |  2 +-
 2 files changed, 43 insertions(+), 15 deletions(-)

diff --git a/srcpkgs/dkms/files/kernel.d/dkms.postinst b/srcpkgs/dkms/files/kernel.d/dkms.postinst
index bfeaf34fccf..5cfc630de1c 100644
--- a/srcpkgs/dkms/files/kernel.d/dkms.postinst
+++ b/srcpkgs/dkms/files/kernel.d/dkms.postinst
@@ -40,49 +40,77 @@ for _mod_ in /var/lib/dkms/*; do
 	done
 done
 
+# This section builds and install the available modules.
+#
+# If either building or installing a module fails, a warning is
+# printed and it is skipped, but the script still tries to build
+# the other modules.
+#
+# The list of available modules is in the form
+# [module1, modulever1, module2, modulever2, ...]
+#
 set -- ${DKMS_MODULES}
-while [ $# -ne 0 ]; do
+while [ $# -gt 1 ]; do
 	module="$1"
-	modulever="$2"
+	shift
+	modulever="$1"
+	shift
+
+	# If adding a module, depmod is necessary unless dkms runs it
+	do_depmod="yes"
 
 	status=$(dkms status -m ${module} -v ${modulever} -k ${VERSION})
 	if [ $(echo "$status"|grep -c ": built") -eq 0 ]; then
 		# Check if the module is still there.
 		if [ ! -f usr/src/${module}-${modulever}/dkms.conf ]; then
-			echo "Skipping unexistent DKMS module: ${module}-${modulever}."
-			shift 2
+			echo "Skipping nonexistent DKMS module: ${module}-${modulever}."
 			continue
 		fi
 		# Build the module
 		echo -n "Building DKMS module: ${module}-${modulever}... "
 		dkms build -q -m ${module} -v ${modulever} -k ${VERSION} -a ${ARCH}
 		rval=$?
-		if [ $rval -eq 9 ]; then
+		# If the module was skipped or failed, go to the next module.
+		if [ $rval -eq 0 ]; then
+			echo "done."
+		elif [ $rval -eq 9 ]; then
 			echo "skipped!"
-			shift; shift
 			continue
-		elif [ $rval -eq 0 ]; then
-			echo "done."
 		else
 			echo "FAILED!"
-			exit $?
+			continue
 		fi
 		status=$(dkms status -m ${module} -v ${modulever} -k ${VERSION})
 	fi
 
-	#if the module is built (either pre-built or just now), install it
+	# If the module is built (either pre-built or just now), install it
 	if [ $(echo "$status"|grep -c ": built") -eq 1 ] &&
 	   [ $(echo "$status"|grep -c ": installed") -eq 0 ]; then
 		echo -n "Installing DKMS module: ${module}-${modulever}... "
-	        dkms install -q -m ${module} -v ${modulever} -k ${VERSION} -a ${ARCH}
-		if [ $? -eq 0 ]; then
+		dkms install -q -m ${module} -v ${modulever} -k ${VERSION} -a ${ARCH}
+		rval=$?
+		# If the module failed installation, go to the next module.
+		if [ $rval -eq 0 ]; then
+			# dkms runs depmod as part of the installation
+			unset do_depmod
 			echo "done."
 		else
 			echo "FAILED!"
-			exit $?
+			continue
 		fi
 	fi
-	shift; shift
 done
 
+if [ -n "$do_depmod" ]; then
+	echo -n "Generating kernel module dependency lists... "
+	depmod -a ${VERSION}
+	rval=$?
+	if [ $rval -eq 0 ]; then
+		echo "done."
+	else
+		echo "FAILED!"
+		exit $rval
+	fi
+fi
+
 exit 0
diff --git a/srcpkgs/dkms/template b/srcpkgs/dkms/template
index 521dfde1114..32345b9b421 100644
--- a/srcpkgs/dkms/template
+++ b/srcpkgs/dkms/template
@@ -2,7 +2,7 @@
 pkgname=dkms
 reverts="2.8.2_1"
 version=2.8.1
-revision=3
+revision=4
 conf_files="/etc/dkms/framework.conf"
 depends="bash kmod gcc make coreutils linux-headers"
 short_desc="Dynamic Kernel Modules System"

Re: [PR PATCH] [Merged]: dkms: improvements to post-install hook: run depmod if necessary and better handle DKMS build/install failures

[ahesford]

[-- Attachment #1: Type: text/plain, Size: 1786 bytes --]

There's a merged pull request on the void-packages repository

dkms: improvements to post-install hook: run depmod if necessary and better handle DKMS build/install failures
https://github.com/void-linux/void-packages/pull/23392

Description:
Normally, `dkms install` will run depmod with each installed module to update module dependency lists. When force-reinstalling kernel packages, `dkms install` will not run because modules are already marked installed, but the kernel package will overwrite `modules.dep` and `modules.dep.bin` with packaged versions that do not include any modules installed by dkms. The kernel post-install hook now tracks whether depmod should be run and will do so if necessary.

To demonstrate the original problem:

1. Install `zfs` and enable the dracut zfs module with `add_dracutmodules+=" zfs "` in `dracut.conf` or a `.conf` file in `/etc/dracut.conf.d`.
2. Rerun `dracut`, e.g., with `xbps-reconfigure -f linuxX.Y` for an appropriate kernel package.
3. Confirm that `zfs.ko.gz` is in the newly generated initramfs.
4. Force-reinstall the kernel package: `xbps-install -f linuxX.Y`.
5. Confirm that `zfs.ko.gz` is *missing* from the newly generated initramfs.

With the changes in the hook, the force-installation of `linuxX.Y` should display `Generating kernel module dependency lists... done.` after "building" all DKMS modules (note: the modules were already built for the reinstalled kernel, so the `dkms build` run by the hook is basically a no-op). The resulting initramfs will contain `zfs.ko.gz` as expected.

Also, I propagated some return codes from `dkms` through the hook, which I believe was the original intention. (Before the fix, `exit $?` after an `echo` would generally always exit 0 because `echo` is unlikely to fail.)