Re: [PATCH v2] kernel.bbclass: hoist up "unset S" bbfatal from kernel-yocto.bbclass to kernel.bbclass

[Bruce Ashfield <bruce.ashfield@gmail.com>]

In message: [PATCH v2] kernel.bbclass: hoist up "unset S" bbfatal from kernel-yocto.bbclass to kernel.bbclass
on 26/06/2023 luca.ceresoli@bootlin.com wrote:

> From: Luca Ceresoli <luca.ceresoli@bootlin.com>
> 
> Writing a simple recipe that inherits kernel.bbclass and downloads a kernel
> tarball (e.g. a mainline release from kernel.org) via http or ftp fails
> with either:
> 
>   ERROR: linux-acme-6.3.3-r0 do_configure: oe_runmake failed
>   ...
>   | make: *** No rule to make target 'oldnoconfig'.  Stop.
> 
> or (seen on a different setup, based on kirkstone):
> 
>   ... do_populate_lic: QA Issue: ... LIC_FILES_CHKSUM points to an invalid file: .../work-shared/.../kernel-source/COPYING [license-checksum]
> 
> This happens when not setting S in the recipe. In this case, kernel.bbclass
> sets it to ${STAGING_KERNEL_DIR}
> (${TMPDIR}/work-shared/${MACHINE}/kernel-source).  This means that in
> do_symlink_kernsrc(), the 'if s != kernsrc' never triggers and thus the
> kernel tree will not be moved into work-shared, which results in an empty
> work-shared/.../kernel-source directory.
> 
> Setting S in recipes is usually not required when downloading a tarball, so
> it is not obvious here and the error message does not point to the problem
> or its solution.
> 
> There is such a check in kernel-yocto.bbclass though. Move it to
> kernel.bbclass so that even kernel recipes not based on kernel-yocto can
> benefit from it.
> 
> The check is moved:
> 
>  - from the beginning of do_kernel_checkout() in kernel-yocto
>  - to the end of do_symlink_kernsrc() in kernel.bbclass
> 
> and since do_kernel_checkout is executed 'after do_symlink_kernsrc', the
> code flow does not change in a relevant way when using linux-yocto.

That's the current task flow, but the checkout has moved in the past,
and could move again.

My "don't fix it if it isn't broken" inclination tells me to just
as for the new check to be added for the use case, and leave the
one in kernel-yocto as a backup, in case the task ordering changes
or some other strange path through the code appears later.

But either way, the change looks ok to me.

Bruce

> 
> As an additional benefit, the check is now taking place both when
> downloading a tarball and when downloading from git, so even when using git
> the recipe writer will be presented the explanatory error message.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> 
> ---
> 
> Changed in v2:
>  - don't check for WORKDIR/git dir existence (Suggested by Richard)
> ---
>  meta/classes-recipe/kernel-yocto.bbclass | 8 --------
>  meta/classes-recipe/kernel.bbclass       | 6 ++++++
>  2 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/meta/classes-recipe/kernel-yocto.bbclass b/meta/classes-recipe/kernel-yocto.bbclass
> index 4ac977b12207..3f2ce17aeb88 100644
> --- a/meta/classes-recipe/kernel-yocto.bbclass
> +++ b/meta/classes-recipe/kernel-yocto.bbclass
> @@ -394,16 +394,8 @@ do_kernel_checkout() {
>  		# case: we have no git repository at all. 
>  		# To support low bandwidth options for building the kernel, we'll just 
>  		# convert the tree to a git repo and let the rest of the process work unchanged
> -		
> -		# if ${S} hasn't been set to the proper subdirectory a default of "linux" is 
> -		# used, but we can't initialize that empty directory. So check it and throw a
> -		# clear error
>  
>  	        cd ${S}
> -		if [ ! -f "Makefile" ]; then
> -			bberror "S is not set to the linux source directory. Check "
> -			bbfatal "the recipe and set S to the proper extracted subdirectory"
> -		fi
>  		rm -f .gitignore
>  		git init
>  		check_git_config
> diff --git a/meta/classes-recipe/kernel.bbclass b/meta/classes-recipe/kernel.bbclass
> index e82b696d1a14..75f43cb1134e 100644
> --- a/meta/classes-recipe/kernel.bbclass
> +++ b/meta/classes-recipe/kernel.bbclass
> @@ -195,6 +195,12 @@ python do_symlink_kernsrc () {
>              import shutil
>              shutil.move(s, kernsrc)
>              os.symlink(kernsrc, s)
> +
> +    # Setting S is required with this class when fetching a tarball because
> +    # we cannot figure out automatically the extracted directory name. The
> +    # check is beneficial even when using git so don't check for git here.
> +    if not os.path.exists(os.path.join(s, "Makefile")):
> +        bb.fatal("S is not set to the linux source directory. Check the recipe and set S to the proper extracted subdirectory.")
>  }
>  # do_patch is normally ordered before do_configure, but
>  # externalsrc.bbclass deletes do_patch, breaking the dependency of
> -- 
> 2.34.1
>