[bitbake][RFC PATCH 0/1] Proposed fix for layer branch mismatch issue

[bitbake][RFC PATCH 0/1] Proposed fix for layer branch mismatch issue

[Paul Eggleton]

RFC - ccing the OE-Core list for review. If we apply this, do people
think we can commit to updating the LAYERVERSION in OE-Core at least on
every release and setting versioned LAYERDEPENDS in other layers so that
users get a sensible error if they use incompatible branches? (This also
provides us with a mechanism to handle these types of issues across
changes in master, as well.)

Thoughts?


Please review the following changes for suitability for inclusion. If you have
any objections or suggestions for improvement, please respond to the patches. If
you agree with the changes, please provide your Acked-by.

The following changes since commit 9b5b1bd7d77e3f5886f6c557d3b750de1f6d6025:

  siggen: Ensure taskdata default functions exist in base class (2015-01-23 14:39:40 +0000)

are available in the git repository at:

  git://git.yoctoproject.org/poky-contrib paule/layerdepends
  http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=paule/layerdepends

Paul Eggleton (1):
  cooker: rework LAYERDEPENDS versioning so that it is actually useful

 lib/bb/cooker.py | 36 +++++++++---------------------------
 lib/bb/utils.py  | 22 ++++++++++++++++++++++
 2 files changed, 31 insertions(+), 27 deletions(-)

-- 
1.9.3

[RFC bitbake][RFC PATCH 1/1] cooker: rework LAYERDEPENDS versioning so that it is actually useful

[Paul Eggleton]

We've had versioned dependency support in LAYERDEPENDS for quite a long
time, but I can say with pretty good certainty that almost nobody has
used it because it was too strict - the specified version had to exactly
match the version in your configuration or you would get an error; there
was no "greater than or equal" option, which is usually what you will
want given that LAYERVERSION does get bumped.

However, users mismatching layer branches and then having their builds
fail later on with some incomprehensible error is still a pretty common
problem. To address this, I have reworked LAYERDEPENDS version
specifications to use the more familiar "dependency (>= version)" syntax
as used with package dependencies, support non-integer versions, and
clarified the error message a little. If we then take care to bump the
version on every breaking change, it is at least possible to have layers
depend on these changes when they update to match; we can now even
support a major.minor scheme to allow retrospectively adding a version
limiter to old branches when a new branch is created and yet still allow
the old branch minor version to be bumped if needed.

