Re: [PATCH] buildhistory: Fix do_package race issues

[pmi183@gmail.com]

[-- 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