| Message ID | 20240621103414.2842509-2-alex.kanavin@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [01/11] devtool/upgrade: rename RECIPE_UPDATE_EXTRA_TASKS -> RECIPE_UPGRADE_EXTRA_TASKS | expand |
On Fri, 2024-06-21 at 12:34 +0200, Alexander Kanavin via lists.openembedded.org wrote: > From: Alexander Kanavin <alex@linutronix.de> > > This is modeled on RECIPE_UPGRADE_EXTRA_TASKS with the difference > being that the tasks run in 'devtool finish', and only when > finishing an upgrade operation. This allows recipes to define > custom operations, such as renaming additional supplementary > recipes (such as *-initial, *-native etc), or any other custom > handling that devtool cannot know about. > > The idea is to teach glib, cmake, mesa, systemd, libva, rt-tests, > go, rust, etc etc about upgrading themselves properly with this, > improving success rates in AUH. Whilst I understand the desire, I'm not sure this lives up to way devtool works. By that I mean that devtool upgrade usually does testing of the result but with what you implement here, the second recipe is never actually tested so things could easily break after devtool finish and you wouldn't know without further testing. Thinking about this a bit more, we should be able to ask bitbake which other recipes "share" files since I think the list of included files is actually cached by bitbake. Yes, we may need API but in theory this means devtool can know which other recipes it needs to upgrade at the same time, and when doing so, run them through the same process as normal upgrades. So yes, it will be a bit more work but the end result should be more generic and scalable? Cheers, Richard
On Mon, 24 Jun 2024 at 22:33, Richard Purdie <richard.purdie@linuxfoundation.org> wrote: > Whilst I understand the desire, I'm not sure this lives up to way > devtool works. > > By that I mean that devtool upgrade usually does testing of the result > but with what you implement here, the second recipe is never actually > tested so things could easily break after devtool finish and you > wouldn't know without further testing. Yes. Supplementary .bb renaming only works if those additional recipes do not add licensing statements, or custom patches (and AUH also won't test-build them separately). It was something of a low hanging fruit I wanted to try, as for things like mesa, qemu, cmake and others in the patchset I sent this does turn reliably failing AUH updates into working ones (as long as only the renaming is needed). > Thinking about this a bit more, we should be able to ask bitbake which > other recipes "share" files since I think the list of included files is > actually cached by bitbake. Yes, we may need API but in theory this > means devtool can know which other recipes it needs to upgrade at the > same time, and when doing so, run them through the same process as > normal upgrades. > > So yes, it will be a bit more work but the end result should be more > generic and scalable? I would first try to simply list the additional recipes directly in the main one: RECIPE_UPGRADE_ADDITIONAL = "qemu-native qemu-system-native" and see if devtool can be easily made to iterate over them: devtool upgrade qemu - internally performs upgrade qemu-native qemu-system-native devtool finish qemu - does the same for finish operation. And since these share includes and patches and other items in SRC_URI, we need to ensure they don't step on each other. Although there will be multiple overwriting in 'devtool finish' which is okay as long as one updated thing is overwritten with the same updated thing. Alex
On Tue, 2024-06-25 at 11:19 +0200, Alexander Kanavin wrote: > On Mon, 24 Jun 2024 at 22:33, Richard Purdie > <richard.purdie@linuxfoundation.org> wrote: > > Whilst I understand the desire, I'm not sure this lives up to way > > devtool works. > > > > By that I mean that devtool upgrade usually does testing of the result > > but with what you implement here, the second recipe is never actually > > tested so things could easily break after devtool finish and you > > wouldn't know without further testing. > > Yes. Supplementary .bb renaming only works if those additional recipes > do not add licensing statements, or custom patches (and AUH also won't > test-build them separately). It was something of a low hanging fruit I > wanted to try, as for things like mesa, qemu, cmake and others in the > patchset I sent this does turn reliably failing AUH updates into > working ones (as long as only the renaming is needed). > > > Thinking about this a bit more, we should be able to ask bitbake which > > other recipes "share" files since I think the list of included files is > > actually cached by bitbake. Yes, we may need API but in theory this > > means devtool can know which other recipes it needs to upgrade at the > > same time, and when doing so, run them through the same process as > > normal upgrades. > > > > So yes, it will be a bit more work but the end result should be more > > generic and scalable? > > I would first try to simply list the additional recipes directly in > the main one: > RECIPE_UPGRADE_ADDITIONAL = "qemu-native qemu-system-native" > and see if devtool can be easily made to iterate over them: > > devtool upgrade qemu > - internally performs upgrade qemu-native qemu-system-native > > devtool finish qemu > - does the same for finish operation. > > And since these share includes and patches and other items in SRC_URI, > we need to ensure they don't step on each other. Although there will > be multiple overwriting in 'devtool finish' which is okay as long as > one updated thing is overwritten with the same updated thing. I don't think we even need to list the recipes like this. When devtool upgrade runs, we get a list of files which change. We can then query bitbake with that list of changed files and ask "which recipes also depend on these?". Based upon that list we can then know what else might need to change (e.g. if any of those recipe names have the same PV in) and also which recipe targets also need to be tested as part of the upgrade. Cheers, Richard
On Tue, 25 Jun 2024 at 13:06, Richard Purdie <richard.purdie@linuxfoundation.org> wrote: > I don't think we even need to list the recipes like this. When devtool > upgrade runs, we get a list of files which change. We can then query > bitbake with that list of changed files and ask "which recipes also > depend on these?". Right. Can you give me a lead on how this can be obtained? (e.g. where is the data structure it's in, or a function that returns it) Alex
On Tue, 2024-06-25 at 13:34 +0200, Alexander Kanavin wrote: > On Tue, 25 Jun 2024 at 13:06, Richard Purdie > <richard.purdie@linuxfoundation.org> wrote: > > > I don't think we even need to list the recipes like this. When > > devtool > > upgrade runs, we get a list of files which change. We can then > > query > > bitbake with that list of changed files and ask "which recipes also > > depend on these?". > > Right. Can you give me a lead on how this can be obtained? (e.g. > where > is the data structure it's in, or a function that returns it) In tinfoil you can see the code in TinfoilCookerAdapter/TinfoilRecipeCacheAdapter i.e.: self.recipecaches[mc].inherits i.e. the result of: attrvalue = self.tinfoil.run_command('getRecipeInherits', self.mc) or {} I appreciate it calls this "inherits" but this is actually a list of any file included by the base recipe so any bbclass, .bb, or .inc file that was included should be listed here. The True/False associated with it is whether the file existed or not so that if it changed, we know to reparse. Cheers, Richard
On Tue, 25 Jun 2024 at 13:54, Richard Purdie <richard.purdie@linuxfoundation.org> wrote: > I appreciate it calls this "inherits" but this is actually a list of > any file included by the base recipe so any bbclass, .bb, or .inc file > that was included should be listed here. The True/False associated with > it is whether the file existed or not so that if it changed, we know to > reparse. This doesn't seem to be correct. I added + logger.info(tinfoil.cooker_data.inherits['/srv/work/alex/poky/meta/recipes-devtools/cmake/cmake_3.29.3.bb']) + logger.info(tinfoil.cooker_data.inherits['/srv/work/alex/poky/meta/recipes-devtools/cmake/cmake-native_3.29.3.bb']) to devtool to see how the data in that attribute looks like, and unfortunately it contains only .bbclass filenames and not .inc or any other dependent files: INFO: ['/srv/work/alex/poky/meta/classes-global/base.bbclass', '/srv/work/alex/poky/meta/classes-global/patch.bbclass', '/srv/work/alex/poky/meta/classes/terminal.bbclass', '/srv/work/alex/poky/meta/classes-global/staging.bbclass', '/srv/work/alex/poky/meta/classes-global/mirrors.bbclass', '/srv/work/alex/poky/meta/classes-global/utils.bbclass', '/srv/work/alex/poky/meta/classes-global/utility-tasks.bbclass', ... etc. - no cmake.inc mentioned anywhere. So assuming I didn't miss something, this either needs further bitbake/tinfoil extensions (even more work), or we can just write manual hints for lockstep upgrades into recipes (there's not that many of them so I'm leaning that way tbh). Alex
On Wed, 2024-07-03 at 14:28 +0200, Alexander Kanavin wrote: > On Tue, 25 Jun 2024 at 13:54, Richard Purdie > <richard.purdie@linuxfoundation.org> wrote: > > > I appreciate it calls this "inherits" but this is actually a list > > of > > any file included by the base recipe so any bbclass, .bb, or .inc > > file > > that was included should be listed here. The True/False associated > > with > > it is whether the file existed or not so that if it changed, we > > know to > > reparse. > > This doesn't seem to be correct. I added > > + > logger.info(tinfoil.cooker_data.inherits['/srv/work/alex/poky/meta/re > cipes-devtools/cmake/cmake_3.29.3.bb']) > + > logger.info(tinfoil.cooker_data.inherits['/srv/work/alex/poky/meta/re > cipes-devtools/cmake/cmake-native_3.29.3.bb']) > > to devtool to see how the data in that attribute looks like, and > unfortunately it contains only .bbclass filenames and not .inc or any > other dependent files: > > INFO: ['/srv/work/alex/poky/meta/classes-global/base.bbclass', > '/srv/work/alex/poky/meta/classes-global/patch.bbclass', > '/srv/work/alex/poky/meta/classes/terminal.bbclass', > '/srv/work/alex/poky/meta/classes-global/staging.bbclass', > '/srv/work/alex/poky/meta/classes-global/mirrors.bbclass', > '/srv/work/alex/poky/meta/classes-global/utils.bbclass', > '/srv/work/alex/poky/meta/classes-global/utility-tasks.bbclass', ... > > etc. - no cmake.inc mentioned anywhere. > > So assuming I didn't miss something, this either needs further > bitbake/tinfoil extensions (even more work), or we can just write > manual hints for lockstep upgrades into recipes (there's not that > many > of them so I'm leaning that way tbh). Sorry, I've got confused. Look at tinfoil.cooker_data.file_depends Cheers, Richard
On Wed, 3 Jul 2024 at 15:31, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> Sorry, I've got confused. Look at tinfoil.cooker_data.file_depends
AttributeError: TinfoilRecipeCacheAdapter instance has no attribute
'file_depends'
I had looked into the code, it seems that data is internal to bitbake,
and isn't exposed to tinfoil.
It's also placed into BBINCLUDED per-recipe, but that would require
iterating over "recipes = tinfoil.all_recipe_files(variants=False)" in
every upgrade operation, which might slow things down a bit too much.
Worth trying though?
Alex
On Thu, 2024-07-04 at 12:52 +0200, Alexander Kanavin wrote: > On Wed, 3 Jul 2024 at 15:31, Richard Purdie > <richard.purdie@linuxfoundation.org> wrote: > > > Sorry, I've got confused. Look at tinfoil.cooker_data.file_depends > > AttributeError: TinfoilRecipeCacheAdapter instance has no attribute > 'file_depends' > > I had looked into the code, it seems that data is internal to > bitbake, and isn't exposed to tinfoil. We might consider exposing it? There data is there and in the cache so in theory we should be able to do something with it, even if we filter the data so we don't expose the internal data structure (only list files that exist via tinfoil). Cheers, Richard
diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py index 1d0fe137887..d22bfe8e4a9 100644 --- a/scripts/lib/devtool/standard.py +++ b/scripts/lib/devtool/standard.py @@ -2088,7 +2088,6 @@ def _get_layer(layername, d): logger.warning("Consider using path instead of base name to specify layer:\n\t\t%s" % '\n\t\t'.join(layer_paths)) return os.path.abspath(layer_paths[0]) - def finish(args, config, basepath, workspace): """Entry point for the devtool 'finish' subcommand""" import bb @@ -2120,6 +2119,7 @@ def finish(args, config, basepath, workspace): destlayerdir = _get_layer(args.destination, tinfoil.config_data) recipefile = rd.getVar('FILE') + upgrade_finish_tasks = (rd.getVar('RECIPE_UPGRADE_FINISH_EXTRA_TASKS') or '').split() recipedir = os.path.dirname(recipefile) origlayerdir = oe.recipeutils.find_layerdir(recipefile) @@ -2265,6 +2265,9 @@ def finish(args, config, basepath, workspace): else: _reset([args.recipename], no_clean=no_clean, remove_work=remove_work, config=config, basepath=basepath, workspace=workspace) + if removing_original: + for t in upgrade_finish_tasks: + exec_build_env_command(config.init_path, basepath, 'bitbake -c {} {}'.format(t, args.recipename)) return 0