[PATCH rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB

[PATCH rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB

[Michal Suchanek]

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 | 8 ++++----
 scripts/package/mkspec      | 1 +
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/scripts/package/kernel.spec b/scripts/package/kernel.spec
index 3eee0143e0c5..15f49c5077db 100644
--- a/scripts/package/kernel.spec
+++ b/scripts/package/kernel.spec
@@ -67,7 +67,7 @@ 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}%{MODLIB}/build
 %if %{with_devel}
 %{make} %{makeflags} run-command KBUILD_RUN_COMMAND='${srctree}/scripts/package/install-extmod-build %{buildroot}/usr/src/kernels/%{KERNELRELEASE}'
 %endif
@@ -98,8 +98,8 @@ fi
 
 %files
 %defattr (-, root, root)
-/lib/modules/%{KERNELRELEASE}
-%exclude /lib/modules/%{KERNELRELEASE}/build
+%{MODLIB}
+%exclude %{MODLIB}/build
 /boot/*
 
 %files headers
@@ -110,5 +110,5 @@ fi
 %files devel
 %defattr (-, root, root)
 /usr/src/kernels/%{KERNELRELEASE}
-/lib/modules/%{KERNELRELEASE}/build
+%{MODLIB}/build
 %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.42.0

Re: [PATCH rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB

[Nathan Chancellor]

On Thu, Oct 05, 2023 at 05:07:28PM +0200, Michal Suchanek wrote:
> 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>

This appears to work for me.

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  scripts/package/kernel.spec | 8 ++++----
>  scripts/package/mkspec      | 1 +
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/scripts/package/kernel.spec b/scripts/package/kernel.spec
> index 3eee0143e0c5..15f49c5077db 100644
> --- a/scripts/package/kernel.spec
> +++ b/scripts/package/kernel.spec
> @@ -67,7 +67,7 @@ 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}%{MODLIB}/build
>  %if %{with_devel}
>  %{make} %{makeflags} run-command KBUILD_RUN_COMMAND='${srctree}/scripts/package/install-extmod-build %{buildroot}/usr/src/kernels/%{KERNELRELEASE}'
>  %endif
> @@ -98,8 +98,8 @@ fi
>  
>  %files
>  %defattr (-, root, root)
> -/lib/modules/%{KERNELRELEASE}
> -%exclude /lib/modules/%{KERNELRELEASE}/build
> +%{MODLIB}
> +%exclude %{MODLIB}/build
>  /boot/*
>  
>  %files headers
> @@ -110,5 +110,5 @@ fi
>  %files devel
>  %defattr (-, root, root)
>  /usr/src/kernels/%{KERNELRELEASE}
> -/lib/modules/%{KERNELRELEASE}/build
> +%{MODLIB}/build
>  %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.42.0
> 

Re: [PATCH rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB

[Masahiro Yamada]

On Fri, Oct 6, 2023 at 12:49 AM Michal Suchanek <msuchanek@suse.de> wrote:
>
> 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


In other words, do you want to say

"If defining this middle part as a variable had been accepted,
this patch would have been unneeded." ?


I do not think so.


If your original patch were accepted, how would this patch look like?

kernel.spec needs to know the module directory somehow.


Would you add the following in scripts/package/mkspec ?

%define MODLIB $(pkg-config --print-variables kmod 2>/dev/null | grep
'^module_directory$' >/dev/null && pkg-config
--variable=module_directory kmod || echo /lib/modules)








>
> 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 | 8 ++++----
>  scripts/package/mkspec      | 1 +
>  2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/package/kernel.spec b/scripts/package/kernel.spec
> index 3eee0143e0c5..15f49c5077db 100644
> --- a/scripts/package/kernel.spec
> +++ b/scripts/package/kernel.spec
> @@ -67,7 +67,7 @@ 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}%{MODLIB}/build
>  %if %{with_devel}
>  %{make} %{makeflags} run-command KBUILD_RUN_COMMAND='${srctree}/scripts/package/install-extmod-build %{buildroot}/usr/src/kernels/%{KERNELRELEASE}'
>  %endif
> @@ -98,8 +98,8 @@ fi
>
>  %files
>  %defattr (-, root, root)
> -/lib/modules/%{KERNELRELEASE}
> -%exclude /lib/modules/%{KERNELRELEASE}/build
> +%{MODLIB}
> +%exclude %{MODLIB}/build
>  /boot/*
>
>  %files headers
> @@ -110,5 +110,5 @@ fi
>  %files devel
>  %defattr (-, root, root)
>  /usr/src/kernels/%{KERNELRELEASE}
> -/lib/modules/%{KERNELRELEASE}/build
> +%{MODLIB}/build
>  %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.42.0
>


--
Best Regards
Masahiro Yamada

Re: [PATCH rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB

[Michal Suchánek]

Hello,

On Mon, Oct 09, 2023 at 05:31:02PM +0900, Masahiro Yamada wrote:
> On Fri, Oct 6, 2023 at 12:49 AM Michal Suchanek <msuchanek@suse.de> wrote:
> >
> > 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
> 
> 
> In other words, do you want to say
> 
> "If defining this middle part as a variable had been accepted,
> this patch would have been unneeded." ?

If it were accepted I would not have to guess what the middle part is,
and could use the variable that unambiguosly defines it instead.

Thanks

Michal

> 
> 
> If your original patch were accepted, how would this patch look like?
> 
> kernel.spec needs to know the module directory somehow.
> 
> 
> Would you add the following in scripts/package/mkspec ?
> 
> %define MODLIB $(pkg-config --print-variables kmod 2>/dev/null | grep
> '^module_directory$' >/dev/null && pkg-config
> --variable=module_directory kmod || echo /lib/modules)
> 
> 
> 
> 
> 
> 
> 
> 
> >
> > 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 | 8 ++++----
> >  scripts/package/mkspec      | 1 +
> >  2 files changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/scripts/package/kernel.spec b/scripts/package/kernel.spec
> > index 3eee0143e0c5..15f49c5077db 100644
> > --- a/scripts/package/kernel.spec
> > +++ b/scripts/package/kernel.spec
> > @@ -67,7 +67,7 @@ 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}%{MODLIB}/build
> >  %if %{with_devel}
> >  %{make} %{makeflags} run-command KBUILD_RUN_COMMAND='${srctree}/scripts/package/install-extmod-build %{buildroot}/usr/src/kernels/%{KERNELRELEASE}'
> >  %endif
> > @@ -98,8 +98,8 @@ fi
> >
> >  %files
> >  %defattr (-, root, root)
> > -/lib/modules/%{KERNELRELEASE}
> > -%exclude /lib/modules/%{KERNELRELEASE}/build
> > +%{MODLIB}
> > +%exclude %{MODLIB}/build
> >  /boot/*
> >
> >  %files headers
> > @@ -110,5 +110,5 @@ fi
> >  %files devel
> >  %defattr (-, root, root)
> >  /usr/src/kernels/%{KERNELRELEASE}
> > -/lib/modules/%{KERNELRELEASE}/build
> > +%{MODLIB}/build
> >  %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.42.0
> >
> 
> 
> --
> Best Regards
> Masahiro Yamada

Re: [PATCH rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB

[Masahiro Yamada]

On Mon, Oct 9, 2023 at 5:52 PM Michal Suchánek <msuchanek@suse.de> wrote:
>
> Hello,
>
> On Mon, Oct 09, 2023 at 05:31:02PM +0900, Masahiro Yamada wrote:
> > On Fri, Oct 6, 2023 at 12:49 AM Michal Suchanek <msuchanek@suse.de> wrote:
> > >
> > > 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
> >
> >
> > In other words, do you want to say
> >
> > "If defining this middle part as a variable had been accepted,
> > this patch would have been unneeded." ?
>
> If it were accepted I would not have to guess what the middle part is,
> and could use the variable that unambiguosly defines it instead.


How?

scripts/package/kernel.spec hardcodes 'lib/modules'
in a couple of places.

I am asking how to derive the module path.




-- 
Best Regards
Masahiro Yamada

Re: [PATCH rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB

[Michal Suchánek]

On Mon, Oct 09, 2023 at 09:34:10PM +0900, Masahiro Yamada wrote:
> On Mon, Oct 9, 2023 at 5:52 PM Michal Suchánek <msuchanek@suse.de> wrote:
> >
> > Hello,
> >
> > On Mon, Oct 09, 2023 at 05:31:02PM +0900, Masahiro Yamada wrote:
> > > On Fri, Oct 6, 2023 at 12:49 AM Michal Suchanek <msuchanek@suse.de> wrote:
> > > >
> > > > 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
> > >
> > >
> > > In other words, do you want to say
> > >
> > > "If defining this middle part as a variable had been accepted,
> > > this patch would have been unneeded." ?
> >
> > If it were accepted I would not have to guess what the middle part is,
> > and could use the variable that unambiguosly defines it instead.
> 
> 
> How?
> 
> scripts/package/kernel.spec hardcodes 'lib/modules'
> in a couple of places.
> 
> I am asking how to derive the module path.

Not sure what you are asking here. The path is hardcoded, everywhere.

The current Makefile has

MODLIB	= $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE)

and there is no reliable way to learn what the middle part was after the
fact - $(INSTALL_MOD_PATH) can be non-empty.

The rejected patch was changing this to a variable, and also default to
adjusting the content to what kmod exports in pkgconfig after applying a
proposed patch to make this hardcoded part configurable:

export KERNEL_MODULE_DIRECTORY := $(shell pkg-config --print-variables kmod 2>/dev/null | grep '^module_directory$$' >/dev/null && pkg-config --variable=module_directory kmod || echo /lib/modules)

MODLIB	= $(INSTALL_MOD_PATH)$(KERNEL_MODULE_DIRECTORY)/$(KERNELRELEASE)

It would be completely posible to only define the middle part as a
variable that could then be used in rpm-pkg:

export KERNEL_MODULE_DIRECTORY := /lib/modules

MODLIB	= $(INSTALL_MOD_PATH)$(KERNEL_MODULE_DIRECTORY)/$(KERNELRELEASE)

Thanks

Michal

Re: [PATCH rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB

[Masahiro Yamada]

On Mon, Oct 9, 2023 at 11:07 PM Michal Suchánek <msuchanek@suse.de> wrote:
>
> On Mon, Oct 09, 2023 at 09:34:10PM +0900, Masahiro Yamada wrote:
> > On Mon, Oct 9, 2023 at 5:52 PM Michal Suchánek <msuchanek@suse.de> wrote:
> > >
> > > Hello,
> > >
> > > On Mon, Oct 09, 2023 at 05:31:02PM +0900, Masahiro Yamada wrote:
> > > > On Fri, Oct 6, 2023 at 12:49 AM Michal Suchanek <msuchanek@suse.de> wrote:
> > > > >
> > > > > 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
> > > >
> > > >
> > > > In other words, do you want to say
> > > >
> > > > "If defining this middle part as a variable had been accepted,
> > > > this patch would have been unneeded." ?
> > >
> > > If it were accepted I would not have to guess what the middle part is,
> > > and could use the variable that unambiguosly defines it instead.
> >
> >
> > How?
> >
> > scripts/package/kernel.spec hardcodes 'lib/modules'
> > in a couple of places.
> >
> > I am asking how to derive the module path.
>
> Not sure what you are asking here. The path is hardcoded, everywhere.
>
> The current Makefile has
>
> MODLIB  = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE)
>
> and there is no reliable way to learn what the middle part was after the
> fact - $(INSTALL_MOD_PATH) can be non-empty.
>
> The rejected patch was changing this to a variable, and also default to
> adjusting the content to what kmod exports in pkgconfig after applying a
> proposed patch to make this hardcoded part configurable:
>
> export KERNEL_MODULE_DIRECTORY := $(shell pkg-config --print-variables kmod 2>/dev/null | grep '^module_directory$$' >/dev/null && pkg-config --variable=module_directory kmod || echo /lib/modules)
>
> MODLIB  = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_DIRECTORY)/$(KERNELRELEASE)
>
> It would be completely posible to only define the middle part as a
> variable that could then be used in rpm-pkg:
>
> export KERNEL_MODULE_DIRECTORY := /lib/modules
>
> MODLIB  = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_DIRECTORY)/$(KERNELRELEASE)
>
> Thanks
>
> Michal
>
>


Let me add more context to my question.


I am interested in the timing when
'pkg-config --print-variables kmod | grep module_directory'
is executed.



1.  Build a SRPM on machine A

2.  Copy the SRPM from machine A to machine B

3.  Run rpmbuild on machine B to build the SRPM into a RPM

4.  Copy the RPM from machine B to machine C

5.  Install the RPM to machine C


Of course, we are most interested in the module path
of machine C, but it is difficult/impossible to
guess it at the time of building.

We can assume machine B == machine C.

We are the second most interested in the module
path on machine B.

The module path of machine A is not important.


So, I am asking where you would inject
'pkg-config --print-variables kmod | grep module_directory'.


-- 
Best Regards
Masahiro Yamada

Re: [PATCH rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB

[Jan Engelhardt]

On Monday 2023-10-09 17:14, Masahiro Yamada wrote:
>
>Let me add more context to my question.
>
>I am interested in the timing when
>'pkg-config --print-variables kmod | grep module_directory'
>is executed.
>
>1.  Build a SRPM on machine A
>2.  Copy the SRPM from machine A to machine B
>3.  Run rpmbuild on machine B to build the SRPM into a RPM
>4.  Copy the RPM from machine B to machine C
>5.  Install the RPM to machine C

In over 20 years of Linux distros existing, the one thing that
everyone has learned is that installing foreign RPM packages (or any
other format) is probably not going to work for one reason or
another. Different package names in Require: lines (just think of the
switch from modutils to kmod), different ABIs..

The overwhelming amount of package production that is going on these
days targets distro(A) == distro(B) == distro(C).


Yeah, the kernel package is kinda special because the files in it
are freestanding executables, but still..

Re: [PATCH rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB

[Michal Suchánek]

On Tue, Oct 10, 2023 at 12:14:01AM +0900, Masahiro Yamada wrote:
> On Mon, Oct 9, 2023 at 11:07 PM Michal Suchánek <msuchanek@suse.de> wrote:
> >
> > On Mon, Oct 09, 2023 at 09:34:10PM +0900, Masahiro Yamada wrote:
> > > On Mon, Oct 9, 2023 at 5:52 PM Michal Suchánek <msuchanek@suse.de> wrote:
> > > >
> > > > Hello,
> > > >
> > > > On Mon, Oct 09, 2023 at 05:31:02PM +0900, Masahiro Yamada wrote:
> > > > > On Fri, Oct 6, 2023 at 12:49 AM Michal Suchanek <msuchanek@suse.de> wrote:
> > > > > >
> > > > > > 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
> > > > >
> > > > >
> > > > > In other words, do you want to say
> > > > >
> > > > > "If defining this middle part as a variable had been accepted,
> > > > > this patch would have been unneeded." ?
> > > >
> > > > If it were accepted I would not have to guess what the middle part is,
> > > > and could use the variable that unambiguosly defines it instead.
> > >
> > >
> > > How?
> > >
> > > scripts/package/kernel.spec hardcodes 'lib/modules'
> > > in a couple of places.
> > >
> > > I am asking how to derive the module path.
> >
> > Not sure what you are asking here. The path is hardcoded, everywhere.
> >
> > The current Makefile has
> >
> > MODLIB  = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE)
> >
> > and there is no reliable way to learn what the middle part was after the
> > fact - $(INSTALL_MOD_PATH) can be non-empty.
> >
> > The rejected patch was changing this to a variable, and also default to
> > adjusting the content to what kmod exports in pkgconfig after applying a
> > proposed patch to make this hardcoded part configurable:
> >
> > export KERNEL_MODULE_DIRECTORY := $(shell pkg-config --print-variables kmod 2>/dev/null | grep '^module_directory$$' >/dev/null && pkg-config --variable=module_directory kmod || echo /lib/modules)
> >
> > MODLIB  = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_DIRECTORY)/$(KERNELRELEASE)
> >
> > It would be completely posible to only define the middle part as a
> > variable that could then be used in rpm-pkg:
> >
> > export KERNEL_MODULE_DIRECTORY := /lib/modules
> >
> > MODLIB  = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_DIRECTORY)/$(KERNELRELEASE)
> >
> > Thanks
> >
> > Michal
> >
> >
> 
> 
> Let me add more context to my question.
> 
> 
> I am interested in the timing when
> 'pkg-config --print-variables kmod | grep module_directory'
> is executed.
> 
> 
> 
> 1.  Build a SRPM on machine A
> 
> 2.  Copy the SRPM from machine A to machine B
> 
> 3.  Run rpmbuild on machine B to build the SRPM into a RPM
> 
> 4.  Copy the RPM from machine B to machine C
> 
> 5.  Install the RPM to machine C

As far as I am aware the typical use case is two step:

1. run make rpm-pkg on machine A
2. install the binary rpm on machine C that might not have build tools
   or powerful enough CPU

While it's theoretically possible to use the srpm to rebuild the binary
rpm independently of the kernel git tree I am not aware of people
commonly doing this.

If rebuilding the source rpm on a different machine from where the git
tree is located, and possibly on a different distribution is desirable
then the detection of the KERNEL_MODULE_DIRECTORY should be added in the
rpm spec file as well.

> Of course, we are most interested in the module path
> of machine C, but it is difficult/impossible to
> guess it at the time of building.
> 
> We can assume machine B == machine C.
> 
> We are the second most interested in the module
> path on machine B.
> 
> The module path of machine A is not important.
> 
> So, I am asking where you would inject
> 'pkg-config --print-variables kmod | grep module_directory'.

I don't. I don't think there will be a separate machine B.

And I can't really either - so far any attempt at adding support for
this has been rejected.

Technically the KERNEL_MODULE_DIRECTORY could be set in two steps - one
giving the script to run, and one running it, and then it could be run
independently in the SRPM as well.

Thanks

Michal

Re: [PATCH rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB

[Masahiro Yamada]

> >
> > Let me add more context to my question.
> >
> >
> > I am interested in the timing when
> > 'pkg-config --print-variables kmod | grep module_directory'
> > is executed.
> >
> >
> >
> > 1.  Build a SRPM on machine A
> >
> > 2.  Copy the SRPM from machine A to machine B
> >
> > 3.  Run rpmbuild on machine B to build the SRPM into a RPM
> >
> > 4.  Copy the RPM from machine B to machine C
> >
> > 5.  Install the RPM to machine C
>
> As far as I am aware the typical use case is two step:
>
> 1. run make rpm-pkg on machine A
> 2. install the binary rpm on machine C that might not have build tools
>    or powerful enough CPU
>
> While it's theoretically possible to use the srpm to rebuild the binary
> rpm independently of the kernel git tree I am not aware of people
> commonly doing this.



If I correctly understand commit
8818039f959b2efc0d6f2cb101f8061332f0c77e,
those Redhat guys pack a SRPM on a local machine,
then send it to their build server called 'koji'.

Otherwise, there is no reason
to have 'make srcrpm-pkg'.



I believe "A == B" is not always true,
but we can assume "distro(A) == distro(B)" is always met
for simplicity.

So, I am OK with configuration at the SRPM time.





> If rebuilding the source rpm on a different machine from where the git
> tree is located, and possibly on a different distribution is desirable
> then the detection of the KERNEL_MODULE_DIRECTORY should be added in the
> rpm spec file as well.
>
> > Of course, we are most interested in the module path
> > of machine C, but it is difficult/impossible to
> > guess it at the time of building.
> >
> > We can assume machine B == machine C.
> >
> > We are the second most interested in the module
> > path on machine B.
> >
> > The module path of machine A is not important.
> >
> > So, I am asking where you would inject
> > 'pkg-config --print-variables kmod | grep module_directory'.
>
> I don't. I don't think there will be a separate machine B.
>
> And I can't really either - so far any attempt at adding support for
> this has been rejected.
>
> Technically the KERNEL_MODULE_DIRECTORY could be set in two steps - one
> giving the script to run, and one running it, and then it could be run
> independently in the SRPM as well.


At first, I thought your patch [1] was very ugly,
but I do not think it is so ugly if cleanly implemented.

It won't hurt to allow users to specify the middle part of MODLIB.


There are two options.




[A]  Add 'MOD_PREFIX' to specify the middle part of MODLIB


The top Makefile will look as follows:


MODLIB = $(INSTALL_MOD_PATH)$(MOD_PREFIX)/lib/modules/$(KERNELRELEASE)
export MODLIB


It is easier than specifying the entire MODLIB, but you still need
to manually pass "MOD_PREFIX=/usr" from an env variable or
the command line.

If MOD_PREFIX is not given, MODLIB is the same as the current one.




[B] Support a dynamic configuration as well



MOD_PREFIX ?= $(shell pkg-config --variable=module_prefix libkmod 2>/dev/null)
export MOD_PREFIX

MODLIB = $(INSTALL_MOD_PATH)$(MOD_PREFIX)/lib/modules/$(KERNELRELEASE)
export MODLIB




If MOD_PREFIX is given from an env variable or from the command line,
it is respected.

If "pkg-config --variable=module_prefix libkmod" works,
that configuration is applied.

Otherwise, MOD_PREFIX is empty, i.e. fall back to the current behavior.






I prefer 'MOD_PREFIX' to 'KERNEL_MODULE_DIRECTORY' in your patch [1]
because "|| echo /lib/modules" can be omitted.

I do not think we will have such a crazy distro that
installs modules under /opt/ directory.




I could not understand why you inserted
"--print-variables kmod 2>/dev/null | grep '^module_directory$$' >/dev/null"
but I guess the reason is the same.
"pkg-config --variable=module_directory kmod" always succeeds,
so "|| echo /lib/modules" is never processed.


I do not know why you parsed kmod.pc instead of libkmod.pc [2]



[1] https://lore.kernel.org/linux-kbuild/20230718120348.383-1-msuchanek@suse.de/
[2] https://github.com/kmod-project/kmod/blob/v31/configure.ac#L295



--
Best Regards
Masahiro Yamada

Re: [PATCH rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB

[Michal Suchánek]

On Tue, Oct 17, 2023 at 07:15:50PM +0900, Masahiro Yamada wrote:
> > >
> > > Let me add more context to my question.
> > >
> > >
> > > I am interested in the timing when
> > > 'pkg-config --print-variables kmod | grep module_directory'
> > > is executed.
> > >
> > >
> > >
> > > 1.  Build a SRPM on machine A
> > >
> > > 2.  Copy the SRPM from machine A to machine B
> > >
> > > 3.  Run rpmbuild on machine B to build the SRPM into a RPM
> > >
> > > 4.  Copy the RPM from machine B to machine C
> > >
> > > 5.  Install the RPM to machine C
> >
> > As far as I am aware the typical use case is two step:
> >
> > 1. run make rpm-pkg on machine A
> > 2. install the binary rpm on machine C that might not have build tools
> >    or powerful enough CPU
> >
> > While it's theoretically possible to use the srpm to rebuild the binary
> > rpm independently of the kernel git tree I am not aware of people
> > commonly doing this.
> 
> 
> 
> If I correctly understand commit
> 8818039f959b2efc0d6f2cb101f8061332f0c77e,
> those Redhat guys pack a SRPM on a local machine,
> then send it to their build server called 'koji'.
> 
> Otherwise, there is no reason
> to have 'make srcrpm-pkg'.
> 
> 
> 
> I believe "A == B" is not always true,
> but we can assume "distro(A) == distro(B)" is always met
> for simplicity.
> 
> So, I am OK with configuration at the SRPM time.

Even if the distro does not match it will likely work to configure SRPM
for non-matching distro and then build it on the target distro but I have
not tested it.

> > If rebuilding the source rpm on a different machine from where the git
> > tree is located, and possibly on a different distribution is desirable
> > then the detection of the KERNEL_MODULE_DIRECTORY should be added in the
> > rpm spec file as well.
> >
> > > Of course, we are most interested in the module path
> > > of machine C, but it is difficult/impossible to
> > > guess it at the time of building.
> > >
> > > We can assume machine B == machine C.
> > >
> > > We are the second most interested in the module
> > > path on machine B.
> > >
> > > The module path of machine A is not important.
> > >
> > > So, I am asking where you would inject
> > > 'pkg-config --print-variables kmod | grep module_directory'.
> >
> > I don't. I don't think there will be a separate machine B.
> >
> > And I can't really either - so far any attempt at adding support for
> > this has been rejected.
> >
> > Technically the KERNEL_MODULE_DIRECTORY could be set in two steps - one
> > giving the script to run, and one running it, and then it could be run
> > independently in the SRPM as well.
> 
> 
> At first, I thought your patch [1] was very ugly,
> but I do not think it is so ugly if cleanly implemented.
> 
> It won't hurt to allow users to specify the middle part of MODLIB.
> 
> 
> There are two options.
> 
> 
> [A]  Add 'MOD_PREFIX' to specify the middle part of MODLIB
> 
> 
> The top Makefile will look as follows:
> 
> 
> MODLIB = $(INSTALL_MOD_PATH)$(MOD_PREFIX)/lib/modules/$(KERNELRELEASE)
> export MODLIB
> 
> 
> It is easier than specifying the entire MODLIB, but you still need
> to manually pass "MOD_PREFIX=/usr" from an env variable or
> the command line.
> 
> If MOD_PREFIX is not given, MODLIB is the same as the current one.
> 
> [B] Support a dynamic configuration as well
> 
> 
> MOD_PREFIX ?= $(shell pkg-config --variable=module_prefix libkmod 2>/dev/null)
> export MOD_PREFIX
> 
> MODLIB = $(INSTALL_MOD_PATH)$(MOD_PREFIX)/lib/modules/$(KERNELRELEASE)
> export MODLIB

That's basically the same thing as the patch that has been rejected.

I used := to prevent calling pkg-config every time MODLIB is used but it
might not be the most flexible wrt overrides.

> If MOD_PREFIX is given from an env variable or from the command line,
> it is respected.
> 
> If "pkg-config --variable=module_prefix libkmod" works,
> that configuration is applied.
> 
> Otherwise, MOD_PREFIX is empty, i.e. fall back to the current behavior.
> 
> 
> I prefer 'MOD_PREFIX' to 'KERNEL_MODULE_DIRECTORY' in your patch [1]
> because "|| echo /lib/modules" can be omitted.
> 
> I do not think we will have such a crazy distro that
> installs modules under /opt/ directory.

However, I can easily imagine a distribution that would want to put
modules in /usr/lib-amd64-linux/modules.

> I could not understand why you inserted
> "--print-variables kmod 2>/dev/null | grep '^module_directory$$' >/dev/null"
> but I guess the reason is the same.
> "pkg-config --variable=module_directory kmod" always succeeds,
> so "|| echo /lib/modules" is never processed.

Yes, that's the semantics of the tool. The jq version was slightly less
convoluted but required additional tool for building the kernel.

> I do not know why you parsed kmod.pc instead of libkmod.pc [2]

Because it's kmod property, not libkmod property.

Distributions would install libkmod.pc only with development files
whereas the kmod.pc should be installed with the binaries.

Thanks

Michal

> 
> 
> [1] https://lore.kernel.org/linux-kbuild/20230718120348.383-1-msuchanek@suse.de/
> [2] https://github.com/kmod-project/kmod/blob/v31/configure.ac#L295

Re: [PATCH rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB

[Masahiro Yamada]

On Tue, Oct 17, 2023 at 7:44 PM Michal Suchánek <msuchanek@suse.de> wrote:
>
> On Tue, Oct 17, 2023 at 07:15:50PM +0900, Masahiro Yamada wrote:
> > > >
> > > > Let me add more context to my question.
> > > >
> > > >
> > > > I am interested in the timing when
> > > > 'pkg-config --print-variables kmod | grep module_directory'
> > > > is executed.
> > > >
> > > >
> > > >
> > > > 1.  Build a SRPM on machine A
> > > >
> > > > 2.  Copy the SRPM from machine A to machine B
> > > >
> > > > 3.  Run rpmbuild on machine B to build the SRPM into a RPM
> > > >
> > > > 4.  Copy the RPM from machine B to machine C
> > > >
> > > > 5.  Install the RPM to machine C
> > >
> > > As far as I am aware the typical use case is two step:
> > >
> > > 1. run make rpm-pkg on machine A
> > > 2. install the binary rpm on machine C that might not have build tools
> > >    or powerful enough CPU
> > >
> > > While it's theoretically possible to use the srpm to rebuild the binary
> > > rpm independently of the kernel git tree I am not aware of people
> > > commonly doing this.
> >
> >
> >
> > If I correctly understand commit
> > 8818039f959b2efc0d6f2cb101f8061332f0c77e,
> > those Redhat guys pack a SRPM on a local machine,
> > then send it to their build server called 'koji'.
> >
> > Otherwise, there is no reason
> > to have 'make srcrpm-pkg'.
> >
> >
> >
> > I believe "A == B" is not always true,
> > but we can assume "distro(A) == distro(B)" is always met
> > for simplicity.
> >
> > So, I am OK with configuration at the SRPM time.
>
> Even if the distro does not match it will likely work to configure SRPM
> for non-matching distro and then build it on the target distro but I have
> not tested it.



Your approach specifies %{MODLIB} as a fixed string
when generating kernel.spec, i.e. at the SRPM time.


 %files
 %defattr (-, root, root)
-/lib/modules/%{KERNELRELEASE}
-%exclude /lib/modules/%{KERNELRELEASE}/build
+%{MODLIB}
+%exclude %{MODLIB}/build
 /boot/*


Then, how to change the path later?






I do not know if the relocatable package
is a sensible solution because the kernel package has /boot/

http://ftp.rpm.org/api/4.4.2.2/relocatable.html


We might be able to tweak installation paths in %post section.

Or perhaps, %{shell } can defer the module path detection
until building RPM.

%define MOD_PREFIX    %{shell pkg-config --variable=module_prefix
libkmod 2>/dev/null}


Overall, I did not find a cool solution.



>
> > > If rebuilding the source rpm on a different machine from where the git
> > > tree is located, and possibly on a different distribution is desirable
> > > then the detection of the KERNEL_MODULE_DIRECTORY should be added in the
> > > rpm spec file as well.
> > >
> > > > Of course, we are most interested in the module path
> > > > of machine C, but it is difficult/impossible to
> > > > guess it at the time of building.
> > > >
> > > > We can assume machine B == machine C.
> > > >
> > > > We are the second most interested in the module
> > > > path on machine B.
> > > >
> > > > The module path of machine A is not important.
> > > >
> > > > So, I am asking where you would inject
> > > > 'pkg-config --print-variables kmod | grep module_directory'.
> > >
> > > I don't. I don't think there will be a separate machine B.
> > >
> > > And I can't really either - so far any attempt at adding support for
> > > this has been rejected.
> > >
> > > Technically the KERNEL_MODULE_DIRECTORY could be set in two steps - one
> > > giving the script to run, and one running it, and then it could be run
> > > independently in the SRPM as well.
> >
> >
> > At first, I thought your patch [1] was very ugly,
> > but I do not think it is so ugly if cleanly implemented.
> >
> > It won't hurt to allow users to specify the middle part of MODLIB.
> >
> >
> > There are two options.
> >
> >
> > [A]  Add 'MOD_PREFIX' to specify the middle part of MODLIB
> >
> >
> > The top Makefile will look as follows:
> >
> >
> > MODLIB = $(INSTALL_MOD_PATH)$(MOD_PREFIX)/lib/modules/$(KERNELRELEASE)
> > export MODLIB
> >
> >
> > It is easier than specifying the entire MODLIB, but you still need
> > to manually pass "MOD_PREFIX=/usr" from an env variable or
> > the command line.
> >
> > If MOD_PREFIX is not given, MODLIB is the same as the current one.
> >
> > [B] Support a dynamic configuration as well
> >
> >
> > MOD_PREFIX ?= $(shell pkg-config --variable=module_prefix libkmod 2>/dev/null)
> > export MOD_PREFIX
> >
> > MODLIB = $(INSTALL_MOD_PATH)$(MOD_PREFIX)/lib/modules/$(KERNELRELEASE)
> > export MODLIB
>
> That's basically the same thing as the patch that has been rejected.
>
> I used := to prevent calling pkg-config every time MODLIB is used but it
> might not be the most flexible wrt overrides.




That's good you care about the cost of $(shell ) invocations.

:= is evaluated one time at maximum, but one time at minimum.

$(shell ) is always invoked for non-build targets as
"make clean", "make help", etc.
That is what I care about.


?= is a recursive variable.

The workaround for one-time evaluation is here,
https://savannah.gnu.org/bugs/index.php?64746#comment2

However, that is not a problem because I can do it properly somehow,
for example, with "private export".







>
> > If MOD_PREFIX is given from an env variable or from the command line,
> > it is respected.
> >
> > If "pkg-config --variable=module_prefix libkmod" works,
> > that configuration is applied.
> >
> > Otherwise, MOD_PREFIX is empty, i.e. fall back to the current behavior.
> >
> >
> > I prefer 'MOD_PREFIX' to 'KERNEL_MODULE_DIRECTORY' in your patch [1]
> > because "|| echo /lib/modules" can be omitted.
> >
> > I do not think we will have such a crazy distro that
> > installs modules under /opt/ directory.
>
> However, I can easily imagine a distribution that would want to put
> modules in /usr/lib-amd64-linux/modules.


Sorry, it is not easy for me.

What is the background of your thought?



>
> > I could not understand why you inserted
> > "--print-variables kmod 2>/dev/null | grep '^module_directory$$' >/dev/null"
> > but I guess the reason is the same.
> > "pkg-config --variable=module_directory kmod" always succeeds,
> > so "|| echo /lib/modules" is never processed.
>
> Yes, that's the semantics of the tool. The jq version was slightly less
> convoluted but required additional tool for building the kernel.


It IS convoluted.



>
> > I do not know why you parsed kmod.pc instead of libkmod.pc [2]
>
> Because it's kmod property, not libkmod property.
>
> Distributions would install libkmod.pc only with development files
> whereas the kmod.pc should be installed with the binaries.


This is up to the kmod maintainer.

If they agree, I do not mind where the configuration comes from.








> Thanks
>
> Michal
>
> >
> >
> > [1] https://lore.kernel.org/linux-kbuild/20230718120348.383-1-msuchanek@suse.de/
> > [2] https://github.com/kmod-project/kmod/blob/v31/configure.ac#L295



-- 
Best Regards
Masahiro Yamada

Re: [PATCH rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB

[Michal Suchánek]

On Tue, Oct 17, 2023 at 09:05:29PM +0900, Masahiro Yamada wrote:
> On Tue, Oct 17, 2023 at 7:44 PM Michal Suchánek <msuchanek@suse.de> wrote:
> >
> > On Tue, Oct 17, 2023 at 07:15:50PM +0900, Masahiro Yamada wrote:
> > > > >
> > > > > Let me add more context to my question.
> > > > >
> > > > >
> > > > > I am interested in the timing when
> > > > > 'pkg-config --print-variables kmod | grep module_directory'
> > > > > is executed.
> > > > >
> > > > >
> > > > >
> > > > > 1.  Build a SRPM on machine A
> > > > >
> > > > > 2.  Copy the SRPM from machine A to machine B
> > > > >
> > > > > 3.  Run rpmbuild on machine B to build the SRPM into a RPM
> > > > >
> > > > > 4.  Copy the RPM from machine B to machine C
> > > > >
> > > > > 5.  Install the RPM to machine C
> > > >
> > > > As far as I am aware the typical use case is two step:
> > > >
> > > > 1. run make rpm-pkg on machine A
> > > > 2. install the binary rpm on machine C that might not have build tools
> > > >    or powerful enough CPU
> > > >
> > > > While it's theoretically possible to use the srpm to rebuild the binary
> > > > rpm independently of the kernel git tree I am not aware of people
> > > > commonly doing this.
> > >
> > >
> > >
> > > If I correctly understand commit
> > > 8818039f959b2efc0d6f2cb101f8061332f0c77e,
> > > those Redhat guys pack a SRPM on a local machine,
> > > then send it to their build server called 'koji'.
> > >
> > > Otherwise, there is no reason
> > > to have 'make srcrpm-pkg'.
> > >
> > >
> > >
> > > I believe "A == B" is not always true,
> > > but we can assume "distro(A) == distro(B)" is always met
> > > for simplicity.
> > >
> > > So, I am OK with configuration at the SRPM time.
> >
> > Even if the distro does not match it will likely work to configure SRPM
> > for non-matching distro and then build it on the target distro but I have
> > not tested it.
> 
> 
> 
> Your approach specifies %{MODLIB} as a fixed string
> when generating kernel.spec, i.e. at the SRPM time.
> 
> 
>  %files
>  %defattr (-, root, root)
> -/lib/modules/%{KERNELRELEASE}
> -%exclude /lib/modules/%{KERNELRELEASE}/build
> +%{MODLIB}
> +%exclude %{MODLIB}/build
>  /boot/*
> 
> 
> Then, how to change the path later?

Why would you need to change the path later?

The SRPM has sources, it does not need to build on the system on which
it is authored if it is intended for another distribution.

Of course, you would need to know for what distribution and where it
wants its modules so that you can specify the location when creating the
SRPM.

> > > > If rebuilding the source rpm on a different machine from where the git
> > > > tree is located, and possibly on a different distribution is desirable
> > > > then the detection of the KERNEL_MODULE_DIRECTORY should be added in the
> > > > rpm spec file as well.
> > > >
> > > > > Of course, we are most interested in the module path
> > > > > of machine C, but it is difficult/impossible to
> > > > > guess it at the time of building.
> > > > >
> > > > > We can assume machine B == machine C.
> > > > >
> > > > > We are the second most interested in the module
> > > > > path on machine B.
> > > > >
> > > > > The module path of machine A is not important.
> > > > >
> > > > > So, I am asking where you would inject
> > > > > 'pkg-config --print-variables kmod | grep module_directory'.
> > > >
> > > > I don't. I don't think there will be a separate machine B.
> > > >
> > > > And I can't really either - so far any attempt at adding support for
> > > > this has been rejected.
> > > >
> > > > Technically the KERNEL_MODULE_DIRECTORY could be set in two steps - one
> > > > giving the script to run, and one running it, and then it could be run
> > > > independently in the SRPM as well.
> > >
> > >
> > > At first, I thought your patch [1] was very ugly,
> > > but I do not think it is so ugly if cleanly implemented.
> > >
> > > It won't hurt to allow users to specify the middle part of MODLIB.
> > >
> > >
> > > There are two options.
> > >
> > >
> > > [A]  Add 'MOD_PREFIX' to specify the middle part of MODLIB
> > >
> > >
> > > The top Makefile will look as follows:
> > >
> > >
> > > MODLIB = $(INSTALL_MOD_PATH)$(MOD_PREFIX)/lib/modules/$(KERNELRELEASE)
> > > export MODLIB
> > >
> > >
> > > It is easier than specifying the entire MODLIB, but you still need
> > > to manually pass "MOD_PREFIX=/usr" from an env variable or
> > > the command line.
> > >
> > > If MOD_PREFIX is not given, MODLIB is the same as the current one.
> > >
> > > [B] Support a dynamic configuration as well
> > >
> > >
> > > MOD_PREFIX ?= $(shell pkg-config --variable=module_prefix libkmod 2>/dev/null)
> > > export MOD_PREFIX
> > >
> > > MODLIB = $(INSTALL_MOD_PATH)$(MOD_PREFIX)/lib/modules/$(KERNELRELEASE)
> > > export MODLIB
> >
> > That's basically the same thing as the patch that has been rejected.
> >
> > I used := to prevent calling pkg-config every time MODLIB is used but it
> > might not be the most flexible wrt overrides.
> 
> That's good you care about the cost of $(shell ) invocations.
> 
> := is evaluated one time at maximum, but one time at minimum.
> 
> $(shell ) is always invoked for non-build targets as
> "make clean", "make help", etc.
> That is what I care about.
> 
> 
> ?= is a recursive variable.
> 
> The workaround for one-time evaluation is here,
> https://savannah.gnu.org/bugs/index.php?64746#comment2
> 
> However, that is not a problem because I can do it properly somehow,
> for example, with "private export".

That's good to know.

> > > If MOD_PREFIX is given from an env variable or from the command line,
> > > it is respected.
> > >
> > > If "pkg-config --variable=module_prefix libkmod" works,
> > > that configuration is applied.
> > >
> > > Otherwise, MOD_PREFIX is empty, i.e. fall back to the current behavior.
> > >
> > >
> > > I prefer 'MOD_PREFIX' to 'KERNEL_MODULE_DIRECTORY' in your patch [1]
> > > because "|| echo /lib/modules" can be omitted.
> > >
> > > I do not think we will have such a crazy distro that
> > > installs modules under /opt/ directory.
> >
> > However, I can easily imagine a distribution that would want to put
> > modules in /usr/lib-amd64-linux/modules.
> 
> 
> Sorry, it is not easy for me.
> 
> What is the background of your thought?

That's where every other library and module would go on distributions
that care about ability to install packages for multiple architectures
at the same time. AFAIK the workaround is to inclclude the CPU
architecture in extraversion for the kernel to fit.

> >
> > > I could not understand why you inserted
> > > "--print-variables kmod 2>/dev/null | grep '^module_directory$$' >/dev/null"
> > > but I guess the reason is the same.
> > > "pkg-config --variable=module_directory kmod" always succeeds,
> > > so "|| echo /lib/modules" is never processed.
> >
> > Yes, that's the semantics of the tool. The jq version was slightly less
> > convoluted but required additional tool for building the kernel.
> 
> 
> It IS convoluted.

That's unfortunate result of how the pkgconfig tool works. By now it is
even too late to complain to the tool author because it's been like that
forever, best bet is to to use it as is or pick a different tool for
configuration.

> > > I do not know why you parsed kmod.pc instead of libkmod.pc [2]
> >
> > Because it's kmod property, not libkmod property.
> >
> > Distributions would install libkmod.pc only with development files
> > whereas the kmod.pc should be installed with the binaries.
> 
> 
> This is up to the kmod maintainer.
> 
> If they agree, I do not mind where the configuration comes from.

So far it has not been commented on. Maybe it's time for a ping.

Thanks

Michal

> > > [1] https://lore.kernel.org/linux-kbuild/20230718120348.383-1-msuchanek@suse.de/
> > > [2] https://github.com/kmod-project/kmod/blob/v31/configure.ac#L295

Re: [PATCH rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB

[Masahiro Yamada]

On Tue, Oct 17, 2023 at 9:27 PM Michal Suchánek <msuchanek@suse.de> wrote:
>
> On Tue, Oct 17, 2023 at 09:05:29PM +0900, Masahiro Yamada wrote:
> > On Tue, Oct 17, 2023 at 7:44 PM Michal Suchánek <msuchanek@suse.de> wrote:
> > >
> > > On Tue, Oct 17, 2023 at 07:15:50PM +0900, Masahiro Yamada wrote:
> > > > > >
> > > > > > Let me add more context to my question.
> > > > > >
> > > > > >
> > > > > > I am interested in the timing when
> > > > > > 'pkg-config --print-variables kmod | grep module_directory'
> > > > > > is executed.
> > > > > >
> > > > > >
> > > > > >
> > > > > > 1.  Build a SRPM on machine A
> > > > > >
> > > > > > 2.  Copy the SRPM from machine A to machine B
> > > > > >
> > > > > > 3.  Run rpmbuild on machine B to build the SRPM into a RPM
> > > > > >
> > > > > > 4.  Copy the RPM from machine B to machine C
> > > > > >
> > > > > > 5.  Install the RPM to machine C
> > > > >
> > > > > As far as I am aware the typical use case is two step:
> > > > >
> > > > > 1. run make rpm-pkg on machine A
> > > > > 2. install the binary rpm on machine C that might not have build tools
> > > > >    or powerful enough CPU
> > > > >
> > > > > While it's theoretically possible to use the srpm to rebuild the binary
> > > > > rpm independently of the kernel git tree I am not aware of people
> > > > > commonly doing this.
> > > >
> > > >
> > > >
> > > > If I correctly understand commit
> > > > 8818039f959b2efc0d6f2cb101f8061332f0c77e,
> > > > those Redhat guys pack a SRPM on a local machine,
> > > > then send it to their build server called 'koji'.
> > > >
> > > > Otherwise, there is no reason
> > > > to have 'make srcrpm-pkg'.
> > > >
> > > >
> > > >
> > > > I believe "A == B" is not always true,
> > > > but we can assume "distro(A) == distro(B)" is always met
> > > > for simplicity.
> > > >
> > > > So, I am OK with configuration at the SRPM time.
> > >
> > > Even if the distro does not match it will likely work to configure SRPM
> > > for non-matching distro and then build it on the target distro but I have
> > > not tested it.
> >
> >
> >
> > Your approach specifies %{MODLIB} as a fixed string
> > when generating kernel.spec, i.e. at the SRPM time.
> >
> >
> >  %files
> >  %defattr (-, root, root)
> > -/lib/modules/%{KERNELRELEASE}
> > -%exclude /lib/modules/%{KERNELRELEASE}/build
> > +%{MODLIB}
> > +%exclude %{MODLIB}/build
> >  /boot/*
> >
> >
> > Then, how to change the path later?
>
> Why would you need to change the path later?
>
> The SRPM has sources, it does not need to build on the system on which
> it is authored if it is intended for another distribution.
>
> Of course, you would need to know for what distribution and where it
> wants its modules so that you can specify the location when creating the
> SRPM.



Simply I wrongly understood your description.

If you manage to correctly configure for the target distro
when building SRPM, that's fine.





>
> > > > > If rebuilding the source rpm on a different machine from where the git
> > > > > tree is located, and possibly on a different distribution is desirable
> > > > > then the detection of the KERNEL_MODULE_DIRECTORY should be added in the
> > > > > rpm spec file as well.
> > > > >
> > > > > > Of course, we are most interested in the module path
> > > > > > of machine C, but it is difficult/impossible to
> > > > > > guess it at the time of building.
> > > > > >
> > > > > > We can assume machine B == machine C.
> > > > > >
> > > > > > We are the second most interested in the module
> > > > > > path on machine B.
> > > > > >
> > > > > > The module path of machine A is not important.
> > > > > >
> > > > > > So, I am asking where you would inject
> > > > > > 'pkg-config --print-variables kmod | grep module_directory'.
> > > > >
> > > > > I don't. I don't think there will be a separate machine B.
> > > > >
> > > > > And I can't really either - so far any attempt at adding support for
> > > > > this has been rejected.
> > > > >
> > > > > Technically the KERNEL_MODULE_DIRECTORY could be set in two steps - one
> > > > > giving the script to run, and one running it, and then it could be run
> > > > > independently in the SRPM as well.
> > > >
> > > >
> > > > At first, I thought your patch [1] was very ugly,
> > > > but I do not think it is so ugly if cleanly implemented.
> > > >
> > > > It won't hurt to allow users to specify the middle part of MODLIB.
> > > >
> > > >
> > > > There are two options.
> > > >
> > > >
> > > > [A]  Add 'MOD_PREFIX' to specify the middle part of MODLIB
> > > >
> > > >
> > > > The top Makefile will look as follows:
> > > >
> > > >
> > > > MODLIB = $(INSTALL_MOD_PATH)$(MOD_PREFIX)/lib/modules/$(KERNELRELEASE)
> > > > export MODLIB
> > > >
> > > >
> > > > It is easier than specifying the entire MODLIB, but you still need
> > > > to manually pass "MOD_PREFIX=/usr" from an env variable or
> > > > the command line.
> > > >
> > > > If MOD_PREFIX is not given, MODLIB is the same as the current one.
> > > >
> > > > [B] Support a dynamic configuration as well
> > > >
> > > >
> > > > MOD_PREFIX ?= $(shell pkg-config --variable=module_prefix libkmod 2>/dev/null)
> > > > export MOD_PREFIX
> > > >
> > > > MODLIB = $(INSTALL_MOD_PATH)$(MOD_PREFIX)/lib/modules/$(KERNELRELEASE)
> > > > export MODLIB
> > >
> > > That's basically the same thing as the patch that has been rejected.
> > >
> > > I used := to prevent calling pkg-config every time MODLIB is used but it
> > > might not be the most flexible wrt overrides.
> >
> > That's good you care about the cost of $(shell ) invocations.
> >
> > := is evaluated one time at maximum, but one time at minimum.
> >
> > $(shell ) is always invoked for non-build targets as
> > "make clean", "make help", etc.
> > That is what I care about.
> >
> >
> > ?= is a recursive variable.
> >
> > The workaround for one-time evaluation is here,
> > https://savannah.gnu.org/bugs/index.php?64746#comment2
> >
> > However, that is not a problem because I can do it properly somehow,
> > for example, with "private export".
>
> That's good to know.
>
> > > > If MOD_PREFIX is given from an env variable or from the command line,
> > > > it is respected.
> > > >
> > > > If "pkg-config --variable=module_prefix libkmod" works,
> > > > that configuration is applied.
> > > >
> > > > Otherwise, MOD_PREFIX is empty, i.e. fall back to the current behavior.
> > > >
> > > >
> > > > I prefer 'MOD_PREFIX' to 'KERNEL_MODULE_DIRECTORY' in your patch [1]
> > > > because "|| echo /lib/modules" can be omitted.
> > > >
> > > > I do not think we will have such a crazy distro that
> > > > installs modules under /opt/ directory.
> > >
> > > However, I can easily imagine a distribution that would want to put
> > > modules in /usr/lib-amd64-linux/modules.
> >
> >
> > Sorry, it is not easy for me.
> >
> > What is the background of your thought?
>
> That's where every other library and module would go on distributions
> that care about ability to install packages for multiple architectures
> at the same time. AFAIK the workaround is to inclclude the CPU
> architecture in extraversion for the kernel to fit.


In my system (Ubuntu), I see the directory paths

/usr/aarch64-linux-gnu/lib/
/usr/i686-linux-gnu/lib/
/usr/x86_64-linux-gnu/lib/

If there were such a crazy distro that supports multiple kernel arches
within a single image, modules might be installed:
/usr/x86_64-linux-gnu/lib/module/<version>/

I have never seen a distro with /usr/lib-<triplet> hierarchy.

But, I have no idea, since this discussion is hypothetical after all.


> > >
> > > > I could not understand why you inserted
> > > > "--print-variables kmod 2>/dev/null | grep '^module_directory$$' >/dev/null"
> > > > but I guess the reason is the same.
> > > > "pkg-config --variable=module_directory kmod" always succeeds,
> > > > so "|| echo /lib/modules" is never processed.
> > >
> > > Yes, that's the semantics of the tool. The jq version was slightly less
> > > convoluted but required additional tool for building the kernel.
> >
> >
> > It IS convoluted.
>
> That's unfortunate result of how the pkgconfig tool works. By now it is
> even too late to complain to the tool author because it's been like that
> forever, best bet is to to use it as is or pick a different tool for
> configuration.


"pkg-config --variable=<name>" returns its value.
It is pretty simple, and I do not think it is a big problem.

Your code is long, but the reason is that you implemented
it in that way.


If you go with KERNEL_MODULE_DIRECTORY for max flexibility,

  KERNEL_MODULE_DIRECTORY := $(or $(shell pkg-config
--variable=module_directory kmod 2>/dev/null),/lib/modules)

should work with less characters and less process forks.

But, now I started to prefer confining the long code
into the shell script, "scripts/modinst-dir",
and calling it where needed.





>
> > > > I do not know why you parsed kmod.pc instead of libkmod.pc [2]
> > >
> > > Because it's kmod property, not libkmod property.
> > >
> > > Distributions would install libkmod.pc only with development files
> > > whereas the kmod.pc should be installed with the binaries.
> >
> >
> > This is up to the kmod maintainer.
> >
> > If they agree, I do not mind where the configuration comes from.
>
> So far it has not been commented on. Maybe it's time for a ping.
>
> Thanks
>
> Michal
>
> > > > [1] https://lore.kernel.org/linux-kbuild/20230718120348.383-1-msuchanek@suse.de/
> > > > [2] https://github.com/kmod-project/kmod/blob/v31/configure.ac#L295
--
Best Regards
Masahiro Yamada

Re: [PATCH rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB

[Michal Suchánek]

On Tue, Oct 17, 2023 at 11:46:45PM +0900, Masahiro Yamada wrote:
> On Tue, Oct 17, 2023 at 9:27 PM Michal Suchánek <msuchanek@suse.de> wrote:
> >
> > On Tue, Oct 17, 2023 at 09:05:29PM +0900, Masahiro Yamada wrote:
> > > On Tue, Oct 17, 2023 at 7:44 PM Michal Suchánek <msuchanek@suse.de> wrote:
> > > >
> > > > On Tue, Oct 17, 2023 at 07:15:50PM +0900, Masahiro Yamada wrote:

> > > > > If MOD_PREFIX is given from an env variable or from the command line,
> > > > > it is respected.
> > > > >
> > > > > If "pkg-config --variable=module_prefix libkmod" works,
> > > > > that configuration is applied.
> > > > >
> > > > > Otherwise, MOD_PREFIX is empty, i.e. fall back to the current behavior.
> > > > >
> > > > >
> > > > > I prefer 'MOD_PREFIX' to 'KERNEL_MODULE_DIRECTORY' in your patch [1]
> > > > > because "|| echo /lib/modules" can be omitted.
> > > > >
> > > > > I do not think we will have such a crazy distro that
> > > > > installs modules under /opt/ directory.
> > > >
> > > > However, I can easily imagine a distribution that would want to put
> > > > modules in /usr/lib-amd64-linux/modules.
> > >
> > >
> > > Sorry, it is not easy for me.
> > >
> > > What is the background of your thought?
> >
> > That's where every other library and module would go on distributions
> > that care about ability to install packages for multiple architectures
> > at the same time. AFAIK the workaround is to inclclude the CPU
> > architecture in extraversion for the kernel to fit.
> 
> 
> In my system (Ubuntu), I see the directory paths
> 
> /usr/aarch64-linux-gnu/lib/
> /usr/i686-linux-gnu/lib/
> /usr/x86_64-linux-gnu/lib/
> 
> If there were such a crazy distro that supports multiple kernel arches
> within a single image, modules might be installed:
> /usr/x86_64-linux-gnu/lib/module/<version>/

For me it's /usr/lib/i386-linux-gnu/.

Did they change the scheme at some point?

> > > >
> > > > > I could not understand why you inserted
> > > > > "--print-variables kmod 2>/dev/null | grep '^module_directory$$' >/dev/null"
> > > > > but I guess the reason is the same.
> > > > > "pkg-config --variable=module_directory kmod" always succeeds,
> > > > > so "|| echo /lib/modules" is never processed.
> > > >
> > > > Yes, that's the semantics of the tool. The jq version was slightly less
> > > > convoluted but required additional tool for building the kernel.
> > >
> > >
> > > It IS convoluted.
> >
> > That's unfortunate result of how the pkgconfig tool works. By now it is
> > even too late to complain to the tool author because it's been like that
> > forever, best bet is to to use it as is or pick a different tool for
> > configuration.
> 
> "pkg-config --variable=<name>" returns its value.
> It is pretty simple, and I do not think it is a big problem.
> 
> Your code is long, but the reason is that you implemented
> it in that way.
> 
> 
> If you go with KERNEL_MODULE_DIRECTORY for max flexibility,
> 
>   KERNEL_MODULE_DIRECTORY := $(or $(shell pkg-config
> --variable=module_directory kmod 2>/dev/null),/lib/modules)
> 
> should work with less characters and less process forks.

And assumes that the module_directory cannot be empty.

Which may or may not be a reasonable assumption, the script as proposed
in the patch does not rely on it.

> But, now I started to prefer confining the long code
> into the shell script, "scripts/modinst-dir",
> and calling it where needed.

That's also an option.

Thanks

Michal

> > > > > [1] https://lore.kernel.org/linux-kbuild/20230718120348.383-1-msuchanek@suse.de/
> > > > > [2] https://github.com/kmod-project/kmod/blob/v31/configure.ac#L295

Re: [PATCH rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB

[Jan Engelhardt]

On Tuesday 2023-10-17 17:10, Michal Suchánek wrote:
>
>> In my system (Ubuntu), I see the directory paths
>> 
>> /usr/aarch64-linux-gnu/lib/
>> /usr/i686-linux-gnu/lib/
>> /usr/x86_64-linux-gnu/lib/
>> 
>> If there were such a crazy distro that supports multiple kernel arches
>> within a single image, modules might be installed:
>> /usr/x86_64-linux-gnu/lib/module/<version>/
>
>For me it's /usr/lib/i386-linux-gnu/.
>
>Did they change the scheme at some point?

It's a complicated mumble-jumble. Prior art exists as in:

 /opt/vendorThing/bin/...
 /usr/X11R6/lib/libXi.so.6 [host binary]
 /usr/x86_64-w64-mingw32/bin/as [host binary]
 /usr/x86_64-w64-mingw32/sys-root/mingw/bin/as.exe [foreign binary]
 /usr/platform/SUNW,Ultra-2/lib/libprtdiag_psr.so.1 [looks foreign]

The use of suffix-based naming must have been established sometime
near the end of the 90s or the start of 2000s as the first biarch
Linux distros emerged. Probably in gcc or glibc sources one will find
the root of where the use of suffix identifiers like /usr/lib64
started. Leaves the question open "why".

Re: [PATCH rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB

[Michal Suchánek]

On Wed, Oct 18, 2023 at 03:12:41AM +0200, Jan Engelhardt wrote:
> On Tuesday 2023-10-17 17:10, Michal Suchánek wrote:
> >
> >> In my system (Ubuntu), I see the directory paths
> >> 
> >> /usr/aarch64-linux-gnu/lib/
> >> /usr/i686-linux-gnu/lib/
> >> /usr/x86_64-linux-gnu/lib/
> >> 
> >> If there were such a crazy distro that supports multiple kernel arches
> >> within a single image, modules might be installed:
> >> /usr/x86_64-linux-gnu/lib/module/<version>/
> >
> >For me it's /usr/lib/i386-linux-gnu/.
> >
> >Did they change the scheme at some point?
> 
> It's a complicated mumble-jumble. Prior art exists as in:
> 
>  /opt/vendorThing/bin/...
>  /usr/X11R6/lib/libXi.so.6 [host binary]
>  /usr/x86_64-w64-mingw32/bin/as [host binary]
>  /usr/x86_64-w64-mingw32/sys-root/mingw/bin/as.exe [foreign binary]
>  /usr/platform/SUNW,Ultra-2/lib/libprtdiag_psr.so.1 [looks foreign]
> 
> The use of suffix-based naming must have been established sometime
> near the end of the 90s or the start of 2000s as the first biarch
> Linux distros emerged. Probably in gcc or glibc sources one will find
> the root of where the use of suffix identifiers like /usr/lib64
> started. Leaves the question open "why".

That's pretty clear: to be able to install libraries for multiple
architectures at the same time.

Thanks

Michal

Re: [PATCH rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB

[Jan Engelhardt]

On Friday 2023-11-10 18:44, Michal Suchánek wrote:
>> It's a complicated mumble-jumble. Prior art exists as in:
>> 
>>  /opt/vendorThing/bin/...
>>  /usr/X11R6/lib/libXi.so.6 [host binary]
>>  /usr/x86_64-w64-mingw32/bin/as [host binary]
>>  /usr/x86_64-w64-mingw32/sys-root/mingw/bin/as.exe [foreign binary]
>>  /usr/platform/SUNW,Ultra-2/lib/libprtdiag_psr.so.1 [looks foreign]
>> 
>> The use of suffix-based naming must have been established sometime
>> near the end of the 90s or the start of 2000s as the first biarch
>> Linux distros emerged. Probably in gcc or glibc sources one will find
>> the root of where the use of suffix identifiers like /usr/lib64
>> started. Leaves the question open "why".
>
>That's pretty clear: to be able to install libraries for multiple
>architectures at the same time.

Well, what I tried to express or imply was something like:

“ we could (should?) have used /usr/<triplet>/lib rather than
  /usr/lib<suffixortriplet> all along, because at some point, there *will* be
  someone who wants to provide not only arch-different libraries, but *also*
  arch-different binaries (for whatever reason).

[PATCH v6 1/2] depmod: Handle installing modules under a different directory

[Michal Suchanek]

Some distributions aim at shipping all files in /usr.

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

When kmod provides kmod.pc, use it to determine the correct module
installation path.

With kmod that does not provide the config /lib/modules is used as
before.

While pkg-config does not return an error when a variable does not exist
the kmod configure script puts some effort into ensuring that
module_directory is non-empty. With that empty module_directory from
pkg-config can be used to detect absence of the variable.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
v6:
 - use ?= instead of := to make it easier to override the value
 - use shorter expression for determining the module directory assuming
   it's non-empty
---
 Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 511b5616aa41..84f32bd563d4 100644
--- a/Makefile
+++ b/Makefile
@@ -1081,7 +1081,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_DIRECTORY ?= $(or $(shell pkg-config --variable=module_directory kmod 2>/dev/null),/lib/modules)
+
+MODLIB	= $(INSTALL_MOD_PATH)$(KERNEL_MODULE_DIRECTORY)/$(KERNELRELEASE)
 export MODLIB
 
 PHONY += prepare0
-- 
2.42.0

Re: [PATCH v6 1/2] depmod: Handle installing modules under a different directory

[Masahiro Yamada]

On Thu, Dec 7, 2023 at 4:48 AM Michal Suchanek <msuchanek@suse.de> wrote:
>
> Some distributions aim at shipping all files in /usr.
>
> The path under which kernel modules are installed is hardcoded to /lib
> which conflicts with this goal.
>
> When kmod provides kmod.pc, use it to determine the correct module
> installation path.
>
> With kmod that does not provide the config /lib/modules is used as
> before.
>
> While pkg-config does not return an error when a variable does not exist
> the kmod configure script puts some effort into ensuring that
> module_directory is non-empty. With that empty module_directory from
> pkg-config can be used to detect absence of the variable.
>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> v6:
>  - use ?= instead of := to make it easier to override the value


"KERNEL_MODULE_DIRECTORY=/local/usr/lib/modules make modules_install"
will override the install destination, but
depmod will not be not aware of it.

How to avoid the depmod error?














>  - use shorter expression for determining the module directory assuming
>    it's non-empty
> ---
>  Makefile | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 511b5616aa41..84f32bd563d4 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1081,7 +1081,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_DIRECTORY ?= $(or $(shell pkg-config --variable=module_directory kmod 2>/dev/null),/lib/modules)
> +
> +MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_DIRECTORY)/$(KERNELRELEASE)
>  export MODLIB
>
>  PHONY += prepare0
> --
> 2.42.0
>
>


--
Best Regards
Masahiro Yamada

Re: [PATCH v6 1/2] depmod: Handle installing modules under a different directory

[Woody Suwalski]

Masahiro Yamada wrote:
> On Thu, Dec 7, 2023 at 4:48 AM Michal Suchanek <msuchanek@suse.de> wrote:
>> Some distributions aim at shipping all files in /usr.
>>
>> The path under which kernel modules are installed is hardcoded to /lib
>> which conflicts with this goal.
>>
>> When kmod provides kmod.pc, use it to determine the correct module
>> installation path.
>>
>> With kmod that does not provide the config /lib/modules is used as
>> before.
>>
>> While pkg-config does not return an error when a variable does not exist
>> the kmod configure script puts some effort into ensuring that
>> module_directory is non-empty. With that empty module_directory from
>> pkg-config can be used to detect absence of the variable.
>>
>> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
>> ---
>> v6:
>>   - use ?= instead of := to make it easier to override the value
>
> "KERNEL_MODULE_DIRECTORY=/local/usr/lib/modules make modules_install"
> will override the install destination, but
> depmod will not be not aware of it.
>
> How to avoid the depmod error?
>
>
I think the depmod -b option can be used to ran depmod in an arbitrary 
location:

The following options are useful for people managing distributions:
     -b, --basedir=DIR    Use an image of a module tree.

Woody

>
>
>
>
>
>
>
>
>
>
>
>>   - use shorter expression for determining the module directory assuming
>>     it's non-empty
>> ---
>>   Makefile | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 511b5616aa41..84f32bd563d4 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1081,7 +1081,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_DIRECTORY ?= $(or $(shell pkg-config --variable=module_directory kmod 2>/dev/null),/lib/modules)
>> +
>> +MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_DIRECTORY)/$(KERNELRELEASE)
>>   export MODLIB
>>
>>   PHONY += prepare0
>> --
>> 2.42.0
>>
>>
>
> --
> Best Regards
> Masahiro Yamada

Re: [PATCH v6 1/2] depmod: Handle installing modules under a different directory

[Michal Suchánek]

Hello!

On Mon, Dec 11, 2023 at 03:43:44AM +0900, Masahiro Yamada wrote:
> On Thu, Dec 7, 2023 at 4:48 AM Michal Suchanek <msuchanek@suse.de> wrote:
> >
> > Some distributions aim at shipping all files in /usr.
> >
> > The path under which kernel modules are installed is hardcoded to /lib
> > which conflicts with this goal.
> >
> > When kmod provides kmod.pc, use it to determine the correct module
> > installation path.
> >
> > With kmod that does not provide the config /lib/modules is used as
> > before.
> >
> > While pkg-config does not return an error when a variable does not exist
> > the kmod configure script puts some effort into ensuring that
> > module_directory is non-empty. With that empty module_directory from
> > pkg-config can be used to detect absence of the variable.
> >
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> > v6:
> >  - use ?= instead of := to make it easier to override the value
> 
> 
> "KERNEL_MODULE_DIRECTORY=/local/usr/lib/modules make modules_install"
> will override the install destination, but
> depmod will not be not aware of it.

At the same time if you know what you are doing you can build a src rpm
for another system that uses a different location.

> How to avoid the depmod error?

Not override the variable?

Thanks

Michal

> >  - use shorter expression for determining the module directory assuming
> >    it's non-empty
> > ---
> >  Makefile | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 511b5616aa41..84f32bd563d4 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1081,7 +1081,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_DIRECTORY ?= $(or $(shell pkg-config --variable=module_directory kmod 2>/dev/null),/lib/modules)
> > +
> > +MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_DIRECTORY)/$(KERNELRELEASE)
> >  export MODLIB
> >
> >  PHONY += prepare0
> > --
> > 2.42.0
> >
> >
> 
> 
> --
> Best Regards
> Masahiro Yamada

Re: [PATCH v6 1/2] depmod: Handle installing modules under a different directory

[Masahiro Yamada]

On Mon, Dec 11, 2023 at 6:07 AM Michal Suchánek <msuchanek@suse.de> wrote:
>
> Hello!
>
> On Mon, Dec 11, 2023 at 03:43:44AM +0900, Masahiro Yamada wrote:
> > On Thu, Dec 7, 2023 at 4:48 AM Michal Suchanek <msuchanek@suse.de> wrote:
> > >
> > > Some distributions aim at shipping all files in /usr.
> > >
> > > The path under which kernel modules are installed is hardcoded to /lib
> > > which conflicts with this goal.
> > >
> > > When kmod provides kmod.pc, use it to determine the correct module
> > > installation path.
> > >
> > > With kmod that does not provide the config /lib/modules is used as
> > > before.
> > >
> > > While pkg-config does not return an error when a variable does not exist
> > > the kmod configure script puts some effort into ensuring that
> > > module_directory is non-empty. With that empty module_directory from
> > > pkg-config can be used to detect absence of the variable.
> > >
> > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > ---
> > > v6:
> > >  - use ?= instead of := to make it easier to override the value
> >
> >
> > "KERNEL_MODULE_DIRECTORY=/local/usr/lib/modules make modules_install"
> > will override the install destination, but
> > depmod will not be not aware of it.
>
> At the same time if you know what you are doing you can build a src rpm
> for another system that uses a different location.
>
> > How to avoid the depmod error?
>
> Not override the variable?




You are not answering my question.


You intentionally changed := to ?=.

This implies that KERNEL_MODULE_DIRECTORY is an interface to users,
and should be documented in Documentation/kbuild/kbuild.rst


However, it never works if it is overridden from the env variable
or make command line because there is no way to let depmod know
the fact that KERNEL_MODULE_DIRECTORY has been overridden.



In my understanding, depmod does not provide an option to
specify the module directory from a command line option, does it?
If not, is it reasonable to add a new option to depmod?


depmod provides the "-b basedir" option, but it only allows
adding a prefix to the default "/lib/modules/<version>".

(My original idea to provide the prefix_part, it would have worked
like  -b "${INSTALL_MOD_PATH}${MOD_PREFIX}", which you refused)




















> Thanks
>
> Michal
>
> > >  - use shorter expression for determining the module directory assuming
> > >    it's non-empty
> > > ---
> > >  Makefile | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index 511b5616aa41..84f32bd563d4 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -1081,7 +1081,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_DIRECTORY ?= $(or $(shell pkg-config --variable=module_directory kmod 2>/dev/null),/lib/modules)
> > > +
> > > +MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_DIRECTORY)/$(KERNELRELEASE)
> > >  export MODLIB
> > >
> > >  PHONY += prepare0
> > > --
> > > 2.42.0
> > >
> > >
> >
> >
> > --
> > Best Regards
> > Masahiro Yamada
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH v6 1/2] depmod: Handle installing modules under a different directory

[Michal Suchánek]

On Mon, Dec 11, 2023 at 01:29:15PM +0900, Masahiro Yamada wrote:
> On Mon, Dec 11, 2023 at 6:07 AM Michal Suchánek <msuchanek@suse.de> wrote:
> >
> > Hello!
> >
> > On Mon, Dec 11, 2023 at 03:43:44AM +0900, Masahiro Yamada wrote:
> > > On Thu, Dec 7, 2023 at 4:48 AM Michal Suchanek <msuchanek@suse.de> wrote:
> > > >
> > > > Some distributions aim at shipping all files in /usr.
> > > >
> > > > The path under which kernel modules are installed is hardcoded to /lib
> > > > which conflicts with this goal.
> > > >
> > > > When kmod provides kmod.pc, use it to determine the correct module
> > > > installation path.
> > > >
> > > > With kmod that does not provide the config /lib/modules is used as
> > > > before.
> > > >
> > > > While pkg-config does not return an error when a variable does not exist
> > > > the kmod configure script puts some effort into ensuring that
> > > > module_directory is non-empty. With that empty module_directory from
> > > > pkg-config can be used to detect absence of the variable.
> > > >
> > > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > > ---
> > > > v6:
> > > >  - use ?= instead of := to make it easier to override the value
> > >
> > >
> > > "KERNEL_MODULE_DIRECTORY=/local/usr/lib/modules make modules_install"
> > > will override the install destination, but
> > > depmod will not be not aware of it.
> >
> > At the same time if you know what you are doing you can build a src rpm
> > for another system that uses a different location.
> >
> > > How to avoid the depmod error?
> >
> > Not override the variable?
> 
> You are not answering my question.
> You intentionally changed := to ?=.
> 
> This implies that KERNEL_MODULE_DIRECTORY is an interface to users,
> and should be documented in Documentation/kbuild/kbuild.rst

That's reasonable

> However, it never works if it is overridden from the env variable
> or make command line because there is no way to let depmod know
> the fact that KERNEL_MODULE_DIRECTORY has been overridden.

And there should not. kmod is not aware, kbuild is. That's the
direction of information flow. kmod defines where it looks for the
modules, and kbuild shoukld install the modules there.

If the user knows better (eg. possibility of building src-rpm for a
different you brought up) they can override the autodetection.

> In my understanding, depmod does not provide an option to
> specify the module directory from a command line option, does it?

No it does not.

> If not, is it reasonable to add a new option to depmod?

I don't think so. The module directory is intentionally in a fixed
location. It can be set at compile time, and that's it.

Then when running depmod on the target distribution either kbuild and
kmod agree on the location or the build fails. That's the intended
outcome.

kmod recently grew the ability to use modules outside of module
directory. For that to work internally the path to these out-of-kernel
modules is stored as absolute path, and the path of modules that are in
the module directory is stored relative to the module directory.

Setting the module directory location dynamically still should not break
this but I am not sure it's a great idea. In the end modprobe needs to
find those modules, and if depmod puts the modules.dep in arbitrary
location it will not.

> depmod provides the "-b basedir" option, but it only allows
> adding a prefix to the default "/lib/modules/<version>".

Yes, that's for installation into a staging directory, and there again
the modules that are inside the module directory are considedred
'in-kernel'. Not sure how well this even works with 'out-of-kernel'
modules.

> (My original idea to provide the prefix_part, it would have worked
> like  -b "${INSTALL_MOD_PATH}${MOD_PREFIX}", which you refused)

It's not clear that adding a prefix covers all use cases. It is an
arbitrary limitation that the module path must end with '/lib/modules'.

It may allow taking some shortcuts in some places but is unnecessarily
limiting.

Thanks

Michal

Re: [PATCH v6 1/2] depmod: Handle installing modules under a different directory

[Masahiro Yamada]

On Tue, Dec 12, 2023 at 10:03 PM Michal Suchánek <msuchanek@suse.de> wrote:
>
> On Mon, Dec 11, 2023 at 01:29:15PM +0900, Masahiro Yamada wrote:
> > On Mon, Dec 11, 2023 at 6:07 AM Michal Suchánek <msuchanek@suse.de> wrote:
> > >
> > > Hello!
> > >
> > > On Mon, Dec 11, 2023 at 03:43:44AM +0900, Masahiro Yamada wrote:
> > > > On Thu, Dec 7, 2023 at 4:48 AM Michal Suchanek <msuchanek@suse.de> wrote:
> > > > >
> > > > > Some distributions aim at shipping all files in /usr.
> > > > >
> > > > > The path under which kernel modules are installed is hardcoded to /lib
> > > > > which conflicts with this goal.
> > > > >
> > > > > When kmod provides kmod.pc, use it to determine the correct module
> > > > > installation path.
> > > > >
> > > > > With kmod that does not provide the config /lib/modules is used as
> > > > > before.
> > > > >
> > > > > While pkg-config does not return an error when a variable does not exist
> > > > > the kmod configure script puts some effort into ensuring that
> > > > > module_directory is non-empty. With that empty module_directory from
> > > > > pkg-config can be used to detect absence of the variable.
> > > > >
> > > > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > > > ---
> > > > > v6:
> > > > >  - use ?= instead of := to make it easier to override the value
> > > >
> > > >
> > > > "KERNEL_MODULE_DIRECTORY=/local/usr/lib/modules make modules_install"
> > > > will override the install destination, but
> > > > depmod will not be not aware of it.
> > >
> > > At the same time if you know what you are doing you can build a src rpm
> > > for another system that uses a different location.
> > >
> > > > How to avoid the depmod error?
> > >
> > > Not override the variable?
> >
> > You are not answering my question.
> > You intentionally changed := to ?=.
> >
> > This implies that KERNEL_MODULE_DIRECTORY is an interface to users,
> > and should be documented in Documentation/kbuild/kbuild.rst
>
> That's reasonable
>
> > However, it never works if it is overridden from the env variable
> > or make command line because there is no way to let depmod know
> > the fact that KERNEL_MODULE_DIRECTORY has been overridden.
>
> And there should not. kmod is not aware, kbuild is. That's the
> direction of information flow. kmod defines where it looks for the
> modules, and kbuild shoukld install the modules there.


Then, you cannot explain why KERNEL_MODULE_DIRECTORY should be exposed
as a user interface.



The MODULE_DIRECTORY in depmod is determined when kmod is compiled.

Kbuild takes KERNEL_MODULE_DIRECTORY from pkg-config.


If these two do not agree, it never works.





> If the user knows better (eg. possibility of building src-rpm for a
> different you brought up) they can override the autodetection.


No, it does not work.


The user has no way to override the MODULE_DIRECTORY in depmod.





> > In my understanding, depmod does not provide an option to
> > specify the module directory from a command line option, does it?
>
> No it does not.
>
> > If not, is it reasonable to add a new option to depmod?
>
> I don't think so. The module directory is intentionally in a fixed
> location. It can be set at compile time, and that's it.
>
> Then when running depmod on the target distribution either kbuild and
> kmod agree on the location or the build fails. That's the intended
> outcome.
>
> kmod recently grew the ability to use modules outside of module
> directory. For that to work internally the path to these out-of-kernel
> modules is stored as absolute path, and the path of modules that are in
> the module directory is stored relative to the module directory.
>
> Setting the module directory location dynamically still should not break
> this but I am not sure it's a great idea. In the end modprobe needs to
> find those modules, and if depmod puts the modules.dep in arbitrary
> location it will not.


That is true when modules are compiled and installed on the local machine.


If you create an SRPM with KERNEL_MODULE_DIRECTORY,
builders must follow it.





>
> > depmod provides the "-b basedir" option, but it only allows
> > adding a prefix to the default "/lib/modules/<version>".
>
> Yes, that's for installation into a staging directory, and there again
> the modules that are inside the module directory are considedred
> 'in-kernel'. Not sure how well this even works with 'out-of-kernel'
> modules.
>
> > (My original idea to provide the prefix_part, it would have worked
> > like  -b "${INSTALL_MOD_PATH}${MOD_PREFIX}", which you refused)
>
> It's not clear that adding a prefix covers all use cases. It is an
> arbitrary limitation that the module path must end with '/lib/modules'.
>
> It may allow taking some shortcuts in some places but is unnecessarily
> limiting.
>
> Thanks
>
> Michal



-- 
Best Regards
Masahiro Yamada

[PATCH v6 2/2] kbuild: rpm-pkg: Fix build with non-default MODLIB

[Michal Suchanek]

The default MODLIB value is composed of three variables

MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_DIRECTORY)/$(KERNELRELEASE)

However, the kernel.spec hadcodes the default value of
$(KERNEL_MODULE_DIRECTORY), and changed value is not reflected when
building the package.

Pass KERNEL_MODULE_DIRECTORY to kernel.spec to fix this problem.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
Build on top of the previous patch adding KERNEL_MODULE_DIRECTORY
---
 scripts/package/kernel.spec | 8 ++++----
 scripts/package/mkspec      | 1 +
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/scripts/package/kernel.spec b/scripts/package/kernel.spec
index 3eee0143e0c5..12996ed365f8 100644
--- a/scripts/package/kernel.spec
+++ b/scripts/package/kernel.spec
@@ -67,7 +67,7 @@ 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}%{KERNEL_MODULE_DIRECTORY}/%{KERNELRELEASE}/build
 %if %{with_devel}
 %{make} %{makeflags} run-command KBUILD_RUN_COMMAND='${srctree}/scripts/package/install-extmod-build %{buildroot}/usr/src/kernels/%{KERNELRELEASE}'
 %endif
@@ -98,8 +98,8 @@ fi
 
 %files
 %defattr (-, root, root)
-/lib/modules/%{KERNELRELEASE}
-%exclude /lib/modules/%{KERNELRELEASE}/build
+%{KERNEL_MODULE_DIRECTORY}/%{KERNELRELEASE}
+%exclude %{KERNEL_MODULE_DIRECTORY}/%{KERNELRELEASE}/build
 /boot/*
 
 %files headers
@@ -110,5 +110,5 @@ fi
 %files devel
 %defattr (-, root, root)
 /usr/src/kernels/%{KERNELRELEASE}
-/lib/modules/%{KERNELRELEASE}/build
+%{KERNEL_MODULE_DIRECTORY}/%{KERNELRELEASE}/build
 %endif
diff --git a/scripts/package/mkspec b/scripts/package/mkspec
index ce201bfa8377..e952fa4f2937 100755
--- a/scripts/package/mkspec
+++ b/scripts/package/mkspec
@@ -24,6 +24,7 @@ fi
 cat<<EOF
 %define ARCH ${ARCH}
 %define KERNELRELEASE ${KERNELRELEASE}
+%define KERNEL_MODULE_DIRECTORY ${KERNEL_MODULE_DIRECTORY}
 %define pkg_release $("${srctree}/init/build-version")
 EOF
 
-- 
2.42.0

Re: [PATCH v6 2/2] kbuild: rpm-pkg: Fix build with non-default MODLIB

[Masahiro Yamada]

On Thu, Dec 7, 2023 at 4:48 AM Michal Suchanek <msuchanek@suse.de> wrote:
>
> The default MODLIB value is composed of three variables
>
> MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_DIRECTORY)/$(KERNELRELEASE)
>
> However, the kernel.spec hadcodes the default value of
> $(KERNEL_MODULE_DIRECTORY), and changed value is not reflected when
> building the package.
>
> Pass KERNEL_MODULE_DIRECTORY to kernel.spec to fix this problem.
>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> Build on top of the previous patch adding KERNEL_MODULE_DIRECTORY


The SRPM package created by 'make srcrpm-pkg' may not work
if rpmbuild is executed in a different machine.



%{make} %{makeflags} INSTALL_MOD_PATH=%{buildroot}
KERNEL_MODULE_DIRECTORY=%{KERNEL_MODULE_DIRECTORY} modules_install


will align with the specified install destination,
but depmod will still fail.
(same issue as 1/2)









> ---
>  scripts/package/kernel.spec | 8 ++++----
>  scripts/package/mkspec      | 1 +
>  2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/package/kernel.spec b/scripts/package/kernel.spec
> index 3eee0143e0c5..12996ed365f8 100644
> --- a/scripts/package/kernel.spec
> +++ b/scripts/package/kernel.spec
> @@ -67,7 +67,7 @@ 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}%{KERNEL_MODULE_DIRECTORY}/%{KERNELRELEASE}/build
>  %if %{with_devel}
>  %{make} %{makeflags} run-command KBUILD_RUN_COMMAND='${srctree}/scripts/package/install-extmod-build %{buildroot}/usr/src/kernels/%{KERNELRELEASE}'
>  %endif
> @@ -98,8 +98,8 @@ fi
>
>  %files
>  %defattr (-, root, root)
> -/lib/modules/%{KERNELRELEASE}
> -%exclude /lib/modules/%{KERNELRELEASE}/build
> +%{KERNEL_MODULE_DIRECTORY}/%{KERNELRELEASE}
> +%exclude %{KERNEL_MODULE_DIRECTORY}/%{KERNELRELEASE}/build
>  /boot/*
>
>  %files headers
> @@ -110,5 +110,5 @@ fi
>  %files devel
>  %defattr (-, root, root)
>  /usr/src/kernels/%{KERNELRELEASE}
> -/lib/modules/%{KERNELRELEASE}/build
> +%{KERNEL_MODULE_DIRECTORY}/%{KERNELRELEASE}/build
>  %endif
> diff --git a/scripts/package/mkspec b/scripts/package/mkspec
> index ce201bfa8377..e952fa4f2937 100755
> --- a/scripts/package/mkspec
> +++ b/scripts/package/mkspec
> @@ -24,6 +24,7 @@ fi
>  cat<<EOF
>  %define ARCH ${ARCH}
>  %define KERNELRELEASE ${KERNELRELEASE}
> +%define KERNEL_MODULE_DIRECTORY ${KERNEL_MODULE_DIRECTORY}
>  %define pkg_release $("${srctree}/init/build-version")
>  EOF
>
> --
> 2.42.0
>
>


--
Best Regards
Masahiro Yamada

Re: [PATCH v6 2/2] kbuild: rpm-pkg: Fix build with non-default MODLIB

[Michal Suchánek]

On Mon, Dec 11, 2023 at 03:44:35AM +0900, Masahiro Yamada wrote:
> On Thu, Dec 7, 2023 at 4:48 AM Michal Suchanek <msuchanek@suse.de> wrote:
> >
> > The default MODLIB value is composed of three variables
> >
> > MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_DIRECTORY)/$(KERNELRELEASE)
> >
> > However, the kernel.spec hadcodes the default value of
> > $(KERNEL_MODULE_DIRECTORY), and changed value is not reflected when
> > building the package.
> >
> > Pass KERNEL_MODULE_DIRECTORY to kernel.spec to fix this problem.
> >
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> > Build on top of the previous patch adding KERNEL_MODULE_DIRECTORY
> 
> 
> The SRPM package created by 'make srcrpm-pkg' may not work
> if rpmbuild is executed in a different machine.

That's why there is an option to override KERNEL_MODULE_DIRECTORY?

Thanks

Michal

> 
> 
> 
> %{make} %{makeflags} INSTALL_MOD_PATH=%{buildroot}
> KERNEL_MODULE_DIRECTORY=%{KERNEL_MODULE_DIRECTORY} modules_install
> 
> 
> will align with the specified install destination,
> but depmod will still fail.
> (same issue as 1/2)
> 
> 
> 
> 
> 
> 
> 
> 
> 
> > ---
> >  scripts/package/kernel.spec | 8 ++++----
> >  scripts/package/mkspec      | 1 +
> >  2 files changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/scripts/package/kernel.spec b/scripts/package/kernel.spec
> > index 3eee0143e0c5..12996ed365f8 100644
> > --- a/scripts/package/kernel.spec
> > +++ b/scripts/package/kernel.spec
> > @@ -67,7 +67,7 @@ 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}%{KERNEL_MODULE_DIRECTORY}/%{KERNELRELEASE}/build
> >  %if %{with_devel}
> >  %{make} %{makeflags} run-command KBUILD_RUN_COMMAND='${srctree}/scripts/package/install-extmod-build %{buildroot}/usr/src/kernels/%{KERNELRELEASE}'
> >  %endif
> > @@ -98,8 +98,8 @@ fi
> >
> >  %files
> >  %defattr (-, root, root)
> > -/lib/modules/%{KERNELRELEASE}
> > -%exclude /lib/modules/%{KERNELRELEASE}/build
> > +%{KERNEL_MODULE_DIRECTORY}/%{KERNELRELEASE}
> > +%exclude %{KERNEL_MODULE_DIRECTORY}/%{KERNELRELEASE}/build
> >  /boot/*
> >
> >  %files headers
> > @@ -110,5 +110,5 @@ fi
> >  %files devel
> >  %defattr (-, root, root)
> >  /usr/src/kernels/%{KERNELRELEASE}
> > -/lib/modules/%{KERNELRELEASE}/build
> > +%{KERNEL_MODULE_DIRECTORY}/%{KERNELRELEASE}/build
> >  %endif
> > diff --git a/scripts/package/mkspec b/scripts/package/mkspec
> > index ce201bfa8377..e952fa4f2937 100755
> > --- a/scripts/package/mkspec
> > +++ b/scripts/package/mkspec
> > @@ -24,6 +24,7 @@ fi
> >  cat<<EOF
> >  %define ARCH ${ARCH}
> >  %define KERNELRELEASE ${KERNELRELEASE}
> > +%define KERNEL_MODULE_DIRECTORY ${KERNEL_MODULE_DIRECTORY}
> >  %define pkg_release $("${srctree}/init/build-version")
> >  EOF
> >
> > --
> > 2.42.0
> >
> >
> 
> 
> --
> Best Regards
> Masahiro Yamada

Re: [PATCH v6 2/2] kbuild: rpm-pkg: Fix build with non-default MODLIB

[Masahiro Yamada]

On Mon, Dec 11, 2023 at 6:09 AM Michal Suchánek <msuchanek@suse.de> wrote:
>
> On Mon, Dec 11, 2023 at 03:44:35AM +0900, Masahiro Yamada wrote:
> > On Thu, Dec 7, 2023 at 4:48 AM Michal Suchanek <msuchanek@suse.de> wrote:
> > >
> > > The default MODLIB value is composed of three variables
> > >
> > > MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_DIRECTORY)/$(KERNELRELEASE)
> > >
> > > However, the kernel.spec hadcodes the default value of
> > > $(KERNEL_MODULE_DIRECTORY), and changed value is not reflected when
> > > building the package.
> > >
> > > Pass KERNEL_MODULE_DIRECTORY to kernel.spec to fix this problem.
> > >
> > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > ---
> > > Build on top of the previous patch adding KERNEL_MODULE_DIRECTORY
> >
> >
> > The SRPM package created by 'make srcrpm-pkg' may not work
> > if rpmbuild is executed in a different machine.
>
> That's why there is an option to override KERNEL_MODULE_DIRECTORY?


Yes.
But, as I pointed out in 1/2, depmod must follow the packager's decision.

'make srcrpm-pkg' creates a SRPM on machine A.
'rpmbuild' builds it into binary RPMs on machine B.

If A and B disagree about kmod.pc, depmod will fail
because there is no code to force the decision made
on machine A.












> Thanks
>
> Michal
>
> >
> >
> >
> > %{make} %{makeflags} INSTALL_MOD_PATH=%{buildroot}
> > KERNEL_MODULE_DIRECTORY=%{KERNEL_MODULE_DIRECTORY} modules_install
> >
> >
> > will align with the specified install destination,
> > but depmod will still fail.
> > (same issue as 1/2)
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > > ---
> > >  scripts/package/kernel.spec | 8 ++++----
> > >  scripts/package/mkspec      | 1 +
> > >  2 files changed, 5 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/scripts/package/kernel.spec b/scripts/package/kernel.spec
> > > index 3eee0143e0c5..12996ed365f8 100644
> > > --- a/scripts/package/kernel.spec
> > > +++ b/scripts/package/kernel.spec
> > > @@ -67,7 +67,7 @@ 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}%{KERNEL_MODULE_DIRECTORY}/%{KERNELRELEASE}/build
> > >  %if %{with_devel}
> > >  %{make} %{makeflags} run-command KBUILD_RUN_COMMAND='${srctree}/scripts/package/install-extmod-build %{buildroot}/usr/src/kernels/%{KERNELRELEASE}'
> > >  %endif
> > > @@ -98,8 +98,8 @@ fi
> > >
> > >  %files
> > >  %defattr (-, root, root)
> > > -/lib/modules/%{KERNELRELEASE}
> > > -%exclude /lib/modules/%{KERNELRELEASE}/build
> > > +%{KERNEL_MODULE_DIRECTORY}/%{KERNELRELEASE}
> > > +%exclude %{KERNEL_MODULE_DIRECTORY}/%{KERNELRELEASE}/build
> > >  /boot/*
> > >
> > >  %files headers
> > > @@ -110,5 +110,5 @@ fi
> > >  %files devel
> > >  %defattr (-, root, root)
> > >  /usr/src/kernels/%{KERNELRELEASE}
> > > -/lib/modules/%{KERNELRELEASE}/build
> > > +%{KERNEL_MODULE_DIRECTORY}/%{KERNELRELEASE}/build
> > >  %endif
> > > diff --git a/scripts/package/mkspec b/scripts/package/mkspec
> > > index ce201bfa8377..e952fa4f2937 100755
> > > --- a/scripts/package/mkspec
> > > +++ b/scripts/package/mkspec
> > > @@ -24,6 +24,7 @@ fi
> > >  cat<<EOF
> > >  %define ARCH ${ARCH}
> > >  %define KERNELRELEASE ${KERNELRELEASE}
> > > +%define KERNEL_MODULE_DIRECTORY ${KERNEL_MODULE_DIRECTORY}
> > >  %define pkg_release $("${srctree}/init/build-version")
> > >  EOF
> > >
> > > --
> > > 2.42.0
> > >
> > >
> >
> >
> > --
> > Best Regards
> > Masahiro Yamada



--
Best Regards
Masahiro Yamada

Re: [PATCH v6 2/2] kbuild: rpm-pkg: Fix build with non-default MODLIB

[Michal Suchánek]

On Mon, Dec 11, 2023 at 01:33:23PM +0900, Masahiro Yamada wrote:
> On Mon, Dec 11, 2023 at 6:09 AM Michal Suchánek <msuchanek@suse.de> wrote:
> >
> > On Mon, Dec 11, 2023 at 03:44:35AM +0900, Masahiro Yamada wrote:
> > > On Thu, Dec 7, 2023 at 4:48 AM Michal Suchanek <msuchanek@suse.de> wrote:
> > > >
> > > > The default MODLIB value is composed of three variables
> > > >
> > > > MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_DIRECTORY)/$(KERNELRELEASE)
> > > >
> > > > However, the kernel.spec hadcodes the default value of
> > > > $(KERNEL_MODULE_DIRECTORY), and changed value is not reflected when
> > > > building the package.
> > > >
> > > > Pass KERNEL_MODULE_DIRECTORY to kernel.spec to fix this problem.
> > > >
> > > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > > ---
> > > > Build on top of the previous patch adding KERNEL_MODULE_DIRECTORY
> > >
> > >
> > > The SRPM package created by 'make srcrpm-pkg' may not work
> > > if rpmbuild is executed in a different machine.
> >
> > That's why there is an option to override KERNEL_MODULE_DIRECTORY?
> 
> 
> Yes.
> But, as I pointed out in 1/2, depmod must follow the packager's decision.
> 
> 'make srcrpm-pkg' creates a SRPM on machine A.
> 'rpmbuild' builds it into binary RPMs on machine B.
> 
> If A and B disagree about kmod.pc, depmod will fail
> because there is no code to force the decision made
> on machine A.

There is. It's the ?= in the top Makefile.

Currently the test that determines the module directory uses make logic
so it's not possible to pass on the shell magic before executing it so
it could be executed inside the rpm spec file as well.

OUtsourcing it into an external script would mean that the sources need
to be unpacked before the script can be executed. That would require
using dynamically generated file list in the spec file because the
module location would not be known at spec parse time. Possible but
convoluted.

In the end I do not think this is a problem that needs solving. Most
distributions that build kernel packages would use their own packaging
files, not rpm-pkg. That limits rpm-pkg to ad-hoc use when people want
to build one-off test kernel. It's reasonable to do on the same
distribution as the target system. The option to do so on a distribution
with different module directory is available if somebody really needs
that.

Thanks

Michal

Re: [PATCH v6 2/2] kbuild: rpm-pkg: Fix build with non-default MODLIB

[Masahiro Yamada]

On Tue, Dec 12, 2023 at 10:12 PM Michal Suchánek <msuchanek@suse.de> wrote:
>
> On Mon, Dec 11, 2023 at 01:33:23PM +0900, Masahiro Yamada wrote:
> > On Mon, Dec 11, 2023 at 6:09 AM Michal Suchánek <msuchanek@suse.de> wrote:
> > >
> > > On Mon, Dec 11, 2023 at 03:44:35AM +0900, Masahiro Yamada wrote:
> > > > On Thu, Dec 7, 2023 at 4:48 AM Michal Suchanek <msuchanek@suse.de> wrote:
> > > > >
> > > > > The default MODLIB value is composed of three variables
> > > > >
> > > > > MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_DIRECTORY)/$(KERNELRELEASE)
> > > > >
> > > > > However, the kernel.spec hadcodes the default value of
> > > > > $(KERNEL_MODULE_DIRECTORY), and changed value is not reflected when
> > > > > building the package.
> > > > >
> > > > > Pass KERNEL_MODULE_DIRECTORY to kernel.spec to fix this problem.
> > > > >
> > > > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > > > ---
> > > > > Build on top of the previous patch adding KERNEL_MODULE_DIRECTORY
> > > >
> > > >
> > > > The SRPM package created by 'make srcrpm-pkg' may not work
> > > > if rpmbuild is executed in a different machine.
> > >
> > > That's why there is an option to override KERNEL_MODULE_DIRECTORY?
> >
> >
> > Yes.
> > But, as I pointed out in 1/2, depmod must follow the packager's decision.
> >
> > 'make srcrpm-pkg' creates a SRPM on machine A.
> > 'rpmbuild' builds it into binary RPMs on machine B.
> >
> > If A and B disagree about kmod.pc, depmod will fail
> > because there is no code to force the decision made
> > on machine A.
>
> There is. It's the ?= in the top Makefile.


Nope.


Only Kbuild follows the specified KERNEL_MODULE_DIRECTORY.


depmod still uses the MODULE_DRECTORY determined
when it was compiled.


>
> Currently the test that determines the module directory uses make logic
> so it's not possible to pass on the shell magic before executing it so
> it could be executed inside the rpm spec file as well.
>
> OUtsourcing it into an external script would mean that the sources need
> to be unpacked before the script can be executed. That would require
> using dynamically generated file list in the spec file because the
> module location would not be known at spec parse time. Possible but
> convoluted.


I do not require that.


This is simple; builders must follow the packager's decision.

To make it work, depmod must follow MODULE_DIRECTORY
given from an external environment.





> In the end I do not think this is a problem that needs solving. Most
> distributions that build kernel packages would use their own packaging
> files, not rpm-pkg. That limits rpm-pkg to ad-hoc use when people want
> to build one-off test kernel. It's reasonable to do on the same
> distribution as the target system. The option to do so on a distribution
> with different module directory is available if somebody really needs
> that.
>
> Thanks
>
> Michal



-- 
Best Regards
Masahiro Yamada