On Fri, Jul 14, 2023 at 09:37:04PM +0200, Nicolas Schier wrote:

On Fri, Jul 14, 2023 at 05:10:42PM +0200 Michal Suchánek wrote:

quoted

On Fri, Jul 14, 2023 at 04:54:49PM +0200, Nicolas Schier wrote:

quoted

On Fri, Jul 14, 2023 at 04:30:02PM +0200, Michal Such�nek wrote:

quoted

Hello,

On Fri, Jul 14, 2023 at 04:05:10PM +0200, Nicolas Schier wrote:

quoted

On Fri, Jul 14, 2023 at 02:21:08PM +0200 Michal Suchanek wrote:

quoted

Some distributions aim at not shipping any files in / outside of usr.

For me, preventing negation often makes things easier, e.g.: "... aim at
shipping files only below /usr".

quoted

The path under which kernel modules are installed is hardcoded to /lib
which conflicts with this goal.

When kmod provides the config command, use it to determine the correct
module installation prefix.

This is a prefix under which the modules are searched by kmod on the
system, and is separate from the temporary staging location already
supported by INSTALL_MOD_PATH.

With kmod that does not provide the config command empty prefix is used
as before.

Signed-off-by: Michal Suchanek <redacted>
---
v2: Avoid error on systems with kmod that does not support config
command
v3: More verbose commit message
---
 Makefile          | 4 +++-
 scripts/depmod.sh | 8 ++++----
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 47690c28456a..b1fea135bdec 100644
--- a/Makefile
+++ b/Makefile

@@ -1165,7 +1165,9 @@ export INSTALL_DTBS_PATH ?= $(INSTALL_PATH)/dtbs/$(KERNELRELEASE)
 # makefile but the argument can be passed to make if needed.
 #
 
-MODLIB	= $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE)
+export KERNEL_MODULE_PREFIX := $(shell kmod config &> /dev/null && kmod config | jq -r .module_prefix)

oh, should this be 'jq -r .prefix' (w/o ".module") to match your other patches?

No, this aligns perfectly fine, prefix is where kmod is installed.

quoted

quoted

quoted

quoted

All other calls of `jq` that I could find are located at tools/; as this here
is evaluated on each invocation, this should probably be documented in
Documentation/process/changes.rst?

(Absence of `jq` will cause error messages, even with CONFIG_MODULES=n.)

That's a good point.

quoted

quoted

+
+MODLIB	= $(INSTALL_MOD_PATH)$(KERNEL_MODULE_PREFIX)/lib/modules/$(KERNELRELEASE)
 export MODLIB
 
 PHONY += prepare0

diff --git a/scripts/depmod.sh b/scripts/depmod.sh
index 3643b4f896ed..88ac79056153 100755
--- a/scripts/depmod.sh
+++ b/scripts/depmod.sh

@@ -27,16 +27,16 @@ fi
 # numbers, so we cheat with a symlink here
 depmod_hack_needed=true
 tmp_dir=$(mktemp -d ${TMPDIR:-/tmp}/depmod.XXXXXX)
-mkdir -p "$tmp_dir/lib/modules/$KERNELRELEASE"
+mkdir -p "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE"
 if "$DEPMOD" -b "$tmp_dir" $KERNELRELEASE 2>/dev/null; then
-	if test -e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep" -o \
-		-e "$tmp_dir/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
+	if test -e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep" -o \
+		-e "$tmp_dir$KERNEL_MODULE_PREFIX/lib/modules/$KERNELRELEASE/modules.dep.bin"; then
 		depmod_hack_needed=false
 	fi
 fi

I'd like to come back to the statement from Masahiro: Is the check above,
against some very old versions of depmod [1], the only reason for this patch?  

If we could remove that, would

    make INSTALL_MOD_PATH="$(kmod config | jq -r .module_prefix)" modules_install

be sufficient?

No, the INSTALL_MOD_PATH is passed as the -b argument to depmod while
the newly added part is not because it's integral part of where the
modules are installed on the system, and not the staging area path.

Ah, thanks.  So just for my understanding, could this be a (non-gentle)
alternative version of your patch, w/o modifying top-level Makefile?

diff --git a/scripts/depmod.sh b/scripts/depmod.sh
index 3643b4f896ed..72c819de0669 100755
--- a/scripts/depmod.sh
+++ b/scripts/depmod.sh

@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
 # SPDX-License-Identifier: GPL-2.0
 #
 # A depmod wrapper used by the toplevel Makefile

@@ -23,6 +23,8 @@ if [ -z $(command -v $DEPMOD) ]; then
        exit 0
 fi
 
+kmod_version=$(( $(kmod --version | sed -rne 's/^kmod version ([0-9]+).*$/\1/p') ))
+
 # older versions of depmod require the version string to start with three
 # numbers, so we cheat with a symlink here
 depmod_hack_needed=true

@@ -35,6 +37,13 @@ if "$DEPMOD" -b "$tmp_dir" $KERNELRELEASE 2>/dev/null; then
        fi
 fi
 rm -rf "$tmp_dir"
+
+if [ "${kmod_version}" -gt 32 ]; then
+       kmod_prefix="$(kmod config | jq -r .module_prefix)"
+       INSTALL_MOD_PATH="${INSTALL_MOD_PATH#${kmod_prefix}"
+       depmod_hack_needed=false
+fi
+
 if $depmod_hack_needed; then
        symlink="$INSTALL_MOD_PATH/lib/modules/99.98.$KERNELRELEASE"
        ln -s "$KERNELRELEASE" "$symlink"

(untested, and assuming that kmod module prefix is in kmod >= 32)

It can be detected by running the 'kmod config' command first and
ignoring the output when it fails which the above patch already did.
The version check does not sound very reliable.

quoted

Or are I am still missing something?

MODLIB still needs to include the extra prefix so that files are
installed in the correct location. And that's defined in the toplevel
Makefile.

Well, I think that depends.  Technically, you are right; and if we want
to support system with a non-empty kmod prefix fully transparently, then
patching top-level Makefile will probably be necessary.  

As for me, I am not convinced yet, that the fully transparent way to support
PREFIX/lib/modules/ is the best way forward.  I think it might be better to
first only make script/depmod.sh fit for a kmod prefix and require an adjusted
INSTALL_MOD_PATH for modules_install.

Nevermind, I will update the patch to change the whole path. That will
make it crystal clear that no amount of fiddling with INSTALL_MOD_PATH
will work.

Which concrete distributions did you have in mind while composing the patches?

If you STFW for usrmerge you can find that a number of distributions is
in different stages of experimenting with packing all shipped files into
/usr.

In the early experiment stages when the distribution is built with /lib
/bin, and /sbin in place and massaged after the fact to remove these
directories it does not matter much where files were installed
initially.

In later stages when the distribution is built without these extra
directories they are missing in the staging area for building packages,
and creating them is an error because files in these directories are not
to be shipped in packages. At this point the kernel insisting that
modules must be in /lib is sticking out like a sore thumb, it's on every
system.

Sure, there is a number of hacks that can be used to work around the
problem. Still it's a problem the needs to be addressed for usrmerge to
fully work.

openSUSE is maybe 75% there - it's nowhere near complete but the
packages that insist on using these directories outside of /usr are
becoming problematic.

Thanks

Michal