pixbufcache: Use sceneQueueComplete event to simplify usage

RFC PATCH In review Richard Purdie 3 participants 6 messages Aug 2, 2014 – Sep 1, 2014

lore

Share & tools

lore

Richard Purdie

author

2014-08-02 08:54 UTC #0 lore raw

RFC PATCH 1/1 pixbufcache: Use sceneQueueComplete event to simplify usage

[This is an RFC which depends on a patch to bitbake to operate]

Currently, we have a mess of dependencies for pixbufcache and even then it breaks since they might be controlled by PACKAGECONFIG.

Instead, this patch proposes an alternative approach where we allow "fixups" from a sceneQueueComplete() event at the end of the setscene process. We signal the need for these using simply stamp files.

The one downside is that the processing code needs to be in a global event handler like base.bbclass rather than pixbufcache.bbclass but this is probably a price worth paying to avoid the dependency mess?

Signed-off-by Richard Purdie <[email protected]>

meta/classes/base.bbclass +12 -1

index 4972a98..cd4debe 100644
@@ -284,8 +284,10 @@ def buildcfg_neededvars(d):
         bb.fatal('The following variable(s) were not set: %s\nPlease set them directly, or choose a MACHINE or DISTRO that sets them.' % ', '.join(pesteruser))
 
 addhandler base_eventhandler
-base_eventhandler[eventmask] = "bb.event.ConfigParsed bb.event.BuildStarted bb.event.RecipePreFinalise"
+base_eventhandler[eventmask] = "bb.event.ConfigParsed bb.event.BuildStarted bb.event.RecipePreFinalise bb.runqueue.sceneQueueComplete"
 python base_eventhandler() {
+    import bb.runqueue
+
     if isinstance(e, bb.event.ConfigParsed):
         e.data.setVar("NATIVELSBSTRING", lsb_distro_identifier(e.data))
         e.data.setVar('BB_VERSION', bb.__version__)

@@ -324,6 +326,15 @@ python base_eventhandler() {
             e.data.delVar("PREFERRED_PROVIDER_virtual/${TARGET_PREFIX}g++")
             e.data.delVar("PREFERRED_PROVIDER_virtual/${TARGET_PREFIX}compilerlibs")
 
+    if isinstance(e, bb.runqueue.sceneQueueComplete):
+        pbfile = e.data.expand("${STAGING_DIR}/needpixbuf")
+        if os.path.exists(pbfile):
+            bb.build.exec_func("gdkpixbuf_complete", e.data)
+            os.remove(pbfile)
+}
+
+gdkpixbuf_complete() {
+	GDK_PIXBUF_FATAL_LOADER=1 ${STAGING_BINDIR_NATIVE}/gdk-pixbuf-query-loaders --update-cache || exit 1
 }
 
 addtask configure after do_patch

meta/classes/pixbufcache.bbclass +2 ~~-13~~

index b8d75bd..19b4982 100644
@@ -52,21 +52,10 @@ python populate_packages_append() {
 #
 SSTATEPOSTINSTFUNCS_append_class-native = " pixbufcache_sstate_postinst"
 
+# See base.bbclass for the other half of this
 pixbufcache_sstate_postinst() {
 	if [ "${BB_CURRENTTASK}" = "populate_sysroot" -o "${BB_CURRENTTASK}" = "populate_sysroot_setscene" ]
 	then
-		GDK_PIXBUF_FATAL_LOADER=1 gdk-pixbuf-query-loaders --update-cache || exit 1
+		touch ${STAGING_DIR}/needpixbuf
 	fi
 }
-
-# Add all of the dependencies of gdk-pixbuf as dependencies of
-# do_populate_sysroot_setscene so that pixbufcache_sstate_postinst can work
-# (otherwise gdk-pixbuf-query-loaders may not exist or link). Only add
-# gdk-pixbuf-native if we're not building gdk-pixbuf itself.
-#
-# Packages that use this class should extend this variable with their runtime
-# dependencies.
-PIXBUFCACHE_SYSROOT_DEPS = ""
-PIXBUFCACHE_SYSROOT_DEPS_class-native = "${@['gdk-pixbuf-native:do_populate_sysroot_setscene', '']['${BPN}' == 'gdk-pixbuf']} glib-2.0-native:do_populate_sysroot_setscene libffi-native:do_populate_sysroot_setscene libpng-native:do_populate_sysroot_setscene zlib-native:do_populate_sysroot_setscene"
-do_populate_sysroot_setscene[depends] += "${PIXBUFCACHE_SYSROOT_DEPS}"
-do_populate_sysroot[depends] += "${@d.getVar('PIXBUFCACHE_SYSROOT_DEPS', True).replace('_setscene','')}"

Jacob Kroon

2014-08-17 08:33 UTC #1 lore raw

Quoted 9 lines · Richard Purdie

[This is an RFC which depends on a patch to bitbake to operate]
Currently, we have a mess of dependencies for pixbufcache and even then it breaks since they might be controlled by PACKAGECONFIG.
Instead, this patch proposes an alternative approach where we allow "fixups" from a sceneQueueComplete() event at the end of the setscene process. We signal the need for these using simply stamp files.

Quoted 7 lines

The one downside is that the processing code needs to be in a global event handler like base.bbclass rather than pixbufcache.bbclass but this is probably a price worth paying to avoid the dependency mess?
Signed-off-by Richard Purdie <[email protected]>

Instead of having the sstate_postinst() touch "needpixbuf", could we make it write out the actual script that needs to be executed ? Then base.bbclass could iterate over any scripts installed by any sstate_postinst() and execute them. At least this would better isolate the pixbuf-code to pixbufcache.bbclass, and make the snippet in base.bbclass more generic.
/Jacob

Richard Purdie

author

2014-08-17 08:55 UTC #2 lore raw

Quoted 27 lines · Jacob Kroon

On Sat, Aug 2, 2014 at 10:54 AM, Richard Purdie <[email protected]> wrote:
[This is an RFC which depends on a patch to bitbake to operate]
Currently, we have a mess of dependencies for pixbufcache and even then
it breaks since they might be controlled by PACKAGECONFIG.
Instead, this patch proposes an alternative approach where we allow
"fixups" from a sceneQueueComplete() event at the end of the setscene
process. We signal the need for these using simply stamp files.
The one downside is that the processing code needs to be in a global
event handler like base.bbclass rather than pixbufcache.bbclass but this
is probably a price worth paying to avoid the dependency mess?
Signed-off-by: Richard Purdie
<[email protected]>

Quoted 7 lines

Instead of having the sstate_postinst() touch "needpixbuf", could we make it write out the actual script that needs to be executed ?
Then base.bbclass could iterate over any scripts installed by any sstate_postinst() and execute them. At least this would better isolate the pixbuf-code to pixbufcache.bbclass, and make the snippet in base.bbclass more generic.

It does become trickier to avoid races when writing out that script. Touching the file is rather easy from a lock perspective :)

Cheers,

Richard

Jacob Kroon

2014-08-17 11:51 UTC #3 lore raw

Quoted 31 lines · Richard Purdie

Quoted 30 lines · Jacob Kroon
On Sat, Aug 2, 2014 at 10:54 AM, Richard Purdie <[email protected]> wrote:
[This is an RFC which depends on a patch to bitbake to operate]
Currently, we have a mess of dependencies for pixbufcache and even then
it breaks since they might be controlled by PACKAGECONFIG.
Instead, this patch proposes an alternative approach where we allow
"fixups" from a sceneQueueComplete() event at the end of the setscene
process. We signal the need for these using simply stamp files.
The one downside is that the processing code needs to be in a global
event handler like base.bbclass rather than pixbufcache.bbclass but this
is probably a price worth paying to avoid the dependency mess?
Signed-off-by: Richard Purdie
<[email protected]>
Instead of having the sstate_postinst() touch "needpixbuf", could we make it write out the actual script that needs to be executed ?

Quoted 9 lines

Quoted 4 lines
Then base.bbclass could iterate over any scripts installed by any sstate_postinst() and execute them. At least this would better isolate the pixbuf-code to pixbufcache.bbclass, and make the snippet in base.bbclass more generic.
It does become trickier to avoid races when writing out that script. Touching the file is rather easy from a lock perspective :)

