On Tue, Oct 17, 2023 at 9:27 PM Michal Suchánek <msuchanek@xxxxxxx> 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@xxxxxxx> 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@xxxxxxx/ > > > > [2] https://github.com/kmod-project/kmod/blob/v31/configure.ac#L295 -- Best Regards Masahiro Yamada