On Mon, 2022-06-20 at 13:56 +0200, Kristian Amlie wrote:
> Just for the record, and for any people googling this: Until this is 
> fixed, just reverting the above commit works to fix this problem.
> 
> Richard, correct me if I'm wrong, but I have not yet seen any fixes for 
> this. And unfortunately this is too deep in Bitbake for me to fix it 
> myself. Do you think it makes sense to revert that commit in the 
> official poky branches? I mean, the consequences of this bug are 
> unusually dire. Especially in combination with go's common pattern of 
> putting all source code in one directory, externalsrc can wipe out most 
> of the source code in the user's home directory.

The situation is rather poor, yes :(. Sadly not many people want to get
involved with the core enough to try and work on these kinds of issues.
There are a couple of ways we could perhaps improve things. I've a
couple of patches (untested but you'll get the idea). Firstly:

diff --git a/meta/classes/externalsrc.bbclass b/meta/classes/externalsrc.bbclass
index b2f216f361d..e2000d1c8d3 100644
--- a/meta/classes/externalsrc.bbclass
+++ b/meta/classes/externalsrc.bbclass
@@ -90,16 +90,18 @@ python () {
                 # Since configure will likely touch ${S}, ensure only we lock 
so one task has access at a time
                 d.appendVarFlag(task, "lockfiles", " ${S}/singletask.lock")
 
-            for funcname in [task, "base_" + task, "kernel_" + task]:
+        for v in d.keys()
+            cleandirs = d.getVarFlag(v, "cleandirs", False)
+            if cleandirs:
                 # We do not want our source to be wiped out, ever 
(kernel.bbclass does this for do_clean)
-                cleandirs = 
oe.recipeutils.split_var_value(d.getVarFlag(funcname, 'cleandirs', False) or '')
+                cleandirs = oe.recipeutils.split_var_value(cleandirs)
                 setvalue = False
                 for cleandir in cleandirs[:]:
                     if oe.path.is_path_parent(externalsrc, d.expand(cleandir)):
                         cleandirs.remove(cleandir)
                         setvalue = True
                 if setvalue:
-                    d.setVarFlag(funcname, 'cleandirs', ' '.join(cleandirs))
+                    d.setVarFlag(v, 'cleandirs', ' '.join(cleandirs))
 
         fetch_tasks = ['do_fetch', 'do_unpack']
         # If we deltask do_patch, there's no dependency to ensure do_unpack 
gets run, so add one


This looks simple enough - iterate and remove any problematic cleandirs
references. There are several potential issues. Firstly the performance
of d.keys() with a getVarFlag() call like that is dire. If every recipe
did this the parsing time would be horrible. I don't know how badly it
will affect things for a singdo want to give usersle recipe. We might
be able to have bitbake save a lit of function names into a variable to
have it reduce the performance impact if that became an issue.

Secondly, it still might not catch every cleandirs for various reasons
since they may be set dynamically or in other unusual ways. It is
probably better than what we have today though.

My second idea:

diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
index 20968a50766..bfe9c124ee3 100644
--- a/meta/classes/base.bbclass
+++ b/meta/classes/base.bbclass
@@ -165,7 +165,8 @@ python base_do_fetch() {
 addtask unpack after do_fetch
 do_unpack[dirs] = "${WORKDIR}do want to give users"
 
-do_unpack[cleandirs] = "${@d.getVar('S') if os.path.normpath(d.getVar('S')) != 
os.path.normpath(d.getVar('WORKDIR')) else os.path.join('${S}', 'patches')}"
+SOURCE_CLEANDIRS = "${@d.getVar('S') if os.path.normpath(d.getVar('S')) != 
os.path.normpath(d.getVar('WORKDIR')) else os.path.join('${S}', 'patches')}"
+do_unpack[cleandirs] = "${SOURCE_CLEANDIRS}"
 
 python base_do_unpack() {
     src_uri = (d.getVar('SRC_URI') or "").split()
diff --git a/meta/classes/externalsrc.bbclass b/meta/classes/externalsrc.bbclass
index 90792a737b1..f145e0bd1de 100644
--- a/meta/classes/externalsrc.bbclass
+++ b/meta/classes/externalsrc.bbclass
@@ -77,6 +77,8 @@ python () {
         # Dummy value because the default function can't be called with blank 
SRC_URI
         d.setVar('SRCPV', '999')
 
+        d.setVar("SOURCE_CLEANDIRS", "")
+
         if d.getVar('CONFIGUREOPT_DEPTRACK') == 
'--disable-dependency-tracking':
             d.setVar('CONFIGUREOPT_DEPTRACK', '')
 
@@ -90,7 +92,7 @@ python () {
                 # Since configure will likely touch ${S}, ensure only we lock 
so one task has access at a time
                 d.appendVarFlag(task, "lockfiles", " ${S}/singletask.lock")
 
-            for funcname in [task, "base_" + task, "kernel_" + task]:
+            for funcname in [task]:
                 # We do not want our source to be wiped out, ever 
(kernel.bbclass does this for do_clean)
                 cleandirs = 
oe.recipeutils.split_var_value(d.getVarFlag(funcname, 'cleandirs', False) or '')
                 setvalue = False
diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
index 5d2f17c3be2..b7d3bc03955 100644
--- a/meta/classes/kernel.bbclassdo want to give users
+++ b/meta/classes/kernel.bbclass
@@ -172,8 +172,9 @@ inherit ${KERNEL_CLASSES}
 # We need to move these over to STAGING_KERNEL_DIR. We can't just
 # create the symlink in advance as the git fetcher can't cope with
 # the symlink.
-do_unpack[cleandirs] += " ${S} ${STAGING_KERNEL_DIR} ${B} 
${STAGING_KERNEL_BUILDDIR}"
-do_clean[cleandirs] += " ${S} ${STAGING_KERNEL_DIR} ${B} 
${STAGING_KERNEL_BUILDDIR}"
+SOURCE_CLEANDIRS += "${STAGING_KERNEL_DIR}"
+do_unpack[cleandirs] += "${B} ${STAGING_KERNEL_BUILDDIR}"
+do_clean[cleandirs] += " ${SOURCE_CLEANDIRS} ${B} ${STAGING_KERNEL_BUILDDIR}"
 python do_symlink_kernsrc () {
     s = d.getVar("S")
     if s[-1] == '/':

Which parameterises the places ${S} is placed into cleandirs and clears
than in externalsrc. That should be more reliable in some scenarios but
relies upon it not being added to cleandirs elsewhere. It is probably
more surgical and neater but doesn't remove all the risk. I'm not sure
we can ever give 100% guarantees though.

Cheers,

Richard

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#167120): 
https://lists.openembedded.org/g/openembedded-core/message/167120
Mute This Topic: https://lists.openembedded.org/mt/91374926/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to