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

[Masahiro Yamada <masahiroy@kernel.org>]

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



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


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


Then, how to change the path later?






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

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


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

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

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


Overall, I did not find a cool solution.



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




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

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

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


?= is a recursive variable.

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

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







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


Sorry, it is not easy for me.

What is the background of your thought?



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


It IS convoluted.



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


This is up to the kmod maintainer.

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








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



-- 
Best Regards
Masahiro Yamada