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

Message ID 20231005150728.3429-1-msuchanek@suse.de
State New
Headers
Series [rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB |

Commit Message

Michal Suchánek Oct. 5, 2023, 3:07 p.m. UTC
  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(-)
  

Comments

Nathan Chancellor Oct. 6, 2023, 4:58 p.m. UTC | #1
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
>
  
Masahiro Yamada Oct. 9, 2023, 8:31 a.m. UTC | #2
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
  
Michal Suchánek Oct. 9, 2023, 8:52 a.m. UTC | #3
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
  
Masahiro Yamada Oct. 9, 2023, 12:34 p.m. UTC | #4
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.
  
Michal Suchánek Oct. 9, 2023, 2:07 p.m. UTC | #5
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
  
Masahiro Yamada Oct. 9, 2023, 3:14 p.m. UTC | #6
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'.
  
Jan Engelhardt Oct. 9, 2023, 4:01 p.m. UTC | #7
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..
  
Michal Suchánek Oct. 10, 2023, 10:15 a.m. UTC | #8
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
  
Masahiro Yamada Oct. 17, 2023, 10:15 a.m. UTC | #9
> >
> > 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
  
Michal Suchánek Oct. 17, 2023, 10:44 a.m. UTC | #10
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
  
Masahiro Yamada Oct. 17, 2023, 12:05 p.m. UTC | #11
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
  
Michal Suchánek Oct. 17, 2023, 12:27 p.m. UTC | #12
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
  
Masahiro Yamada Oct. 17, 2023, 2:46 p.m. UTC | #13
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
  
Michal Suchánek Oct. 17, 2023, 3:10 p.m. UTC | #14
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
  
Jan Engelhardt Oct. 18, 2023, 1:12 a.m. UTC | #15
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".
  
Michal Suchánek Nov. 10, 2023, 5:44 p.m. UTC | #16
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
  
Jan Engelhardt Nov. 10, 2023, 5:57 p.m. UTC | #17
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

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