Don't use set() for datastore variables
cover Don't use set() for datastore variables
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(-)
Signature · 1 line
1.7.6
PATCH 1/1 cache|cooker|parse: switch __depends from a set to a dict
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.
--- 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(-)
lib/bb/cache.py +6 -5
@@ -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",
lib/bb/cooker.py +4 -4
@@ -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)
lib/bb/parse/__init__.py +2 -2
@@ -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):
lib/bb/ui/hob.py +4 -4
@@ -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
Signature · 1 line
1.7.6
Quoted 8 lines · 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.
"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
Signature · 3 lines
Joshua Lock
Yocto Project "Johannes factotum"
Intel Open Source Technology CentreQuoted 21 lines · Joshua Lock
Quoted 8 lines · 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.
"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.
Quoted 2 lines
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,