[RFC PATCH 0/1] Don't use set() for datastore variables

[RFC PATCH 0/1] Don't use set() for datastore variables

[Joshua Lock]

This is an RFC patch.

I recently implemented a check within the hob gui to ensure that it's running with the
required pre and post configuration files. However this check uses a variable from the data
store which is a Python set container.

The Python xmlrpclib is unable to marshal sets and therefore the patch I added breaks hob
when using the xmlrpc server backend.

I've assumed that storing variables in the datastore that cannot be accessed using the
getVariable API with each of our servers is a bad thing and the attached patch changes the
use of set to a dict.

There are, of course, alternative workarounds for this issue. For example, we could create a
new variable to list file dependencies, but that seems a little wasteful? Or we could add
some logic to the xmlrpc server to ensure it doesn't try and marshal unsupported types?

Comments?

Regards,
Joshua

The following changes since commit 24272dae15ccf641ece11ef5a6e2bfa3ebb6f5f9:

  lib/bb/siggen.py: return a string from noop get_taskhash (2011-09-05 20:16:08 +0100)

are available in the git repository at:
  git://github.com/incandescant/bitbake hob
  https://github.com/incandescant/bitbake/tree/hob

Joshua Lock (1):
  cache|cooker|parse: switch __depends from a set to a dict

 lib/bb/cache.py          |   11 ++++++-----
 lib/bb/cooker.py         |    8 ++++----
 lib/bb/parse/__init__.py |    4 ++--
 lib/bb/ui/hob.py         |    8 ++++----
 4 files changed, 16 insertions(+), 15 deletions(-)

-- 
1.7.6

[PATCH 1/1] cache|cooker|parse: switch __depends from a set to a dict

[Joshua Lock]

The set container cannot be marshalled by Python's xmlrpclib meaning that
we cannot interact with the __depends variable via the [get|set]Variable
command API.

This patch changes the code to use a dict instead of a set to store the
__depends mapping of filename to mtime.

Signed-off-by: Joshua Lock <josh@linux.intel.com>
---
 lib/bb/cache.py          |   11 ++++++-----
 lib/bb/cooker.py         |    8 ++++----
 lib/bb/parse/__init__.py |    4 ++--
 lib/bb/ui/hob.py         |    8 ++++----
 4 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/lib/bb/cache.py b/lib/bb/cache.py
index d495f9e..ba77f50 100644
--- a/lib/bb/cache.py
+++ b/lib/bb/cache.py
@@ -43,7 +43,7 @@ except ImportError:
     logger.info("Importing cPickle failed. "
                 "Falling back to a very slow implementation.")
 
-__cache_version__ = "142"
+__cache_version__ = "143"
 
 def getCacheFile(path, filename):
     return os.path.join(path, filename)
@@ -282,7 +282,7 @@ class Cache(object):
         newest_mtime = 0
         deps = bb.data.getVar("__base_depends", data)
 
-        old_mtimes = [old_mtime for _, old_mtime in deps]
+        old_mtimes = deps.values()
         old_mtimes.append(newest_mtime)
         newest_mtime = max(old_mtimes)
 
@@ -406,12 +406,12 @@ class Cache(object):
         """Parse the specified filename, returning the recipe information"""
         infos = []
         datastores = cls.load_bbfile(filename, appends, configdata)
-        depends = set()
+        depends = {}
         for variant, data in sorted(datastores.iteritems(),
                                     key=lambda i: i[0],
                                     reverse=True):
             virtualfn = cls.realfn2virtual(filename, variant)
-            depends |= (data.getVar("__depends", False) or set())
+            depends.update(data.getVar("__depends", False) or {})
             if depends and not variant:
                 data.setVar("__depends", depends)
 
@@ -512,8 +512,9 @@ class Cache(object):
         # Check dependencies are still valid
         depends = info_array[0].file_depends
         if depends:
-            for f, old_mtime in depends:
+            for f in depends:
                 fmtime = bb.parse.cached_mtime_noerror(f)
