| Message ID | 20231005150728.3429-1-msuchanek@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB | expand |
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 >
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
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
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.
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
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'.
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..
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
> > > > 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
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
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
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
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
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
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".
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
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).
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
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(-)