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

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

[luca.ceresoli]

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 me moved into work/shared, which results in an empty
work-shared/.../kernel-source directory.

When downloading a tarball it is usually not required to set S in recipes,
so this 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, so move it to
kernel.bbclass so that also 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.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 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 108b7e675212..ceb451b69969 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 9c8036f4df01..5ed4a2e03c72 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)
+
+    # When not using git, we cannot figure out automatically the extracted
+    # directory. So check it and throw a clear error.
+    if not os.path.isdir(os.path.join(d.getVar("WORKDIR"), "git")) and \
+       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

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

[Richard Purdie]

Hi Luca,

On Mon, 2023-06-05 at 16:13 +0200, Luca Ceresoli via lists.openembedded.org 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 me moved into work/shared, which results in an empty
> work-shared/.../kernel-source directory.
> 
> When downloading a tarball it is usually not required to set S in recipes,
> so this 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, so move it to
> kernel.bbclass so that also 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.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>  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 108b7e675212..ceb451b69969 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 9c8036f4df01..5ed4a2e03c72 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)
> +
> +    # When not using git, we cannot figure out automatically the extracted
> +    # directory. So check it and throw a clear error.
> +    if not os.path.isdir(os.path.join(d.getVar("WORKDIR"), "git")) and \
> +       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

Sorry this has taken a bit of time to get to.

We can't check for "git" as the workdir directory in this test as it
can be overridden in the fetcher. We'll need to find some other better
approach. Can't we just use ${S}/Makefile ?

Please also copy Bruce on these changes going forward.

Cheers,

Richard

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

[Luca Ceresoli]

Hello Richard,

thanks for reviewing!

On Fri, 23 Jun 2023 12:07:43 +0100
"Richard Purdie" <richard.purdie@linuxfoundation.org> wrote:

> Hi Luca,
> 
> On Mon, 2023-06-05 at 16:13 +0200, Luca Ceresoli via lists.openembedded.org 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 me moved into work/shared, which results in an empty
> > work-shared/.../kernel-source directory.
> > 
> > When downloading a tarball it is usually not required to set S in recipes,
> > so this 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, so move it to
> > kernel.bbclass so that also 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.
> > 
> > Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> > ---
> >  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 108b7e675212..ceb451b69969 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 9c8036f4df01..5ed4a2e03c72 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)
> > +
> > +    # When not using git, we cannot figure out automatically the extracted
> > +    # directory. So check it and throw a clear error.
> > +    if not os.path.isdir(os.path.join(d.getVar("WORKDIR"), "git")) and \
> > +       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  
> 
> Sorry this has taken a bit of time to get to.

No problem.

> We can't check for "git" as the workdir directory in this test as it
> can be overridden in the fetcher. We'll need to find some other better
> approach. Can't we just use ${S}/Makefile ?

I'm not sure I got your point, however I agree. :-)

I'm not sure I got your point because in my patch I did try to mimick as
far as possible the same logic as the original code, which is:

        if [ -d "${WORKDIR}/git/" ]; then
                # case: git repository

		...
        else
                # case: we have no git repository at all. 

		...

		### This is the check I have moved to kernel.bbclass ###
                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
		###

		...
	fi

So the bberror/bbfatal I moved is de facto inside a (if ! -d
${WORKDIR}/git && ! -f ${S}/Makefile) check, which is what I replicated
in kernel.bbclass.

On the other hand I agree that we could only check for ${S}/Makefile,
which is what I did in a draft version, and has the small side effect
of printing  the explanatory error message _also_ in the git case,
instead of a long-lined log concluded by "No rule to make target
'oldnoconfig'. Stop.".

So it looks like I'll be sending a v2 with simply the ${WORKDIR}/git
check removed.

> Please also copy Bruce on these changes going forward.

Sure, I will.

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com