[PATCH] buildhistory: Fix do_package race issues

[PATCH] buildhistory: Fix do_package race issues

[Richard Purdie]

The buildhistory_list_pkg_files function uses data from do_package, not
do_packagedata. Usally the two are restored together but it may see
a half complete directory or other races issues depending on timing.

Rework the function so that it uses the correct task dependencies. This
should avoid races but means the data is only restored to buildhistory
if the do_package or do_package_setscene tasks are restored.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 meta/classes/buildhistory.bbclass | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/meta/classes/buildhistory.bbclass b/meta/classes/buildhistory.bbclass
index 64df432f136..daa96f3b63b 100644
--- a/meta/classes/buildhistory.bbclass
+++ b/meta/classes/buildhistory.bbclass
@@ -91,13 +91,19 @@ buildhistory_emit_sysroot() {
 python buildhistory_emit_pkghistory() {
     if d.getVar('BB_CURRENTTASK') in ['populate_sysroot', 'populate_sysroot_setscene']:
         bb.build.exec_func("buildhistory_emit_sysroot", d)
-
-    if not d.getVar('BB_CURRENTTASK') in ['packagedata', 'packagedata_setscene']:
         return 0
 
     if not "package" in (d.getVar('BUILDHISTORY_FEATURES') or "").split():
         return 0
 
+    if d.getVar('BB_CURRENTTASK') in ['package', 'package_setscene']:
+        # Create files-in-<package-name>.txt files containing a list of files of each recipe's package
+        bb.build.exec_func("buildhistory_list_pkg_files", d)
+        return 0
+
+    if not d.getVar('BB_CURRENTTASK') in ['packagedata', 'packagedata_setscene']:
+        return 0
+
     import re
     import json
     import shlex
@@ -319,8 +325,6 @@ python buildhistory_emit_pkghistory() {
 
         write_pkghistory(pkginfo, d)
 
-    # Create files-in-<package-name>.txt files containing a list of files of each recipe's package
-    bb.build.exec_func("buildhistory_list_pkg_files", d)
     oe.qa.exit_if_errors(d)
 }
 
-- 
2.32.0

Re: [OE-core] [PATCH] buildhistory: Fix do_package race issues

[Alexander Kanavin]

[-- Attachment #1: Type: text/plain, Size: 2665 bytes --]

Thanks Richard, this should remove occasional false failures from AUH runs
:)

Alex

On Tue, 23 Nov 2021 at 14:59, Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> The buildhistory_list_pkg_files function uses data from do_package, not
> do_packagedata. Usally the two are restored together but it may see
> a half complete directory or other races issues depending on timing.
>
> Rework the function so that it uses the correct task dependencies. This
> should avoid races but means the data is only restored to buildhistory
> if the do_package or do_package_setscene tasks are restored.
>
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>  meta/classes/buildhistory.bbclass | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/meta/classes/buildhistory.bbclass
> b/meta/classes/buildhistory.bbclass
> index 64df432f136..daa96f3b63b 100644
> --- a/meta/classes/buildhistory.bbclass
> +++ b/meta/classes/buildhistory.bbclass
> @@ -91,13 +91,19 @@ buildhistory_emit_sysroot() {
>  python buildhistory_emit_pkghistory() {
>      if d.getVar('BB_CURRENTTASK') in ['populate_sysroot',
> 'populate_sysroot_setscene']:
>          bb.build.exec_func("buildhistory_emit_sysroot", d)
> -
> -    if not d.getVar('BB_CURRENTTASK') in ['packagedata',
> 'packagedata_setscene']:
>          return 0
>
>      if not "package" in (d.getVar('BUILDHISTORY_FEATURES') or "").split():
>          return 0
>
> +    if d.getVar('BB_CURRENTTASK') in ['package', 'package_setscene']:
> +        # Create files-in-<package-name>.txt files containing a list of
> files of each recipe's package
> +        bb.build.exec_func("buildhistory_list_pkg_files", d)
> +        return 0
> +
> +    if not d.getVar('BB_CURRENTTASK') in ['packagedata',
> 'packagedata_setscene']:
> +        return 0
> +
>      import re
>      import json
>      import shlex
> @@ -319,8 +325,6 @@ python buildhistory_emit_pkghistory() {
>
>          write_pkghistory(pkginfo, d)
>
> -    # Create files-in-<package-name>.txt files containing a list of files
> of each recipe's package
> -    bb.build.exec_func("buildhistory_list_pkg_files", d)
>      oe.qa.exit_if_errors(d)
>  }
>
> --
> 2.32.0
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#158603):
> https://lists.openembedded.org/g/openembedded-core/message/158603
> Mute This Topic: https://lists.openembedded.org/mt/87258776/1686489
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> alex.kanavin@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>

[-- Attachment #2: Type: text/html, Size: 3951 bytes --]

Re: [PATCH] buildhistory: Fix do_package race issues

[pmi183]

[-- Attachment #1: Type: text/plain, Size: 3498 bytes --]

Hi Richard,

While using buildhistory, i faced an issue with files-in-package.txt missing and digging into the logs i found out:

find: ‘/home/user/src/poky-master/build/tmp/work/core2-64-poky-linux/base-passwd/3.6.3/packages-split/*’: No such file or directory

Calling `buildhistory_list_pkg_files`  from do_package seems to be accessing the dir before being ready and fails.

Additionally, using `BUILDHISTORY_RESET` along with `BUILDHISTORY_PRESERVE` looks to fail to preserve files since there is nothing to handle buildhistory/old dir and ends up losing all files marked to preserve.

Thank you,
Pedro

----------------------------------------

meta/classes/buildhistory.bbclass | 29 ++++++++++++++++++++++++-----
1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/meta/classes/buildhistory.bbclass b/meta/classes/buildhistory.bbclass
index fd53e92402..4962c53cae 100644
--- a/meta/classes/buildhistory.bbclass
+++ b/meta/classes/buildhistory.bbclass
@@ -98,11 +98,6 @@ python buildhistory_emit_pkghistory() {
if not "package" in (d.getVar('BUILDHISTORY_FEATURES') or "").split():
return 0

-    if d.getVar('BB_CURRENTTASK') in ['package', 'package_setscene']:
-        # Create files-in-<package-name>.txt files containing a list of files of each recipe's package
-        bb.build.exec_func("buildhistory_list_pkg_files", d)
-        return 0
-
if not d.getVar('BB_CURRENTTASK') in ['packagedata', 'packagedata_setscene']:
return 0

@@ -110,6 +105,7 @@ python buildhistory_emit_pkghistory() {
import json
import shlex
import errno
+    import shutil

pkghistdir = d.getVar('BUILDHISTORY_DIR_PACKAGE')
oldpkghistdir = d.getVar('BUILDHISTORY_OLD_DIR_PACKAGE')
@@ -223,6 +219,20 @@ python buildhistory_emit_pkghistory() {
items.sort()
return ' '.join(items)

+    def copypreservedoldpkgdatafiles(pkg, preserve):
+        if os.path.exists(os.path.join(oldpkghistdir, pkg)):
+            listofobjs = os.listdir(os.path.join(oldpkghistdir, pkg))
+            for obj in listofobjs:
+                if obj not in preserve:
+                    continue
+                try:
+                    bb.utils.mkdirhier(os.path.join(pkghistdir, pkg))
+                    shutil.copyfile(os.path.join(oldpkghistdir, pkg, obj), os.path.join(pkghistdir, pkg, obj))
+                except IOError as e:
+                    bb.note("Unable to copy file. %s" % e)
+                except EnvironmentError as e:
+                    bb.note("Unable to copy file. %s" % e)
+
pn = d.getVar('PN')
pe = d.getVar('PE') or "0"
pv = d.getVar('PV')
@@ -250,6 +260,11 @@ python buildhistory_emit_pkghistory() {
if not os.path.exists(pkghistdir):
bb.utils.mkdirhier(pkghistdir)
else:
+        reset = d.getVar("BUILDHISTORY_RESET")
+        if reset:
+            for pkg in packagelist:
+                copypreservedoldpkgdatafiles(pkg, preserve)
+
# Remove files for packages that no longer exist
for item in os.listdir(pkghistdir):
if item not in preserve:
@@ -327,6 +342,10 @@ python buildhistory_emit_pkghistory() {

write_pkghistory(pkginfo, d)

+    # Only executed when running task `packagedata`
+    if d.getVar('BB_CURRENTTASK') == 'packagedata':
+        bb.build.exec_func("buildhistory_list_pkg_files", d)
+
oe.qa.exit_if_errors(d)
}

--
2.34.1

[-- Attachment #2: Type: text/html, Size: 5088 bytes --]

Re: [OE-core] [PATCH] buildhistory: Fix do_package race issues

[Alexander Kanavin]

[-- Attachment #1: Type: text/plain, Size: 4364 bytes --]

Can you demonstrate how the issue can be observed on current poky master
please?

Alex

On Mon 13. May 2024 at 11.47, pmi183 via lists.openembedded.org <pmi183=
gmail.com@lists.openembedded.org> wrote:

> Hi Richard,
>
> While using buildhistory, i faced an issue with files-in-package.txt
> missing and digging into the logs i found out:
>
>
>
> find:
> ‘/home/user/src/poky-master/build/tmp/work/core2-64-poky-linux/base-passwd/3.6.3/packages-split/*’:
> No such file or directory
> Calling `buildhistory_list_pkg_files`  from do_package seems to be
> accessing the dir before being ready and fails.
>
> Additionally, using `BUILDHISTORY_RESET` along with
> `BUILDHISTORY_PRESERVE` looks to fail to preserve files since there is
> nothing to handle buildhistory/old dir and ends up losing all files marked
> to preserve.
>
> Thank you,
> Pedro
>
> ----------------------------------------
>
>  meta/classes/buildhistory.bbclass | 29 ++++++++++++++++++++++++-----
>  1 file changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/meta/classes/buildhistory.bbclass
> b/meta/classes/buildhistory.bbclass
> index fd53e92402..4962c53cae 100644
> --- a/meta/classes/buildhistory.bbclass
> +++ b/meta/classes/buildhistory.bbclass
> @@ -98,11 +98,6 @@ python buildhistory_emit_pkghistory() {
>      if not "package" in (d.getVar('BUILDHISTORY_FEATURES') or "").split():
>          return 0
>
> -    if d.getVar('BB_CURRENTTASK') in ['package', 'package_setscene']:
> -        # Create files-in-<package-name>.txt files containing a list of
> files of each recipe's package
> -        bb.build.exec_func("buildhistory_list_pkg_files", d)
> -        return 0
> -
>      if not d.getVar('BB_CURRENTTASK') in ['packagedata',
> 'packagedata_setscene']:
>          return 0
>
> @@ -110,6 +105,7 @@ python buildhistory_emit_pkghistory() {
>      import json
>      import shlex
>      import errno
> +    import shutil
>
>      pkghistdir = d.getVar('BUILDHISTORY_DIR_PACKAGE')
>      oldpkghistdir = d.getVar('BUILDHISTORY_OLD_DIR_PACKAGE')
> @@ -223,6 +219,20 @@ python buildhistory_emit_pkghistory() {
>          items.sort()
>          return ' '.join(items)
>
> +    def copypreservedoldpkgdatafiles(pkg, preserve):
> +        if os.path.exists(os.path.join(oldpkghistdir, pkg)):
> +            listofobjs = os.listdir(os.path.join(oldpkghistdir, pkg))
> +            for obj in listofobjs:
> +                if obj not in preserve:
> +                    continue
> +                try:
> +                    bb.utils.mkdirhier(os.path.join(pkghistdir, pkg))
> +                    shutil.copyfile(os.path.join(oldpkghistdir, pkg,
> obj), os.path.join(pkghistdir, pkg, obj))
> +                except IOError as e:
> +                    bb.note("Unable to copy file. %s" % e)
> +                except EnvironmentError as e:
> +                    bb.note("Unable to copy file. %s" % e)
> +
>      pn = d.getVar('PN')
>      pe = d.getVar('PE') or "0"
>      pv = d.getVar('PV')
> @@ -250,6 +260,11 @@ python buildhistory_emit_pkghistory() {
>      if not os.path.exists(pkghistdir):
>          bb.utils.mkdirhier(pkghistdir)
>      else:
> +        reset = d.getVar("BUILDHISTORY_RESET")
> +        if reset:
> +            for pkg in packagelist:
> +                copypreservedoldpkgdatafiles(pkg, preserve)
> +
>          # Remove files for packages that no longer exist
>          for item in os.listdir(pkghistdir):
>              if item not in preserve:
> @@ -327,6 +342,10 @@ python buildhistory_emit_pkghistory() {
>
>          write_pkghistory(pkginfo, d)
>
> +    # Only executed when running task `packagedata`
> +    if d.getVar('BB_CURRENTTASK') == 'packagedata':
> +        bb.build.exec_func("buildhistory_list_pkg_files", d)
> +
>      oe.qa.exit_if_errors(d)
>  }
>
> --
> 2.34.1
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#199226):
> https://lists.openembedded.org/g/openembedded-core/message/199226
> Mute This Topic: https://lists.openembedded.org/mt/87258776/1686489
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> alex.kanavin@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>

[-- Attachment #2: Type: text/html, Size: 5760 bytes --]

Re: [PATCH] buildhistory: Fix do_package race issues

[pmi183]

[-- Attachment #1: Type: text/plain, Size: 303 bytes --]

Hi,

If you execute:
rm -rf tmp/ && bitbake base-passwd -c cleansstate && bitbake base-passwd

Check the log.do_package of the recipe all you will see the error there and if you check `buildhistory/packages/` you wont find `files-in-package.txt` for this package for example.

Thank you,
Pedro

[-- Attachment #2: Type: text/html, Size: 486 bytes --]

Re: [OE-core] [PATCH] buildhistory: Fix do_package race issues

[Alexander Kanavin]

On Mon, 13 May 2024 at 12:05, pmi183 via lists.openembedded.org
<pmi183=gmail.com@lists.openembedded.org> wrote:
> If you execute:
> rm -rf tmp/ && bitbake base-passwd -c cleansstate && bitbake base-passwd
>
> Check the log.do_package of the recipe all you will see the error there and if you check `buildhistory/packages/` you wont find `files-in-package.txt` for this package for example.

Thanks, I see it now. This however raises further questions:
- why is it not a hard error?
- why is it unable to find anything in packages-split/, if it executes
after populate_packages which places things there?

Can you investigate these? Moving things to packagedata seems like
fixing the symptom, not the root issue.

Alex

Re: [PATCH] buildhistory: Fix do_package race issues

[pmi183]

[-- Attachment #1: Type: text/plain, Size: 1464 bytes --]

Hi Alex,

I did some investigation on this topic,

- why is it not a hard error?

That is because find command in `buildhistory_list_pkg_files` is tested inside the loop for, hiding this issue and for loop is always returning 0 even if `find` fails.

> 
> .....
> buildhistory_list_pkg_files() {
> # Create individual files-in-package for each recipe's package
> pkgdirlist=$(find ${PKGDEST}/* -maxdepth 0 -type d)
> 
> for pkgdir in pkgdirlist; do
> .....
> 

- why is it unable to find anything in packages-split/, if it executes after populate_packages which places things there?

As far as i could understand, there is some concurrency issue between package.bbclass and buildhistory.bbclass. Taking in consideration that `buildhistory_list_pkg_files` is triggered by checking if `BB_CURRENTTASK` is `package` and at the same time `PACKAGESPLITFUNCS` is triggered on `do_package`, this leads to the issue with missing files since buildhistory bbclass might be looking into something that wasnt yet created. In my findings, `buildhistory_list_pkg_files` should be called in the end of `do_package` execution where `packages-split` directory is created and populated.

Is there any clear reason why we cant move `buildhistory_list_pkg_files` to `packagedata` task since its the next task to be executed?

Any suggestion or any idea how can i, from buildhistory.bbclass, determine if `PACKAGESPLITFUNCS` was already executed?

Thank you

[-- Attachment #2: Type: text/html, Size: 4663 bytes --]

Re: [OE-core] [PATCH] buildhistory: Fix do_package race issues

[Alexander Kanavin]

On Wed, 15 May 2024 at 11:36, pmi183 via lists.openembedded.org
<pmi183=gmail.com@lists.openembedded.org> wrote:
> - why is it unable to find anything in packages-split/, if it executes after populate_packages which places things there?
>
> As far as i could understand, there is some concurrency issue between package.bbclass and buildhistory.bbclass. Taking in consideration that `buildhistory_list_pkg_files` is triggered by checking if `BB_CURRENTTASK` is `package` and at the same time `PACKAGESPLITFUNCS` is triggered on `do_package`, this leads to the issue with missing files since buildhistory bbclass might be looking into something that wasnt yet created. In my findings, `buildhistory_list_pkg_files` should be called in the end of `do_package` execution where `packages-split` directory is created and populated.
>
> Is there any clear reason why we cant move `buildhistory_list_pkg_files` to `packagedata` task since its the next task to be executed?

The reason is that the issue is not fully understood, and this move
might be working around the real issue. The logs I saw indicate that
package_split completes before buildhistory_list_pkg_files starts.
This needs to be looked into further.

Alex

Re: [PATCH] buildhistory: Fix do_package race issues

[pmi183]

[-- Attachment #1: Type: text/plain, Size: 4180 bytes --]

Hi Alex,

I made some debug in the flow and i saw a timing issue and since there's no reason to generate files-in-package.txt before sstate-cache operations i moved the call to the postfunc of sstate:

---
meta/classes-global/sstate.bbclass |  5 +++++
meta/classes/buildhistory.bbclass  | 18 +++++++++++++++---
meta/lib/oe/packagedata.py         |  2 +-
3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/meta/classes-global/sstate.bbclass b/meta/classes-global/sstate.bbclass
index 76a7b59636..7e3c3e3c4f 100644
--- a/meta/classes-global/sstate.bbclass
+++ b/meta/classes-global/sstate.bbclass
@@ -104,6 +104,7 @@ SSTATEPOSTCREATEFUNCS = ""
SSTATEPREINSTFUNCS = ""
SSTATEPOSTUNPACKFUNCS = "sstate_hardcode_path_unpack"
SSTATEPOSTINSTFUNCS = ""
+SSTATEPOSTFUNCS = ""
EXTRA_STAGING_FIXMES ?= "HOSTTOOLS_DIR"

# Check whether sstate exists for tasks that support sstate and are in the
@@ -805,6 +806,10 @@ python sstate_task_postfunc () {
sstate_installpkgdir(shared_state, d)

bb.utils.remove(d.getVar("SSTATE_BUILDDIR"), recurse=True)
+
+    for postfunc in (d.getVar('SSTATEPOSTFUNCS') or '').split():
+        # All hooks should run in the SSTATE_INSTDIR
+        bb.build.exec_func(postfunc, d, (sstateinst,))
}
sstate_task_postfunc[dirs] = "${WORKDIR}"

diff --git a/meta/classes/buildhistory.bbclass b/meta/classes/buildhistory.bbclass
index fd53e92402..054c213bf5 100644
--- a/meta/classes/buildhistory.bbclass
+++ b/meta/classes/buildhistory.bbclass
@@ -58,6 +58,9 @@ SSTATEPOSTUNPACKFUNCS:append = " buildhistory_emit_outputsigs"
sstate_installpkgdir[vardepsexclude] += "buildhistory_emit_outputsigs"
SSTATEPOSTUNPACKFUNCS[vardepvalueexclude] .= "| buildhistory_emit_outputsigs"

+SSTATEPOSTFUNCS:append = " buildhistory_emit_filesinpackage"
+SSTATEPOSTFUNCS[vardepvalueexclude] .= "| buildhistory_emit_filesinpackage"
+
# All items excepts those listed here will be removed from a recipe's
# build history directory by buildhistory_emit_pkghistory(). This is
# necessary because some of these items (package directories, files that
@@ -87,6 +90,16 @@ buildhistory_emit_sysroot() {
buildhistory_list_files_no_owners $BASE ${BUILDHISTORY_DIR_PACKAGE}/sysroot
}

+#
+# Write out files-in-package for this package
+#
+python buildhistory_emit_filesinpackage() {
+    if d.getVar('BB_CURRENTTASK') in ['package', 'package_setscene']:
+        # Create files-in-<package-name>.txt files containing a list of files of each recipe's package
+        bb.build.exec_func("buildhistory_list_pkg_files", d)
+        return 0
+}
+
#
# Write out metadata about this package for comparison when writing future packages
#
@@ -99,8 +112,6 @@ python buildhistory_emit_pkghistory() {
return 0

if d.getVar('BB_CURRENTTASK') in ['package', 'package_setscene']:
-        # Create files-in-<package-name>.txt files containing a list of files of each recipe's package
-        bb.build.exec_func("buildhistory_list_pkg_files", d)
return 0

if not d.getVar('BB_CURRENTTASK') in ['packagedata', 'packagedata_setscene']:
@@ -599,7 +610,8 @@ buildhistory_list_files_no_owners() {

buildhistory_list_pkg_files() {
# Create individual files-in-package for each recipe's package
-       for pkgdir in $(find ${PKGDEST}/* -maxdepth 0 -type d); do
+    pkgdirlist=$(find ${PKGDEST}/* -maxdepth 0 -type d)
+       for pkgdir in ${pkgdirlist}; do
pkgname=$(basename $pkgdir)
outfolder="${BUILDHISTORY_DIR_PACKAGE}/$pkgname"
outfile="$outfolder/files-in-package.txt"
diff --git a/meta/lib/oe/packagedata.py b/meta/lib/oe/packagedata.py
index 2d1d6ddeb7..e8c503b43b 100644
--- a/meta/lib/oe/packagedata.py
+++ b/meta/lib/oe/packagedata.py
@@ -309,7 +309,7 @@ fi
subdata_file = pkgdatadir + "/runtime/%s" % pkg
with open(subdata_file, 'w') as sf:
for var in (d.getVar('PKGDATA_VARS') or "").split():
-                val = write_if_exists(sf, pkg, var)
+                write_if_exists(sf, pkg, var)

write_if_exists(sf, pkg, 'FILERPROVIDESFLIST')
for dfile in sorted((d.getVar('FILERPROVIDESFLIST:' + pkg) or "").split()):
--
2.34.1

[-- Attachment #2: Type: text/html, Size: 5542 bytes --]

Re: [OE-core] [PATCH] buildhistory: Fix do_package race issues

[Jose Quaresma]

[-- Attachment #1: Type: text/plain, Size: 5433 bytes --]

Hi Pedro,

This will add the sstate-cache as a dependency of the buildhistory
which can be correct if it is enabled or in use
but IMO the buildhistory should also work without it, when the sstate is
not in use.

Could you please test one build with: bitbake --no-setscene ... ?

Jose

pmi183 via lists.openembedded.org <pmi183=gmail.com@lists.openembedded.org>
escreveu (segunda, 20/05/2024 à(s) 09:10):

> Hi Alex,
>
> I made some debug in the flow and i saw a timing issue and since there's
> no reason to generate files-in-package.txt before sstate-cache operations i
> moved the call to the postfunc of sstate:
>
> ---
>  meta/classes-global/sstate.bbclass |  5 +++++
>  meta/classes/buildhistory.bbclass  | 18 +++++++++++++++---
>  meta/lib/oe/packagedata.py         |  2 +-
>  3 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/meta/classes-global/sstate.bbclass
> b/meta/classes-global/sstate.bbclass
> index 76a7b59636..7e3c3e3c4f 100644
> --- a/meta/classes-global/sstate.bbclass
> +++ b/meta/classes-global/sstate.bbclass
> @@ -104,6 +104,7 @@ SSTATEPOSTCREATEFUNCS = ""
>  SSTATEPREINSTFUNCS = ""
>  SSTATEPOSTUNPACKFUNCS = "sstate_hardcode_path_unpack"
>  SSTATEPOSTINSTFUNCS = ""
> +SSTATEPOSTFUNCS = ""
>  EXTRA_STAGING_FIXMES ?= "HOSTTOOLS_DIR"
>
>  # Check whether sstate exists for tasks that support sstate and are in the
> @@ -805,6 +806,10 @@ python sstate_task_postfunc () {
>      sstate_installpkgdir(shared_state, d)
>
>      bb.utils.remove(d.getVar("SSTATE_BUILDDIR"), recurse=True)
> +
> +    for postfunc in (d.getVar('SSTATEPOSTFUNCS') or '').split():
> +        # All hooks should run in the SSTATE_INSTDIR
> +        bb.build.exec_func(postfunc, d, (sstateinst,))
>  }
>  sstate_task_postfunc[dirs] = "${WORKDIR}"
>
> diff --git a/meta/classes/buildhistory.bbclass
> b/meta/classes/buildhistory.bbclass
> index fd53e92402..054c213bf5 100644
> --- a/meta/classes/buildhistory.bbclass
> +++ b/meta/classes/buildhistory.bbclass
> @@ -58,6 +58,9 @@ SSTATEPOSTUNPACKFUNCS:append = "
> buildhistory_emit_outputsigs"
>  sstate_installpkgdir[vardepsexclude] += "buildhistory_emit_outputsigs"
>  SSTATEPOSTUNPACKFUNCS[vardepvalueexclude] .= "|
> buildhistory_emit_outputsigs"
>
> +SSTATEPOSTFUNCS:append = " buildhistory_emit_filesinpackage"
> +SSTATEPOSTFUNCS[vardepvalueexclude] .= "|
> buildhistory_emit_filesinpackage"
> +
>  # All items excepts those listed here will be removed from a recipe's
>  # build history directory by buildhistory_emit_pkghistory(). This is
>  # necessary because some of these items (package directories, files that
> @@ -87,6 +90,16 @@ buildhistory_emit_sysroot() {
>         buildhistory_list_files_no_owners $BASE
> ${BUILDHISTORY_DIR_PACKAGE}/sysroot
>  }
>
> +#
> +# Write out files-in-package for this package
> +#
> +python buildhistory_emit_filesinpackage() {
> +    if d.getVar('BB_CURRENTTASK') in ['package', 'package_setscene']:
> +        # Create files-in-<package-name>.txt files containing a list of
> files of each recipe's package
> +        bb.build.exec_func("buildhistory_list_pkg_files", d)
> +        return 0
> +}
> +
>  #
>  # Write out metadata about this package for comparison when writing
> future packages
>  #
> @@ -99,8 +112,6 @@ python buildhistory_emit_pkghistory() {
>          return 0
>
>      if d.getVar('BB_CURRENTTASK') in ['package', 'package_setscene']:
> -        # Create files-in-<package-name>.txt files containing a list of
> files of each recipe's package
> -        bb.build.exec_func("buildhistory_list_pkg_files", d)
>          return 0
>
>      if not d.getVar('BB_CURRENTTASK') in ['packagedata',
> 'packagedata_setscene']:
> @@ -599,7 +610,8 @@ buildhistory_list_files_no_owners() {
>
>  buildhistory_list_pkg_files() {
>         # Create individual files-in-package for each recipe's package
> -       for pkgdir in $(find ${PKGDEST}/* -maxdepth 0 -type d); do
> +    pkgdirlist=$(find ${PKGDEST}/* -maxdepth 0 -type d)
> +       for pkgdir in ${pkgdirlist}; do
>                 pkgname=$(basename $pkgdir)
>                 outfolder="${BUILDHISTORY_DIR_PACKAGE}/$pkgname"
>                 outfile="$outfolder/files-in-package.txt"
> diff --git a/meta/lib/oe/packagedata.py b/meta/lib/oe/packagedata.py
> index 2d1d6ddeb7..e8c503b43b 100644
> --- a/meta/lib/oe/packagedata.py
> +++ b/meta/lib/oe/packagedata.py
> @@ -309,7 +309,7 @@ fi
>          subdata_file = pkgdatadir + "/runtime/%s" % pkg
>          with open(subdata_file, 'w') as sf:
>              for var in (d.getVar('PKGDATA_VARS') or "").split():
> -                val = write_if_exists(sf, pkg, var)
> +                write_if_exists(sf, pkg, var)
>
>              write_if_exists(sf, pkg, 'FILERPROVIDESFLIST')
>              for dfile in sorted((d.getVar('FILERPROVIDESFLIST:' + pkg) or
> "").split()):
> --
> 2.34.1
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#199555):
> https://lists.openembedded.org/g/openembedded-core/message/199555
> Mute This Topic: https://lists.openembedded.org/mt/87258776/5052612
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> quaresma.jose@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>

-- 
Best regards,

José Quaresma

[-- Attachment #2: Type: text/html, Size: 6940 bytes --]

Re: [OE-core] [PATCH] buildhistory: Fix do_package race issues

[Alexander Kanavin]

This does not seem to address the question I asked. Please do not send
more code that moves things around, but diagnose the original issue
properly.

Alex

On Mon, 20 May 2024 at 10:10, pmi183 via lists.openembedded.org
<pmi183=gmail.com@lists.openembedded.org> wrote:
>
> Hi Alex,
>
> I made some debug in the flow and i saw a timing issue and since there's no reason to generate files-in-package.txt before sstate-cache operations i moved the call to the postfunc of sstate:
>
> ---
>  meta/classes-global/sstate.bbclass |  5 +++++
>  meta/classes/buildhistory.bbclass  | 18 +++++++++++++++---
>  meta/lib/oe/packagedata.py         |  2 +-
>  3 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/meta/classes-global/sstate.bbclass b/meta/classes-global/sstate.bbclass
> index 76a7b59636..7e3c3e3c4f 100644
> --- a/meta/classes-global/sstate.bbclass
> +++ b/meta/classes-global/sstate.bbclass
> @@ -104,6 +104,7 @@ SSTATEPOSTCREATEFUNCS = ""
>  SSTATEPREINSTFUNCS = ""
>  SSTATEPOSTUNPACKFUNCS = "sstate_hardcode_path_unpack"
>  SSTATEPOSTINSTFUNCS = ""
> +SSTATEPOSTFUNCS = ""
>  EXTRA_STAGING_FIXMES ?= "HOSTTOOLS_DIR"
>
>  # Check whether sstate exists for tasks that support sstate and are in the
> @@ -805,6 +806,10 @@ python sstate_task_postfunc () {
>      sstate_installpkgdir(shared_state, d)
>
>      bb.utils.remove(d.getVar("SSTATE_BUILDDIR"), recurse=True)
> +
> +    for postfunc in (d.getVar('SSTATEPOSTFUNCS') or '').split():
> +        # All hooks should run in the SSTATE_INSTDIR
> +        bb.build.exec_func(postfunc, d, (sstateinst,))
>  }
>  sstate_task_postfunc[dirs] = "${WORKDIR}"
>
> diff --git a/meta/classes/buildhistory.bbclass b/meta/classes/buildhistory.bbclass
> index fd53e92402..054c213bf5 100644
> --- a/meta/classes/buildhistory.bbclass
> +++ b/meta/classes/buildhistory.bbclass
> @@ -58,6 +58,9 @@ SSTATEPOSTUNPACKFUNCS:append = " buildhistory_emit_outputsigs"
>  sstate_installpkgdir[vardepsexclude] += "buildhistory_emit_outputsigs"
>  SSTATEPOSTUNPACKFUNCS[vardepvalueexclude] .= "| buildhistory_emit_outputsigs"
>
> +SSTATEPOSTFUNCS:append = " buildhistory_emit_filesinpackage"
> +SSTATEPOSTFUNCS[vardepvalueexclude] .= "| buildhistory_emit_filesinpackage"
> +
>  # All items excepts those listed here will be removed from a recipe's
>  # build history directory by buildhistory_emit_pkghistory(). This is
>  # necessary because some of these items (package directories, files that
> @@ -87,6 +90,16 @@ buildhistory_emit_sysroot() {
>         buildhistory_list_files_no_owners $BASE ${BUILDHISTORY_DIR_PACKAGE}/sysroot
>  }
>
> +#
> +# Write out files-in-package for this package
> +#
> +python buildhistory_emit_filesinpackage() {
> +    if d.getVar('BB_CURRENTTASK') in ['package', 'package_setscene']:
> +        # Create files-in-<package-name>.txt files containing a list of files of each recipe's package
> +        bb.build.exec_func("buildhistory_list_pkg_files", d)
> +        return 0
> +}
> +
>  #
>  # Write out metadata about this package for comparison when writing future packages
>  #
> @@ -99,8 +112,6 @@ python buildhistory_emit_pkghistory() {
>          return 0
>
>      if d.getVar('BB_CURRENTTASK') in ['package', 'package_setscene']:
> -        # Create files-in-<package-name>.txt files containing a list of files of each recipe's package
> -        bb.build.exec_func("buildhistory_list_pkg_files", d)
>          return 0
>
>      if not d.getVar('BB_CURRENTTASK') in ['packagedata', 'packagedata_setscene']:
> @@ -599,7 +610,8 @@ buildhistory_list_files_no_owners() {
>
>  buildhistory_list_pkg_files() {
>         # Create individual files-in-package for each recipe's package
> -       for pkgdir in $(find ${PKGDEST}/* -maxdepth 0 -type d); do
> +    pkgdirlist=$(find ${PKGDEST}/* -maxdepth 0 -type d)
> +       for pkgdir in ${pkgdirlist}; do
>                 pkgname=$(basename $pkgdir)
>                 outfolder="${BUILDHISTORY_DIR_PACKAGE}/$pkgname"
>                 outfile="$outfolder/files-in-package.txt"
> diff --git a/meta/lib/oe/packagedata.py b/meta/lib/oe/packagedata.py
> index 2d1d6ddeb7..e8c503b43b 100644
> --- a/meta/lib/oe/packagedata.py
> +++ b/meta/lib/oe/packagedata.py
> @@ -309,7 +309,7 @@ fi
>          subdata_file = pkgdatadir + "/runtime/%s" % pkg
>          with open(subdata_file, 'w') as sf:
>              for var in (d.getVar('PKGDATA_VARS') or "").split():
> -                val = write_if_exists(sf, pkg, var)
> +                write_if_exists(sf, pkg, var)
>
>              write_if_exists(sf, pkg, 'FILERPROVIDESFLIST')
>              for dfile in sorted((d.getVar('FILERPROVIDESFLIST:' + pkg) or "").split()):
> --
> 2.34.1
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#199555): https://lists.openembedded.org/g/openembedded-core/message/199555
> Mute This Topic: https://lists.openembedded.org/mt/87258776/1686489
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kanavin@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>

Re: [PATCH] buildhistory: Fix do_package race issues

[pmi183]

[-- Attachment #1: Type: text/plain, Size: 1378 bytes --]

Hi Alex,

So problem starts at sstate.bbclass:406 (sstate_install(ss, d)), this should be running after the next for loop (for plain in ss['plaindirs']:).

Having this fixed, jumping to the buildhistory.bbclass:602, find should be out of the loop to give an hard error if it misses.
Then inside the for loop, while checking if output folder exists (buildhistory.bbclass:607), this doesnt make sense to me the way it is running:
- the comment refers "Make sure the output folder exists...", but we are not creating it if we need, just skipping if doesnt exists.
- this folder is being created on do_packagedata (buildhistory.bbclass:396) to write down latest file.

So before spending more time on this topic i have a few questions to understand a little bit of what should be the strategy to fix this:

- If we want to keep files-in-package.txt being created on do_package, shouldnt `buildhistory_list_pkg_files()` (buildhistory.bbclass:600) be responsible to create the folder on buildhistory side?
- If we keep `buildhistory_list_pkg_files` being triggered by do_package is there any case of do_package_setscene trigger this? I couldnt find any case to support this option (buildhistory.bbclass:101).
- If not we dont need in fact to create files-in-package.txt on do_package, should files-in-package.txt creation be moved to do_packagedata?

Thanks,

Pedro

[-- Attachment #2: Type: text/html, Size: 1464 bytes --]

Re: [PATCH] buildhistory: Fix do_package race issues

[pmi183]

[-- Attachment #1: Type: text/plain, Size: 1377 bytes --]

Hi Alex,

So problem starts at sstate.bbclass:406 (sstate_install(ss, d)), this should be running after the next for loop (for plain in ss['plaindirs']:).

Having this fixed, jumping to the buildhistory.bbclass:602, find should be out of the loop to give a hard error if it misses.
Then inside the for loop, while checking if output folder exists (buildhistory.bbclass:607), this doesnt make sense to me the way it is running:
- the comment refers "Make sure the output folder exists...", but we are not creating it if we need, just skipping if doesnt exists.
- this folder is being created on do_packagedata (buildhistory.bbclass:396) to write down latest file.

So before spending more time on this topic i have a few questions to understand a little bit of what should be the strategy to fix this:

- If we want to keep files-in-package.txt being created on do_package, shouldnt `buildhistory_list_pkg_files()` (buildhistory.bbclass:600) be responsible to create the folder on buildhistory side?
- If we keep `buildhistory_list_pkg_files` being triggered by do_package is there any case of do_package_setscene trigger this? I couldnt find any case to support this option (buildhistory.bbclass:101).
- If not we dont need in fact to create files-in-package.txt on do_package, should files-in-package.txt creation be moved to do_packagedata?

Thanks,

Pedro

[-- Attachment #2: Type: text/html, Size: 1463 bytes --]

Re: [OE-core] [PATCH] buildhistory: Fix do_package race issues

[Alexander Kanavin]

Hello Pedro,

is it possible to separate all needed changes into a set of nice,
small, simple, easy to review commits? That would go a long way
towards fixing all issues. I don't have a hard opinion on the best way
to fix this, I just want to ensure there are no mysteries in what is
happening, and the fixing isn't in the form of gigantic, invasive,
single commit. If you need to move code *and* change what it does,
move in one commit, change in another, for example.

buildhistory is an old class, has been worked on by many people over
the years, and the original author (Paul Eggleton) isn't involved in
yocto any longer unfortunately. So the fixes may have been done to
resolve 'pressing problems' rather than with long term quality of the
code in mind.

Also note there are selftests for it:
alex@Zen2:/srv/storage/alex/yocto/build-64-alt$ oe-selftest -l|grep -i
buildhistory
2024-05-24 10:06:56,818 - oe-selftest - INFO -
buildoptions.BuildhistoryTests.test_buildhistory_basic
2024-05-24 10:06:56,818 - oe-selftest - INFO -
buildoptions.BuildhistoryTests.test_buildhistory_buildtime_pr_backwards
2024-05-24 10:06:56,818 - oe-selftest - INFO -
buildoptions.BuildhistoryTests.test_fileinfo
2024-05-24 10:06:56,830 - oe-selftest - INFO -
oelib.buildhistory.TestBlobParsing.test_blob_to_dict
2024-05-24 10:06:56,830 - oe-selftest - INFO -
oelib.buildhistory.TestBlobParsing.test_compare_dict_blobs
2024-05-24 10:06:56,830 - oe-selftest - INFO -
oelib.buildhistory.TestBlobParsing.test_compare_dict_blobs_default
2024-05-24 10:06:56,831 - oe-selftest - INFO -
oelib.buildhistory.TestFileListCompare.test_compare_file_lists
2024-05-24 10:06:56,832 - oe-selftest - INFO -
oescripts.BuildhistoryDiffTests.test_buildhistory_diff

So if you make sure they continue to pass, that'd be good. If you can
add to them, that's better.


Alex



On Thu, 23 May 2024 at 17:24, pmi183 via lists.openembedded.org
<pmi183=gmail.com@lists.openembedded.org> wrote:
>
> [Edited Message Follows]
>
> Hi Alex,
>
> So problem starts at sstate.bbclass:406 (sstate_install(ss, d)), this should be running after the next for loop (for plain in ss['plaindirs']:).
>
> Having this fixed, jumping to the buildhistory.bbclass:602, find should be out of the loop to give a hard error if it misses.
> Then inside the for loop, while checking if output folder exists (buildhistory.bbclass:607), this doesnt make sense to me the way it is running:
>  - the comment refers "Make sure the output folder exists...", but we are not creating it if we need, just skipping if doesnt exists.
>  - this folder is being created on do_packagedata (buildhistory.bbclass:396) to write down latest file.
>
> So before spending more time on this topic i have a few questions to understand a little bit of what should be the strategy to fix this:
>
> - If we want to keep files-in-package.txt being created on do_package, shouldnt `buildhistory_list_pkg_files()` (buildhistory.bbclass:600) be responsible to create the folder on buildhistory side?
> - If we keep `buildhistory_list_pkg_files` being triggered by do_package is there any case of do_package_setscene trigger this? I couldnt find any case to support this option (buildhistory.bbclass:101).
> - If not we dont need in fact to create files-in-package.txt on do_package, should files-in-package.txt creation be moved to do_packagedata?
>
> Thanks,
>
> Pedro
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#199810): https://lists.openembedded.org/g/openembedded-core/message/199810
> Mute This Topic: https://lists.openembedded.org/mt/87258776/1686489
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kanavin@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>

Re: [PATCH] buildhistory: Fix do_package race issues

[pmi183]

[-- Attachment #1.1: Type: text/plain, Size: 908 bytes --]

Hi Alex,

As requested, i created a set of small patches with the changes and unit tests to support these changes.
Also i tested against the unit tests mentioned to be sure that everything keeps running fine. I decided not to copy paste
the content of the patches here otherwise the message would get huge since we already discussed before the content.

Patch 1 - fixes race condition detected while executing functions registered on `SSTATEPOSTINSTFUNCS`
Patch 2 - fixes `find` usage, avoiding hiding errors on the command execution and creates buildhistory output folder if doesnt exist.
Patch 3 - fixes usage of 2 features combined, `BUILDHISTORY_PRESERVE` and `BUILDHISTORY_RESET`, restoring files to buildhistory main folder.
Patch 4 - adds unit tests to validate files-in-package.txt generation and feature combination from patch 3.

I hope this goes towards your vision.

Thanks,
Pedro

[-- Attachment #1.2: Type: text/html, Size: 964 bytes --]

[-- Attachment #2: 0001-sstate-fixing-possible-race-codition.patch --]
[-- Type: application/octet-stream, Size: 1204 bytes --]

From a53bb614e1b2256d7b39c01812ac8a84d9a9ce63 Mon Sep 17 00:00:00 2001
From: Ninette Adhikari <ninette@thehoodiefirm.com>
Date: Wed, 22 May 2024 16:52:52 +0200
Subject: [PATCH 1/3] sstate: fixing possible race codition

Fixes race condition between parallel execution of
functions registered in `SSTATEPOSTINSTFUNCS` and
rename operations on folders related to sstate

Signed-off-by: Pedro Ferreira <pmi183@gmail.com>
---
 meta/classes-global/sstate.bbclass | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/meta/classes-global/sstate.bbclass b/meta/classes-global/sstate.bbclass
index 76a7b59636..9887169e4f 100644
--- a/meta/classes-global/sstate.bbclass
+++ b/meta/classes-global/sstate.bbclass
@@ -403,7 +403,6 @@ def sstate_installpkgdir(ss, d):
     for state in ss['dirs']:
         prepdir(state[1])
         bb.utils.rename(sstateinst + state[0], state[1])
-    sstate_install(ss, d)
 
     for plain in ss['plaindirs']:
         workdir = d.getVar('WORKDIR')
@@ -416,6 +415,8 @@ def sstate_installpkgdir(ss, d):
         prepdir(dest)
         bb.utils.rename(src, dest)
 
+    sstate_install(ss, d)
+
     return True
 
 python sstate_hardcode_path_unpack () {
-- 
2.25.1


[-- Attachment #3: 0002-buildhistory-files-in-package.txt-missing.patch --]
[-- Type: application/octet-stream, Size: 1541 bytes --]

From 81517270baa269e57a871deba8fbb5d276b9afb5 Mon Sep 17 00:00:00 2001
From: Pedro Ferreira <Pedro.MS.Ferreira@ctw.bmwgroup.com>
Date: Tue, 28 May 2024 08:46:43 +0000
Subject: [PATCH 2/3] buildhistory: files-in-package.txt missing

Unhides issue using find command to retrieve tree of
folders on `PKGDEST` and ensures creation of `outfolder`
since this responsability is under task `packageData` and
`files-in-package.txt` is created on task `package`

Signed-off-by: Pedro Ferreira <pmi183@gmail.com>
---
 meta/classes/buildhistory.bbclass | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/meta/classes/buildhistory.bbclass b/meta/classes/buildhistory.bbclass
index fd53e92402..1f4396989a 100644
--- a/meta/classes/buildhistory.bbclass
+++ b/meta/classes/buildhistory.bbclass
@@ -599,15 +599,13 @@ buildhistory_list_files_no_owners() {
 
 buildhistory_list_pkg_files() {
 	# Create individual files-in-package for each recipe's package
-	for pkgdir in $(find ${PKGDEST}/* -maxdepth 0 -type d); do
+    pkgdirlist = $(find ${PKGDEST}/* -maxdepth 0 -type d)
+	for pkgdir in $pkgdirlist; do
 		pkgname=$(basename $pkgdir)
 		outfolder="${BUILDHISTORY_DIR_PACKAGE}/$pkgname"
 		outfile="$outfolder/files-in-package.txt"
 		# Make sure the output folder exists so we can create the file
-		if [ ! -d $outfolder ] ; then
-			bbdebug 2 "Folder $outfolder does not exist, file $outfile not created"
-			continue
-		fi
+        mkdir -p $outfolder
 		buildhistory_list_files $pkgdir $outfile fakeroot
 	done
 }
-- 
2.25.1


[-- Attachment #4: 0003-buildhistory-preserve-files-while-buildhitory-reset-.patch --]
[-- Type: application/octet-stream, Size: 2864 bytes --]

From a1efb46c2bba7724eb6e5f3cf447523f7118d9db Mon Sep 17 00:00:00 2001
From: Pedro Ferreira <Pedro.MS.Ferreira@ctw.bmwgroup.com>
Date: Tue, 28 May 2024 08:55:44 +0000
Subject: [PATCH 3/3] buildhistory: preserve files while buildhitory reset
 enabled

When `BUILDHISTORY_RESET` is enabled, files mentioned on
`BUILDHISTORY_PRESERVE` are not being kept. This leads to
file loss.
Adds on task `packageData` a routine to copy files from
`oldpkghistdir` to `pkghistdir`

Signed-off-by: Pedro Ferreira <pmi183@gmail.com>
---
 meta/classes/buildhistory.bbclass | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/meta/classes/buildhistory.bbclass b/meta/classes/buildhistory.bbclass
index 1f4396989a..c707f71657 100644
--- a/meta/classes/buildhistory.bbclass
+++ b/meta/classes/buildhistory.bbclass
@@ -110,6 +110,7 @@ python buildhistory_emit_pkghistory() {
     import json
     import shlex
     import errno
+    import shutil
 
     pkghistdir = d.getVar('BUILDHISTORY_DIR_PACKAGE')
     oldpkghistdir = d.getVar('BUILDHISTORY_OLD_DIR_PACKAGE')
@@ -223,6 +224,20 @@ python buildhistory_emit_pkghistory() {
         items.sort()
         return ' '.join(items)
 
+    def preservebuildhistoryfiles(pkg, preserve):
+        if os.path.exists(os.path.join(oldpkghistdir, pkg)):
+            listofobjs = os.listdir(os.path.join(oldpkghistdir, pkg))
+            for obj in listofobjs:
+                if obj not in preserve:
+                    continue
+                try:
+                    bb.utils.mkdirhier(os.path.join(pkghistdir, pkg))
+                    shutil.copyfile(os.path.join(oldpkghistdir, pkg, obj), os.path.join(pkghistdir, pkg, obj))
+                except IOError as e:
+                    bb.note("Unable to copy file. %s" % e)
+                except EnvironmentError as e:
+                    bb.note("Unable to copy file. %s" % e)
+
     pn = d.getVar('PN')
     pe = d.getVar('PE') or "0"
     pv = d.getVar('PV')
@@ -250,6 +265,11 @@ python buildhistory_emit_pkghistory() {
     if not os.path.exists(pkghistdir):
         bb.utils.mkdirhier(pkghistdir)
     else:
+        # Copy all files marked to preserve
+        if d.getVar("BUILDHISTORY_RESET"):
+            for pkg in packagelist:
+                preservebuildhistoryfiles(pkg, preserve)
+
         # Remove files for packages that no longer exist
         for item in os.listdir(pkghistdir):
             if item not in preserve:
@@ -599,7 +619,7 @@ buildhistory_list_files_no_owners() {
 
 buildhistory_list_pkg_files() {
 	# Create individual files-in-package for each recipe's package
-    pkgdirlist = $(find ${PKGDEST}/* -maxdepth 0 -type d)
+    pkgdirlist=$(find ${PKGDEST}/* -maxdepth 0 -type d)
 	for pkgdir in $pkgdirlist; do
 		pkgname=$(basename $pkgdir)
 		outfolder="${BUILDHISTORY_DIR_PACKAGE}/$pkgname"
-- 
2.25.1


[-- Attachment #5: 0004-buildhistory-unit-tests-for-files-in-package.txt.patch --]
[-- Type: application/octet-stream, Size: 2597 bytes --]

From 207f9049bb0e9ff0950061ff8651cea0160a3174 Mon Sep 17 00:00:00 2001
From: Pedro Ferreira <Pedro.MS.Ferreira@ctw.bmwgroup.com>
Date: Wed, 29 May 2024 08:27:05 +0000
Subject: [PATCH 4/4] buildhistory: unit tests for files-in-package.txt

Adds unit tests to cover file creation and reset
plus preserve feature.

Signed-off-by: Pedro Ferreira <pmi183@gmail.com>
---
 meta/lib/oeqa/selftest/cases/buildoptions.py | 26 ++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/meta/lib/oeqa/selftest/cases/buildoptions.py b/meta/lib/oeqa/selftest/cases/buildoptions.py
index 31dafaa9c5..e05bac7c6c 100644
--- a/meta/lib/oeqa/selftest/cases/buildoptions.py
+++ b/meta/lib/oeqa/selftest/cases/buildoptions.py
@@ -176,6 +176,32 @@ class BuildhistoryTests(BuildhistoryBase):
             self.assertEqual(data['FILELIST'], '')
         self.assertEqual(int(data['PKGSIZE']), 0)
 
+    def test_files_in_package_txt_creation(self):
+        self.config_buildhistory()
+        bitbake('xcursor-transparent-theme -c cleansstate')
+        bitbake('xcursor-transparent-theme')
+        history_dir = get_bb_var('BUILDHISTORY_DIR_PACKAGE', 'xcursor-transparent-theme')
+        self.assertTrue(os.path.isdir(history_dir), 'buildhistory dir was not created.')
+        subfolders = [ fobject.path for fobject in os.scandir(history_dir) if fobject.is_dir() ]
+        self.assertTrue(len(subfolders), "No folders inside package.")
+        for subpath in subfolders:
+            self.assertTrue(os.path.isfile(os.path.join(subpath, 'files-in-package.txt')))
+
+    def test_files_in_package_txt_reset_and_preserved(self):
+        self.config_buildhistory()
+        self.append_config("BUILDHISTORY_RESET = '1'")
+        self.append_config("BUILDHISTORY_PRESERVE:append = ' files-in-package.txt'")
+        bitbake('xcursor-transparent-theme -c cleansstate')
+        bitbake('xcursor-transparent-theme')
+        shutil.rmtree(get_bb_var('TMPDIR'))
+        bitbake('xcursor-transparent-theme')
+        history_dir = get_bb_var('BUILDHISTORY_DIR_PACKAGE', 'xcursor-transparent-theme')
+        self.assertTrue(os.path.isdir(history_dir), 'buildhistory dir was not created.')
+        subfolders = [ fobject.path for fobject in os.scandir(history_dir) if fobject.is_dir() ]
+        self.assertTrue(len(subfolders), "No folders inside package.")
+        for subpath in subfolders:
+            self.assertTrue(os.path.isfile(os.path.join(subpath, 'files-in-package.txt')))
+
 class ArchiverTest(OESelftestTestCase):
     def test_arch_work_dir_and_export_source(self):
         """
-- 
2.25.1

Re: [PATCH] buildhistory: Fix do_package race issues

[pmi183]

[-- Attachment #1.1: Type: text/plain, Size: 890 bytes --]

Hi Alex,

As requested, i created a set of small patches with the changes and unit tests to support these changes.
Also i tested against the unit tests mentioned to be sure that everything keeps running fine. I decided not to copy paste
the content of the patches here otherwise the message would get huge since we already discussed before the content.

Patch 1 - fixes race condition detected while executing functions registered on `SSTATEPOSTINSTFUNCS`
Patch 2 - fixes `find` usage, avoiding hiding errors on the command execution and creates buildhistory output folder if doesnt exist.
Patch 3 - fixes usage of 2 features combined, `BUILDHISTORY_PRESERVE` and `BUILDHISTORY_RESET`, restoring files to buildhistory main folder.
Patch 4 - adds unit tests to validate files-in-package.txt generation and feature combination from patch 3.

I hope this goes towards your vision.

[-- Attachment #1.2: Type: text/html, Size: 11160 bytes --]

[-- Attachment #2: 0001-sstate-fixing-possible-race-codition.patch --]
[-- Type: application/octet-stream, Size: 1204 bytes --]

From a53bb614e1b2256d7b39c01812ac8a84d9a9ce63 Mon Sep 17 00:00:00 2001
From: Ninette Adhikari <ninette@thehoodiefirm.com>
Date: Wed, 22 May 2024 16:52:52 +0200
Subject: [PATCH 1/3] sstate: fixing possible race codition

Fixes race condition between parallel execution of
functions registered in `SSTATEPOSTINSTFUNCS` and
rename operations on folders related to sstate

Signed-off-by: Pedro Ferreira <pmi183@gmail.com>
---
 meta/classes-global/sstate.bbclass | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/meta/classes-global/sstate.bbclass b/meta/classes-global/sstate.bbclass
index 76a7b59636..9887169e4f 100644
--- a/meta/classes-global/sstate.bbclass
+++ b/meta/classes-global/sstate.bbclass
@@ -403,7 +403,6 @@ def sstate_installpkgdir(ss, d):
     for state in ss['dirs']:
         prepdir(state[1])
         bb.utils.rename(sstateinst + state[0], state[1])
-    sstate_install(ss, d)
 
     for plain in ss['plaindirs']:
         workdir = d.getVar('WORKDIR')
@@ -416,6 +415,8 @@ def sstate_installpkgdir(ss, d):
         prepdir(dest)
         bb.utils.rename(src, dest)
 
+    sstate_install(ss, d)
+
     return True
 
 python sstate_hardcode_path_unpack () {
-- 
2.25.1


[-- Attachment #3: 0002-buildhistory-files-in-package.txt-missing.patch --]
[-- Type: application/octet-stream, Size: 1539 bytes --]

From 81517270baa269e57a871deba8fbb5d276b9afb5 Mon Sep 17 00:00:00 2001
From: Pedro Ferreira <Pedro.MS.Ferreira@ctw.bmwgroup.com>
Date: Tue, 28 May 2024 08:46:43 +0000
Subject: [PATCH 2/3] buildhistory: files-in-package.txt missing

Unhides issue using find command to retrieve tree of
folders on `PKGDEST` and ensures creation of `outfolder`
since this responsability is under task `packageData` and
`files-in-package.txt` is created on task `package`

Signed-off-by: Pedro Ferreira <pmi183@gmail.com>
---
 meta/classes/buildhistory.bbclass | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/meta/classes/buildhistory.bbclass b/meta/classes/buildhistory.bbclass
index fd53e92402..1f4396989a 100644
--- a/meta/classes/buildhistory.bbclass
+++ b/meta/classes/buildhistory.bbclass
@@ -599,15 +599,13 @@ buildhistory_list_files_no_owners() {
 
 buildhistory_list_pkg_files() {
 	# Create individual files-in-package for each recipe's package
-	for pkgdir in $(find ${PKGDEST}/* -maxdepth 0 -type d); do
+    pkgdirlist=$(find ${PKGDEST}/* -maxdepth 0 -type d)
+	for pkgdir in $pkgdirlist; do
 		pkgname=$(basename $pkgdir)
 		outfolder="${BUILDHISTORY_DIR_PACKAGE}/$pkgname"
 		outfile="$outfolder/files-in-package.txt"
 		# Make sure the output folder exists so we can create the file
-		if [ ! -d $outfolder ] ; then
-			bbdebug 2 "Folder $outfolder does not exist, file $outfile not created"
-			continue
-		fi
+        mkdir -p $outfolder
 		buildhistory_list_files $pkgdir $outfile fakeroot
 	done
 }
-- 
2.25.1


[-- Attachment #4: 0003-buildhistory-preserve-files-while-buildhitory-reset-.patch --]
[-- Type: application/octet-stream, Size: 2864 bytes --]

From a1efb46c2bba7724eb6e5f3cf447523f7118d9db Mon Sep 17 00:00:00 2001
From: Pedro Ferreira <Pedro.MS.Ferreira@ctw.bmwgroup.com>
Date: Tue, 28 May 2024 08:55:44 +0000
Subject: [PATCH 3/3] buildhistory: preserve files while buildhitory reset
 enabled

When `BUILDHISTORY_RESET` is enabled, files mentioned on
`BUILDHISTORY_PRESERVE` are not being kept. This leads to
file loss.
Adds on task `packageData` a routine to copy files from
`oldpkghistdir` to `pkghistdir`

Signed-off-by: Pedro Ferreira <pmi183@gmail.com>
---
 meta/classes/buildhistory.bbclass | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/meta/classes/buildhistory.bbclass b/meta/classes/buildhistory.bbclass
index 1f4396989a..c707f71657 100644
--- a/meta/classes/buildhistory.bbclass
+++ b/meta/classes/buildhistory.bbclass
@@ -110,6 +110,7 @@ python buildhistory_emit_pkghistory() {
     import json
     import shlex
     import errno
+    import shutil
 
     pkghistdir = d.getVar('BUILDHISTORY_DIR_PACKAGE')
     oldpkghistdir = d.getVar('BUILDHISTORY_OLD_DIR_PACKAGE')
@@ -223,6 +224,20 @@ python buildhistory_emit_pkghistory() {
         items.sort()
         return ' '.join(items)
 
+    def preservebuildhistoryfiles(pkg, preserve):
+        if os.path.exists(os.path.join(oldpkghistdir, pkg)):
+            listofobjs = os.listdir(os.path.join(oldpkghistdir, pkg))
+            for obj in listofobjs:
+                if obj not in preserve:
+                    continue
+                try:
+                    bb.utils.mkdirhier(os.path.join(pkghistdir, pkg))
+                    shutil.copyfile(os.path.join(oldpkghistdir, pkg, obj), os.path.join(pkghistdir, pkg, obj))
+                except IOError as e:
+                    bb.note("Unable to copy file. %s" % e)
+                except EnvironmentError as e:
+                    bb.note("Unable to copy file. %s" % e)
+
     pn = d.getVar('PN')
     pe = d.getVar('PE') or "0"
     pv = d.getVar('PV')
@@ -250,6 +265,11 @@ python buildhistory_emit_pkghistory() {
     if not os.path.exists(pkghistdir):
         bb.utils.mkdirhier(pkghistdir)
     else:
+        # Copy all files marked to preserve
+        if d.getVar("BUILDHISTORY_RESET"):
+            for pkg in packagelist:
+                preservebuildhistoryfiles(pkg, preserve)
+
         # Remove files for packages that no longer exist
         for item in os.listdir(pkghistdir):
             if item not in preserve:
@@ -599,7 +619,7 @@ buildhistory_list_files_no_owners() {
 
 buildhistory_list_pkg_files() {
 	# Create individual files-in-package for each recipe's package
-    pkgdirlist = $(find ${PKGDEST}/* -maxdepth 0 -type d)
+    pkgdirlist=$(find ${PKGDEST}/* -maxdepth 0 -type d)
 	for pkgdir in $pkgdirlist; do
 		pkgname=$(basename $pkgdir)
 		outfolder="${BUILDHISTORY_DIR_PACKAGE}/$pkgname"
-- 
2.25.1


[-- Attachment #5: 0004-buildhistory-unit-tests-for-files-in-package.txt.patch --]
[-- Type: application/octet-stream, Size: 2597 bytes --]

From 207f9049bb0e9ff0950061ff8651cea0160a3174 Mon Sep 17 00:00:00 2001
From: Pedro Ferreira <Pedro.MS.Ferreira@ctw.bmwgroup.com>
Date: Wed, 29 May 2024 08:27:05 +0000
Subject: [PATCH 4/4] buildhistory: unit tests for files-in-package.txt

Adds unit tests to cover file creation and reset
plus preserve feature.

Signed-off-by: Pedro Ferreira <pmi183@gmail.com>
---
 meta/lib/oeqa/selftest/cases/buildoptions.py | 26 ++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/meta/lib/oeqa/selftest/cases/buildoptions.py b/meta/lib/oeqa/selftest/cases/buildoptions.py
index 31dafaa9c5..e05bac7c6c 100644
--- a/meta/lib/oeqa/selftest/cases/buildoptions.py
+++ b/meta/lib/oeqa/selftest/cases/buildoptions.py
@@ -176,6 +176,32 @@ class BuildhistoryTests(BuildhistoryBase):
             self.assertEqual(data['FILELIST'], '')
         self.assertEqual(int(data['PKGSIZE']), 0)
 
+    def test_files_in_package_txt_creation(self):
+        self.config_buildhistory()
+        bitbake('xcursor-transparent-theme -c cleansstate')
+        bitbake('xcursor-transparent-theme')
+        history_dir = get_bb_var('BUILDHISTORY_DIR_PACKAGE', 'xcursor-transparent-theme')
+        self.assertTrue(os.path.isdir(history_dir), 'buildhistory dir was not created.')
+        subfolders = [ fobject.path for fobject in os.scandir(history_dir) if fobject.is_dir() ]
+        self.assertTrue(len(subfolders), "No folders inside package.")
+        for subpath in subfolders:
+            self.assertTrue(os.path.isfile(os.path.join(subpath, 'files-in-package.txt')))
+
+    def test_files_in_package_txt_reset_and_preserved(self):
+        self.config_buildhistory()
+        self.append_config("BUILDHISTORY_RESET = '1'")
+        self.append_config("BUILDHISTORY_PRESERVE:append = ' files-in-package.txt'")
+        bitbake('xcursor-transparent-theme -c cleansstate')
+        bitbake('xcursor-transparent-theme')
+        shutil.rmtree(get_bb_var('TMPDIR'))
+        bitbake('xcursor-transparent-theme')
+        history_dir = get_bb_var('BUILDHISTORY_DIR_PACKAGE', 'xcursor-transparent-theme')
+        self.assertTrue(os.path.isdir(history_dir), 'buildhistory dir was not created.')
+        subfolders = [ fobject.path for fobject in os.scandir(history_dir) if fobject.is_dir() ]
+        self.assertTrue(len(subfolders), "No folders inside package.")
+        for subpath in subfolders:
+            self.assertTrue(os.path.isfile(os.path.join(subpath, 'files-in-package.txt')))
+
 class ArchiverTest(OESelftestTestCase):
     def test_arch_work_dir_and_export_source(self):
         """
-- 
2.25.1

Re: [OE-core] [PATCH] buildhistory: Fix do_package race issues

[Alexander Kanavin]

On Wed, 29 May 2024 at 11:01, pmi183 via lists.openembedded.org
<pmi183=gmail.com@lists.openembedded.org> wrote:
> As requested, i created a set of small patches with the changes and unit tests to support these changes.
> Also i tested against the unit tests mentioned to be sure that everything keeps running fine. I decided not to copy paste
> the content of the patches here otherwise the message would get huge since we already discussed before the content.

Hello,

please resend the patches as direct mailing list messages; it's hard
to comment on attachments.


Alex

Re: [PATCH] buildhistory: Fix do_package race issues

[pmi183]

[-- Attachment #1: Type: text/plain, Size: 6759 bytes --]

Hi Alex,

Patch 1 - fixes race condition detected while executing functions registered on `SSTATEPOSTINSTFUNCS`
---
meta/classes-global/sstate.bbclass | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/meta/classes-global/sstate.bbclass b/meta/classes-global/sstate.bbclass
index 76a7b59636..9887169e4f 100644
--- a/meta/classes-global/sstate.bbclass
+++ b/meta/classes-global/sstate.bbclass
@@ -403,7 +403,6 @@ def sstate_installpkgdir(ss, d):
for state in ss['dirs']:
prepdir(state[1])
bb.utils.rename(sstateinst + state[0], state[1])
-    sstate_install(ss, d)

for plain in ss['plaindirs']:
workdir = d.getVar('WORKDIR')
@@ -416,6 +415,8 @@ def sstate_installpkgdir(ss, d):
prepdir(dest)
bb.utils.rename(src, dest)

+    sstate_install(ss, d)
+
return True

python sstate_hardcode_path_unpack () {
--
2.25.1

Patch 2 - fixes `find` usage, avoiding hiding errors on the command execution and creates buildhistory output folder if doesnt exist.
---
meta/classes/buildhistory.bbclass | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/meta/classes/buildhistory.bbclass b/meta/classes/buildhistory.bbclass
index fd53e92402..59be4516b2 100644
--- a/meta/classes/buildhistory.bbclass
+++ b/meta/classes/buildhistory.bbclass
@@ -599,15 +599,13 @@ buildhistory_list_files_no_owners() {

buildhistory_list_pkg_files() {
# Create individual files-in-package for each recipe's package
-       for pkgdir in $(find ${PKGDEST}/* -maxdepth 0 -type d); do
+    pkgdirlist=$(find ${PKGDEST}/* -maxdepth 0 -type d)
+       for pkgdir in $pkgdirlist; do
pkgname=$(basename $pkgdir)
outfolder="${BUILDHISTORY_DIR_PACKAGE}/$pkgname"
outfile="$outfolder/files-in-package.txt"
# Make sure the output folder exists so we can create the file
-               if [ ! -d $outfolder ] ; then
-                       bbdebug 2 "Folder $outfolder does not exist, file $outfile not created"
-                       continue
-               fi
+        mkdir -p $outfolder
buildhistory_list_files $pkgdir $outfile fakeroot
done
}
--
2.25.1

Patch 3 - fixes usage of 2 features combined, `BUILDHISTORY_PRESERVE` and `BUILDHISTORY_RESET`, restoring files to buildhistory main folder.
---
meta/classes/buildhistory.bbclass | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/meta/classes/buildhistory.bbclass b/meta/classes/buildhistory.bbclass
index 59be4516b2..c707f71657 100644
--- a/meta/classes/buildhistory.bbclass
+++ b/meta/classes/buildhistory.bbclass
@@ -110,6 +110,7 @@ python buildhistory_emit_pkghistory() {
import json
import shlex
import errno
+    import shutil

pkghistdir = d.getVar('BUILDHISTORY_DIR_PACKAGE')
oldpkghistdir = d.getVar('BUILDHISTORY_OLD_DIR_PACKAGE')
@@ -223,6 +224,20 @@ python buildhistory_emit_pkghistory() {
items.sort()
return ' '.join(items)

+    def preservebuildhistoryfiles(pkg, preserve):
+        if os.path.exists(os.path.join(oldpkghistdir, pkg)):
+            listofobjs = os.listdir(os.path.join(oldpkghistdir, pkg))
+            for obj in listofobjs:
+                if obj not in preserve:
+                    continue
+                try:
+                    bb.utils.mkdirhier(os.path.join(pkghistdir, pkg))
+                    shutil.copyfile(os.path.join(oldpkghistdir, pkg, obj), os.path.join(pkghistdir, pkg, obj))
+                except IOError as e:
+                    bb.note("Unable to copy file. %s" % e)
+                except EnvironmentError as e:
+                    bb.note("Unable to copy file. %s" % e)
+
pn = d.getVar('PN')
pe = d.getVar('PE') or "0"
pv = d.getVar('PV')
@@ -250,6 +265,11 @@ python buildhistory_emit_pkghistory() {
if not os.path.exists(pkghistdir):
bb.utils.mkdirhier(pkghistdir)
else:
+        # Copy all files marked to preserve
+        if d.getVar("BUILDHISTORY_RESET"):
+            for pkg in packagelist:
+                preservebuildhistoryfiles(pkg, preserve)
+
# Remove files for packages that no longer exist
for item in os.listdir(pkghistdir):
if item not in preserve:
--
2.25.1

Patch 4 - adds unit tests to validate files-in-package.txt generation and feature combination from patch 3.
---
meta/lib/oeqa/selftest/cases/buildoptions.py | 26 ++++++++++++++++++++
1 file changed, 26 insertions(+)

diff --git a/meta/lib/oeqa/selftest/cases/buildoptions.py b/meta/lib/oeqa/selftest/cases/buildoptions.py
index 31dafaa9c5..e05bac7c6c 100644
--- a/meta/lib/oeqa/selftest/cases/buildoptions.py
+++ b/meta/lib/oeqa/selftest/cases/buildoptions.py
@@ -176,6 +176,32 @@ class BuildhistoryTests(BuildhistoryBase):
self.assertEqual(data['FILELIST'], '')
self.assertEqual(int(data['PKGSIZE']), 0)

+    def test_files_in_package_txt_creation(self):
+        self.config_buildhistory()
+        bitbake('xcursor-transparent-theme -c cleansstate')
+        bitbake('xcursor-transparent-theme')
+        history_dir = get_bb_var('BUILDHISTORY_DIR_PACKAGE', 'xcursor-transparent-theme')
+        self.assertTrue(os.path.isdir(history_dir), 'buildhistory dir was not created.')
+        subfolders = [ fobject.path for fobject in os.scandir(history_dir) if fobject.is_dir() ]
+        self.assertTrue(len(subfolders), "No folders inside package.")
+        for subpath in subfolders:
+            self.assertTrue(os.path.isfile(os.path.join(subpath, 'files-in-package.txt')))
+
+    def test_files_in_package_txt_reset_and_preserved(self):
+        self.config_buildhistory()
+        self.append_config("BUILDHISTORY_RESET = '1'")
+        self.append_config("BUILDHISTORY_PRESERVE:append = ' files-in-package.txt'")
+        bitbake('xcursor-transparent-theme -c cleansstate')
+        bitbake('xcursor-transparent-theme')
+        shutil.rmtree(get_bb_var('TMPDIR'))
+        bitbake('xcursor-transparent-theme')
+        history_dir = get_bb_var('BUILDHISTORY_DIR_PACKAGE', 'xcursor-transparent-theme')
+        self.assertTrue(os.path.isdir(history_dir), 'buildhistory dir was not created.')
+        subfolders = [ fobject.path for fobject in os.scandir(history_dir) if fobject.is_dir() ]
+        self.assertTrue(len(subfolders), "No folders inside package.")
+        for subpath in subfolders:
+            self.assertTrue(os.path.isfile(os.path.join(subpath, 'files-in-package.txt')))
+
class ArchiverTest(OESelftestTestCase):
def test_arch_work_dir_and_export_source(self):
"""
--
2.25.1

[-- Attachment #2: Type: text/html, Size: 12755 bytes --]

Re: [OE-core] [PATCH] buildhistory: Fix do_package race issues

[Alexander Kanavin]

On Wed, 29 May 2024 at 13:57, pmi183 via lists.openembedded.org
<pmi183=gmail.com@lists.openembedded.org> wrote:

> Patch 1 - fixes race condition detected while executing functions registered on `SSTATEPOSTINSTFUNCS`

The patches needs to be sent as separate messages in a series, like
every other patch submitted here ('git send-email' does it).

> ---
>  meta/classes-global/sstate.bbclass | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/meta/classes-global/sstate.bbclass b/meta/classes-global/sstate.bbclass
> index 76a7b59636..9887169e4f 100644
> --- a/meta/classes-global/sstate.bbclass
> +++ b/meta/classes-global/sstate.bbclass
> @@ -403,7 +403,6 @@ def sstate_installpkgdir(ss, d):
>      for state in ss['dirs']:
>          prepdir(state[1])
>          bb.utils.rename(sstateinst + state[0], state[1])
> -    sstate_install(ss, d)
>
>      for plain in ss['plaindirs']:
>          workdir = d.getVar('WORKDIR')
> @@ -416,6 +415,8 @@ def sstate_installpkgdir(ss, d):
>          prepdir(dest)
>          bb.utils.rename(src, dest)
>
> +    sstate_install(ss, d)
> +
>      return True
>
>  python sstate_hardcode_path_unpack () {

sstate.bbclass is a highly sensitive piece in yocto, and any changes
need to be strictly justified. Specifically, you need to explain what
is racing against each other, demonstrate how to reproduce and observe
the problem that the patch is fixing, and (ideally) provide a test
case (in oe-selftest) that fails without the change and passes with
it.

> Patch 2 - fixes `find` usage, avoiding hiding errors on the command execution and creates buildhistory output folder if doesnt exist.
>  buildhistory_list_pkg_files() {
>         # Create individual files-in-package for each recipe's package
> -       for pkgdir in $(find ${PKGDEST}/* -maxdepth 0 -type d); do
> +    pkgdirlist=$(find ${PKGDEST}/* -maxdepth 0 -type d)
> +       for pkgdir in $pkgdirlist; do
>                 pkgname=$(basename $pkgdir)
>                 outfolder="${BUILDHISTORY_DIR_PACKAGE}/$pkgname"
>                 outfile="$outfolder/files-in-package.txt"
>                 # Make sure the output folder exists so we can create the file
> -               if [ ! -d $outfolder ] ; then
> -                       bbdebug 2 "Folder $outfolder does not exist, file $outfile not created"
> -                       continue
> -               fi
> +        mkdir -p $outfolder
>                 buildhistory_list_files $pkgdir $outfile fakeroot
>         done

Indentation problems here. Is the folder is supposed to be created
elsewhere, and sometimes it is not? Why?

>  }
> --
> 2.25.1
>
> Patch 3 - fixes usage of 2 features combined, `BUILDHISTORY_PRESERVE` and `BUILDHISTORY_RESET`, restoring files to buildhistory main folder.

Needs a better explanation too. What wasn't previously working? How
does the code change address it?

Don't be afraid to be verbose and write multiple paragraphs in your
commit messages.

> Patch 4 - adds unit tests to validate files-in-package.txt generation and feature combination from patch 3.
> ---
>  meta/lib/oeqa/selftest/cases/buildoptions.py | 26 ++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>
> diff --git a/meta/lib/oeqa/selftest/cases/buildoptions.py b/meta/lib/oeqa/selftest/cases/buildoptions.py
> index 31dafaa9c5..e05bac7c6c 100644
> --- a/meta/lib/oeqa/selftest/cases/buildoptions.py
> +++ b/meta/lib/oeqa/selftest/cases/buildoptions.py
> @@ -176,6 +176,32 @@ class BuildhistoryTests(BuildhistoryBase):
>              self.assertEqual(data['FILELIST'], '')
>          self.assertEqual(int(data['PKGSIZE']), 0)
>
> +    def test_files_in_package_txt_creation(self):
> +        self.config_buildhistory()
> +        bitbake('xcursor-transparent-theme -c cleansstate')

Thanks for taking the trouble to write the tests, it's good to see.
Sadly cleansstate is not allowed in selftests because that operation
must be performed offline with no other builds running. Is it possible
to test without that?

Alex

Re: [OE-core] [PATCH] buildhistory: Fix do_package race issues

[Richard Purdie]

On Wed, 2024-05-29 at 14:17 +0200, Alexander Kanavin via lists.openembedded.org wrote:
> On Wed, 29 May 2024 at 13:57, pmi183 via lists.openembedded.org
> <pmi183=gmail.com@lists.openembedded.org> wrote:
> 
> > Patch 1 - fixes race condition detected while executing functions registered on `SSTATEPOSTINSTFUNCS`
> 
> The patches needs to be sent as separate messages in a series, like
> every other patch submitted here ('git send-email' does it).
> 
> > ---
> >  meta/classes-global/sstate.bbclass | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/meta/classes-global/sstate.bbclass b/meta/classes-global/sstate.bbclass
> > index 76a7b59636..9887169e4f 100644
> > --- a/meta/classes-global/sstate.bbclass
> > +++ b/meta/classes-global/sstate.bbclass
> > @@ -403,7 +403,6 @@ def sstate_installpkgdir(ss, d):
> >      for state in ss['dirs']:
> >          prepdir(state[1])
> >          bb.utils.rename(sstateinst + state[0], state[1])
> > -    sstate_install(ss, d)
> > 
> >      for plain in ss['plaindirs']:
> >          workdir = d.getVar('WORKDIR')
> > @@ -416,6 +415,8 @@ def sstate_installpkgdir(ss, d):
> >          prepdir(dest)
> >          bb.utils.rename(src, dest)
> > 
> > +    sstate_install(ss, d)
> > +
> >      return True
> > 
> >  python sstate_hardcode_path_unpack () {
> 
> sstate.bbclass is a highly sensitive piece in yocto, and any changes
> need to be strictly justified. Specifically, you need to explain what
> is racing against each other, demonstrate how to reproduce and observe
> the problem that the patch is fixing, and (ideally) provide a test
> case (in oe-selftest) that fails without the change and passes with
> it.

I did look at the sstate code and this part of the problem
specifically. I think there is an issue here and this probably is the
nearly the right fix.

This is the only call site for sstate_install and the "for plain in
ss['plaindirs']:" block probably needs to move into sstate_install
before the SSTATEPOSTINSTFUNCS function calls. I say that as the code
is then within the lock operations. It probably doesn't technically
need to be but it would be what most users would expect.

I'm not sure we've ever guaranteed that SSTATEPOSTINSTFUNCS can see
plaindirs but the buildhistory code clearly depends on that. Quite
often that would happen to work but I can see "from sstate" codepaths
where it wouldn't.

I haven't looked into the other patches in much detail but I did notice
that patch 2 has shell whitespace problems that one of the later
patches fixes.

Cheers,

Richard