Fixes [YOCTO #5991].

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 lib/bb/cooker.py | 36 +++++++++---------------------------
 lib/bb/utils.py  | 22 ++++++++++++++++++++++
 2 files changed, 31 insertions(+), 27 deletions(-)

diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
index f77c6c0..87c6385 100644
--- a/lib/bb/cooker.py
+++ b/lib/bb/cooker.py
@@ -1094,44 +1094,26 @@ class BBCooker:
                     collection_priorities[c] = None
 
                 # Check dependencies and store information for priority calculation
+                dep_re = re.compile('([<>=]*)([0-9]+)')
                 deps = self.data.getVar("LAYERDEPENDS_%s" % c, True)
                 if deps:
-                    depnamelist = []
-                    deplist = deps.split()
-                    for dep in deplist:
-                        depsplit = dep.split(':')
-                        if len(depsplit) > 1:
-                            try:
-                                depver = int(depsplit[1])
-                            except ValueError:
-                                parselog.error("invalid version value in LAYERDEPENDS_%s: \"%s\"", c, dep)
-                                errors = True
-                                continue
-                        else:
-                            depver = None
-                        dep = depsplit[0]
-                        depnamelist.append(dep)
-
+                    deplist = bb.utils.explode_dep_versions2(deps)
+                    for dep, oplist in deplist.iteritems():
                         if dep in collection_list:
-                            if depver:
+                            for opstr in oplist:
                                 layerver = self.data.getVar("LAYERVERSION_%s" % dep, True)
+                                (op, depver) = opstr.split()
                                 if layerver:
-                                    try:
-                                        lver = int(layerver)
-                                    except ValueError:
-                                        parselog.error("invalid value for LAYERVERSION_%s: \"%s\"", c, layerver)
-                                        errors = True
-                                        continue
-                                    if lver != depver:
-                                        parselog.error("Layer '%s' depends on version %d of layer '%s', but version %d is enabled in your configuration", c, depver, dep, lver)
+                                    if not bb.utils.vercmp_string_op(layerver, depver, op):
+                                        parselog.error("Layer '%s' depends on version %s of layer '%s', but version %s is currently enabled in your configuration. Check that you are using the correct matching versions/branches of these two layers.", c, opstr, dep, layerver)
                                         errors = True
                                 else:
-                                    parselog.error("Layer '%s' depends on version %d of layer '%s', which exists in your configuration but does not specify a version", c, depver, dep)
+                                    parselog.error("Layer '%s' depends on version %s of layer '%s', which exists in your configuration but does not specify a version. Check that you are using the correct matching versions/branches of these two layers.", c, opstr, dep)
                                     errors = True
                         else:
                             parselog.error("Layer '%s' depends on layer '%s', but this layer is not enabled in your configuration", c, dep)
                             errors = True
-                    collection_depends[c] = depnamelist
+                    collection_depends[c] = deplist.keys()
                 else:
                     collection_depends[c] = []
 
diff --git a/lib/bb/utils.py b/lib/bb/utils.py
index 90090b2..4ad23c9 100644
--- a/lib/bb/utils.py
+++ b/lib/bb/utils.py
@@ -128,6 +128,28 @@ def vercmp_string(a, b):
     tb = split_version(b)
     return vercmp(ta, tb)
 
+def vercmp_string_op(a, b, op):
+    """
+    Compare two versions and check if the specified comparison operator matches the result of the comparison.
+    This function is fairly liberal about what operators it will accept since there are a variety of styles
+    depending on the context.
+    """
+    res = vercmp_string(a, b)
+    if op in ('=', '=='):
+        return res == 0
+    elif op == '<=':
+        return res <= 0
+    elif op == '>=':
+        return res >= 0
+    elif op in ('>', '>>'):
+        return res > 0
+    elif op in ('<', '<<'):
+        return res < 0
+    elif op == '!=':
+        return res != 0
+    else:
+        raise Exception('vercmp_string_op: unsupported operation %s' % op)
+
 def explode_deps(s):
     """
     Take an RDEPENDS style string of format:
-- 
1.9.3

Re: [RFC bitbake][RFC PATCH 1/1] cooker: rework LAYERDEPENDS versioning so that it is actually useful

[Mark Hatle]

On 1/29/15 7:49 AM, Paul Eggleton wrote:
> We've had versioned dependency support in LAYERDEPENDS for quite a long
> time, but I can say with pretty good certainty that almost nobody has
> used it because it was too strict - the specified version had to exactly
> match the version in your configuration or you would get an error; there
> was no "greater than or equal" option, which is usually what you will
> want given that LAYERVERSION does get bumped.
> 
> However, users mismatching layer branches and then having their builds
> fail later on with some incomprehensible error is still a pretty common
> problem. To address this, I have reworked LAYERDEPENDS version
> specifications to use the more familiar "dependency (>= version)" syntax
> as used with package dependencies, support non-integer versions, and
> clarified the error message a little. If we then take care to bump the
> version on every breaking change, it is at least possible to have layers
> depend on these changes when they update to match; we can now even
> support a major.minor scheme to allow retrospectively adding a version
> limiter to old branches when a new branch is created and yet still allow
> the old branch minor version to be bumped if needed.
> 
> Fixes [YOCTO #5991].
> 
> Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>

I've run into this exact support situation as well.  I think we need to do
something like this >, =, <, etc.. so that we can add the necessary keys to
layers to more clearly indicate what is and isn't supported (combination wise.)

I'm all for this!

--Mark

Re: [bitbake-devel] [RFC bitbake][RFC PATCH 1/1] cooker: rework LAYERDEPENDS versioning so that it is actually useful

[Mark Hatle]

On 1/29/15 7:49 AM, Paul Eggleton wrote:
> We've had versioned dependency support in LAYERDEPENDS for quite a long
> time, but I can say with pretty good certainty that almost nobody has
> used it because it was too strict - the specified version had to exactly
> match the version in your configuration or you would get an error; there
> was no "greater than or equal" option, which is usually what you will
> want given that LAYERVERSION does get bumped.
> 
> However, users mismatching layer branches and then having their builds
> fail later on with some incomprehensible error is still a pretty common
> problem. To address this, I have reworked LAYERDEPENDS version
> specifications to use the more familiar "dependency (>= version)" syntax
> as used with package dependencies, support non-integer versions, and
> clarified the error message a little. If we then take care to bump the
> version on every breaking change, it is at least possible to have layers
> depend on these changes when they update to match; we can now even
> support a major.minor scheme to allow retrospectively adding a version
> limiter to old branches when a new branch is created and yet still allow
> the old branch minor version to be bumped if needed.
> 
> Fixes [YOCTO #5991].
> 
> Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>

I've run into this exact support situation as well.  I think we need to do
something like this >, =, <, etc.. so that we can add the necessary keys to
layers to more clearly indicate what is and isn't supported (combination wise.)

I'm all for this!

--Mark