11372 2017-04-18 10:31:10 +0000 image.bbclass forces some flags for do_image_X tasks 2017-06-29 07:58:53 +0000 1 1 7 Build System, Metadata & Runtime OE-Core core 2.3 x86 Multiple RESOLVED FIXED Medium+ normal 2.4 M2 1 ola.x.nilsson eduard.bartosh eduard.bartosh meta.mr.watcher meta.watcher ross.burton --- --- Don't know oldest_to_newest 72582 0 ola.x.nilsson 2017-04-18 10:31:10 +0000 The anonymous python function in image.bbclass that creates that do_image_X functions sets a number of flags for each of these functions. Some of those flags can be useful to set or append to from an image_Y.bbclass that is inherited using the IMAGE_CLASSES variable. However, image.bbclass sets all flags using setVarFlag which will overwrite any previous value. The current flags that could be a problem are prefuncs, postfuncs, and subimages. 74137 1 eduard.bartosh 2017-06-14 09:10:41 +0000 Fixed by using appendVarFlag instead of setVarFlag: http://lists.openembedded.org/pipermail/openembedded-core/2017-June/138103.html please, review 74140 2 ola.x.nilsson 2017-06-14 12:16:31 +0000 I'm not sure this is enough. A simple anon python function in image_X.bbclass cannot just appendVarFlag to do_image_X[postfuncs] if it important that the function is executed after the functions added by image.bbclass. This: image_X.bbclass: IMAGE_CMD_X = "make_X_image" python() { d.appendVarFlag('do_image_X', 'postfuncs', ' after_symlinks_function') } MACHINE.conf: IMAGE_CLASSES += "image_X" IMAGE_FSTYPES += "X" after_symlinks_function will be executed before create_symlinks as the anon python in image_X.bbclass will execute first. Is it possible to move the anon function to before the inherit statement? Otherwise I can think of these three solutions, but I'm not really happy with either of them: 1) Add variables IMAGE_X_BEFORE_POSTFUNC amd IMAGE_X_AFTER_POSTFUNC to each varflag respectively. appendVarFlag(task, 'postfuncs', '${IMAGE_X_BEFORE_POSTFUNCS} .... ${IMAGE_X_AFTER_POSTFUNCS}' 2) do not set varflags in image.bbclass. Use a variable like IMAGE_CMD_POSTFUNCS in the generated function instead. 3) Add some kind of hook in the anon python function. 74202 3 eduard.bartosh 2017-06-15 14:04:34 +0000 (In reply to comment #2) > I'm not sure this is enough. > > A simple anon python function in image_X.bbclass cannot just appendVarFlag > to do_image_X[postfuncs] if it important that the function is executed after > the functions added by image.bbclass. > > This: > > image_X.bbclass: > > IMAGE_CMD_X = "make_X_image" > > python() { > d.appendVarFlag('do_image_X', 'postfuncs', ' after_symlinks_function') > } > > after_symlinks_function will be executed before create_symlinks as the anon > python in image_X.bbclass will execute first. > > Is it possible to move the anon function to before the inherit statement? Considering current complexity of image.bbclass code I'm hesitating to move it. It can cause unpredictable effects. I can try if we don't come up with something better. I'm not sure it's a good idea to assume that postfuncs are run in one or another order. What if yo need to run your function before create_symlinks? If you want to enforce order would it be acceptable to use setVarFlag and take care of the order in your postfunc, e.g. python() { d.setVarFlag('do_image_X', 'postfuncs', 'my_postfunc') } python my_postfunc () { create_symlinks() # put your code here } > Otherwise I can think of these three solutions, but I'm not really happy > with either of them: > > 1) Add variables IMAGE_X_BEFORE_POSTFUNC amd IMAGE_X_AFTER_POSTFUNC to each > varflag respectively. > > appendVarFlag(task, 'postfuncs', '${IMAGE_X_BEFORE_POSTFUNCS} .... > ${IMAGE_X_AFTER_POSTFUNCS}' > > 2) do not set varflags in image.bbclass. Use a variable like > IMAGE_CMD_POSTFUNCS in the generated function instead. > > 3) Add some kind of hook in the anon python function. I'm not happy about my proposal as well to be honest. 74222 4 ola.x.nilsson 2017-06-15 15:12:58 +0000 > Considering current complexity of image.bbclass code I'm hesitating to move it. > It can cause unpredictable effects. I can try if we don't come up > with something better. Understandable. > I'm not sure it's a good idea to assume that postfuncs are run in one > or another order. What if yo need to run your function > before create_symlinks? > > If you want to enforce order would it be acceptable to use setVarFlag > and take care of the order in your postfunc, e.g. > > python() { > d.setVarFlag('do_image_X', 'postfuncs', 'my_postfunc') > } > > python my_postfunc () { > create_symlinks() > # put your code here > } This wont work because of the ordering. Since image_X.bbclass will be inherited before the problematic anon in image.bbclass we would end up with "my_postfunc create_symlinks" I've actually done create_symlinks_append() { other_func() } but I felt unclean afterwards :) How about this (without your cleanup): d.setVar('do_image_%s' % t, '\n'.join(cmds)) d.setVarFlag('do_image_%s' % t, 'func', '1') d.setVarFlag('do_image_%s' % t, 'fakeroot', '1') d.setVarFlag('do_image_%s' % t, 'prefuncs', '${IMAGE_' + t.upper() + '_PREFUNCS}') d.appendVar('IMAGE_' + t.upper() + '_PREFUNCS', ' ' + debug + ' set_image_size') d.setVarFlag('do_image_%s' % t, 'postfuncs', '${IMAGE_' + t.upper() + '_POSTFUNCS}') d.appendVar('IMAGE_' + t.upper() + '_POSTFUNCS', create_symlinks') d.setVarFlag('do_image_%s' % t, 'subimages', ' '.join(subimages)) d.appendVarFlag('do_image_%s' % t, 'vardeps', ' '.join(vardeps)) d.appendVarFlag('do_image_%s' % t, 'vardepsexclude', 'DATETIME') If IMAGE_X_PREFUNCS is set by image_X.bbclass the value will be appended to, and image_X.bbclass can use IMAGE_X_PREFUNCS_append if it needs to. We can let this rest for a while and I will try out your patches properly and see how it works. 74233 5 eduard.bartosh 2017-06-15 15:36:56 +0000 > How about this That could work, but I don't like introducing 2 more variables. How about this? @@ -497,12 +497,16 @@ python () { d.setVarFlag(task, 'func', '1') d.setVarFlag(task, 'fakeroot', '1') - d.appendVarFlag(task, 'prefuncs', ' ' + debug + ' set_image_size') - d.appendVarFlag(task, 'postfuncs', ' create_symlinks') + prefuncs = d.getVarFlag(task, 'prefuncs') or '' + d.setVarFlag(task, 'prefuncs', debug + ' set_image_size ' + prefuncs) + postfuncs = d.getVarFlag(task, 'postfuncs') or '' + d.setVarFlag(task, 'postfuncs', 'create_symlinks ' + postfuncs) d.appendVarFlag(task, 'subimages', ' ' + ' '.join(subimages)) d.appendVarFlag(task, 'vardeps', ' ' + ' '.join(vardeps)) d.appendVarFlag(task, 'vardepsexclude', 'DATETIME') I'm using updated code as my patch is in master already. 74243 6 ola.x.nilsson 2017-06-15 16:49:28 +0000 Isn't that the same as prependVarFlag? And then it wont be possible to add anything before the functions added by image.bbclass. How about if prefuncs are added with append and postfuncs with prepend, and then image_X.bbclass can add code with either pre/post-funcs or do_image_x_append or _prepend. The execution order would be image_X prefuncs image prefuncs do_image_X_prepend from image_X.bbclass do_image_X from image.bbclass do_image_X_append from image_X.bbclass image postfuncs image_X postfuncs Then it would be possible to achieve any order, but the default implementation is still simple. Obviously not many image types will use this, so I think you are right in keeping it simple. 74271 7 eduard.bartosh 2017-06-16 09:17:41 +0000 (In reply to comment #6) > Isn't that the same as prependVarFlag? > And then it wont be possible to add anything before the functions added by > image.bbclass. > > How about if prefuncs are added with append and postfuncs with prepend, and > then image_X.bbclass can add code with either pre/post-funcs or > do_image_x_append or _prepend. The execution order would be > > image_X prefuncs > image prefuncs > do_image_X_prepend from image_X.bbclass > do_image_X from image.bbclass > do_image_X_append from image_X.bbclass > image postfuncs > image_X postfuncs > > Then it would be possible to achieve any order, but the default > implementation is still simple. > > Obviously not many image types will use this, so I think you are right in > keeping it simple. Thank you! That's a nice idea! So, this tiny patch is what should do the trick, right? diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass index 9a04b43..be8c9dd 100644 --- a/meta/classes/image.bbclass +++ b/meta/classes/image.bbclass @@ -498,7 +498,7 @@ python () { d.setVarFlag(task, 'fakeroot', '1') d.appendVarFlag(task, 'prefuncs', ' ' + debug + ' set_image_size') - d.appendVarFlag(task, 'postfuncs', ' create_symlinks') + d.prependVarFlag(task, 'postfuncs', ' create_symlinks') d.appendVarFlag(task, 'subimages', ' ' + ' '.join(subimages)) d.appendVarFlag(task, 'vardeps', ' ' + ' '.join(vardeps)) d.appendVarFlag(task, 'vardepsexclude', 'DATETIME') 74272 8 ola.x.nilsson 2017-06-16 10:06:09 +0000 (In reply to comment #7) > (In reply to comment #6) > Thank you! That's a nice idea! > > So, this tiny patch is what should do the trick, right? > > diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass > index 9a04b43..be8c9dd 100644 > --- a/meta/classes/image.bbclass > +++ b/meta/classes/image.bbclass > @@ -498,7 +498,7 @@ python () { > d.setVarFlag(task, 'fakeroot', '1') > > d.appendVarFlag(task, 'prefuncs', ' ' + debug + ' set_image_size') > - d.appendVarFlag(task, 'postfuncs', ' create_symlinks') > + d.prependVarFlag(task, 'postfuncs', ' create_symlinks') > d.appendVarFlag(task, 'subimages', ' ' + ' '.join(subimages)) > d.appendVarFlag(task, 'vardeps', ' ' + ' '.join(vardeps)) > d.appendVarFlag(task, 'vardepsexclude', 'DATETIME') I think so, yes. 74277 9 eduard.bartosh 2017-06-16 12:10:05 +0000 ok, sent for review: http://lists.openembedded.org/pipermail/openembedded-core/2017-June/138339.html 74622 10 eduard.bartosh 2017-06-29 07:58:53 +0000 The fix has been accepted to poky master. Commit id: 6fa6099b493f9fb164bda5f66e678984da83989e