Patchwork [bitbake-devel] bitbake/utils: Convert vercmp_string() to use vercmp internally

login
register
mail settings
Submitter Richard Purdie
Date May 4, 2012, 2:03 p.m.
Message ID <1336140213.23777.4.camel@ted>
Download mbox | patch
Permalink /patch/27063/
State New
Headers show

Comments

Richard Purdie - May 4, 2012, 2:03 p.m.
Having two different version comparision algorithms in bitbake has never seemed
like a sensible idea. Worryingly, they also return different results to each other.

The vercmp_string API is relatively unused with no users in OE-Core or BitBake
itself for example. This patch converts it to use vercmp internalls, bringing
consitency to the comparisions which is easy now we have other recently added
functions. Yes, this changes behaviour but in this case I'd prefer we were
consistent than having two different comparisions.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
Chris Larson - May 4, 2012, 2:31 p.m.
On Fri, May 4, 2012 at 7:03 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> Having two different version comparision algorithms in bitbake has never seemed
> like a sensible idea. Worryingly, they also return different results to each other.
>
> The vercmp_string API is relatively unused with no users in OE-Core or BitBake
> itself for example. This patch converts it to use vercmp internalls, bringing
> consitency to the comparisions which is easy now we have other recently added
> functions. Yes, this changes behaviour but in this case I'd prefer we were
> consistent than having two different comparisions.
>
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>

This seems sane to me (aside: minor typo: internalls). I recall that
we attempted this once in the past with regular OE and got bitten hard
by the actual users in the metadata, but clearly that's no longer a
concern.

Acked-by: Christopher Larson <chris_larson@mentor.com>
Richard Purdie - May 4, 2012, 2:47 p.m.
On Fri, 2012-05-04 at 07:31 -0700, Chris Larson wrote:
> On Fri, May 4, 2012 at 7:03 AM, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> > Having two different version comparision algorithms in bitbake has never seemed
> > like a sensible idea. Worryingly, they also return different results to each other.
> >
> > The vercmp_string API is relatively unused with no users in OE-Core or BitBake
> > itself for example. This patch converts it to use vercmp internalls, bringing
> > consitency to the comparisions which is easy now we have other recently added
> > functions. Yes, this changes behaviour but in this case I'd prefer we were
> > consistent than having two different comparisions.
> >
> > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> 
> This seems sane to me (aside: minor typo: internalls). I recall that
> we attempted this once in the past with regular OE and got bitten hard
> by the actual users in the metadata, but clearly that's no longer a
> concern.

There are a couple of lib.oe.utils() functions that do call this api but
those themselves don't seem to be used anywhere I could spot.

The way I've changed things is API compatible, it just behaves
consistently with the other version functions and differently to how it
did before in some cases. I think last time we tried this, we tried to
remove it so this approach should be ok...

Cheers,

Richard

Patch

diff --git a/bitbake/lib/bb/utils.py b/bitbake/lib/bb/utils.py
index 1d98bca..7a73419 100644
--- a/bitbake/lib/bb/utils.py
+++ b/bitbake/lib/bb/utils.py
@@ -108,130 +108,10 @@  def vercmp(ta, tb):
         r = vercmp_part(ra, rb)
     return r
 
-_package_weights_ = {"pre":-2, "p":0, "alpha":-4, "beta":-3, "rc":-1}    # dicts are unordered
-_package_ends_ = ["pre", "p", "alpha", "beta", "rc", "cvs", "bk", "HEAD" ]            # so we need ordered list
-
-def relparse(myver):
-    """Parses the last elements of a version number into a triplet, that can
-    later be compared.
-    """
-
-    number = 0
-    p1 = 0
-    p2 = 0
-    mynewver = myver.split('_')
-    if len(mynewver) == 2:
-        # an _package_weights_
-        number = float(mynewver[0])
-        match = 0
-        for x in _package_ends_:
-            elen = len(x)
-            if mynewver[1][:elen] == x:
-                match = 1
-                p1 = _package_weights_[x]
-                try:
-                    p2 = float(mynewver[1][elen:])
-                except:
-                    p2 = 0
-                break
-        if not match:
-            # normal number or number with letter at end
-            divider = len(myver)-1
-            if myver[divider:] not in "1234567890":
-                # letter at end
-                p1 = ord(myver[divider:])
-                number = float(myver[0:divider])
-            else:
-                number = float(myver)
-    else:
-        # normal number or number with letter at end
-        divider = len(myver)-1
-        if myver[divider:] not in "1234567890":
-            #letter at end
-            p1 = ord(myver[divider:])
-            number = float(myver[0:divider])
-        else:
-            number = float(myver)
-    return [number, p1, p2]
-
-__vercmp_cache__ = {}
-
-def vercmp_string(val1, val2):
-    """This takes two version strings and returns an integer to tell you whether
-    the versions are the same, val1>val2 or val2>val1.
-    """
-
-    # quick short-circuit
-    if val1 == val2:
-        return 0
-    valkey = val1 + " " + val2
-
-    # cache lookup
-    try:
-        return __vercmp_cache__[valkey]
-        try:
-            return - __vercmp_cache__[val2 + " " + val1]
-        except KeyError:
-            pass
-    except KeyError:
-        pass
-
-    # consider 1_p2 vc 1.1
-    # after expansion will become (1_p2,0) vc (1,1)
-    # then 1_p2 is compared with 1 before 0 is compared with 1
-    # to solve the bug we need to convert it to (1,0_p2)
-    # by splitting _prepart part and adding it back _after_expansion
-
-    val1_prepart = val2_prepart = ''
-    if val1.count('_'):
-        val1, val1_prepart = val1.split('_', 1)
-    if val2.count('_'):
-        val2, val2_prepart = val2.split('_', 1)
-
-    # replace '-' by '.'
-    # FIXME: Is it needed? can val1/2 contain '-'?
-
-    val1 = val1.split("-")
-    if len(val1) == 2:
-        val1[0] = val1[0] + "." + val1[1]
-    val2 = val2.split("-")
-    if len(val2) == 2:
-        val2[0] = val2[0] + "." + val2[1]
-
-    val1 = val1[0].split('.')
-    val2 = val2[0].split('.')
-
-    # add back decimal point so that .03 does not become "3" !
-    for x in xrange(1, len(val1)):
-        if val1[x][0] == '0' :
-            val1[x] = '.' + val1[x]
-    for x in xrange(1, len(val2)):
-        if val2[x][0] == '0' :
-            val2[x] = '.' + val2[x]
-
-    # extend varion numbers
-    if len(val2) < len(val1):
-        val2.extend(["0"]*(len(val1)-len(val2)))
-    elif len(val1) < len(val2):
-        val1.extend(["0"]*(len(val2)-len(val1)))
-
-    # add back _prepart tails
-    if val1_prepart:
-        val1[-1] += '_' + val1_prepart
-    if val2_prepart:
-        val2[-1] += '_' + val2_prepart
-    # The above code will extend version numbers out so they
-    # have the same number of digits.
-    for x in xrange(0, len(val1)):
-        cmp1 = relparse(val1[x])
-        cmp2 = relparse(val2[x])
-        for y in xrange(0, 3):
-            myret = cmp1[y] - cmp2[y]
-            if myret != 0:
-                __vercmp_cache__[valkey] = myret
-                return myret
-    __vercmp_cache__[valkey] = 0
-    return 0
+def vercmp_string(a, b):
+    ta = split_version(a)
+    tb = split_version(b)
+    return vercmp(ta, tb)
 
 def explode_deps(s):
     """