[PATCH 1/2] buildhistory: fix package output folder creation

[PATCH 1/2] buildhistory: fix package output folder creation

[Pedro Ferreira]

This fix garantees that output package folder exists on
buildhistory folder to avoid missing files-in-package.txt
creation during task `package` execution.

This issue happens because the output folder is being
created on task `packagedata` before generating `latest` file.

Also it ensures that in case of `find` fails we leave with
a hard error instead of hidding the error on the for loop.

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 c3d049aea3..ad151092c9 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.34.1

The information in this communication may contain confidential or legally privileged information. It is intended solely for the use of the individual or entity it addresses and others authorized to receive it. If you are not an intended recipient, you are hereby notified that any disclosure, copying, distribution or action in reliance on the contents of this information is strictly prohibited and may be unlawful. If you have received this communication by error, please notify us immediately by responding to this e-mail and then delete it from your system. Critical TechWorks is not liable for the proper and complete transmission of the information in this communication nor for any delay in its receipt

This e-mail is environmentally friendly, just like Critical TechWorks, which lives in a paper-free atmosphere. Therefore, please consider the environment before printing it!

[PATCH 2/2] buildhistory: Restoring files from preserve list

[Pedro Ferreira]

This fix will ensure that, when we activate feature
`BUILDHISTORY_RESET`, files marked to keep on feature
`BUILDHISTORY_PRESERVE` will indeed exist is buildhistory
final path since they are moved to buildhistory/old but
not restored at any point.

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

diff --git a/meta/classes/buildhistory.bbclass b/meta/classes/buildhistory.bbclass
index ad151092c9..349b7dfba2 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,14 @@ python buildhistory_emit_pkghistory() {
     if not os.path.exists(pkghistdir):
         bb.utils.mkdirhier(pkghistdir)
     else:
+        # We need to make sure that all files kept in
+        # buildhistory/old are restored successfully
+        # otherwise next block of code wont have files to
+        # check and purge
+        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.34.1

The information in this communication may contain confidential or legally privileged information. It is intended solely for the use of the individual or entity it addresses and others authorized to receive it. If you are not an intended recipient, you are hereby notified that any disclosure, copying, distribution or action in reliance on the contents of this information is strictly prohibited and may be unlawful. If you have received this communication by error, please notify us immediately by responding to this e-mail and then delete it from your system. Critical TechWorks is not liable for the proper and complete transmission of the information in this communication nor for any delay in its receipt

This e-mail is environmentally friendly, just like Critical TechWorks, which lives in a paper-free atmosphere. Therefore, please consider the environment before printing it!

Re: [OE-core] [PATCH 1/2] buildhistory: fix package output folder creation

[Richard Purdie]

On Fri, 2024-08-02 at 15:00 +0100, Pedro Ferreira via lists.openembedded.org wrote:
> This fix garantees that output package folder exists on
> buildhistory folder to avoid missing files-in-package.txt
> creation during task `package` execution.
> 
> This issue happens because the output folder is being
> created on task `packagedata` before generating `latest` file.
> 
> Also it ensures that in case of `find` fails we leave with
> a hard error instead of hidding the error on the for loop.
> 
> Signed-off-by: Pedro Ferreira <pmi183@gmail.com>
> ---
>  meta/classes/buildhistory.bbclass | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)

How would someone reproduce the bug being fixed here? 

I'm a little worried that this might really be a build race or a
dependency problem. If that is the case, creating the directory doesn't
seem the right thing to do and is just working around the problem?

Cheers,

Richard

RE: [OE-core] [PATCH 1/2] buildhistory: fix package output folder creation

[Pedro Silva Ferreira]

Without the fix you can check that with:
Set on local.conf : INHERIT += "buildhistory"
Run: bitbake -c cleansstate base-passwd && bitbake base-passwd

Taking a look on buildhistory you will see that for base-passwd packages there is no files-in-package.txt

Reason:
On log.do_package, theres a log,"DEBUG: Shell function buildhistory_list_pkg_files finished", which indicates that the main shell function for the `files-in-package.txt` was triggered an so the function expects that the recipe package folder exists it will skip `files-in-package.txt` creation.

On `packageData` task, there a python function `write_pkghistory` that runs for all packages from a recipe package and this function is protected to create the folder if this doesn’t exists.
When writing `latest` we are protecting it from failing to write down the file but for `files-in-package.txt` we don’t.

My best regards,

Pedro Ferreira

