7668 2015-04-27 23:07:23 +0000 Feature request: inject 'd' as well as 'e' into event handlers 2015-09-10 07:21:08 +0000 1 1 7 Build System, Metadata & Runtime BitBake bitbake unspecified x86 Multiple RESOLVED FIXED Medium enhancement 1.9 M1 1 kergoth richard.purdie bluelightning bogdanx.a.voiculescu poky.bs.watcher poky.watcher georgex.l.musat --- --- Yes (doc changes required) oldest_to_newest 50541 0 kergoth 2015-04-27 23:07:23 +0000 e.data.getVar() gets a bit old in a large event handler, and it means a simple handler has to be modified if switching between an event handler (e.g. RecipeParsed) and anonymous python. I think it would make sense to restore the 'd' convention here to align with python elsewhere. It'd just be a convenience, d==e.data, to avoid the common pattern of setting it at the top of the event handler. 51818 1 richard.purdie 2015-06-19 07:48:59 +0000 I had a look at doing this, it turns out to be quite hard/ugly. The issue is that the event handler code is precompiled and takes a single argument, e. In order to get the same behaviour as anonymous python, we'd need to pass d into the locals() of the function but to do that, as far as I can tell we need exec/eval. exec/eval don't work with functions which take parameters. There are tricks you can use to exec/eval functions with parameters but they work by wrapping the function, then we lose the ability to influence locals again. I also tried injecting d into globals() however globals() is module specific and we therefore can't access globals past the first function exec where a different namespace takes over. This left __builtins__ and something like: diff --git a/bitbake/lib/bb/event.py b/bitbake/lib/bb/event.py index f755eb2..80e3796 100644 --- a/bitbake/lib/bb/event.py +++ b/bitbake/lib/bb/event.py @@ -72,6 +72,7 @@ _eventfilter = None def execute_handler(name, handler, event, d): event.data = d + __builtins__['d'] = d try: ret = handler(event) except bb.parse.SkipRecipe: @@ -87,6 +88,7 @@ def execute_handler(name, handler, event, d): logger.error("Execution of event handler '%s' failed (exit code %s)" % (name, exc.code)) finally: del event.data + del __builtins__['d'] def fire_class_handlers(event, d): if isinstance(event, logging.LogRecord): does work. Do we accept that ugliness though? 51857 2 kergoth 2015-06-22 17:44:03 +0000 Is there a reason we don't just add a second argument? I realize it's probably not conceptually ideal, since setting d is just a convenience, but it would work. As far as I know, the only code which compiles the handler functions is bb.event.register(), so it should be trivial to change the signature of the handler functions. Alternatively, we could inject d = e.data as the first line of the functions, but then we'd need a way to adjust the line numbers for tracebacks, so probably not worthwhile. diff --git i/bitbake/lib/bb/event.py w/bitbake/lib/bb/event.py index ba0de11..1399ba5 100644 --- i/bitbake/lib/bb/event.py +++ w/bitbake/lib/bb/event.py @@ -73,7 +73,7 @@ _eventfilter = None def execute_handler(name, handler, event, d): event.data = d try: - ret = handler(event) + ret = handler(event, d) except (bb.parse.SkipRecipe, bb.BBHandledException): raise except Exception: @@ -180,9 +180,9 @@ def register(name, handler, mask=[]): if handler is not None: # handle string containing python code if isinstance(handler, basestring): - tmp = "def %s(e):\n%s" % (name, handler) + tmp = "def %s(e, d):\n%s" % (name, handler) try: - code = compile(tmp, "%s(e)" % name, "exec") + code = compile(tmp, "%s(e, d)" % name, "exec") except SyntaxError: logger.error("Unable to register event handler '%s':\n%s", name, ''.join(traceback.format_exc(limit=0))) 51935 3 richard.purdie 2015-06-25 15:21:49 +0000 http://git.yoctoproject.org/cgit.cgi/poky/commit/?id=df123ddab43a221cc26028e0aff211ac34c1eafc(In reply to comment #2) > Is there a reason we don't just add a second argument? I realize it's > probably not conceptually ideal, since setting d is just a convenience, but > it would work. As far as I know, the only code which compiles the handler > functions is bb.event.register(), so it should be trivial to change the > signature of the handler functions. > > Alternatively, we could inject d = e.data as the first line of the > functions, but then we'd need a way to adjust the line numbers for > tracebacks, so probably not worthwhile. > > diff --git i/bitbake/lib/bb/event.py w/bitbake/lib/bb/event.py > index ba0de11..1399ba5 100644 > --- i/bitbake/lib/bb/event.py > +++ w/bitbake/lib/bb/event.py > @@ -73,7 +73,7 @@ _eventfilter = None > def execute_handler(name, handler, event, d): > event.data = d > try: > - ret = handler(event) > + ret = handler(event, d) > except (bb.parse.SkipRecipe, bb.BBHandledException): > raise > except Exception: > @@ -180,9 +180,9 @@ def register(name, handler, mask=[]): > if handler is not None: > # handle string containing python code > if isinstance(handler, basestring): > - tmp = "def %s(e):\n%s" % (name, handler) > + tmp = "def %s(e, d):\n%s" % (name, handler) > try: > - code = compile(tmp, "%s(e)" % name, "exec") > + code = compile(tmp, "%s(e, d)" % name, "exec") > except SyntaxError: > logger.error("Unable to register event handler '%s':\n%s", > name, > ''.join(traceback.format_exc(limit=0))) The reason I didn't do this is that that under that if block, the register function would appear to take functions as handlers as well as strings. Those functions would then break. At the time I assumed something used that code path, perhaps it doesn't though? 51953 4 richard.purdie 2015-06-26 08:31:59 +0000 http://git.yoctoproject.org/cgit.cgi/poky/commit/?id=df123ddab43a221cc26028e0aff211ac34c1eafc http://git.yoctoproject.org/cgit.cgi/poky/commit/?id=8c3dd2d9eb2c593d5b7cf28d7627e6725ab7dbfe Still considering the change Chris proposed though...