diff mbox series

[v5] kernel.bbclass: make do_symlink_kernelsrc reentrant

Message ID CAHUKmYaFA8_eHrSCqknXqDfPfkwySdjqxWKvo6thuAAmD4guNw@mail.gmail.com
State New
Headers show
Series [v5] kernel.bbclass: make do_symlink_kernelsrc reentrant | expand

Commit Message

Etienne Cordonnier Dec. 21, 2023, 9:49 p.m. UTC 
From: Etienne Cordonnier <ecordonnier@snap.com>

The function do_symlink_kernsrc is not reentrant in the case where S is
defined
to a non-default value. This causes build-failures e.g. when building
linux-yocto, then updating
poky to a commit which modifies kernel.bbclass, and then building
linux-yocto again.

Bugzilla: https://bugzilla.yoctoproject.org/show_bug.cgi?id=15325

Tested with a recipe "my-custom-linux" which unpacks sources to a custom
${S} directory
and ran symlink_kernsrc several times:
$ bitbake -f -c symlink_kernsrc my-custom-linux

Signed-off-by: Etienne Cordonnier <ecordonnier@snap.com>
---
 meta/classes-recipe/kernel.bbclass | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

 }

Comments

Richard Purdie Feb. 9, 2024, 5:36 p.m. UTC | #1 
On Thu, 2023-12-21 at 22:49 +0100, Etienne Cordonnier via
lists.openembedded.org wrote:
> From: Etienne Cordonnier <ecordonnier@snap.com>
> 
> The function do_symlink_kernsrc is not reentrant in the case where S is defined
> to a non-default value. This causes build-failures e.g. when building linux-yocto, then updating
> poky to a commit which modifies kernel.bbclass, and then building linux-yocto again.
> 
> Bugzilla: https://bugzilla.yoctoproject.org/show_bug.cgi?id=15325
> 
> Tested with a recipe "my-custom-linux" which unpacks sources to a custom ${S} directory
> and ran symlink_kernsrc several times:
> $ bitbake -f -c symlink_kernsrc my-custom-linux

Sorry for the delay in getting back to review this patch. I'm extremely
worried about adding complexity into this function, particularly as
that complexity is now adding in significant overhead.

Firstly, can I ask why you're using a non-default directory for ${S}?
Is this as a way of doing a kind of external source usage?

Having spent time looking at what this code is doing, in normal usage,
S gets set to STAGING_KERNEL_DIR, the source is unpacked there and none
of this code triggers.

For EXTERNALSRC, a symlink is put in place. The code should remove a
symlink if present and create it with the current setup. In that sense,
this patch isn't doing the right thing.

I'm very tempted to say the "using a non default S value" just error
out.

Whilst I understand how we got to this level of complexity, I think it
will just end up causing us pain in the long run and I'd rather just
not support it.

Cheers,

Richard
diff mbox series

Patch

diff --git a/meta/classes-recipe/kernel.bbclass
b/meta/classes-recipe/kernel.bbclass
index 9ff37f5c38..45b63f1fa1 100644
--- a/meta/classes-recipe/kernel.bbclass
+++ b/meta/classes-recipe/kernel.bbclass
@@ -189,11 +189,17 @@  python do_symlink_kernsrc () {
             # drop trailing slash, so that os.symlink(kernsrc, s) doesn't
use s as
             # directory name and fail
             s = s[:-1]
-        if d.getVar("EXTERNALSRC"):
+        if d.getVar("EXTERNALSRC") and not os.path.islink(s):
             # With EXTERNALSRC S will not be wiped so we can symlink to it
             os.symlink(s, kernsrc)
         else:
             import shutil
+            # perform idempotent/reentrant copy
+            s_copy = s + ".orig"
+            if not os.path.isdir(s_copy):
+                shutil.copytree(s, s_copy, ignore_dangling_symlinks=True)
+            bb.utils.remove(s, recurse=True)
+            shutil.copytree(s_copy, s, ignore_dangling_symlinks=True)
             shutil.move(s, kernsrc)
             os.symlink(kernsrc, s)