-----Original Message-----
From: Richard Purdie <richard.purdie@linuxfoundation.org>
Sent: Friday, August 2, 2024 3:33 PM
To: Pedro Silva Ferreira <Pedro.Silva.Ferreira@criticaltechworks.com>; openembedded-core@lists.openembedded.org
Cc: Pedro Ferreira <pmi183@gmail.com>
Subject: Re: [OE-core] [PATCH 1/2] buildhistory: fix package output folder creation

Notice: This e-mail has originated from an external email service, so do not click on any links, nor open any attachments unless you know who the sender is and are sure the content is secure.



On Fri, 2024-08-02 at 15:00 +0100, Pedro Ferreira via lists.openembedded.org wrote:
> This fix garantees that output package folder exists on buildhistory
> folder to avoid missing files-in-package.txt creation during task
> `package` execution.
>
> This issue happens because the output folder is being created on task
> `packagedata` before generating `latest` file.
>
> Also it ensures that in case of `find` fails we leave with a hard
> error instead of hidding the error on the for loop.
>
> Signed-off-by: Pedro Ferreira <pmi183@gmail.com>
> ---
>  meta/classes/buildhistory.bbclass | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)

How would someone reproduce the bug being fixed here?

I'm a little worried that this might really be a build race or a dependency problem. If that is the case, creating the directory doesn't seem the right thing to do and is just working around the problem?

Cheers,

Richard
The information in this communication may contain confidential or legally privileged information. It is intended solely for the use of the individual or entity it addresses and others authorized to receive it. If you are not an intended recipient, you are hereby notified that any disclosure, copying, distribution or action in reliance on the contents of this information is strictly prohibited and may be unlawful. If you have received this communication by error, please notify us immediately by responding to this e-mail and then delete it from your system. Critical TechWorks is not liable for the proper and complete transmission of the information in this communication nor for any delay in its receipt

This e-mail is environmentally friendly, just like Critical TechWorks, which lives in a paper-free atmosphere. Therefore, please consider the environment before printing it!

Re: [OE-core] [PATCH 1/2] buildhistory: fix package output folder creation

[Alexandre Belloni]

Hello,

This patch is missing your From:

https://docs.yoctoproject.org/dev/contributor-guide/submit-changes.html#fixing-your-from-identity


On 02/08/2024 15:00:07+0100, Pedro Ferreira via lists.openembedded.org wrote:
> This fix garantees that output package folder exists on
> buildhistory folder to avoid missing files-in-package.txt
> creation during task `package` execution.
> 
> This issue happens because the output folder is being
> created on task `packagedata` before generating `latest` file.
> 
> Also it ensures that in case of `find` fails we leave with
> a hard error instead of hidding the error on the for loop.
> 
> 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 c3d049aea3..ad151092c9 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

Also, the patch doesn't apply because tabs have been changed to spaces
here.

>                 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.34.1
> 
> The information in this communication may contain confidential or legally privileged information. It is intended solely for the use of the individual or entity it addresses and others authorized to receive it. If you are not an intended recipient, you are hereby notified that any disclosure, copying, distribution or action in reliance on the contents of this information is strictly prohibited and may be unlawful. If you have received this communication by error, please notify us immediately by responding to this e-mail and then delete it from your system. Critical TechWorks is not liable for the proper and complete transmission of the information in this communication nor for any delay in its receipt
> 
> This e-mail is environmentally friendly, just like Critical TechWorks, which lives in a paper-free atmosphere. Therefore, please consider the environment before printing it!

> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#202911): https://lists.openembedded.org/g/openembedded-core/message/202911
> Mute This Topic: https://lists.openembedded.org/mt/107685168/3617179
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alexandre.belloni@bootlin.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [OE-core] [PATCH 1/2] buildhistory: fix package output folder creation

[Richard Purdie]

On Mon, 2024-08-05 at 14:52 +0000, Pedro Silva Ferreira wrote:
> Without the fix you can check that with:
> Set on local.conf : INHERIT += "buildhistory"
> Run: bitbake -c cleansstate base-passwd && bitbake base-passwd
> 
> Taking a look on buildhistory you will see that for base-passwd
> packages there is no files-in-package.txt
> 
> Reason:
> On log.do_package, theres a log,"DEBUG: Shell function
> buildhistory_list_pkg_files finished", which indicates that the main
> shell function for the `files-in-package.txt` was triggered an so the
> function expects that the recipe package folder exists it will skip
> `files-in-package.txt` creation.
> 
> On `packageData` task, there a python function `write_pkghistory`
> that runs for all packages from a recipe package and this function is
> protected to create the folder if this doesn’t exists.
> When writing `latest` we are protecting it from failing to write down
> the file but for `files-in-package.txt` we don’t.