+                old_mtime = depends[f]
                 # Check if file still exists
                 if old_mtime != 0 and fmtime == 0:
                     logger.debug(2, "Cache: %s's dependency %s was removed",
diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
index a0fcc15..e2bbb48 100644
--- a/lib/bb/cooker.py
+++ b/lib/bb/cooker.py
@@ -627,12 +627,12 @@ class BBCooker:
         # Generate a list of parsed configuration files by searching the files
         # listed in the __depends and __base_depends variables with a .conf suffix.
         conffiles = []
-        dep_files = bb.data.getVar('__depends', self.configuration.data) or set()
-        dep_files.union(bb.data.getVar('__base_depends', self.configuration.data) or set())
+        dep_files = bb.data.getVar('__depends', self.configuration.data) or {}
+        dep_files.update(bb.data.getVar('__base_depends', self.configuration.data) or {})
 
         for f in dep_files:
-            if f[0].endswith(".conf"):
-                conffiles.append(f[0])
+            if f.endswith(".conf"):
+                conffiles.append(f)
 
         _, conf, conffile = path.rpartition("conf/")
         match = os.path.join(conf, conffile)
diff --git a/lib/bb/parse/__init__.py b/lib/bb/parse/__init__.py
index eee8d9c..31a35f9 100644
--- a/lib/bb/parse/__init__.py
+++ b/lib/bb/parse/__init__.py
@@ -62,8 +62,8 @@ def update_mtime(f):
 def mark_dependency(d, f):
     if f.startswith('./'):
         f = "%s/%s" % (os.getcwd(), f[2:])
-    deps = bb.data.getVar('__depends', d) or set()
-    deps.update([(f, cached_mtime(f))])
+    deps = bb.data.getVar('__depends', d) or {}
+    deps[f] =  cached_mtime(f)
     bb.data.setVar('__depends', deps, d)
 
 def supports(fn, data):
diff --git a/lib/bb/ui/hob.py b/lib/bb/ui/hob.py
index 84df37d..39731fa 100644
--- a/lib/bb/ui/hob.py
+++ b/lib/bb/ui/hob.py
@@ -994,12 +994,12 @@ def main (server, eventHandler):
     # We hope to adjust this long term as tracked in Yocto Bugzilla #1441
     # http://bugzilla.pokylinux.org/show_bug.cgi?id=1441
     reqfiles = 0
-    dep_files = server.runCommand(["getVariable", "__depends"]) or set()
-    dep_files.union(server.runCommand(["getVariable", "__base_depends"]) or set())
+    dep_files = server.runCommand(["getVariable", "__depends"]) or {}
+    dep_files.update(server.runCommand(["getVariable", "__base_depends"]) or {})
     for f in dep_files:
-        if f[0].endswith("hob-pre.conf"):
+        if f.endswith("hob-pre.conf"):
             reqfiles = reqfiles + 1
-        elif f[0].endswith("hob-post.conf"):
+        elif f.endswith("hob-post.conf"):
             reqfiles = reqfiles + 1
         if reqfiles == 2:
             break
-- 
1.7.6

Re: [RFC PATCH 0/1] Don't use set() for datastore variables

[Joshua Lock]

On Wed, 2011-09-07 at 14:27 -0700, Joshua Lock wrote:
> This is an RFC patch.
> 
> I recently implemented a check within the hob gui to ensure that it's running with the
> required pre and post configuration files. However this check uses a variable from the data
> store which is a Python set container.
> 
> The Python xmlrpclib is unable to marshal sets and therefore the patch I added breaks hob
> when using the xmlrpc server backend.

"Why can't xmlrpclib marshal a set?" - I pretend to hear you ask.
xmlrpclib will only marshal objects with a type which can be serialized
to XML. A set has no direct XML equivalent, so the marshalling fails.

As alluded to in my original mail, we could approach this in alternative
ways. The most obvious being to add an extra step to serialise the set,
or any other passed objects, to some more appropriate form. We already
use the pickle module in various places and this is an obvious contender
here, however I'm not clear on how we would introduce this *just* for
the xmlrpc server.

Would profiling data make this change more palatable? If so what sort of
profiling would people like to see?

Comments appreciated,
Joshua
-- 
Joshua Lock
        Yocto Project "Johannes factotum"
        Intel Open Source Technology Centre

Re: [RFC PATCH 0/1] Don't use set() for datastore variables

[Bernhard Reutner-Fischer]

On Mon, Sep 12, 2011 at 01:42:51PM -0700, Joshua Lock wrote:
>On Wed, 2011-09-07 at 14:27 -0700, Joshua Lock wrote:
>> This is an RFC patch.
>> 
>> I recently implemented a check within the hob gui to ensure that it's running with the
>> required pre and post configuration files. However this check uses a variable from the data
>> store which is a Python set container.
>> 
>> The Python xmlrpclib is unable to marshal sets and therefore the patch I added breaks hob
>> when using the xmlrpc server backend.
>
>"Why can't xmlrpclib marshal a set?" - I pretend to hear you ask.
>xmlrpclib will only marshal objects with a type which can be serialized
>to XML. A set has no direct XML equivalent, so the marshalling fails.
>
>As alluded to in my original mail, we could approach this in alternative
>ways. The most obvious being to add an extra step to serialise the set,
>or any other passed objects, to some more appropriate form. We already
>use the pickle module in various places and this is an obvious contender
>here, however I'm not clear on how we would introduce this *just* for
>the xmlrpc server.
>
>Would profiling data make this change more palatable? If so what sort of
>profiling would people like to see?

Are you talking about f7c69462b8ba726861898817cc5b13174c78e35a ?

If you want to revert it then you may of course do so.
Without knowing that rpc thing, may i suggest you teach it to deal with
sets or, short of that, just handle them as list on the transport layer?

regards,