Re: [Buildroot] [PATCHv2] package/zfs: Fix cross-platform-compilations

[Romain Naour <romain.naour@gmail.com>]

Hello Yann,

Le 27/08/2021 à 22:48, Yann E. MORIN a écrit :
> Romain, José, All,
> 
> On 2021-08-27 11:17 +0200, Romain Naour spake thusly:
>> From: José Luis Salvador Rufo <salvador.joseluis@gmail.com>
>>
>> This patch fixes the `make` executed from the `kernel.m4` in
>> cross-platform-compilations environments:
>> https://github.com/openzfs/zfs/blob/zfs-2.0.5/config/kernel.m4#L588
>>
>> Fixes:
>> https://gitlab.com/kubu93/buildroot/-/jobs/1522848483
>>
>> Signed-off-by: José Luis Salvador Rufo <salvador.joseluis@gmail.com>
>> Signed-off-by: Romain Naour <romain.naour@gmail.com>
>> ---
>> v2: add a patch to remove DESTDIR used with INSTALL_MOD_PATH
>>     Use LINUX_MAKE_FLAGS for ZFS_MAKE_ENV and ZFS_CONF_ENV
>>     http://lists.busybox.net/pipermail/buildroot/2021-August/621350.html
> 
> I've only had a cursory look at the previous discussion, sorry...
> 
> However, I believe this is incorrect to pass $(LINUX_MAKE_FLAGS) in the
> environment for configure and make.
> 
> Indeed, $(LINUX_MAKE_FLAGS) should really only be used when actually
> building kernel modules, not userland stuff, because it contains
> variables that are not valid otherwise.
> 
> For example, it overrides HOSTCC with a value that is not correct
> outside of bulding the kernel. It also conditionally contains the
> unscoped variable LOADADDR, which name is too generic.
> 
> Also, I don't understand why we need these at configure time, because we
> are using the kernel-module infra to build the kernel module... And
> basically, the two are mostly mutually exclusive...

The configure script is trying to build a kernel module to check if its able to
build zfs modules. Without LINUX_MAKE_FLAGS, it try to build the module for the
host machine that fail to build with old compiler/distribution.

I haven't looked deeply to the buildsystem...

> 
>> ---
>>  ...in-don-t-use-DESTDIR-and-INSTALL_MOD.patch | 44 +++++++++++++++++++
>>  package/zfs/zfs.mk                            |  5 +++
>>  2 files changed, 49 insertions(+)
>>  create mode 100644 package/zfs/0001-module-Makefile.in-don-t-use-DESTDIR-and-INSTALL_MOD.patch
>>
>> diff --git a/package/zfs/0001-module-Makefile.in-don-t-use-DESTDIR-and-INSTALL_MOD.patch b/package/zfs/0001-module-Makefile.in-don-t-use-DESTDIR-and-INSTALL_MOD.patch
>> new file mode 100644
>> index 0000000000..e1d0f91533
>> --- /dev/null
>> +++ b/package/zfs/0001-module-Makefile.in-don-t-use-DESTDIR-and-INSTALL_MOD.patch
>> @@ -0,0 +1,44 @@
>> +From 12851a8dbc0aeb8e48f21f1e0cb7b3123029594e Mon Sep 17 00:00:00 2001
>> +From: Romain Naour <romain.naour@gmail.com>
>> +Date: Thu, 26 Aug 2021 23:12:31 +0200
>> +Subject: [PATCH] module/Makefile.in: don't use DESTDIR and INSTALL_MOD_PATH
>> + together
>> +
>> +DESTDIR and INSTALL_MOD_PATH are usually set with absolute path
>> +by build system (Buildroot), so when used together it concatenates
>> +two absolute path.
>> +
>> +Keep INSTALL_MOD_PATH since it's set when building an out of tree
>> +kernel module.
>> +
>> +Signed-off-by: Romain Naour <romain.naour@gmail.com>
> 
> That patch on its own is correct, though. It should be pushed upstream.

ok,

> 
>> +---
>> + module/Makefile.in | 6 +++---
>> + 1 file changed, 3 insertions(+), 3 deletions(-)
>> +
>> +diff --git a/module/Makefile.in b/module/Makefile.in
>> +index 69caf4857..043f2bdea 100644
>> +--- a/module/Makefile.in
>> ++++ b/module/Makefile.in
>> +@@ -76,15 +76,15 @@ clean: clean-@ac_system@
>> + modules_install-Linux:
>> + 	@# Install the kernel modules
>> + 	$(MAKE) -C @LINUX_OBJ@ M=`pwd` modules_install \
>> +-		INSTALL_MOD_PATH=$(DESTDIR)$(INSTALL_MOD_PATH) \
>> ++		INSTALL_MOD_PATH=$(INSTALL_MOD_PATH) \
>> + 		INSTALL_MOD_DIR=$(INSTALL_MOD_DIR) \
>> + 		KERNELRELEASE=@LINUX_VERSION@
>> + 	@# Remove extraneous build products when packaging
>> +-	kmoddir=$(DESTDIR)$(INSTALL_MOD_PATH)/lib/modules/@LINUX_VERSION@; \
>> ++	kmoddir=$(INSTALL_MOD_PATH)/lib/modules/@LINUX_VERSION@; \
>> + 	if [ -n "$(DESTDIR)" ]; then \
>> + 		find $$kmoddir -name 'modules.*' | xargs $(RM); \
>> + 	fi
>> +-	sysmap=$(DESTDIR)$(INSTALL_MOD_PATH)/boot/System.map-@LINUX_VERSION@; \
>> ++	sysmap=$(INSTALL_MOD_PATH)/boot/System.map-@LINUX_VERSION@; \
>> + 	if [ -f $$sysmap ]; then \
>> + 		depmod -ae -F $$sysmap @LINUX_VERSION@; \
>> + 	fi
>> +-- 
>> +2.31.1
>> +
>> diff --git a/package/zfs/zfs.mk b/package/zfs/zfs.mk
>> index 3b1afb419a..537d4ece15 100644
>> --- a/package/zfs/zfs.mk
>> +++ b/package/zfs/zfs.mk
>> @@ -13,6 +13,11 @@ ZFS_CPE_ID_PRODUCT = openzfs
>>  
>>  ZFS_AUTORECONF = YES
>>  
>> +# cross compile environment for linux kernel module
>> +ZFS_MAKE_ENV = $(LINUX_MAKE_FLAGS)
>> +# `./configure` will execute a `make modules` test case
>> +ZFS_CONF_ENV = $(LINUX_MAKE_FLAGS)
> 
> This hunk I am not too happy about... :-/

I understand.

Best regards,
Romain

> 
> Regards,
> Yann E. MORIN.
> 
>>  ZFS_DEPENDENCIES = libaio openssl udev util-linux zlib
>>  
>>  # sysvinit installs only a commented-out modules-load.d/ config file
>> -- 
>> 2.31.1
>>
>> _______________________________________________
>> buildroot mailing list
>> buildroot@busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot
> 

_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot