diff mbox series

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

Message ID baa3224bece94220dfe7173432143a91f7612c09.1701892062.git.msuchanek@suse.de (mailing list archive)
State New, archived
Headers show
Series [v6,1/2] depmod: Handle installing modules under a different directory | expand

Commit Message

Michal Suchanek Dec. 6, 2023, 7:47 p.m. UTC
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(-)

Comments

Masahiro Yamada Dec. 10, 2023, 6:44 p.m. UTC | #1
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
Michal Suchanek Dec. 10, 2023, 9:08 p.m. UTC | #2
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
Masahiro Yamada Dec. 11, 2023, 4:33 a.m. UTC | #3
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
Michal Suchanek Dec. 12, 2023, 1:12 p.m. UTC | #4
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
Masahiro Yamada Dec. 18, 2023, 2:16 p.m. UTC | #5
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
diff mbox series

Patch

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