[PATCH] buildhistory.bbclass: restore BUILDHISTORY_PRESERVE files

[PATCH] buildhistory.bbclass: restore BUILDHISTORY_PRESERVE files

[Pedro Ferreira]

From: Pedro Ferreira <Pedro.Silva.Ferreira@criticaltechworks.com>

On each build using sstate-cache, buildhistory will move
content to a temporary folder named `old`.
When buildhistory looks for the main dir, it wont find it
and ends up creating it.
As a consequence how code is structured wont restore any
preserved file.

Code block moved to ensure if old dir exists, it will
attempt to restore those files marked to preserve.

Signed-off-by: Pedro Silva Ferreira <Pedro.Silva.Ferreira@criticaltechworks.com>
---
 meta/classes/buildhistory.bbclass | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/meta/classes/buildhistory.bbclass b/meta/classes/buildhistory.bbclass
index d735dd5fb5..b0f395f05e 100644
--- a/meta/classes/buildhistory.bbclass
+++ b/meta/classes/buildhistory.bbclass
@@ -260,14 +260,6 @@ 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:
@@ -280,6 +272,13 @@ python buildhistory_emit_pkghistory() {
                     else:
                         os.unlink(itempath)
 
+    if os.path.exists(oldpkghistdir):
+        # We need to make sure that all files in preserve
+        # are restored from buildhistory/old successfully
+        if d.getVar("BUILDHISTORY_RESET"):
+            for pkg in packagelist:
+                preservebuildhistoryfiles(pkg, preserve)
+
     rcpinfo = RecipeInfo(pn)
     rcpinfo.pe = pe
     rcpinfo.pv = pv
-- 
2.34.1

Re: [OE-core] [PATCH] buildhistory.bbclass: restore BUILDHISTORY_PRESERVE files

[Ross Burton]

Hi Pedro,

> On 15 Jan 2025, at 15:31, Pedro Ferreira via lists.openembedded.org <pmi183=gmail.com@lists.openembedded.org> wrote:
> 
> From: Pedro Ferreira <Pedro.Silva.Ferreira@criticaltechworks.com>
> 
> On each build using sstate-cache, buildhistory will move
> content to a temporary folder named `old`.
> When buildhistory looks for the main dir, it wont find it
> and ends up creating it.
> As a consequence how code is structured wont restore any
> preserved file.
> 
> Code block moved to ensure if old dir exists, it will
> attempt to restore those files marked to preserve.

Do you have a reproducer for this behaviour?  As you have seen the buildhistory file management isn’t trivial and we’d like to understand the failure better.

Cheers,
Ross

Re: [PATCH] buildhistory.bbclass: restore BUILDHISTORY_PRESERVE files

[Pedro Ferreira]

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

Hi Ross,

Here's a simple test do reproduce it.

*Pre-conditions:*
INHERIT += "buildhistory"
BUILDHISTORY_RESET = "1"
BUILDHISTORY_PRESERVE:append = " files-in-package.txt"

*- Starting without sstate cache*
*Run:*
bitbake base-passwd
*Check:*
cat PATH_TO_BUILD_dir/buildhistory/packages/core2-64-poky-linux/base-passwd/base-passwd-dbg/files-in-package.txt
(File exists and content ok)
*Run:*
bitbake base-passwd (it will use sstate cache)
*Check:*
cat PATH_TO_BUILD_dir/buildhistory/packages/core2-64-poky-linux/base-passwd/base-passwd-dbg/files-in-package.txt
(File wont exist)

*- Starting with sstate cache*
*Run:*
bitbake base-passwd --no-setscene
*Check:*
cat PATH_TO_BUILD_dir/buildhistory/packages/core2-64-poky-linux/base-passwd/base-passwd-dbg/files-in-package.txt
(File exists and content ok)
*Run:*
bitbake base-passwd (it will use sstate cache)
*Check:*
cat PATH_TO_BUILD_dir/buildhistory/packages/core2-64-poky-linux/base-passwd/base-passwd-dbg/files-in-package.txt
(File wont exist)

Cheers,
Pedro

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

Re: [PATCH] buildhistory.bbclass: restore BUILDHISTORY_PRESERVE files

[Pedro Ferreira]

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

Simple scenario to test it:

*Pre-conditions:*
INHERIT += "buildhistory"
BUILDHISTORY_RESET = "1"
BUILDHISTORY_PRESERVE:append = " files-in-package.txt"

*Run:*
bitbake -c cleansstate base-passwd && bitbake base-passwd
*Check:*
cat PATH_TO_BUILD_dir/buildhistory/packages/core2-64-poky-linux/base-passwd/base-passwd-dbg/files-in-package.txt
(File exists and content ok)
*Run:*
rm -rf tmp && bitbake base-passwd (it will use sstate cache)
*Check:*
cat PATH_TO_BUILD_dir/buildhistory/packages/core2-64-poky-linux/base-passwd/base-passwd-dbg/files-in-package.txt
(File wont exist)

Cheers,
Pedro

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

Re: [OE-core] [PATCH] buildhistory.bbclass: restore BUILDHISTORY_PRESERVE files

[Ross Burton]

On 6 Mar 2025, at 14:26, Pedro Ferreira via lists.openembedded.org <pedro.silva.ferreira=criticaltechworks.com@lists.openembedded.org> wrote:
> 
> Simple scenario to test it:
>   Pre-conditions:
> INHERIT += "buildhistory"
> BUILDHISTORY_RESET = "1"
> BUILDHISTORY_PRESERVE:append = " files-in-package.txt"
> 
> Run:
> bitbake -c cleansstate base-passwd && bitbake base-passwd
> Check:
> cat PATH_TO_BUILD_dir/buildhistory/packages/core2-64-poky-linux/base-passwd/base-passwd-dbg/files-in-package.txt
> (File exists and content ok)
> Run:
> rm -rf tmp && bitbake base-passwd (it will use sstate cache)
> Check:
> cat PATH_TO_BUILD_dir/buildhistory/packages/core2-64-poky-linux/base-passwd/base-passwd-dbg/files-in-package.txt
> (File wont exist)
>   Cheers,

There’s an even bigger problem with BUILDHISTORY_RESET, where recipes from sstate are just not entering the buildhistory at all…

From a clean buildhistory and build tree, if an entire image can be populated from sstate then there’s just nothing in buildhistory for the packages, as it’s only populated on do_package which won’t actually be re-ran.

Adding files-in-package.txt to preserve feels like a workaround for that problem, am I right? Apart from the fact that it doesn’t actually work when the buildhistory repository is empty from the beginning.

Ross

Re: [OE-core] [PATCH] buildhistory.bbclass: restore BUILDHISTORY_PRESERVE files

[Richard Purdie]

On Wed, 2025-01-15 at 15:31 +0000, Pedro Ferreira via lists.openembedded.org wrote:
> From: Pedro Ferreira <Pedro.Silva.Ferreira@criticaltechworks.com>
> 
> On each build using sstate-cache, buildhistory will move
> content to a temporary folder named `old`.
> When buildhistory looks for the main dir, it wont find it
> and ends up creating it.
> As a consequence how code is structured wont restore any
> preserved file.
> 
> Code block moved to ensure if old dir exists, it will
> attempt to restore those files marked to preserve.
> 
> Signed-off-by: Pedro Silva Ferreira <Pedro.Silva.Ferreira@criticaltechworks.com>
> ---
>  meta/classes/buildhistory.bbclass | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)

