buildhistory: Fix do_package race issues

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(-)

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]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>

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

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]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>

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

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

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

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