Message ID | 20211123135919.26315-1-richard.purdie@linuxfoundation.org |
---|---|
State | Accepted, archived |
Commit | b83823ce44e7531bbd2bfa62062c04147a11f724 |
Headers | show |
Series | buildhistory: Fix do_package race issues | expand |
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
Hi Alex, I made some debug in the flow and i saw a timing issue and since there's no reason to generate files-in-package.txt before sstate-cache operations i moved the call to the postfunc of sstate: --- meta/classes-global/sstate.bbclass | 5 +++++ meta/classes/buildhistory.bbclass | 18 +++++++++++++++--- meta/lib/oe/packagedata.py | 2 +- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/meta/classes-global/sstate.bbclass b/meta/classes-global/sstate.bbclass index 76a7b59636..7e3c3e3c4f 100644 --- a/meta/classes-global/sstate.bbclass +++ b/meta/classes-global/sstate.bbclass @@ -104,6 +104,7 @@ SSTATEPOSTCREATEFUNCS = "" SSTATEPREINSTFUNCS = "" SSTATEPOSTUNPACKFUNCS = "sstate_hardcode_path_unpack" SSTATEPOSTINSTFUNCS = "" +SSTATEPOSTFUNCS = "" EXTRA_STAGING_FIXMES ?= "HOSTTOOLS_DIR" # Check whether sstate exists for tasks that support sstate and are in the @@ -805,6 +806,10 @@ python sstate_task_postfunc () { sstate_installpkgdir(shared_state, d) bb.utils.remove(d.getVar("SSTATE_BUILDDIR"), recurse=True) + + for postfunc in (d.getVar('SSTATEPOSTFUNCS') or '').split(): + # All hooks should run in the SSTATE_INSTDIR + bb.build.exec_func(postfunc, d, (sstateinst,)) } sstate_task_postfunc[dirs] = "${WORKDIR}" diff --git a/meta/classes/buildhistory.bbclass b/meta/classes/buildhistory.bbclass index fd53e92402..054c213bf5 100644 --- a/meta/classes/buildhistory.bbclass +++ b/meta/classes/buildhistory.bbclass @@ -58,6 +58,9 @@ SSTATEPOSTUNPACKFUNCS:append = " buildhistory_emit_outputsigs" sstate_installpkgdir[vardepsexclude] += "buildhistory_emit_outputsigs" SSTATEPOSTUNPACKFUNCS[vardepvalueexclude] .= "| buildhistory_emit_outputsigs" +SSTATEPOSTFUNCS:append = " buildhistory_emit_filesinpackage" +SSTATEPOSTFUNCS[vardepvalueexclude] .= "| buildhistory_emit_filesinpackage" + # All items excepts those listed here will be removed from a recipe's # build history directory by buildhistory_emit_pkghistory(). This is # necessary because some of these items (package directories, files that @@ -87,6 +90,16 @@ buildhistory_emit_sysroot() { buildhistory_list_files_no_owners $BASE ${BUILDHISTORY_DIR_PACKAGE}/sysroot } +# +# Write out files-in-package for this package +# +python buildhistory_emit_filesinpackage() { + 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 +} + # # Write out metadata about this package for comparison when writing future packages # @@ -99,8 +112,6 @@ python buildhistory_emit_pkghistory() { 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']: @@ -599,7 +610,8 @@ 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" diff --git a/meta/lib/oe/packagedata.py b/meta/lib/oe/packagedata.py index 2d1d6ddeb7..e8c503b43b 100644 --- a/meta/lib/oe/packagedata.py +++ b/meta/lib/oe/packagedata.py @@ -309,7 +309,7 @@ fi subdata_file = pkgdatadir + "/runtime/%s" % pkg with open(subdata_file, 'w') as sf: for var in (d.getVar('PKGDATA_VARS') or "").split(): - val = write_if_exists(sf, pkg, var) + write_if_exists(sf, pkg, var) write_if_exists(sf, pkg, 'FILERPROVIDESFLIST') for dfile in sorted((d.getVar('FILERPROVIDESFLIST:' + pkg) or "").split()): -- 2.34.1
Hi Pedro, This will add the sstate-cache as a dependency of the buildhistory which can be correct if it is enabled or in use but IMO the buildhistory should also work without it, when the sstate is not in use. Could you please test one build with: bitbake --no-setscene ... ? Jose pmi183 via lists.openembedded.org <pmi183=gmail.com@lists.openembedded.org> escreveu (segunda, 20/05/2024 à(s) 09:10): > Hi Alex, > > I made some debug in the flow and i saw a timing issue and since there's > no reason to generate files-in-package.txt before sstate-cache operations i > moved the call to the postfunc of sstate: > > --- > meta/classes-global/sstate.bbclass | 5 +++++ > meta/classes/buildhistory.bbclass | 18 +++++++++++++++--- > meta/lib/oe/packagedata.py | 2 +- > 3 files changed, 21 insertions(+), 4 deletions(-) > > diff --git a/meta/classes-global/sstate.bbclass > b/meta/classes-global/sstate.bbclass > index 76a7b59636..7e3c3e3c4f 100644 > --- a/meta/classes-global/sstate.bbclass > +++ b/meta/classes-global/sstate.bbclass > @@ -104,6 +104,7 @@ SSTATEPOSTCREATEFUNCS = "" > SSTATEPREINSTFUNCS = "" > SSTATEPOSTUNPACKFUNCS = "sstate_hardcode_path_unpack" > SSTATEPOSTINSTFUNCS = "" > +SSTATEPOSTFUNCS = "" > EXTRA_STAGING_FIXMES ?= "HOSTTOOLS_DIR" > > # Check whether sstate exists for tasks that support sstate and are in the > @@ -805,6 +806,10 @@ python sstate_task_postfunc () { > sstate_installpkgdir(shared_state, d) > > bb.utils.remove(d.getVar("SSTATE_BUILDDIR"), recurse=True) > + > + for postfunc in (d.getVar('SSTATEPOSTFUNCS') or '').split(): > + # All hooks should run in the SSTATE_INSTDIR > + bb.build.exec_func(postfunc, d, (sstateinst,)) > } > sstate_task_postfunc[dirs] = "${WORKDIR}" > > diff --git a/meta/classes/buildhistory.bbclass > b/meta/classes/buildhistory.bbclass > index fd53e92402..054c213bf5 100644 > --- a/meta/classes/buildhistory.bbclass > +++ b/meta/classes/buildhistory.bbclass > @@ -58,6 +58,9 @@ SSTATEPOSTUNPACKFUNCS:append = " > buildhistory_emit_outputsigs" > sstate_installpkgdir[vardepsexclude] += "buildhistory_emit_outputsigs" > SSTATEPOSTUNPACKFUNCS[vardepvalueexclude] .= "| > buildhistory_emit_outputsigs" > > +SSTATEPOSTFUNCS:append = " buildhistory_emit_filesinpackage" > +SSTATEPOSTFUNCS[vardepvalueexclude] .= "| > buildhistory_emit_filesinpackage" > + > # All items excepts those listed here will be removed from a recipe's > # build history directory by buildhistory_emit_pkghistory(). This is > # necessary because some of these items (package directories, files that > @@ -87,6 +90,16 @@ buildhistory_emit_sysroot() { > buildhistory_list_files_no_owners $BASE > ${BUILDHISTORY_DIR_PACKAGE}/sysroot > } > > +# > +# Write out files-in-package for this package > +# > +python buildhistory_emit_filesinpackage() { > + 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 > +} > + > # > # Write out metadata about this package for comparison when writing > future packages > # > @@ -99,8 +112,6 @@ python buildhistory_emit_pkghistory() { > 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']: > @@ -599,7 +610,8 @@ 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" > diff --git a/meta/lib/oe/packagedata.py b/meta/lib/oe/packagedata.py > index 2d1d6ddeb7..e8c503b43b 100644 > --- a/meta/lib/oe/packagedata.py > +++ b/meta/lib/oe/packagedata.py > @@ -309,7 +309,7 @@ fi > subdata_file = pkgdatadir + "/runtime/%s" % pkg > with open(subdata_file, 'w') as sf: > for var in (d.getVar('PKGDATA_VARS') or "").split(): > - val = write_if_exists(sf, pkg, var) > + write_if_exists(sf, pkg, var) > > write_if_exists(sf, pkg, 'FILERPROVIDESFLIST') > for dfile in sorted((d.getVar('FILERPROVIDESFLIST:' + pkg) or > "").split()): > -- > 2.34.1 > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#199555): > https://lists.openembedded.org/g/openembedded-core/message/199555 > Mute This Topic: https://lists.openembedded.org/mt/87258776/5052612 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [ > quaresma.jose@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- > >
This does not seem to address the question I asked. Please do not send more code that moves things around, but diagnose the original issue properly. Alex On Mon, 20 May 2024 at 10:10, pmi183 via lists.openembedded.org <pmi183=gmail.com@lists.openembedded.org> wrote: > > Hi Alex, > > I made some debug in the flow and i saw a timing issue and since there's no reason to generate files-in-package.txt before sstate-cache operations i moved the call to the postfunc of sstate: > > --- > meta/classes-global/sstate.bbclass | 5 +++++ > meta/classes/buildhistory.bbclass | 18 +++++++++++++++--- > meta/lib/oe/packagedata.py | 2 +- > 3 files changed, 21 insertions(+), 4 deletions(-) > > diff --git a/meta/classes-global/sstate.bbclass b/meta/classes-global/sstate.bbclass > index 76a7b59636..7e3c3e3c4f 100644 > --- a/meta/classes-global/sstate.bbclass > +++ b/meta/classes-global/sstate.bbclass > @@ -104,6 +104,7 @@ SSTATEPOSTCREATEFUNCS = "" > SSTATEPREINSTFUNCS = "" > SSTATEPOSTUNPACKFUNCS = "sstate_hardcode_path_unpack" > SSTATEPOSTINSTFUNCS = "" > +SSTATEPOSTFUNCS = "" > EXTRA_STAGING_FIXMES ?= "HOSTTOOLS_DIR" > > # Check whether sstate exists for tasks that support sstate and are in the > @@ -805,6 +806,10 @@ python sstate_task_postfunc () { > sstate_installpkgdir(shared_state, d) > > bb.utils.remove(d.getVar("SSTATE_BUILDDIR"), recurse=True) > + > + for postfunc in (d.getVar('SSTATEPOSTFUNCS') or '').split(): > + # All hooks should run in the SSTATE_INSTDIR > + bb.build.exec_func(postfunc, d, (sstateinst,)) > } > sstate_task_postfunc[dirs] = "${WORKDIR}" > > diff --git a/meta/classes/buildhistory.bbclass b/meta/classes/buildhistory.bbclass > index fd53e92402..054c213bf5 100644 > --- a/meta/classes/buildhistory.bbclass > +++ b/meta/classes/buildhistory.bbclass > @@ -58,6 +58,9 @@ SSTATEPOSTUNPACKFUNCS:append = " buildhistory_emit_outputsigs" > sstate_installpkgdir[vardepsexclude] += "buildhistory_emit_outputsigs" > SSTATEPOSTUNPACKFUNCS[vardepvalueexclude] .= "| buildhistory_emit_outputsigs" > > +SSTATEPOSTFUNCS:append = " buildhistory_emit_filesinpackage" > +SSTATEPOSTFUNCS[vardepvalueexclude] .= "| buildhistory_emit_filesinpackage" > + > # All items excepts those listed here will be removed from a recipe's > # build history directory by buildhistory_emit_pkghistory(). This is > # necessary because some of these items (package directories, files that > @@ -87,6 +90,16 @@ buildhistory_emit_sysroot() { > buildhistory_list_files_no_owners $BASE ${BUILDHISTORY_DIR_PACKAGE}/sysroot > } > > +# > +# Write out files-in-package for this package > +# > +python buildhistory_emit_filesinpackage() { > + 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 > +} > + > # > # Write out metadata about this package for comparison when writing future packages > # > @@ -99,8 +112,6 @@ python buildhistory_emit_pkghistory() { > 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']: > @@ -599,7 +610,8 @@ 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" > diff --git a/meta/lib/oe/packagedata.py b/meta/lib/oe/packagedata.py > index 2d1d6ddeb7..e8c503b43b 100644 > --- a/meta/lib/oe/packagedata.py > +++ b/meta/lib/oe/packagedata.py > @@ -309,7 +309,7 @@ fi > subdata_file = pkgdatadir + "/runtime/%s" % pkg > with open(subdata_file, 'w') as sf: > for var in (d.getVar('PKGDATA_VARS') or "").split(): > - val = write_if_exists(sf, pkg, var) > + write_if_exists(sf, pkg, var) > > write_if_exists(sf, pkg, 'FILERPROVIDESFLIST') > for dfile in sorted((d.getVar('FILERPROVIDESFLIST:' + pkg) or "").split()): > -- > 2.34.1 > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#199555): https://lists.openembedded.org/g/openembedded-core/message/199555 > 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 Alex, So problem starts at sstate.bbclass:406 (sstate_install(ss, d)), this should be running after the next for loop (for plain in ss['plaindirs']:). Having this fixed, jumping to the buildhistory.bbclass:602, find should be out of the loop to give an hard error if it misses. Then inside the for loop, while checking if output folder exists (buildhistory.bbclass:607), this doesnt make sense to me the way it is running: - the comment refers "Make sure the output folder exists...", but we are not creating it if we need, just skipping if doesnt exists. - this folder is being created on do_packagedata (buildhistory.bbclass:396) to write down latest file. So before spending more time on this topic i have a few questions to understand a little bit of what should be the strategy to fix this: - If we want to keep files-in-package.txt being created on do_package, shouldnt `buildhistory_list_pkg_files()` (buildhistory.bbclass:600) be responsible to create the folder on buildhistory side? - If we keep `buildhistory_list_pkg_files` being triggered by do_package is there any case of do_package_setscene trigger this? I couldnt find any case to support this option (buildhistory.bbclass:101). - If not we dont need in fact to create files-in-package.txt on do_package, should files-in-package.txt creation be moved to do_packagedata? Thanks, Pedro
Hello Pedro, is it possible to separate all needed changes into a set of nice, small, simple, easy to review commits? That would go a long way towards fixing all issues. I don't have a hard opinion on the best way to fix this, I just want to ensure there are no mysteries in what is happening, and the fixing isn't in the form of gigantic, invasive, single commit. If you need to move code *and* change what it does, move in one commit, change in another, for example. buildhistory is an old class, has been worked on by many people over the years, and the original author (Paul Eggleton) isn't involved in yocto any longer unfortunately. So the fixes may have been done to resolve 'pressing problems' rather than with long term quality of the code in mind. Also note there are selftests for it: alex@Zen2:/srv/storage/alex/yocto/build-64-alt$ oe-selftest -l|grep -i buildhistory 2024-05-24 10:06:56,818 - oe-selftest - INFO - buildoptions.BuildhistoryTests.test_buildhistory_basic 2024-05-24 10:06:56,818 - oe-selftest - INFO - buildoptions.BuildhistoryTests.test_buildhistory_buildtime_pr_backwards 2024-05-24 10:06:56,818 - oe-selftest - INFO - buildoptions.BuildhistoryTests.test_fileinfo 2024-05-24 10:06:56,830 - oe-selftest - INFO - oelib.buildhistory.TestBlobParsing.test_blob_to_dict 2024-05-24 10:06:56,830 - oe-selftest - INFO - oelib.buildhistory.TestBlobParsing.test_compare_dict_blobs 2024-05-24 10:06:56,830 - oe-selftest - INFO - oelib.buildhistory.TestBlobParsing.test_compare_dict_blobs_default 2024-05-24 10:06:56,831 - oe-selftest - INFO - oelib.buildhistory.TestFileListCompare.test_compare_file_lists 2024-05-24 10:06:56,832 - oe-selftest - INFO - oescripts.BuildhistoryDiffTests.test_buildhistory_diff So if you make sure they continue to pass, that'd be good. If you can add to them, that's better. Alex On Thu, 23 May 2024 at 17:24, pmi183 via lists.openembedded.org <pmi183=gmail.com@lists.openembedded.org> wrote: > > [Edited Message Follows] > > Hi Alex, > > So problem starts at sstate.bbclass:406 (sstate_install(ss, d)), this should be running after the next for loop (for plain in ss['plaindirs']:). > > Having this fixed, jumping to the buildhistory.bbclass:602, find should be out of the loop to give a hard error if it misses. > Then inside the for loop, while checking if output folder exists (buildhistory.bbclass:607), this doesnt make sense to me the way it is running: > - the comment refers "Make sure the output folder exists...", but we are not creating it if we need, just skipping if doesnt exists. > - this folder is being created on do_packagedata (buildhistory.bbclass:396) to write down latest file. > > So before spending more time on this topic i have a few questions to understand a little bit of what should be the strategy to fix this: > > - If we want to keep files-in-package.txt being created on do_package, shouldnt `buildhistory_list_pkg_files()` (buildhistory.bbclass:600) be responsible to create the folder on buildhistory side? > - If we keep `buildhistory_list_pkg_files` being triggered by do_package is there any case of do_package_setscene trigger this? I couldnt find any case to support this option (buildhistory.bbclass:101). > - If not we dont need in fact to create files-in-package.txt on do_package, should files-in-package.txt creation be moved to do_packagedata? > > Thanks, > > Pedro > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#199810): https://lists.openembedded.org/g/openembedded-core/message/199810 > 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 Alex, As requested, i created a set of small patches with the changes and unit tests to support these changes. Also i tested against the unit tests mentioned to be sure that everything keeps running fine. I decided not to copy paste the content of the patches here otherwise the message would get huge since we already discussed before the content. Patch 1 - fixes race condition detected while executing functions registered on `SSTATEPOSTINSTFUNCS` Patch 2 - fixes `find` usage, avoiding hiding errors on the command execution and creates buildhistory output folder if doesnt exist. Patch 3 - fixes usage of 2 features combined, `BUILDHISTORY_PRESERVE` and `BUILDHISTORY_RESET`, restoring files to buildhistory main folder. Patch 4 - adds unit tests to validate files-in-package.txt generation and feature combination from patch 3. I hope this goes towards your vision. Thanks, Pedro
Hi Alex, As requested, i created a set of small patches with the changes and unit tests to support these changes. Also i tested against the unit tests mentioned to be sure that everything keeps running fine. I decided not to copy paste the content of the patches here otherwise the message would get huge since we already discussed before the content. Patch 1 - fixes race condition detected while executing functions registered on `SSTATEPOSTINSTFUNCS` Patch 2 - fixes `find` usage, avoiding hiding errors on the command execution and creates buildhistory output folder if doesnt exist. Patch 3 - fixes usage of 2 features combined, `BUILDHISTORY_PRESERVE` and `BUILDHISTORY_RESET`, restoring files to buildhistory main folder. Patch 4 - adds unit tests to validate files-in-package.txt generation and feature combination from patch 3. I hope this goes towards your vision.
On Wed, 29 May 2024 at 11:01, pmi183 via lists.openembedded.org <pmi183=gmail.com@lists.openembedded.org> wrote: > As requested, i created a set of small patches with the changes and unit tests to support these changes. > Also i tested against the unit tests mentioned to be sure that everything keeps running fine. I decided not to copy paste > the content of the patches here otherwise the message would get huge since we already discussed before the content. Hello, please resend the patches as direct mailing list messages; it's hard to comment on attachments. Alex
Hi Alex, Patch 1 - fixes race condition detected while executing functions registered on `SSTATEPOSTINSTFUNCS` --- meta/classes-global/sstate.bbclass | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/meta/classes-global/sstate.bbclass b/meta/classes-global/sstate.bbclass index 76a7b59636..9887169e4f 100644 --- a/meta/classes-global/sstate.bbclass +++ b/meta/classes-global/sstate.bbclass @@ -403,7 +403,6 @@ def sstate_installpkgdir(ss, d): for state in ss['dirs']: prepdir(state[1]) bb.utils.rename(sstateinst + state[0], state[1]) - sstate_install(ss, d) for plain in ss['plaindirs']: workdir = d.getVar('WORKDIR') @@ -416,6 +415,8 @@ def sstate_installpkgdir(ss, d): prepdir(dest) bb.utils.rename(src, dest) + sstate_install(ss, d) + return True python sstate_hardcode_path_unpack () { -- 2.25.1 Patch 2 - fixes `find` usage, avoiding hiding errors on the command execution and creates buildhistory output folder if doesnt exist. --- 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 fd53e92402..59be4516b2 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.25.1 Patch 3 - fixes usage of 2 features combined, `BUILDHISTORY_PRESERVE` and `BUILDHISTORY_RESET`, restoring files to buildhistory main folder. --- meta/classes/buildhistory.bbclass | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/meta/classes/buildhistory.bbclass b/meta/classes/buildhistory.bbclass index 59be4516b2..c707f71657 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,11 @@ python buildhistory_emit_pkghistory() { if not os.path.exists(pkghistdir): bb.utils.mkdirhier(pkghistdir) else: + # Copy all files marked to preserve + 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.25.1 Patch 4 - adds unit tests to validate files-in-package.txt generation and feature combination from patch 3. --- meta/lib/oeqa/selftest/cases/buildoptions.py | 26 ++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/meta/lib/oeqa/selftest/cases/buildoptions.py b/meta/lib/oeqa/selftest/cases/buildoptions.py index 31dafaa9c5..e05bac7c6c 100644 --- a/meta/lib/oeqa/selftest/cases/buildoptions.py +++ b/meta/lib/oeqa/selftest/cases/buildoptions.py @@ -176,6 +176,32 @@ class BuildhistoryTests(BuildhistoryBase): self.assertEqual(data['FILELIST'], '') self.assertEqual(int(data['PKGSIZE']), 0) + def test_files_in_package_txt_creation(self): + self.config_buildhistory() + bitbake('xcursor-transparent-theme -c cleansstate') + bitbake('xcursor-transparent-theme') + history_dir = get_bb_var('BUILDHISTORY_DIR_PACKAGE', 'xcursor-transparent-theme') + self.assertTrue(os.path.isdir(history_dir), 'buildhistory dir was not created.') + subfolders = [ fobject.path for fobject in os.scandir(history_dir) if fobject.is_dir() ] + self.assertTrue(len(subfolders), "No folders inside package.") + for subpath in subfolders: + self.assertTrue(os.path.isfile(os.path.join(subpath, 'files-in-package.txt'))) + + def test_files_in_package_txt_reset_and_preserved(self): + self.config_buildhistory() + self.append_config("BUILDHISTORY_RESET = '1'") + self.append_config("BUILDHISTORY_PRESERVE:append = ' files-in-package.txt'") + bitbake('xcursor-transparent-theme -c cleansstate') + bitbake('xcursor-transparent-theme') + shutil.rmtree(get_bb_var('TMPDIR')) + bitbake('xcursor-transparent-theme') + history_dir = get_bb_var('BUILDHISTORY_DIR_PACKAGE', 'xcursor-transparent-theme') + self.assertTrue(os.path.isdir(history_dir), 'buildhistory dir was not created.') + subfolders = [ fobject.path for fobject in os.scandir(history_dir) if fobject.is_dir() ] + self.assertTrue(len(subfolders), "No folders inside package.") + for subpath in subfolders: + self.assertTrue(os.path.isfile(os.path.join(subpath, 'files-in-package.txt'))) + class ArchiverTest(OESelftestTestCase): def test_arch_work_dir_and_export_source(self): """ -- 2.25.1
On Wed, 29 May 2024 at 13:57, pmi183 via lists.openembedded.org <pmi183=gmail.com@lists.openembedded.org> wrote: > Patch 1 - fixes race condition detected while executing functions registered on `SSTATEPOSTINSTFUNCS` The patches needs to be sent as separate messages in a series, like every other patch submitted here ('git send-email' does it). > --- > meta/classes-global/sstate.bbclass | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/meta/classes-global/sstate.bbclass b/meta/classes-global/sstate.bbclass > index 76a7b59636..9887169e4f 100644 > --- a/meta/classes-global/sstate.bbclass > +++ b/meta/classes-global/sstate.bbclass > @@ -403,7 +403,6 @@ def sstate_installpkgdir(ss, d): > for state in ss['dirs']: > prepdir(state[1]) > bb.utils.rename(sstateinst + state[0], state[1]) > - sstate_install(ss, d) > > for plain in ss['plaindirs']: > workdir = d.getVar('WORKDIR') > @@ -416,6 +415,8 @@ def sstate_installpkgdir(ss, d): > prepdir(dest) > bb.utils.rename(src, dest) > > + sstate_install(ss, d) > + > return True > > python sstate_hardcode_path_unpack () { sstate.bbclass is a highly sensitive piece in yocto, and any changes need to be strictly justified. Specifically, you need to explain what is racing against each other, demonstrate how to reproduce and observe the problem that the patch is fixing, and (ideally) provide a test case (in oe-selftest) that fails without the change and passes with it. > Patch 2 - fixes `find` usage, avoiding hiding errors on the command execution and creates buildhistory output folder if doesnt exist. > 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 Indentation problems here. Is the folder is supposed to be created elsewhere, and sometimes it is not? Why? > } > -- > 2.25.1 > > Patch 3 - fixes usage of 2 features combined, `BUILDHISTORY_PRESERVE` and `BUILDHISTORY_RESET`, restoring files to buildhistory main folder. Needs a better explanation too. What wasn't previously working? How does the code change address it? Don't be afraid to be verbose and write multiple paragraphs in your commit messages. > Patch 4 - adds unit tests to validate files-in-package.txt generation and feature combination from patch 3. > --- > meta/lib/oeqa/selftest/cases/buildoptions.py | 26 ++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/meta/lib/oeqa/selftest/cases/buildoptions.py b/meta/lib/oeqa/selftest/cases/buildoptions.py > index 31dafaa9c5..e05bac7c6c 100644 > --- a/meta/lib/oeqa/selftest/cases/buildoptions.py > +++ b/meta/lib/oeqa/selftest/cases/buildoptions.py > @@ -176,6 +176,32 @@ class BuildhistoryTests(BuildhistoryBase): > self.assertEqual(data['FILELIST'], '') > self.assertEqual(int(data['PKGSIZE']), 0) > > + def test_files_in_package_txt_creation(self): > + self.config_buildhistory() > + bitbake('xcursor-transparent-theme -c cleansstate') Thanks for taking the trouble to write the tests, it's good to see. Sadly cleansstate is not allowed in selftests because that operation must be performed offline with no other builds running. Is it possible to test without that? Alex
On Wed, 2024-05-29 at 14:17 +0200, Alexander Kanavin via lists.openembedded.org wrote: > On Wed, 29 May 2024 at 13:57, pmi183 via lists.openembedded.org > <pmi183=gmail.com@lists.openembedded.org> wrote: > > > Patch 1 - fixes race condition detected while executing functions registered on `SSTATEPOSTINSTFUNCS` > > The patches needs to be sent as separate messages in a series, like > every other patch submitted here ('git send-email' does it). > > > --- > > meta/classes-global/sstate.bbclass | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/meta/classes-global/sstate.bbclass b/meta/classes-global/sstate.bbclass > > index 76a7b59636..9887169e4f 100644 > > --- a/meta/classes-global/sstate.bbclass > > +++ b/meta/classes-global/sstate.bbclass > > @@ -403,7 +403,6 @@ def sstate_installpkgdir(ss, d): > > for state in ss['dirs']: > > prepdir(state[1]) > > bb.utils.rename(sstateinst + state[0], state[1]) > > - sstate_install(ss, d) > > > > for plain in ss['plaindirs']: > > workdir = d.getVar('WORKDIR') > > @@ -416,6 +415,8 @@ def sstate_installpkgdir(ss, d): > > prepdir(dest) > > bb.utils.rename(src, dest) > > > > + sstate_install(ss, d) > > + > > return True > > > > python sstate_hardcode_path_unpack () { > > sstate.bbclass is a highly sensitive piece in yocto, and any changes > need to be strictly justified. Specifically, you need to explain what > is racing against each other, demonstrate how to reproduce and observe > the problem that the patch is fixing, and (ideally) provide a test > case (in oe-selftest) that fails without the change and passes with > it. I did look at the sstate code and this part of the problem specifically. I think there is an issue here and this probably is the nearly the right fix. This is the only call site for sstate_install and the "for plain in ss['plaindirs']:" block probably needs to move into sstate_install before the SSTATEPOSTINSTFUNCS function calls. I say that as the code is then within the lock operations. It probably doesn't technically need to be but it would be what most users would expect. I'm not sure we've ever guaranteed that SSTATEPOSTINSTFUNCS can see plaindirs but the buildhistory code clearly depends on that. Quite often that would happen to work but I can see "from sstate" codepaths where it wouldn't. I haven't looked into the other patches in much detail but I did notice that patch 2 has shell whitespace problems that one of the later patches fixes. Cheers, Richard
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) }
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(-)