Makefile | 4 +++- scripts/depmod.sh | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-)
Some distributions aim at not shipping any files in / ustside of usr.
The path under which kernel modules are instaleld is hardcoded to /lib
which conflicts with this goal.
When kmod provides the config command use it to determine the correct
module installation prefix.
On kmod that does not provide the command / is used as before.
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
v2: Avoid error on systems with kmod that does not support config
command
---
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)
+
+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
rm -rf "$tmp_dir"
if $depmod_hack_needed; then
- symlink="$INSTALL_MOD_PATH/lib/modules/99.98.$KERNELRELEASE"
+ symlink="$INSTALL_MOD_PATH$KERNEL_MODULE_PREFIX/lib/modules/99.98.$KERNELRELEASE"
ln -s "$KERNELRELEASE" "$symlink"
KERNELRELEASE=99.98.$KERNELRELEASE
fi
--
2.41.0On Wed, Jul 12, 2023 at 10:45 PM Michal Suchanek <msuchanek@suse.de> wrote:
>
> Some distributions aim at not shipping any files in / ustside of usr.
>
> The path under which kernel modules are instaleld is hardcoded to /lib
> which conflicts with this goal.
>
> When kmod provides the config command use it to determine the correct
> module installation prefix.
>
> On kmod that does not provide the command / is used as before.
>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> v2: Avoid error on systems with kmod that does not support config
> command
> ---
> 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)
> +
> +MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_PREFIX)/lib/modules/$(KERNELRELEASE)
You can do "make modules_install INSTALL_MOD_PATH=/usr/what/ever/prefix"
This patch is unneeded.
> 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
> rm -rf "$tmp_dir"
> if $depmod_hack_needed; then
> - symlink="$INSTALL_MOD_PATH/lib/modules/99.98.$KERNELRELEASE"
> + symlink="$INSTALL_MOD_PATH$KERNEL_MODULE_PREFIX/lib/modules/99.98.$KERNELRELEASE"
> ln -s "$KERNELRELEASE" "$symlink"
> KERNELRELEASE=99.98.$KERNELRELEASE
> fi
> --
> 2.41.0
>
--
Best Regards
Masahiro Yamada
On Wed, Jul 12, 2023 at 11:14:33PM +0900, Masahiro Yamada wrote:
> On Wed, Jul 12, 2023 at 10:45 PM Michal Suchanek <msuchanek@suse.de> wrote:
> >
> > Some distributions aim at not shipping any files in / ustside of usr.
> >
> > The path under which kernel modules are instaleld is hardcoded to /lib
> > which conflicts with this goal.
> >
> > When kmod provides the config command use it to determine the correct
> > module installation prefix.
> >
> > On kmod that does not provide the command / is used as before.
> >
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> > v2: Avoid error on systems with kmod that does not support config
> > command
> > ---
> > 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)
> > +
> > +MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_PREFIX)/lib/modules/$(KERNELRELEASE)
>
> You can do "make modules_install INSTALL_MOD_PATH=/usr/what/ever/prefix"
>
> This patch is unneeded.
It's very much needed.
INSTALL_MOD_PATH is temporary staging location, KERNEL_MODULE_PREFIX is
permanent prefix under which modules are searched.
Note that depmod.sh does not use INSTALL_MOD_PATH but uses
KERNEL_MODULE_PREFIX.
Support for this is added by the corresponding kmod patchset.
Thanks
Michal
> > 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
> > rm -rf "$tmp_dir"
> > if $depmod_hack_needed; then
> > - symlink="$INSTALL_MOD_PATH/lib/modules/99.98.$KERNELRELEASE"
> > + symlink="$INSTALL_MOD_PATH$KERNEL_MODULE_PREFIX/lib/modules/99.98.$KERNELRELEASE"
> > ln -s "$KERNELRELEASE" "$symlink"
> > KERNELRELEASE=99.98.$KERNELRELEASE
> > fi
> > --
> > 2.41.0
> >
>
>
> --
> Best Regards
> Masahiro Yamada
On 12. 07. 23, 15:45, Michal Suchanek wrote:
> Some distributions aim at not shipping any files in / ustside of usr.
"outside".
> The path under which kernel modules are instaleld is hardcoded to /lib
"installed"
> which conflicts with this goal.
>
> When kmod provides the config command use it to determine the correct
> module installation prefix.
>
> On kmod that does not provide the command / is used as before.
Can you spice it with more commas? While the text is understandable
after a while of staring, it's hard to parse.
Like:
When kmod provides the config command, use it to determine the correct
module installation prefix.
On kmod that does not provide the command, / is used as before.
I would also argue here in the commit log on what Masahiro already
pointed out. I.e. that INSTALL_MOD_PATH is useless in this case and why.
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> v2: Avoid error on systems with kmod that does not support config
> command
> ---
> 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)
> +
> +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
> rm -rf "$tmp_dir"
> if $depmod_hack_needed; then
> - symlink="$INSTALL_MOD_PATH/lib/modules/99.98.$KERNELRELEASE"
> + symlink="$INSTALL_MOD_PATH$KERNEL_MODULE_PREFIX/lib/modules/99.98.$KERNELRELEASE"
> ln -s "$KERNELRELEASE" "$symlink"
> KERNELRELEASE=99.98.$KERNELRELEASE
> fi
--
js
suse labs
Some distributions aim at not shipping any files in / outside of usr.
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 <msuchanek@suse.de>
---
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)
+
+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
rm -rf "$tmp_dir"
if $depmod_hack_needed; then
- symlink="$INSTALL_MOD_PATH/lib/modules/99.98.$KERNELRELEASE"
+ symlink="$INSTALL_MOD_PATH$KERNEL_MODULE_PREFIX/lib/modules/99.98.$KERNELRELEASE"
ln -s "$KERNELRELEASE" "$symlink"
KERNELRELEASE=99.98.$KERNELRELEASE
fi
--
2.41.0On Friday 2023-07-14 14:21, Michal Suchanek wrote: >Some distributions aim at not shipping any files in / outside of usr. > >The path under which kernel modules are installed is hardcoded to /lib >which conflicts with this goal. > >+MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_PREFIX)/lib/modules/$(KERNELRELEASE) Ok, so if the problem statement is that hardcoded paths are bad, then why continue to hardcode the "/lib/modules" fragment? Just make it so that KERNEL_MODULE_PREFIX can be set to the exact string "/usr/lib/modules" and not just "/usr".
Hello, On Fri, Jul 14, 2023 at 03:38:18PM +0200, Jan Engelhardt wrote: > > On Friday 2023-07-14 14:21, Michal Suchanek wrote: > > >Some distributions aim at not shipping any files in / outside of usr. > > > >The path under which kernel modules are installed is hardcoded to /lib > >which conflicts with this goal. > > > >+MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_PREFIX)/lib/modules/$(KERNELRELEASE) > > Ok, so if the problem statement is that hardcoded paths are bad, then why > continue to hardcode the "/lib/modules" fragment? Just make it so that > KERNEL_MODULE_PREFIX can be set to the exact string "/usr/lib/modules" and not > just "/usr". That's certainly an option. The feature is modelled after the installation prefix option that can move the whole filesystem hierarchy of installed files under /usr/local, /opt, or any other directory of choice. However, in that case the subdirectories in the hierarchy can be configured as well while in this case /lib/modules remains hardcoded. Making it possible to set the whole path is generally more flexible although there is no need to set the later part for this particular use case. Thanks Michal
On Fri, Jul 14, 2023 at 03:38:18PM +0200, Jan Engelhardt <jengelh@inai.de> wrote: > Ok, so if the problem statement is that hardcoded paths are bad, then why > continue to hardcode the "/lib/modules" fragment? Just make it so that > KERNEL_MODULE_PREFIX can be set to the exact string "/usr/lib/modules" and not > just "/usr". That sounds cleaner but I'm worried it would be a BC break in setups that expect the existing layout under INSTALL_MOD_PATH, wouldn't it? Michal
Hello, On Fri, Jul 14, 2023 at 03:59:17PM +0200, Michal Koutný wrote: > On Fri, Jul 14, 2023 at 03:38:18PM +0200, Jan Engelhardt <jengelh@inai.de> wrote: > > Ok, so if the problem statement is that hardcoded paths are bad, then why > > continue to hardcode the "/lib/modules" fragment? Just make it so that > > KERNEL_MODULE_PREFIX can be set to the exact string "/usr/lib/modules" and not > > just "/usr". > > That sounds cleaner but I'm worried it would be a BC break in setups > that expect the existing layout under INSTALL_MOD_PATH, wouldn't it? It's a break either way, the expected directory righ now is exactly /lib/modules. /usr/lib/modules works to some extent for some use cases only when the compatibility symlink lib -> usr/lib is present. Thanks Michal
On Fri, Jul 14, 2023 at 02:21:08PM +0200 Michal Suchanek wrote:
> 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".
>
> 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 <msuchanek@suse.de>
> ---
> 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)
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.)
> +
> +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?
Kind regards,
Nicolas
[1]: https://lore.kernel.org/linux-kbuild/1307631448-29848-5-git-send-email-mmarek@suse.cz/
> rm -rf "$tmp_dir"
> if $depmod_hack_needed; then
> - symlink="$INSTALL_MOD_PATH/lib/modules/99.98.$KERNELRELEASE"
> + symlink="$INSTALL_MOD_PATH$KERNEL_MODULE_PREFIX/lib/modules/99.98.$KERNELRELEASE"
> ln -s "$KERNELRELEASE" "$symlink"
> KERNELRELEASE=99.98.$KERNELRELEASE
> fi
> --
> 2.41.0
--
epost|xmpp: nicolas@fjasle.eu irc://oftc.net/nsc
↳ gpg: 18ed 52db e34f 860e e9fb c82b 7d97 0932 55a0 ce7f
-- frykten for herren er opphav til kunnskap --
Hello,
On Fri, Jul 14, 2023 at 04:05:10PM +0200, Nicolas Schier wrote:
> On Fri, Jul 14, 2023 at 02:21:08PM +0200 Michal Suchanek wrote:
> > 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".
>
> >
> > 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 <msuchanek@suse.de>
> > ---
> > 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)
>
> 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.
>
> > +
> > +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.
>
> Kind regards,
> Nicolas
>
>
> [1]: https://lore.kernel.org/linux-kbuild/1307631448-29848-5-git-send-email-mmarek@suse.cz/
Was busybox ever fixed to not require the hack?
Thanks
Michal
On Friday 2023-07-14 16:30, Michal Suchánek wrote: >> > >> > -MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE) >> > +export KERNEL_MODULE_PREFIX := $(shell kmod config &> /dev/null && kmod config | jq -r .module_prefix) >> >> 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. Speaking of which, "&>" is a bashism and probably should be replaced.
On Fri, Jul 14, 2023 at 04:30:02PM +0200, Michal Such�nek wrote:
> Hello,
>
> On Fri, Jul 14, 2023 at 04:05:10PM +0200, Nicolas Schier wrote:
> > On Fri, Jul 14, 2023 at 02:21:08PM +0200 Michal Suchanek wrote:
> > > 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".
> >
> > >
> > > 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 <msuchanek@suse.de>
> > > ---
> > > 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)
> >
> > 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.
>
> >
> > > +
> > > +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)
Or are I am still missing something?
> Was busybox ever fixed to not require the hack?
I haven't checked that, yet.
Kind regards,
Nicolas
On Fri, Jul 14, 2023 at 04:54:49PM +0200, Nicolas Schier wrote:
> On Fri, Jul 14, 2023 at 04:30:02PM +0200, Michal Such�nek wrote:
> > Hello,
> >
> > On Fri, Jul 14, 2023 at 04:05:10PM +0200, Nicolas Schier wrote:
> > > On Fri, Jul 14, 2023 at 02:21:08PM +0200 Michal Suchanek wrote:
> > > > 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".
> > >
> > > >
> > > > 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 <msuchanek@suse.de>
> > > > ---
> > > > 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)
> > >
> > > 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.
> >
> > >
> > > > +
> > > > +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.
> 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.
Thanks
Michal
On Fri, Jul 14, 2023 at 05:10:42PM +0200 Michal Suchánek wrote:
> On Fri, Jul 14, 2023 at 04:54:49PM +0200, Nicolas Schier wrote:
> > On Fri, Jul 14, 2023 at 04:30:02PM +0200, Michal Such�nek wrote:
> > > Hello,
> > >
> > > On Fri, Jul 14, 2023 at 04:05:10PM +0200, Nicolas Schier wrote:
> > > > On Fri, Jul 14, 2023 at 02:21:08PM +0200 Michal Suchanek wrote:
> > > > > 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".
> > > >
> > > > >
> > > > > 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 <msuchanek@suse.de>
> > > > > ---
> > > > > 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?
> > > >
> > > > 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.
> > >
> > > >
> > > > > +
> > > > > +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.
>
> > 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.
Which concrete distributions did you have in mind while composing the patches?
Kind regards,
Nicolas
--
epost|xmpp: nicolas@fjasle.eu irc://oftc.net/nsc
↳ gpg: 18ed 52db e34f 860e e9fb c82b 7d97 0932 55a0 ce7f
-- frykten for herren er opphav til kunnskap --
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:
> > On Fri, Jul 14, 2023 at 04:54:49PM +0200, Nicolas Schier wrote:
> > > On Fri, Jul 14, 2023 at 04:30:02PM +0200, Michal Such�nek wrote:
> > > > Hello,
> > > >
> > > > On Fri, Jul 14, 2023 at 04:05:10PM +0200, Nicolas Schier wrote:
> > > > > On Fri, Jul 14, 2023 at 02:21:08PM +0200 Michal Suchanek wrote:
> > > > > > 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".
> > > > >
> > > > > >
> > > > > > 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 <msuchanek@suse.de>
> > > > > > ---
> > > > > > 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.
>
> > > > >
> > > > > 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.
> > > >
> > > > >
> > > > > > +
> > > > > > +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.
> >
> > > 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
On Sat, Jul 15, 2023 at 12:10 AM Michal Suchánek <msuchanek@suse.de> wrote:
>
> On Fri, Jul 14, 2023 at 04:54:49PM +0200, Nicolas Schier wrote:
> > On Fri, Jul 14, 2023 at 04:30:02PM +0200, Michal Such�nek wrote:
> > > Hello,
> > >
> > > On Fri, Jul 14, 2023 at 04:05:10PM +0200, Nicolas Schier wrote:
> > > > On Fri, Jul 14, 2023 at 02:21:08PM +0200 Michal Suchanek wrote:
> > > > > 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".
> > > >
> > > > >
> > > > > 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 <msuchanek@suse.de>
> > > > > ---
> > > > > 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)
> > > >
> > > > 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.
> > >
> > > >
> > > > > +
> > > > > +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.
>
> > 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.
>
> Thanks
>
> Michal
As Documentation/kbuild/kbuild.rst mentions,
you can override MODLIB.
Kbuild already provides the maximum flexibility.
$ make modules_install \
INSTALL_MOD_PATH=/path/to/whatever/staging/dir \
MODLIB=/path/to/whatever/real/install/destination
--
Best Regards
Masahiro Yamada
On Tue, Jul 18, 2023 at 04:14:11AM +0900, Masahiro Yamada wrote:
> On Sat, Jul 15, 2023 at 12:10 AM Michal Suchánek <msuchanek@suse.de> wrote:
> >
> > On Fri, Jul 14, 2023 at 04:54:49PM +0200, Nicolas Schier wrote:
> > > On Fri, Jul 14, 2023 at 04:30:02PM +0200, Michal Such�nek wrote:
> > > > Hello,
> > > >
> > > > On Fri, Jul 14, 2023 at 04:05:10PM +0200, Nicolas Schier wrote:
> > > > > On Fri, Jul 14, 2023 at 02:21:08PM +0200 Michal Suchanek wrote:
> > > > > > 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".
> > > > >
> > > > > >
> > > > > > 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 <msuchanek@suse.de>
> > > > > > ---
> > > > > > 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)
> > > > >
> > > > > 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.
> > > >
> > > > >
> > > > > > +
> > > > > > +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.
> >
> > > 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.
> >
> > Thanks
> >
> > Michal
>
>
>
> As Documentation/kbuild/kbuild.rst mentions,
> you can override MODLIB.
>
> Kbuild already provides the maximum flexibility.
>
>
> $ make modules_install \
> INSTALL_MOD_PATH=/path/to/whatever/staging/dir \
> MODLIB=/path/to/whatever/real/install/destination
That's great that we have another workaround for the case when the
kernel build system and kmod do not agree on the location of the kernel
modules.
However, they should by default agree, and if this feature is merged
into kmod the kernel should also adapt to make sure the modules are
installed where kmod expects them.
Thanks
Michal
The default MODLIB value is composed of two variables and the hardcoded
string '/lib/modules/'.
MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE)
Defining this middle part as a variable was rejected on the basis that
users can pass the whole MODLIB to make, such as
make 'MODLIB=$(INSTALL_MOD_PATH)/usr/lib/modules/$(KERNELRELEASE)'
However, this middle part of MODLIB is independently hardcoded by
rpm-pkg, and when the user alters MODLIB this is not reflected when
building the package.
Given that $(INSTALL_MOD_PATH) is overridden during the rpm package build
it is likely going to be empty. Then MODLIB can be passed to the rpm
package, and used in place of the whole
/usr/lib/modules/$(KERNELRELEASE) part.
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
scripts/package/kernel.spec | 14 +++++++-------
scripts/package/mkspec | 1 +
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/scripts/package/kernel.spec b/scripts/package/kernel.spec
index ac3f2ee6d7a0..1a727f636f67 100644
--- a/scripts/package/kernel.spec
+++ b/scripts/package/kernel.spec
@@ -67,8 +67,8 @@ cp $(%{make} %{makeflags} -s image_name) %{buildroot}/boot/vmlinuz-%{KERNELRELEA
%{make} %{makeflags} INSTALL_HDR_PATH=%{buildroot}/usr headers_install
cp System.map %{buildroot}/boot/System.map-%{KERNELRELEASE}
cp .config %{buildroot}/boot/config-%{KERNELRELEASE}
-ln -fns /usr/src/kernels/%{KERNELRELEASE} %{buildroot}/lib/modules/%{KERNELRELEASE}/build
-ln -fns /usr/src/kernels/%{KERNELRELEASE} %{buildroot}/lib/modules/%{KERNELRELEASE}/source
+ln -fns /usr/src/kernels/%{KERNELRELEASE} %{buildroot}%{MODLIB}/build
+ln -fns /usr/src/kernels/%{KERNELRELEASE} %{buildroot}%{MODLIB}/source
%if %{with_devel}
%{make} %{makeflags} run-command KBUILD_RUN_COMMAND='${srctree}/scripts/package/install-extmod-build %{buildroot}/usr/src/kernels/%{KERNELRELEASE}'
%endif
@@ -99,9 +99,9 @@ fi
%files
%defattr (-, root, root)
-/lib/modules/%{KERNELRELEASE}
-%exclude /lib/modules/%{KERNELRELEASE}/build
-%exclude /lib/modules/%{KERNELRELEASE}/source
+%{MODLIB}
+%exclude %{MODLIB}/build
+%exclude %{MODLIB}/source
/boot/*
%files headers
@@ -112,6 +112,6 @@ fi
%files devel
%defattr (-, root, root)
/usr/src/kernels/%{KERNELRELEASE}
-/lib/modules/%{KERNELRELEASE}/build
-/lib/modules/%{KERNELRELEASE}/source
+%{MODLIB}/build
+%{MODLIB}/source
%endif
diff --git a/scripts/package/mkspec b/scripts/package/mkspec
index d41608efb747..d41b2e5304ac 100755
--- a/scripts/package/mkspec
+++ b/scripts/package/mkspec
@@ -18,6 +18,7 @@ fi
cat<<EOF
%define ARCH ${ARCH}
%define KERNELRELEASE ${KERNELRELEASE}
+%define MODLIB ${MODLIB}
%define pkg_release $("${srctree}/init/build-version")
EOF
--
2.41.0On Fri, Jul 14, 2023 at 11:55 PM Nicolas Schier <nicolas@fjasle.eu> wrote:
>
> On Fri, Jul 14, 2023 at 04:30:02PM +0200, Michal Such�nek wrote:
> > Hello,
> >
> > On Fri, Jul 14, 2023 at 04:05:10PM +0200, Nicolas Schier wrote:
> > > On Fri, Jul 14, 2023 at 02:21:08PM +0200 Michal Suchanek wrote:
> > > > 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".
> > >
> > > >
> > > > 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 <msuchanek@suse.de>
> > > > ---
> > > > 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)
> > >
> > > 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.
> >
> > >
> > > > +
> > > > +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)
>
> Or are I am still missing something?
>
> > Was busybox ever fixed to not require the hack?
>
> I haven't checked that, yet.
I believe we can revert
8fc62e59425389a6d48429b9d146223122743435
bfe5424a8b31624e7a476f959d552999f931e7c7
There is no good reason to keep such old hacky code.
The old module-init-tools project was replaced by kmod.
I do not know whether busybox fixed the issue or not.
Anyway, Linux 3.0 was released 12 years ago.
There was plenty of time to fix the issue if we wanted.
If busybox is still not able to handle the X.Y version form,
it is just that nobody is caring about the busybox's depmod.
--
Best Regards
Masahiro Yamada
© 2016 - 2026 Red Hat, Inc.