Ah.. right, I didn't think of that. Is there no framework already in place in bitbake/OE/Python that we could use for doing atomic write-out-file-if-it-doesn't-exist-yet ?

Jacob Kroon

2014-08-30 14:48 UTC #4 lore raw

In reply to Richard Purdie

Quoted 31 lines · Richard Purdie

Quoted 30 lines · Jacob Kroon
On Sat, Aug 2, 2014 at 10:54 AM, Richard Purdie <[email protected]> wrote:
[This is an RFC which depends on a patch to bitbake to operate]
Currently, we have a mess of dependencies for pixbufcache and even then
it breaks since they might be controlled by PACKAGECONFIG.
Instead, this patch proposes an alternative approach where we allow
"fixups" from a sceneQueueComplete() event at the end of the setscene
process. We signal the need for these using simply stamp files.
The one downside is that the processing code needs to be in a global
event handler like base.bbclass rather than pixbufcache.bbclass but this
is probably a price worth paying to avoid the dependency mess?
Signed-off-by: Richard Purdie
<[email protected]>
Instead of having the sstate_postinst() touch "needpixbuf", could we make it write out the actual script that needs to be executed ?

Quoted 9 lines

Quoted 4 lines
Then base.bbclass could iterate over any scripts installed by any sstate_postinst() and execute them. At least this would better isolate the pixbuf-code to pixbufcache.bbclass, and make the snippet in base.bbclass more generic.
It does become trickier to avoid races when writing out that script. Touching the file is rather easy from a lock perspective :)

How about prefixing the actual written out script with "${PN}-" ? That would prevent races writing the script.
Downside would be that the pixbuf-cache update would be run multiple times.

Burton, Ross

2014-09-01 15:36 UTC #5 lore raw

In reply to Richard Purdie

meta/classes/base.bbclass python base_eventhandler() { · line 333

Quoted 11 lines · Richard Purdie

meta/classes/base.bbclass +5 -0

@@ -324,6 +326,15 @@ python base_eventhandler() {
             e.data.delVar("PREFERRED_PROVIDER_virtual/${TARGET_PREFIX}g++")
             e.data.delVar("PREFERRED_PROVIDER_virtual/${TARGET_PREFIX}compilerlibs")
 
+    if isinstance(e, bb.runqueue.sceneQueueComplete):
+        pbfile = e.data.expand("${STAGING_DIR}/needpixbuf")
+        if os.path.exists(pbfile):
+            bb.build.exec_func("gdkpixbuf_complete", e.data)
+            os.remove(pbfile)

I'm really not keen on base.bbclass having gdk-pixbuf specific knowledge - the problem isn't specific to gdk-pixbuf, it's just obvious there.

Ross