Thanks for the details. Can you confirm this happens with current
master please? I tested it and couldn't reproduce.

I'm wondering if this was fixed by:

https://git.yoctoproject.org/poky/commit/?id=2feb9e20e464088c377fadb9344da28100662130

Cheers,

Richard

RE: [OE-core] [PATCH 1/2] buildhistory: fix package output folder creation

[Pedro Silva Ferreira]

Hi,

Yes I can see the issue using poky[master] on commit `db87ca070c5a4790dcdb9601bcb35037603f6d7f`.
For each package inside recipe_package I can only see latest and latest.pkg_postinst/preinst if applicable. There should be a files-in-package.txt for each package, but as I mentioned, the `recipe_package` folder exists but not `recipe_package/package` folder during `package` task and only on the next task which is creating `latest` file, this folder are created(ex: base-passwd-dbg, base-passwd-dev,...).
FYI: I updated the patches, Signed-off-by was pointing to the wrong email.

My best regards,

Pedro Ferreira

-----Original Message-----
From: Richard Purdie <richard.purdie@linuxfoundation.org>
Sent: Tuesday, August 6, 2024 1:55 PM
To: Pedro Silva Ferreira <Pedro.Silva.Ferreira@criticaltechworks.com>; openembedded-core@lists.openembedded.org
Cc: Pedro Ferreira <pmi183@gmail.com>
Subject: Re: [OE-core] [PATCH 1/2] buildhistory: fix package output folder creation

Notice: This e-mail has originated from an external email service, so do not click on any links, nor open any attachments unless you know who the sender is and are sure the content is secure.



On Mon, 2024-08-05 at 14:52 +0000, Pedro Silva Ferreira wrote:
> Without the fix you can check that with:
> Set on local.conf : INHERIT += "buildhistory"
> Run: bitbake -c cleansstate base-passwd && bitbake base-passwd
>
> Taking a look on buildhistory you will see that for base-passwd
> packages there is no files-in-package.txt
>
> Reason:
> On log.do_package, theres a log,"DEBUG: Shell function
> buildhistory_list_pkg_files finished", which indicates that the main
> shell function for the `files-in-package.txt` was triggered an so the
> function expects that the recipe package folder exists it will skip
> `files-in-package.txt` creation.
>
> On `packageData` task, there a python function `write_pkghistory` that
> runs for all packages from a recipe package and this function is
> protected to create the folder if this doesn’t exists.
> When writing `latest` we are protecting it from failing to write down
> the file but for `files-in-package.txt` we don’t.

Thanks for the details. Can you confirm this happens with current master please? I tested it and couldn't reproduce.

I'm wondering if this was fixed by:

https://git.yoctoproject.org/poky/commit/?id=2feb9e20e464088c377fadb9344da28100662130

Cheers,

Richard
The information in this communication may contain confidential or legally privileged information. It is intended solely for the use of the individual or entity it addresses and others authorized to receive it. If you are not an intended recipient, you are hereby notified that any disclosure, copying, distribution or action in reliance on the contents of this information is strictly prohibited and may be unlawful. If you have received this communication by error, please notify us immediately by responding to this e-mail and then delete it from your system. Critical TechWorks is not liable for the proper and complete transmission of the information in this communication nor for any delay in its receipt

This e-mail is environmentally friendly, just like Critical TechWorks, which lives in a paper-free atmosphere. Therefore, please consider the environment before printing it!

Re: [OE-core] [PATCH 1/2] buildhistory: fix package output folder creation

[Richard Purdie]

On Tue, 2024-08-06 at 15:54 +0000, Pedro Silva Ferreira wrote:
> Yes I can see the issue using poky[master] on commit
> `db87ca070c5a4790dcdb9601bcb35037603f6d7f`.
> For each package inside recipe_package I can only see latest and
> latest.pkg_postinst/preinst if applicable. There should be a files-
> in-package.txt for each package, but as I mentioned, the
> `recipe_package` folder exists but not `recipe_package/package`
> folder during `package` task and only on the next task which is
> creating `latest` file, this folder are created(ex: base-passwd-dbg,
> base-passwd-dev,...).
> FYI: I updated the patches, Signed-off-by was pointing to the wrong
> email.

I was having some trouble understanding the issue from the commit
messages but I was finally able to reproduce it. The key information
is to start from a clean buildhistory and sstate, then run the recipe.
The files-in-package files are then missing.

I've queued the patch for testing with an updated commit message to try
and better explain the race issue between do_package and
do_packagedata.

Cheers,

Richard