diff mbox series

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

Message ID 20240802140008.497492-1-pedro.silva.ferreira@criticaltechworks.com
State New
Headers show
Series [1/2] buildhistory: fix package output folder creation | expand

Commit Message

Pedro Ferreira Aug. 2, 2024, 2 p.m. UTC
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(-)

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

Comments

Richard Purdie Aug. 2, 2024, 2:32 p.m. UTC | #1
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
Pedro Ferreira Aug. 5, 2024, 2:52 p.m. UTC | #2
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!
Alexandre Belloni Aug. 6, 2024, 11:06 a.m. UTC | #3
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]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Richard Purdie Aug. 6, 2024, 12:54 p.m. UTC | #4
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
Pedro Ferreira Aug. 6, 2024, 3:54 p.m. UTC | #5
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!
Richard Purdie Aug. 6, 2024, 8:48 p.m. UTC | #6
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
diff mbox series

Patch

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
 }