Re: [PATCH v2 04/24] linux-module: Do not use shell environment

[Jan Kiszka <jan.kiszka@siemens.com>]

On 19.11.21 13:13, Uladzimir Bely wrote:
> From: Anton Mikanovich <amikan@ilbers.de>
> 
> To make package build process independent of the shell environment we
> should remove KDIR and PN passing through export call.
> KDIR can be prepared during package build. This also will allow not to
> rely on previous builddeps install task.
> To pass PN variable we can just migrate to template-based debian/rules
> file.
> 
> Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
> Signed-off-by: Uladzimir Bely <ubely@ilbers.de>
> ---
>  .../files/debian/{rules => rules.tmpl}        | 14 ++++++++++-
>  meta/recipes-kernel/linux-module/module.inc   | 23 +++++++------------
>  2 files changed, 21 insertions(+), 16 deletions(-)
>  rename meta/recipes-kernel/linux-module/files/debian/{rules => rules.tmpl} (50%)
> 
> diff --git a/meta/recipes-kernel/linux-module/files/debian/rules b/meta/recipes-kernel/linux-module/files/debian/rules.tmpl
> similarity index 50%
> rename from meta/recipes-kernel/linux-module/files/debian/rules
> rename to meta/recipes-kernel/linux-module/files/debian/rules.tmpl
> index 59720b37..d8f1d48a 100755
> --- a/meta/recipes-kernel/linux-module/files/debian/rules
> +++ b/meta/recipes-kernel/linux-module/files/debian/rules.tmpl
> @@ -24,14 +24,26 @@ ifneq (,$(findstring 86,$(DEB_HOST_GNU_CPU)))
>  export ARCH=x86
>  endif
>  
> +# Custom kernels contain the build folder directly.
> +KDIR := $(shell dpkg -L linux-headers-${KERNEL_NAME} | grep "/lib/modules/.*/build")
> +ifeq ($(KDIR),)
> +# Debian kernels install that folder indirectly via a dependency.
> +KERNEL_DEP := $(shell dpkg-query -W -f '$${Depends}' linux-headers-${KERNEL_NAME} | sed 's/.*\(linux-headers-[[:graph:]]*\).*/\1/')

Could we wrap this long line?

> +KDIR := $(shell dpkg -L $(KERNEL_DEP) | grep "/lib/modules/.*/build")
> +endif
> +
> +# With sbuild `dh clean` is called before chroot's apt database updated. So,
> +# KDIR is empty at that moment and the override causes error. Will skip it.
> +ifneq ($(KDIR),)
>  override_dh_auto_clean:
>  	$(MAKE) -C $(KDIR) M=$(PWD) clean
> +endif

This looks wrong, or at least fragile. Why can we live without a proper
clean on the kernel tree?

>  
>  override_dh_auto_build:
>  	$(MAKE) -C $(KDIR) M=$(PWD) modules
>  
>  override_dh_auto_install:
> -	$(MAKE) -C $(KDIR) M=$(PWD) INSTALL_MOD_PATH=$(PWD)/debian/$(PN) modules_install
> +	$(MAKE) -C $(KDIR) M=$(PWD) INSTALL_MOD_PATH=$(PWD)/debian/${PN} modules_install
>  
>  %:
>  	CFLAGS= LDFLAGS= dh $@ --parallel
> diff --git a/meta/recipes-kernel/linux-module/module.inc b/meta/recipes-kernel/linux-module/module.inc
> index 0515621a..5b086233 100644
> --- a/meta/recipes-kernel/linux-module/module.inc
> +++ b/meta/recipes-kernel/linux-module/module.inc
> @@ -22,8 +22,9 @@ AUTOLOAD ?= ""
>  inherit dpkg
>  
>  TEMPLATE_FILES = "debian/control.tmpl \
> -                  debian/changelog.tmpl"
> -TEMPLATE_VARS += "KERNEL_NAME"
> +                  debian/changelog.tmpl \
> +                  debian/rules.tmpl"
> +TEMPLATE_VARS += "KERNEL_NAME PN"
>  
>  do_prepare_build() {
>      cp -r ${WORKDIR}/debian ${S}/
> @@ -31,18 +32,10 @@ do_prepare_build() {
>      for module in "${AUTOLOAD}"; do
>          echo "echo $module >> /etc/modules" >> ${S}/debian/postinst
>      done
> -}
>  
> -dpkg_runbuild_prepend() {
> -    # Custom kernels contain the build folder directly.
> -    export KDIR=$(dpkg -L --root=${BUILDCHROOT_DIR} linux-headers-${KERNEL_NAME} | \
> -                  grep "/lib/modules/.*/build")
> -    if [ -z "$KDIR" ]; then
> -        # Debian kernels install that folder indirectly via a dependency.
> -        KERNEL_DEP=$(dpkg-query -W -f '${Depends}' --admindir=${BUILDCHROOT_DIR}/var/lib/dpkg \
> -                     linux-headers-${KERNEL_NAME} | sed 's/.*\(linux-headers-[[:graph:]]*\).*/\1/')
> -        export KDIR=$(dpkg -L --root=${BUILDCHROOT_DIR} ${KERNEL_DEP} | \
> -                      grep "/lib/modules/.*/build")
> -    fi
> -    export PN=${PN}
> +    # remove templates from the source tree
> +    find ${S}/debian -name *.tmpl | xargs rm -f

Hmm...

> +
> +    # restore execute permissions
> +    chmod a+x ${S}/debian/rules

The mode should be fixed (aligned to the template) in template.bbclass
to avoid bothering every recipe with such boilerplate code. I'm seeing
this in other changes as well.

With all those "do not use shell env" - how will a downstream user
detect that a recipe is not properly converted? That looks like a nice
trap during migration to sbuild (because of silent / non-obvious errors).

Jan

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux