Patchwork [bitbake-devel] parse/cache/cooker: Preserve order in the file inclusion list

login
register
mail settings
Submitter Richard Purdie
Date Nov. 19, 2012, 3:01 p.m.
Message ID <1353337280.3709.167.camel@ted>
Download mbox | patch
Permalink /patch/39249/
State New
Headers show

Comments

Richard Purdie - Nov. 19, 2012, 3:01 p.m.
The data returned by get_file_depends() may me used in contexts like
checksums where order is important. The current usage of sets means
that some of the checksums can change in circumstances they should not.

This patch changes to use lists, thereby removing the problem.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
Martin Jansa - Nov. 19, 2012, 3:09 p.m.
On Mon, Nov 19, 2012 at 03:01:20PM +0000, Richard Purdie wrote:
> The data returned by get_file_depends() may me used in contexts like
> checksums where order is important. The current usage of sets means
> that some of the checksums can change in circumstances they should not.
> 
> This patch changes to use lists, thereby removing the problem.

Should this cover also BBINCLUDED variable in parsing cache?

Sometimes I see all recipes reparsed without any reason (no change in
metadata).

Comparing output of bitbake -p (with added patch to dump parse data,
shows order of BBINCLUDED changing.

e.g. from yesterday:
OE om-gta02@shr ~/shr-core $ diff 20121118.BBINCLUDED 20121119.BBINCLUDED
12c12
< /OE/shr-core/openembedded-core/meta/conf/machine/include/tune-arm920t.inc
---
> /OE/shr-core/openembedded-core/meta/classes/mirrors.bbclass
45a46
> /OE/shr-core/meta-smartphone/meta-shr/conf/distro/shr.conf
51c52
< /OE/shr-core/openembedded-core/meta/classes/mirrors.bbclass
---
> /OE/shr-core/openembedded-core/meta/conf/machine/include/tune-arm920t.inc
54d54
< /OE/shr-core/meta-smartphone/meta-shr/conf/distro/shr.conf
60d59
< /OE/shr-core/openembedded-core/meta/conf/distro/include/tcmode-default.inc
62a62
> /OE/shr-core/openembedded-core/meta/conf/distro/include/tcmode-default.inc

Cheers,

> 
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
> diff --git a/bitbake/lib/bb/cache.py b/bitbake/lib/bb/cache.py
> index 619b9ee..a3c073a 100644
> --- a/bitbake/lib/bb/cache.py
> +++ b/bitbake/lib/bb/cache.py
> @@ -405,12 +405,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 = depends + (data.getVar("__depends", False) or [])
>              if depends and not variant:
>                  data.setVar("__depends", depends)
>  
> diff --git a/bitbake/lib/bb/cooker.py b/bitbake/lib/bb/cooker.py
> index e299059..98d6957 100644
> --- a/bitbake/lib/bb/cooker.py
> +++ b/bitbake/lib/bb/cooker.py
> @@ -692,8 +692,8 @@ 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 = self.configuration.data.getVar('__depends') or set()
> -        dep_files.union(self.configuration.data.getVar('__base_depends') or set())
> +        dep_files = self.configuration.data.getVar('__base_depends') or []
> +        dep_files = dep_files + (self.configuration.data.getVar('__depends') or [])
>  
>          for f in dep_files:
>              if f[0].endswith(".conf"):
> diff --git a/bitbake/lib/bb/parse/__init__.py b/bitbake/lib/bb/parse/__init__.py
> index 7b9c47e..4293d09c 100644
> --- a/bitbake/lib/bb/parse/__init__.py
> +++ b/bitbake/lib/bb/parse/__init__.py
> @@ -73,8 +73,7 @@ def update_mtime(f):
>  def mark_dependency(d, f):
>      if f.startswith('./'):
>          f = "%s/%s" % (os.getcwd(), f[2:])
> -    deps = d.getVar('__depends') or set()
> -    deps.update([(f, cached_mtime(f))])
> +    deps = (d.getVar('__depends') or []) + [(f, cached_mtime(f))]
>      d.setVar('__depends', deps)
>  
>  def supports(fn, data):
> @@ -134,8 +133,8 @@ def vars_from_file(mypkg, d):
>  def get_file_depends(d):
>      '''Return the dependent files'''
>      dep_files = []
> -    depends = d.getVar('__depends', True) or set()
> -    depends = depends.union(d.getVar('__base_depends', True) or set())
> +    depends = d.getVar('__base_depends', True) or []
> +    depends = depends + (d.getVar('__depends', True) or [])
>      for (fn, _) in depends:
>          dep_files.append(os.path.abspath(fn))
>      return " ".join(dep_files)
> 
> 
> 
> _______________________________________________
> bitbake-devel mailing list
> bitbake-devel@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/bitbake-devel
Richard Purdie - Nov. 19, 2012, 3:20 p.m.
On Mon, 2012-11-19 at 16:09 +0100, Martin Jansa wrote:
> On Mon, Nov 19, 2012 at 03:01:20PM +0000, Richard Purdie wrote:
> > The data returned by get_file_depends() may me used in contexts like
> > checksums where order is important. The current usage of sets means
> > that some of the checksums can change in circumstances they should not.
> > 
> > This patch changes to use lists, thereby removing the problem.
> 
> Should this cover also BBINCLUDED variable in parsing cache?
> 
> Sometimes I see all recipes reparsed without any reason (no change in
> metadata).
> 
> Comparing output of bitbake -p (with added patch to dump parse data,
> shows order of BBINCLUDED changing.

Yes, this is exactly why I'm proposing this patch.

Cheers,

Richard

Patch

diff --git a/bitbake/lib/bb/cache.py b/bitbake/lib/bb/cache.py
index 619b9ee..a3c073a 100644
--- a/bitbake/lib/bb/cache.py
+++ b/bitbake/lib/bb/cache.py
@@ -405,12 +405,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 = depends + (data.getVar("__depends", False) or [])
             if depends and not variant:
                 data.setVar("__depends", depends)
 
diff --git a/bitbake/lib/bb/cooker.py b/bitbake/lib/bb/cooker.py
index e299059..98d6957 100644
--- a/bitbake/lib/bb/cooker.py
+++ b/bitbake/lib/bb/cooker.py
@@ -692,8 +692,8 @@  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 = self.configuration.data.getVar('__depends') or set()
-        dep_files.union(self.configuration.data.getVar('__base_depends') or set())
+        dep_files = self.configuration.data.getVar('__base_depends') or []
+        dep_files = dep_files + (self.configuration.data.getVar('__depends') or [])
 
         for f in dep_files:
             if f[0].endswith(".conf"):
diff --git a/bitbake/lib/bb/parse/__init__.py b/bitbake/lib/bb/parse/__init__.py
index 7b9c47e..4293d09c 100644
--- a/bitbake/lib/bb/parse/__init__.py
+++ b/bitbake/lib/bb/parse/__init__.py
@@ -73,8 +73,7 @@  def update_mtime(f):
 def mark_dependency(d, f):
     if f.startswith('./'):
         f = "%s/%s" % (os.getcwd(), f[2:])
-    deps = d.getVar('__depends') or set()
-    deps.update([(f, cached_mtime(f))])
+    deps = (d.getVar('__depends') or []) + [(f, cached_mtime(f))]
     d.setVar('__depends', deps)
 
 def supports(fn, data):
@@ -134,8 +133,8 @@  def vars_from_file(mypkg, d):
 def get_file_depends(d):
     '''Return the dependent files'''
     dep_files = []
-    depends = d.getVar('__depends', True) or set()
-    depends = depends.union(d.getVar('__base_depends', True) or set())
+    depends = d.getVar('__base_depends', True) or []
+    depends = depends + (d.getVar('__depends', True) or [])
     for (fn, _) in depends:
         dep_files.append(os.path.abspath(fn))
     return " ".join(dep_files)