| Message ID | 20250115153149.1827119-1-pmi183@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | buildhistory.bbclass: restore BUILDHISTORY_PRESERVE files | expand |
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
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
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
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
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
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] > -=-=-=-=-=-=-=-=-=-=-=- >
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
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