Thread (31 messages) 31 messages, 5 authors, 2023-12-18

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

From: Michal Suchánek <hidden>
Date: 2023-10-17 15:11:16
Also in: linux-kbuild, lkml

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 [off-list ref] wrote:
quoted
On Tue, Oct 17, 2023 at 09:05:29PM +0900, Masahiro Yamada wrote:
quoted
On Tue, Oct 17, 2023 at 7:44 PM Michal Suchánek [off-list ref] wrote:
quoted
On Tue, Oct 17, 2023 at 07:15:50PM +0900, Masahiro Yamada wrote:
quoted
quoted
quoted
quoted
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?
quoted
quoted
quoted
quoted
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
quoted
quoted
quoted
quoted
[1] https://lore.kernel.org/linux-kbuild/20230718120348.383-1-msuchanek@suse.de/ (local)
[2] https://github.com/kmod-project/kmod/blob/v31/configure.ac#L295
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help