[RFC PATCH 0/2] skipped recipe handling

[RFC PATCH 0/2] skipped recipe handling

[Paul Eggleton]

I noticed bitbake-layers did not handle bbappends for skipped recipes
properly (e.g. in Poky it complained that uclibc_git.bbappend did not
match any recipe). Thus I added a mechanism to bitbake within cooker that
lists skipped recipes and extended bitbake-layers to use it.

This (or something similar) will also be needed for improving the
NoProvider error handling - ideally we should be reporting if the target
being requested has been skipped and why. To this end the SkippedPackage
class includes the reason and DEPENDS/RDEPENDS info (although at the
moment RDEPENDS does not get populated). Some may prefer I remove the 
DEPENDS/RDEPENDS bits until the future patch where they are used, if
so I wouldn't object to removing them.

I also considered adding to skiplist for BBMASKed-out recipes but on
reflection I elected not to do so since I assumed we ought to be
completely ignoring recipes that are masked out in this way.

Patch summary (these are against Poky but they apply cleanly with -p2
against bitbake master):

Paul Eggleton (2):
  bitbake: track skipped packages
  bitbake-layers: handle skipped recipes

 bitbake/bin/bitbake-layers  |   24 +++++++++++++++++++-----
 bitbake/lib/bb/cache.py     |   12 +++++++++---
 bitbake/lib/bb/cooker.py    |   16 ++++++++++++++++
 bitbake/lib/bb/parse/ast.py |   12 ++++++------
 4 files changed, 50 insertions(+), 14 deletions(-)

-- 
1.7.4.1

[RFC PATCH 1/2] bitbake: track skipped packages

[Paul Eggleton]

Add skiplist to cooker that allows access to the list of packages skipped
via SkipPackage (this includes COMPATIBLE_MACHINE, INCOMPATIBLE_LICENSE,
etc.) This can be used to enhance error reporting.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 bitbake/lib/bb/cache.py     |   12 +++++++++---
 bitbake/lib/bb/cooker.py    |   16 ++++++++++++++++
 bitbake/lib/bb/parse/ast.py |   12 ++++++------
 3 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/bitbake/lib/bb/cache.py b/bitbake/lib/bb/cache.py
index d083c51..076262b 100644
--- a/bitbake/lib/bb/cache.py
+++ b/bitbake/lib/bb/cache.py
@@ -59,6 +59,7 @@ recipe_fields = (
     'broken',
     'not_world',
     'skipped',
+    'skipreason',
     'timestamp',
     'packages',
     'packages_dynamic',
@@ -128,11 +129,15 @@ class RecipeInfo(namedtuple('RecipeInfo', recipe_fields)):
 
     @classmethod
     def from_metadata(cls, filename, metadata):
-        if cls.getvar('__SKIPPED', metadata):
+        skipval = cls.getvar('__SKIPPED', metadata)
+        if skipval:
             return cls.make_optional(skipped=True,
+                                     skipreason=skipval,
                                      file_depends=metadata.getVar('__depends', False),
                                      timestamp=bb.parse.cached_mtime(filename),
-                                     variants=cls.listvar('__VARIANTS', metadata) + [''])
+                                     variants=cls.listvar('__VARIANTS', metadata) + [''],
+                                     provides=cls.depvar('PROVIDES', metadata),
+                                     rprovides=cls.depvar('RPROVIDES', metadata))
 
         tasks = metadata.getVar('__BBTASKS', False)
 
@@ -152,6 +157,7 @@ class RecipeInfo(namedtuple('RecipeInfo', recipe_fields)):
             variants         = cls.listvar('__VARIANTS', metadata) + [''],
 
             skipped          = False,
+            skipreason       = None,
             timestamp        = bb.parse.cached_mtime(filename),
             packages         = cls.listvar('PACKAGES', metadata),
             pn               = pn,
@@ -348,7 +354,7 @@ class Cache(object):
         cached, infos = self.load(fn, appends, cfgData)
         for virtualfn, info in infos:
             if info.skipped:
-                logger.debug(1, "Skipping %s", virtualfn)
+                logger.debug(1, "Skipping %s: %s", (virtualfn, info.skipreason) )
                 skipped += 1
             else:
                 self.add_info(virtualfn, info, cacheData, not cached)