I spent a lot of time on Friday looking at this. The BUILDHISTORY_RESET
code path is horrible to understand and basically just hacked into na
don top of the rest of the system. BUILDHISTORY_PRESERVE was never
really intended as a end user API either.

After much consideration, I've sent a patch proposing we remove support
for BUILDHISTORY_RESET. I think the behaviour would be better coded
into the CI integration in different cases.

If we do want in tree support for this "reset" functionality, it needs
to be written in a way where the code can be understood and it needs to
have test cases. It would also need documentation as these code paths
are currently undocumented.

As Ross previously mentioned the interaction with an sstate cache is
also horrible and is another reason I think it is safer for the user to
do what they want/need CI side rather than in the class code. Sorry it
has taken as long to work all this out, the code really is hard to
unravel.

Cheers,

Richard

Re: [OE-core] [PATCH] buildhistory.bbclass: restore BUILDHISTORY_PRESERVE files

[Fabio Berton]

Hi Richard,
Thanks for looking into this. I was wondering if there is a way to know whether BUILDHISTORY_PRESERVE, or another variable, is not intended to be part of the end user API?

Regards,

Fabio

On 6/23/25 21:48, Richard Purdie via lists.openembedded.org wrote:
> On Wed, 2025-01-15 at 15:31 +0000, Pedro Ferreira via lists.openembedded.org wrote:
>> From: Pedro Ferreira <Pedro.Silva.Ferreira@criticaltechworks.com>
>>
>> On each build using sstate-cache, buildhistory will move
>> content to a temporary folder named `old`.
>> When buildhistory looks for the main dir, it wont find it
>> and ends up creating it.
>> As a consequence how code is structured wont restore any
>> preserved file.
>>
>> Code block moved to ensure if old dir exists, it will
>> attempt to restore those files marked to preserve.
>>
>> Signed-off-by: Pedro Silva Ferreira <Pedro.Silva.Ferreira@criticaltechworks.com>
>> ---
>>  meta/classes/buildhistory.bbclass | 15 +++++++--------
>>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> I spent a lot of time on Friday looking at this. The BUILDHISTORY_RESET
> code path is horrible to understand and basically just hacked into na
> don top of the rest of the system. BUILDHISTORY_PRESERVE was never
> really intended as a end user API either.
> 
> After much consideration, I've sent a patch proposing we remove support
> for BUILDHISTORY_RESET. I think the behaviour would be better coded
> into the CI integration in different cases.
> 
> If we do want in tree support for this "reset" functionality, it needs
> to be written in a way where the code can be understood and it needs to
> have test cases. It would also need documentation as these code paths
> are currently undocumented.
> 
> As Ross previously mentioned the interaction with an sstate cache is
> also horrible and is another reason I think it is safer for the user to
> do what they want/need CI side rather than in the class code. Sorry it
> has taken as long to work all this out, the code really is hard to
> unravel.
> 
> Cheers,
> 
> Richard
> 
> 
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#219219): https://lists.openembedded.org/g/openembedded-core/message/219219
> Mute This Topic: https://lists.openembedded.org/mt/110629321/6083838
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [fbberton@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 

Re: [OE-core] [PATCH] buildhistory.bbclass: restore BUILDHISTORY_PRESERVE files

[Richard Purdie]

On Wed, 2025-06-25 at 11:26 +0100, Fabio Berton wrote:
> Thanks for looking into this. I was wondering if there is a way to
> know whether BUILDHISTORY_PRESERVE, or another variable, is not
> intended to be part of the end user API?

This is something we struggle with and don't have well defined. A sign
in this case is that it isn't in the manual though...

Cheers,

Richard