Message ID | 20230626135024.34405-1-luca.ceresoli@bootlin.com |
---|---|
State | New |
Headers | show |
Series | [v2] kernel.bbclass: hoist up "unset S" bbfatal from kernel-yocto.bbclass to kernel.bbclass | expand |
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 >
Hello, This caused the following failures on the AB: stdio: ERROR: linux-yocto-6.1.35+gitAUTOINC+b358c237cf_915f4d2237-r0 do_symlink_kernsrc: S is not set to the linux source directory. Check the recipe and set S to the proper extracted subdirectory. stdio: ERROR: Logfile of failure stored in: /home/pokybuild/yocto-worker/qemuarm/build/build/tmp/work/qemuarm-poky-linux-gnueabi/linux-yocto/6.1.35+gitAUTOINC+b358c237cf_915f4d2237-r0/temp/log.do_symlink_kernsrc.3914759 stdio: ERROR: Task (/home/pokybuild/yocto-worker/qemuarm/build/meta/recipes-kernel/linux/linux-yocto_6.1.bb:do_symlink_kernsrc) failed with exit code '1' stdio: ERROR: Command . ./oe-init-build-env; bitbake core-image-sato core-image-sato-sdk core-image-minimal core-image-minimal-dev core-image-sato:do_populate_sdk -k failed with exit code 1, see errors above. (1687981572.9: 11986.3) On 26/06/2023 15:50:24+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 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. > > 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 > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#183420): https://lists.openembedded.org/g/openembedded-core/message/183420 > Mute This Topic: https://lists.openembedded.org/mt/99787873/3617179 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alexandre.belloni@bootlin.com] > -=-=-=-=-=-=-=-=-=-=-=- >
Hello Richard, Bruce, On Fri, 30 Jun 2023 17:06:51 +0200 Alexandre Belloni <alexandre.belloni@bootlin.com> wrote: > Hello, > > This caused the following failures on the AB: > > stdio: ERROR: linux-yocto-6.1.35+gitAUTOINC+b358c237cf_915f4d2237-r0 do_symlink_kernsrc: S is not set to the linux source directory. Check the recipe and set S to the proper extracted subdirectory. > stdio: ERROR: Logfile of failure stored in: /home/pokybuild/yocto-worker/qemuarm/build/build/tmp/work/qemuarm-poky-linux-gnueabi/linux-yocto/6.1.35+gitAUTOINC+b358c237cf_915f4d2237-r0/temp/log.do_symlink_kernsrc.3914759 > stdio: ERROR: Task (/home/pokybuild/yocto-worker/qemuarm/build/meta/recipes-kernel/linux/linux-yocto_6.1.bb:do_symlink_kernsrc) failed with exit code '1' > stdio: ERROR: Command . ./oe-init-build-env; bitbake core-image-sato core-image-sato-sdk core-image-minimal core-image-minimal-dev core-image-sato:do_populate_sdk -k failed with exit code 1, see errors above. (1687981572.9: 11986.3) v2, where I removed the "not git" condition as discussed[0], fails because the check is *before* do_kernel_checkout, so the move/clone has not been done yet. And cannot be done before in the git case as it could be a bare repo thus no Makefile exists yet anywhere. I think there are two options: 1. Move the check *after* do_kernel_checkout, when the source code must be there 2. Revert to the v1 approach, which checks only in the tarball case as the current code does I think it is better to check sooner rather than later (before git init + git add in the tar case). Not only because the user would be warned as soon as reasonably possible, but also because an error might cause a error path in do_kernel_checkout anyway. So unless there are better ideas I would get back to initial approach, possibly with a different patch to take into account the comments from both of you. [0] https://lore.kernel.org/openembedded-core/674d0a964611cc1e5451318976f17a4a24cbca5a.camel@linuxfoundation.org/#t Luca
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