diff --git a/bitbake/lib/bb/cooker.py b/bitbake/lib/bb/cooker.py
index 0b52f18..9b12e0d 100644
--- a/bitbake/lib/bb/cooker.py
+++ b/bitbake/lib/bb/cooker.py
@@ -55,6 +55,20 @@ class NothingToBuild(Exception):
 class state:
     initial, parsing, running, shutdown, stop = range(5)
 
+
+class SkippedPackage:
+    def __init__(self, info = None, reason = None):
+        self.skipreason = None
+        self.provides = None
+        self.rprovides = None
+
+        if info:
+            self.skipreason = info.skipreason
+            self.provides = info.provides
+            self.rprovides = info.rprovides
+        elif reason:
+            self.skipreason = reason
+
 #============================================================================#
 # BBCooker
 #============================================================================#
@@ -66,6 +80,7 @@ class BBCooker:
     def __init__(self, configuration, server):
         self.status = None
         self.appendlist = {}
+        self.skiplist = {}
 
         if server:
             self.server = server.BitBakeServer(self)
@@ -1198,6 +1213,7 @@ class CookerParser(object):
         for virtualfn, info in result:
             if info.skipped:
                 self.skipped += 1
+                self.cooker.skiplist[virtualfn] = SkippedPackage(info)
             self.bb_cache.add_info(virtualfn, info, self.cooker.status,
                                         parsed=parsed)
         return True
diff --git a/bitbake/lib/bb/parse/ast.py b/bitbake/lib/bb/parse/ast.py
index 375dc61..18e491c 100644
--- a/bitbake/lib/bb/parse/ast.py
+++ b/bitbake/lib/bb/parse/ast.py
@@ -376,8 +376,8 @@ def multi_finalize(fn, d):
     try:
         if not onlyfinalise or "default" in onlyfinalise:
             finalize(fn, d)
-    except bb.parse.SkipPackage:
-        bb.data.setVar("__SKIPPED", True, d)
+    except bb.parse.SkipPackage as e:
+        bb.data.setVar("__SKIPPED", e.args[0], d)
     datastores = {"": safe_d}
 
     versions = (d.getVar("BBVERSIONS", True) or "").split()
@@ -419,8 +419,8 @@ def multi_finalize(fn, d):
             verfunc(pv, d, safe_d)
             try:
                 finalize(fn, d)
-            except bb.parse.SkipPackage:
-                bb.data.setVar("__SKIPPED", True, d)
+            except bb.parse.SkipPackage as e:
+                bb.data.setVar("__SKIPPED", e.args[0], d)
 
         _create_variants(datastores, versions, verfunc)
 
@@ -439,8 +439,8 @@ def multi_finalize(fn, d):
             try:
                 if not onlyfinalise or variant in onlyfinalise:
                     finalize(fn, variant_d, variant)
-            except bb.parse.SkipPackage:
-                bb.data.setVar("__SKIPPED", True, variant_d)
+            except bb.parse.SkipPackage as e:
+                bb.data.setVar("__SKIPPED", e.args[0], variant_d)
 
     if len(datastores) > 1:
         variants = filter(None, datastores.iterkeys())
-- 
1.7.4.1

[RFC PATCH 2/2] bitbake-layers: handle skipped recipes

[Paul Eggleton]

Report bbappends correctly for skipped recipes instead of reporting the
bbappends as not having any matching recipe.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 bitbake/bin/bitbake-layers |   24 +++++++++++++++++++-----
 1 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/bitbake/bin/bitbake-layers b/bitbake/bin/bitbake-layers
index b6106cd..c58aee3 100755
--- a/bitbake/bin/bitbake-layers
+++ b/bitbake/bin/bitbake-layers
@@ -81,6 +81,8 @@ class Commands(cmd.Cmd):
         for pn in self.cooker_data.pkg_pn:
             self.show_appends_for_pn(pn)
 
+        self.show_appends_for_skipped()
+
         self.show_appends_with_no_recipes()
 
     def show_appends_for_pn(self, pn):
@@ -92,17 +94,27 @@ class Commands(cmd.Cmd):
                                              self.cooker_data.pkg_pn)
         best_filename = os.path.basename(best[3])
 
+        self.show_appends_output(filenames, best_filename)
+
+    def show_appends_for_skipped(self):
+        filenames = [os.path.basename(f)
+                    for f in self.cooker.skiplist.iterkeys()]
+        self.show_appends_output(filenames, None, " (skipped)")
+
+    def show_appends_output(self, filenames, best_filename, name_suffix = ''):
         appended, missing = self.get_appends_for_files(filenames)
         if appended:
             for basename, appends in appended:
-                logger.info('%s:', basename)
+                logger.info('%s%s:', basename, name_suffix)
                 for append in appends:
                     logger.info('  %s', append)
 
-            if best_filename in missing:
-                logger.warn('%s: missing append for preferred version',
-                            best_filename)
-                self.returncode |= 1
+            if best_filename:
+                if best_filename in missing:
+                    logger.warn('%s: missing append for preferred version',
+                                best_filename)
+                    self.returncode |= 1
+
 
     def get_appends_for_files(self, filenames):
         appended, notappended = set(), set()
@@ -122,6 +134,8 @@ class Commands(cmd.Cmd):
     def show_appends_with_no_recipes(self):
         recipes = set(os.path.basename(f)
                       for f in self.cooker_data.pkg_fn.iterkeys())
+        recipes |= set(os.path.basename(f)
+                      for f in self.cooker.skiplist.iterkeys())
         appended_recipes = self.cooker_data.appends.iterkeys()
         appends_without_recipes = [self.cooker_data.appends[recipe]
                                    for recipe in appended_recipes
-- 
1.7.4.1

Re: [RFC PATCH 1/2] bitbake: track skipped packages

[Joshua Lock]

On Wed, 2011-06-01 at 18:26 +0100, Paul Eggleton wrote:
> Add skiplist to cooker that allows access to the list of packages skipped
> via SkipPackage (this includes COMPATIBLE_MACHINE, INCOMPATIBLE_LICENSE,
> etc.) This can be used to enhance error reporting.

Neat. Minor coding style nit below.

> 
> Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
> ---
>  bitbake/lib/bb/cache.py     |   12 +++++++++---
>  bitbake/lib/bb/cooker.py    |   16 ++++++++++++++++
>  bitbake/lib/bb/parse/ast.py |   12 ++++++------
>  3 files changed, 31 insertions(+), 9 deletions(-)
> 
> diff --git a/bitbake/lib/bb/cache.py b/bitbake/lib/bb/cache.py
> index d083c51..076262b 100644
> --- a/bitbake/lib/bb/cache.py
> +++ b/bitbake/lib/bb/cache.py
> @@ -59,6 +59,7 @@ recipe_fields = (
>      'broken',
>      'not_world',
>      'skipped',
> +    'skipreason',
>      'timestamp',
>      'packages',
>      'packages_dynamic',
> @@ -128,11 +129,15 @@ class RecipeInfo(namedtuple('RecipeInfo', recipe_fields)):
>  
>      @classmethod
>      def from_metadata(cls, filename, metadata):
> -        if cls.getvar('__SKIPPED', metadata):
> +        skipval = cls.getvar('__SKIPPED', metadata)
> +        if skipval:
>              return cls.make_optional(skipped=True,
> +                                     skipreason=skipval,
>                                       file_depends=metadata.getVar('__depends', False),
>                                       timestamp=bb.parse.cached_mtime(filename),
> -                                     variants=cls.listvar('__VARIANTS', metadata) + [''])
> +                                     variants=cls.listvar('__VARIANTS', metadata) + [''],
> +                                     provides=cls.depvar('PROVIDES', metadata),
> +                                     rprovides=cls.depvar('RPROVIDES', metadata))
>  
>          tasks = metadata.getVar('__BBTASKS', False)
>  
> @@ -152,6 +157,7 @@ class RecipeInfo(namedtuple('RecipeInfo', recipe_fields)):
>              variants         = cls.listvar('__VARIANTS', metadata) + [''],
>  
>              skipped          = False,
> +            skipreason       = None,
>              timestamp        = bb.parse.cached_mtime(filename),
>              packages         = cls.listvar('PACKAGES', metadata),
>              pn               = pn,
> @@ -348,7 +354,7 @@ class Cache(object):
>          cached, infos = self.load(fn, appends, cfgData)
>          for virtualfn, info in infos:
>              if info.skipped:
> -                logger.debug(1, "Skipping %s", virtualfn)
> +                logger.debug(1, "Skipping %s: %s", (virtualfn, info.skipreason) )
>                  skipped += 1
>              else:
>                  self.add_info(virtualfn, info, cacheData, not cached)
> diff --git a/bitbake/lib/bb/cooker.py b/bitbake/lib/bb/cooker.py
> index 0b52f18..9b12e0d 100644
> --- a/bitbake/lib/bb/cooker.py
> +++ b/bitbake/lib/bb/cooker.py
> @@ -55,6 +55,20 @@ class NothingToBuild(Exception):
>  class state:
>      initial, parsing, running, shutdown, stop = range(5)
>  
> +
> +class SkippedPackage:
> +    def __init__(self, info = None, reason = None):
> +        self.skipreason = None
> +        self.provides = None
> +        self.rprovides = None
> +
> +        if info:
> +            self.skipreason = info.skipreason
> +            self.provides = info.provides
> +            self.rprovides = info.rprovides
> +        elif reason:
> +            self.skipreason = reason
> +
>  #============================================================================#
>  # BBCooker
>  #============================================================================#
> @@ -66,6 +80,7 @@ class BBCooker:
>      def __init__(self, configuration, server):
>          self.status = None
>          self.appendlist = {}
> +        self.skiplist = {}
>  
>          if server:
>              self.server = server.BitBakeServer(self)
> @@ -1198,6 +1213,7 @@ class CookerParser(object):
>          for virtualfn, info in result:
>              if info.skipped:
>                  self.skipped += 1
> +                self.cooker.skiplist[virtualfn] = SkippedPackage(info)
>              self.bb_cache.add_info(virtualfn, info, self.cooker.status,
>                                          parsed=parsed)
>          return True
> diff --git a/bitbake/lib/bb/parse/ast.py b/bitbake/lib/bb/parse/ast.py
> index 375dc61..18e491c 100644
> --- a/bitbake/lib/bb/parse/ast.py
> +++ b/bitbake/lib/bb/parse/ast.py
> @@ -376,8 +376,8 @@ def multi_finalize(fn, d):
>      try:
>          if not onlyfinalise or "default" in onlyfinalise:
>              finalize(fn, d)
> -    except bb.parse.SkipPackage:
> -        bb.data.setVar("__SKIPPED", True, d)
> +    except bb.parse.SkipPackage as e:
> +        bb.data.setVar("__SKIPPED", e.args[0], d)
>      datastores = {"": safe_d}
>  
>      versions = (d.getVar("BBVERSIONS", True) or "").split()
> @@ -419,8 +419,8 @@ def multi_finalize(fn, d):
>              verfunc(pv, d, safe_d)
>              try:
>                  finalize(fn, d)
> -            except bb.parse.SkipPackage:
> -                bb.data.setVar("__SKIPPED", True, d)
> +            except bb.parse.SkipPackage as e:
> +                bb.data.setVar("__SKIPPED", e.args[0], d)

This doesn't seem very Pythonic. I think you should just be able to do:
bb.data.setVar("__SKIPPED", e, d)
or possibly:
bb.data.setVar("__SKIPPED", str(e), d)

I know I'm being picky but that e.args[0] stuck out like a sore thumb.

>  
>          _create_variants(datastores, versions, verfunc)
>  
> @@ -439,8 +439,8 @@ def multi_finalize(fn, d):
>              try:
>                  if not onlyfinalise or variant in onlyfinalise:
>                      finalize(fn, variant_d, variant)
> -            except bb.parse.SkipPackage:
> -                bb.data.setVar("__SKIPPED", True, variant_d)
> +            except bb.parse.SkipPackage as e:
> +                bb.data.setVar("__SKIPPED", e.args[0], variant_d)

Same as above.

>  
>      if len(datastores) > 1:
>          variants = filter(None, datastores.iterkeys())

-- 
Joshua Lock
        Yocto Build System Monkey
        Intel Open Source Technology Centre

Re: [RFC PATCH 2/2] bitbake-layers: handle skipped recipes

[Joshua Lock]

On Wed, 2011-06-01 at 18:26 +0100, Paul Eggleton wrote:
> Report bbappends correctly for skipped recipes instead of reporting the
> bbappends as not having any matching recipe.
> 
> Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>

Cool! I know this depends on the 1st patch but

Signed-off-by: Joshua Lock <josh@linux.intel.com>

> ---
>  bitbake/bin/bitbake-layers |   24 +++++++++++++++++++-----
>  1 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/bitbake/bin/bitbake-layers b/bitbake/bin/bitbake-layers
> index b6106cd..c58aee3 100755
> --- a/bitbake/bin/bitbake-layers
> +++ b/bitbake/bin/bitbake-layers
> @@ -81,6 +81,8 @@ class Commands(cmd.Cmd):
>          for pn in self.cooker_data.pkg_pn:
>              self.show_appends_for_pn(pn)
>  
> +        self.show_appends_for_skipped()
> +
>          self.show_appends_with_no_recipes()
>  
>      def show_appends_for_pn(self, pn):
> @@ -92,17 +94,27 @@ class Commands(cmd.Cmd):
>                                               self.cooker_data.pkg_pn)
>          best_filename = os.path.basename(best[3])
>  
> +        self.show_appends_output(filenames, best_filename)
> +
> +    def show_appends_for_skipped(self):
> +        filenames = [os.path.basename(f)
> +                    for f in self.cooker.skiplist.iterkeys()]
> +        self.show_appends_output(filenames, None, " (skipped)")
> +
> +    def show_appends_output(self, filenames, best_filename, name_suffix = ''):
>          appended, missing = self.get_appends_for_files(filenames)
>          if appended:
>              for basename, appends in appended:
> -                logger.info('%s:', basename)
> +                logger.info('%s%s:', basename, name_suffix)
>                  for append in appends:
>                      logger.info('  %s', append)
>  
> -            if best_filename in missing:
> -                logger.warn('%s: missing append for preferred version',
> -                            best_filename)
> -                self.returncode |= 1
> +            if best_filename:
> +                if best_filename in missing:
> +                    logger.warn('%s: missing append for preferred version',
> +                                best_filename)
> +                    self.returncode |= 1
> +
>  
>      def get_appends_for_files(self, filenames):
>          appended, notappended = set(), set()
> @@ -122,6 +134,8 @@ class Commands(cmd.Cmd):
>      def show_appends_with_no_recipes(self):
>          recipes = set(os.path.basename(f)
>                        for f in self.cooker_data.pkg_fn.iterkeys())
> +        recipes |= set(os.path.basename(f)
> +                      for f in self.cooker.skiplist.iterkeys())
>          appended_recipes = self.cooker_data.appends.iterkeys()
>          appends_without_recipes = [self.cooker_data.appends[recipe]
>                                     for recipe in appended_recipes

-- 
Joshua Lock
        Yocto Build System Monkey
        Intel Open Source Technology Centre

Re: [RFC PATCH 1/2] bitbake: track skipped packages

[Paul Eggleton]

On Thursday 02 June 2011 21:01:52 Joshua Lock wrote:
> This doesn't seem very Pythonic. I think you should just be able to do:
> bb.data.setVar("__SKIPPED", e, d)
> or possibly:
> bb.data.setVar("__SKIPPED", str(e), d)

Why is accessing args[0] un-Pythonic? As I understand it, args is provided 
(and documented) as the way to get access to the arguments to the exception, 
and the first argument is being used by us for the reason in the case of 
SkipPackage. At the moment e or str(e) will accomplish the same thing, but 
were we to add another argument then it seems to me that the results would be 
different.

Now I'm not a Python expert by any stretch of the imagination but I'd like to 
understand the reason why this usage might be considered undesirable.

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: [RFC PATCH 1/2] bitbake: track skipped packages

[Joshua Lock]

On Fri, 2011-06-03 at 10:51 +0100, Paul Eggleton wrote:
> On Thursday 02 June 2011 21:01:52 Joshua Lock wrote:
> > This doesn't seem very Pythonic. I think you should just be able to do:
> > bb.data.setVar("__SKIPPED", e, d)
> > or possibly:
> > bb.data.setVar("__SKIPPED", str(e), d)
> 
> Why is accessing args[0] un-Pythonic? As I understand it, args is provided 
> (and documented) as the way to get access to the arguments to the exception, 
> and the first argument is being used by us for the reason in the case of 
> SkipPackage. At the moment e or str(e) will accomplish the same thing, but 
> were we to add another argument then it seems to me that the results would be 
> different.
> 
> Now I'm not a Python expert by any stretch of the imagination but I'd like to 
> understand the reason why this usage might be considered undesirable.
> 

Caveat: also not a Python expert!

I raised the comment as it's a pattern I've not seen often in Python
code I've interacted with. Therefore it stood out/didn't look quite
right.

You are correct that if another argument were added the results would be
different: "If str() or unicode() is called on an instance of this
class, the representation of the argument(s) to the instance are
returned, or the empty string when there were no arguments."
http://docs.python.org/library/exceptions.htm

Happy to retract my nit picking,
Joshua
-- 
Joshua Lock
        Yocto Build System Monkey
        Intel Open Source Technology Centre