All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* Re: [opkg-devel] [opkg-utils PATCH] Opkg.py : Support for tilde in version compare
       [not found] <20200319112625.13804-1-maarten.vanmegen@nedap.com>
@ 2020-03-27 20:34 ` Alejandro del Castillo
  2020-03-30 20:31   ` Maarten van Megen
  0 siblings, 1 reply; 3+ messages in thread
From: Alejandro del Castillo @ 2020-03-27 20:34 UTC (permalink / raw
  To: opkg-devel, Maarten, yocto

Hi Marteen,

thanks for working on this, a couple of comments

On 3/19/20 6:26 AM, Maarten wrote:
> Add support for the special tilde character. The current opkg version
> allows specifying a version appended with a tilde e.g.
> 1.2.2~releasecandidate which is lower in version than 1.2.2 itself. With
> this commit, this is now also supported by the opkg.py script.
> 
> Signed-off-by: Maarten <maarten.vanmegen@nedap.com>
> ---
>   opkg.py | 132 ++++++++++++++++++++++++++++----------------------------
>   1 file changed, 66 insertions(+), 66 deletions(-)
> 
> diff --git a/opkg.py b/opkg.py
> index ba947c2..34af483 100644
> --- a/opkg.py
> +++ b/opkg.py
> @@ -1,5 +1,4 @@
> -#!/usr/bin/env python3
> -# SPDX-License-Identifier: GPL-2.0-or-later

did you intentionally modified the shebang & removed the SPDX identifier?

> +#!/usr/bin/env python
>   #   Copyright (C) 2001 Alexander S. Guy <a7r@andern.org>
>   #                      Andern Research Labs
>   #
> @@ -38,58 +37,86 @@ from __future__ import print_function
>   import tempfile
>   import os
>   import sys
> -import glob
>   import hashlib
>   import re
>   import subprocess
>   from stat import ST_SIZE
>   import arfile
>   import tarfile
> -import textwrap
>   import collections
>   
> +
> +def order(x):
> +    if not x:
> +        return 0
> +    if x == "~":
> +        return -1
> +    if str.isdigit(x):
> +        return 0
> +    if str.isalpha(x):
> +        return ord(x)
> +
> +    return 256 + ord(x)
> +
> +
>   class Version(object):
>       """A class for holding parsed package version information."""
> +
>       def __init__(self, epoch, version):
>           self.epoch = epoch
>           self.version = version
>   
>       def _versioncompare(self, selfversion, refversion):
> +        """
> +        Implementation below is a copy of the opkg version comparison algorithm
> +        https://urldefense.com/v3/__http://git.yoctoproject.org/cgit/cgit.cgi/opkg/tree/libopkg/pkg.c*n933__;Iw!!FbZ0ZwI3Qg!5LDeOr__t_Ow1oYBFtvf1sVRXNIPxT_bNylibWik21KDrv8His3FsfSxBbFI88ZZzzTfeg$
> +        it alternates between number and non number comparisons until a difference is found
> +        digits are compared by value. other characters are sorted lexically using the above method orderOfChar
> +
> +        One slight modification, the original version can return any value, whereas this one is limited to -1, 0, +1
> +        """
>           if not selfversion: selfversion = ""
>           if not refversion: refversion = ""
> -        while 1:
> -            ## first look for non-numeric version component
> -            selfm = re.match('([^0-9]*)(.*)', selfversion)
> -            #print(('selfm', selfm.groups()))
> -            (selfalpha, selfversion) = selfm.groups()
> -            refm = re.match('([^0-9]*)(.*)', refversion)
> -            #print(('refm', refm.groups())
> -            (refalpha, refversion) = refm.groups()
> -            if (selfalpha > refalpha):
> -                return 1
> -            elif (selfalpha < refalpha):
> -                return -1
> -            ## now look for numeric version component
> -            (selfnum, selfversion) = re.match('([0-9]*)(.*)', selfversion).groups()
> -            (refnum, refversion) = re.match('([0-9]*)(.*)', refversion).groups()
> -            #print(('selfnum', selfnum, selfversion)
> -            #print(('refnum', refnum, refversion)
> -            if (selfnum != ''):
> -                selfnum = int(selfnum)
> -            else:
> -                selfnum = -1
> -            if (refnum != ''):
> -                refnum = int(refnum)
> -            else:
> -                refnum = -1
> -            if (selfnum > refnum):
> +
> +        value = list(selfversion)
> +        ref = list(refversion)
> +
> +        while value or ref:
> +            first_diff = 0
> +            # alphanumeric comparison
> +            while (value and not str.isdigit(value[0])) or (ref and not str.isdigit(ref[0])):
> +                vc = order(value.pop(0) if value else None)
> +                rc = order(ref.pop(0) if ref else None)
> +                if vc != rc:
> +                    return -1 if vc < rc else 1
> +
> +            # comparing numbers
> +            # start by skipping 0
> +            while value and value[0] == "0":
> +                value.pop(0)
> +            while ref and ref[0] == "0":
> +                ref.pop(0)
> +
> +            # actual number comparison
> +            while value and str.isdigit(value[0]) and ref and str.isdigit(ref[0]):
> +                if not first_diff:
> +                    first_diff = int(value.pop(0)) - int(ref.pop(0))
> +                else:
> +                    value.pop(0)
> +                    ref.pop(0)
> +
> +            # the one that has a value remaining was the highest number
> +            if value and str.isdigit(value[0]):
>                   return 1
> -            elif (selfnum < refnum):
> +            if ref and str.isdigit(ref[0]):
>                   return -1
> -            if selfversion == '' and refversion == '':
> -                return 0
> +            # in case of equal length numbers look at the first diff
> +            if first_diff:
> +                return 1 if first_diff > 0 else -1
> +        return 0
>   
>       def compare(self, ref):
> +
>           if (self.epoch > ref.epoch):
>               return 1
>           elif (self.epoch < ref.epoch):
> @@ -191,9 +218,6 @@ class Package(object):
>           if name == "md5":
>               self._computeFileMD5()
>               return self.md5
> -        elif name == "sha256":
> -            self._computeFileSHA256()
> -            return self.sha256

this seems unrelated to your patch, was it intentional to remove sha256 
support?

>           elif name == 'size':
>               return self._get_file_size()
>           else:
> @@ -213,20 +237,6 @@ class Package(object):
>               f.close()
>               self.md5 = sum.hexdigest()
>   
> -    def _computeFileSHA256(self):
> -        # compute the SHA256.
> -        if not self.fn:
> -            self.sha256 = 'Unknown'
> -        else:
> -            f = open(self.fn, "rb")
> -            sum = hashlib.sha256()
> -            while True:
> -               data = f.read(1024)
> -               if not data: break
> -               sum.update(data)
> -            f.close()
> -            self.sha256 = sum.hexdigest()
> -

same here

>       def _get_file_size(self):
>           if not self.fn:
>               self.size = 0;
> @@ -259,8 +269,6 @@ class Package(object):
>                       self.size = int(value)
>                   elif name_lowercase == 'md5sum':
>                       self.md5 = value
> -                elif name_lowercase == 'sha256sum':
> -                    self.sha256 = value

and here

>                   elif name_lowercase in self.__dict__:
>                       self.__dict__[name_lowercase] = value
>                   elif all_fields:
> @@ -384,7 +392,6 @@ class Package(object):
>                   error = subprocess.CalledProcessError(retcode, cmd)
>                   error.output = output
>                   raise error
> -            output = output.decode("utf-8")
>               return output
>   
>           if not self.fn:
> @@ -408,12 +415,8 @@ class Package(object):
>               return []
>           f = open(self.fn, "rb")
>           ar = arfile.ArFile(f, self.fn)
> -        try:
> -            tarStream = ar.open("data.tar.gz")
> -            tarf = tarfile.open("data.tar.gz", "r", tarStream)
> -        except IOError:
> -            tarStream = ar.open("data.tar.xz")
> -            tarf = tarfile.open("data.tar.xz", "r:xz", tarStream)
> +        tarStream = ar.open("data.tar.gz")
> +        tarf = tarfile.open("data.tar.gz", "r", tarStream)

this also seem to revert a newer commit (add xz support)

>           self.file_list = tarf.getnames()
>           self.file_list = [["./", ""][a.startswith("./")] + a for a in self.file_list]
>   
> @@ -488,7 +491,7 @@ class Package(object):
>               ref.parsed_version = parse_version(ref.version)
>           return self.parsed_version.compare(ref.parsed_version)
>   
> -    def print(self, checksum):
> +    def __str__(self):

this change also looks unrelated

>           out = ""
>   
>           # XXX - Some checks need to be made, and some exceptions
> @@ -505,10 +508,7 @@ class Package(object):
>           if self.section: out = out + "Section: %s\n" % (self.section)
>           if self.architecture: out = out + "Architecture: %s\n" % (self.architecture)
>           if self.maintainer: out = out + "Maintainer: %s\n" % (self.maintainer)
> -        if 'md5' in checksum:
> -            if self.md5: out = out + "MD5Sum: %s\n" % (self.md5)
> -        if 'sha256' in checksum:
> -            if self.sha256: out = out + "SHA256sum: %s\n" % (self.sha256)
> +        if self.md5: out = out + "MD5Sum: %s\n" % (self.md5)
>           if self.size: out = out + "Size: %d\n" % int(self.size)
>           if self.installed_size: out = out + "InstalledSize: %d\n" % int(self.installed_size)
>           if self.filename: out = out + "Filename: %s\n" % (self.filename)
> @@ -585,14 +585,14 @@ class Packages(object):
>       def __getitem__(self, key):
>           return self.packages[key]
>   
> -if __name__ == "__main__":
>   
> +if __name__ == "__main__":
>       assert Version(0, "1.2.2-r1").compare(Version(0, "1.2.3-r0")) == -1
>       assert Version(0, "1.2.2-r0").compare(Version(0, "1.2.2+cvs20070308-r0")) == -1
>       assert Version(0, "1.2.2+cvs20070308").compare(Version(0, "1.2.2-r0")) == 1
>       assert Version(0, "1.2.2-r0").compare(Version(0, "1.2.2-r0")) == 0
>       assert Version(0, "1.2.2-r5").compare(Version(0, "1.2.2-r0")) == 1
> -
> +    assert Version(0, "1.1.2~r1").compare(Version(0, "1.1.2")) == -1
>       package = Package()
>   
>       package.set_package("FooBar")
> 

-- 
Cheers,

Alejandro

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [opkg-devel] [opkg-utils PATCH] Opkg.py : Support for tilde in version compare
  2020-03-27 20:34 ` [opkg-devel] [opkg-utils PATCH] Opkg.py : Support for tilde in version compare Alejandro del Castillo
@ 2020-03-30 20:31   ` Maarten van Megen
  2020-03-31 21:11     ` Alejandro del Castillo
  0 siblings, 1 reply; 3+ messages in thread
From: Maarten van Megen @ 2020-03-30 20:31 UTC (permalink / raw
  To: Alejandro del Castillo, opkg-devel@googlegroups.com,
	yocto@yoctoproject.org

Hi Alejandro

Thanks for the review. Here is the revised version that addresses the issues with the original patch

Regards,
Maarten

---
 opkg.py | 87 ++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 58 insertions(+), 29 deletions(-)
diff --git a/opkg.py b/opkg.py
index ba947c2..f1d1dcb 100644
--- a/opkg.py
+++ b/opkg.py
@@ -48,6 +48,20 @@ import tarfile
 import textwrap
 import collections
 
+
+def order(x):
+    if not x:
+        return 0
+    if x == "~":
+        return -1
+    if str.isdigit(x):
+        return 0
+    if str.isalpha(x):
+        return ord(x)
+
+    return 256 + ord(x)
+
+
 class Version(object):
     """A class for holding parsed package version information."""
     def __init__(self, epoch, version):
@@ -55,39 +69,53 @@ class Version(object):
         self.version = version
 
     def _versioncompare(self, selfversion, refversion):
+        """
+                Implementation below is a copy of the opkg version comparison algorithm
+                http://git.yoctoproject.org/cgit/cgit.cgi/opkg/tree/libopkg/pkg.c#n933
+                it alternates between number and non number comparisons until a difference is found
+                digits are compared by value. other characters are sorted lexically using the above method orderOfChar
+
+                One slight modification, the original version can return any value, whereas this one is limited to -1, 0, +1
+                """
         if not selfversion: selfversion = ""
         if not refversion: refversion = ""
-        while 1:
-            ## first look for non-numeric version component
-            selfm = re.match('([^0-9]*)(.*)', selfversion)
-            #print(('selfm', selfm.groups()))
-            (selfalpha, selfversion) = selfm.groups()
-            refm = re.match('([^0-9]*)(.*)', refversion)
-            #print(('refm', refm.groups())
-            (refalpha, refversion) = refm.groups()
-            if (selfalpha > refalpha):
-                return 1
-            elif (selfalpha < refalpha):
-                return -1
-            ## now look for numeric version component
-            (selfnum, selfversion) = re.match('([0-9]*)(.*)', selfversion).groups()
-            (refnum, refversion) = re.match('([0-9]*)(.*)', refversion).groups()
-            #print(('selfnum', selfnum, selfversion)
-            #print(('refnum', refnum, refversion)
-            if (selfnum != ''):
-                selfnum = int(selfnum)
-            else:
-                selfnum = -1
-            if (refnum != ''):
-                refnum = int(refnum)
-            else:
-                refnum = -1
-            if (selfnum > refnum):
+
+        value = list(selfversion)
+        ref = list(refversion)
+
+        while value or ref:
+            first_diff = 0
+            # alphanumeric comparison
+            while (value and not str.isdigit(value[0])) or (ref and not str.isdigit(ref[0])):
+                vc = order(value.pop(0) if value else None)
+                rc = order(ref.pop(0) if ref else None)
+                if vc != rc:
+                    return -1 if vc < rc else 1
+
+            # comparing numbers
+            # start by skipping 0
+            while value and value[0] == "0":
+                value.pop(0)
+            while ref and ref[0] == "0":
+                ref.pop(0)
+
+            # actual number comparison
+            while value and str.isdigit(value[0]) and ref and str.isdigit(ref[0]):
+                if not first_diff:
+                    first_diff = int(value.pop(0)) - int(ref.pop(0))
+                else:
+                    value.pop(0)
+                    ref.pop(0)
+
+            # the one that has a value remaining was the highest number
+            if value and str.isdigit(value[0]):
                 return 1
-            elif (selfnum < refnum):
+            if ref and str.isdigit(ref[0]):
                 return -1
-            if selfversion == '' and refversion == '':
-                return 0
+            # in case of equal length numbers look at the first diff
+            if first_diff:
+                return 1 if first_diff > 0 else -1
+        return 0
 
     def compare(self, ref):
         if (self.epoch > ref.epoch):
@@ -592,6 +620,7 @@ if __name__ == "__main__":
     assert Version(0, "1.2.2+cvs20070308").compare(Version(0, "1.2.2-r0")) == 1
     assert Version(0, "1.2.2-r0").compare(Version(0, "1.2.2-r0")) == 0
     assert Version(0, "1.2.2-r5").compare(Version(0, "1.2.2-r0")) == 1
+    assert Version(0, "1.1.2~r1").compare(Version(0, "1.1.2")) == -1
 
     package = Package()
 
--

-----Original Message-----
From: Alejandro del Castillo <alejandro.delcastillo@ni.com> 
Sent: Friday, March 27, 2020 9:34 PM
To: opkg-devel@googlegroups.com; Maarten van Megen <maarten.vanmegen@nedap.com>; yocto@yoctoproject.org
Subject: Re: [opkg-devel] [opkg-utils PATCH] Opkg.py : Support for tilde in version compare

Hi Marteen,

thanks for working on this, a couple of comments

On 3/19/20 6:26 AM, Maarten wrote:
> Add support for the special tilde character. The current opkg version 
> allows specifying a version appended with a tilde e.g.
> 1.2.2~releasecandidate which is lower in version than 1.2.2 itself. 
> With this commit, this is now also supported by the opkg.py script.
> 
> Signed-off-by: Maarten <maarten.vanmegen@nedap.com>
> ---
>   opkg.py | 132 ++++++++++++++++++++++++++++----------------------------
>   1 file changed, 66 insertions(+), 66 deletions(-)
> 
> diff --git a/opkg.py b/opkg.py
> index ba947c2..34af483 100644
> --- a/opkg.py
> +++ b/opkg.py
> @@ -1,5 +1,4 @@
> -#!/usr/bin/env python3
> -# SPDX-License-Identifier: GPL-2.0-or-later

did you intentionally modified the shebang & removed the SPDX identifier?

> +#!/usr/bin/env python
>   #   Copyright (C) 2001 Alexander S. Guy <a7r@andern.org>
>   #                      Andern Research Labs
>   #
> @@ -38,58 +37,86 @@ from __future__ import print_function
>   import tempfile
>   import os
>   import sys
> -import glob
>   import hashlib
>   import re
>   import subprocess
>   from stat import ST_SIZE
>   import arfile
>   import tarfile
> -import textwrap
>   import collections
>   
> +
> +def order(x):
> +    if not x:
> +        return 0
> +    if x == "~":
> +        return -1
> +    if str.isdigit(x):
> +        return 0
> +    if str.isalpha(x):
> +        return ord(x)
> +
> +    return 256 + ord(x)
> +
> +
>   class Version(object):
>       """A class for holding parsed package version information."""
> +
>       def __init__(self, epoch, version):
>           self.epoch = epoch
>           self.version = version
>   
>       def _versioncompare(self, selfversion, refversion):
> +        """
> +        Implementation below is a copy of the opkg version comparison algorithm
> +        https://urldefense.com/v3/__http://git.yoctoproject.org/cgit/cgit.cgi/opkg/tree/libopkg/pkg.c*n933__;Iw!!FbZ0ZwI3Qg!5LDeOr__t_Ow1oYBFtvf1sVRXNIPxT_bNylibWik21KDrv8His3FsfSxBbFI88ZZzzTfeg$
> +        it alternates between number and non number comparisons until a difference is found
> +        digits are compared by value. other characters are sorted 
> + lexically using the above method orderOfChar
> +
> +        One slight modification, the original version can return any value, whereas this one is limited to -1, 0, +1
> +        """
>           if not selfversion: selfversion = ""
>           if not refversion: refversion = ""
> -        while 1:
> -            ## first look for non-numeric version component
> -            selfm = re.match('([^0-9]*)(.*)', selfversion)
> -            #print(('selfm', selfm.groups()))
> -            (selfalpha, selfversion) = selfm.groups()
> -            refm = re.match('([^0-9]*)(.*)', refversion)
> -            #print(('refm', refm.groups())
> -            (refalpha, refversion) = refm.groups()
> -            if (selfalpha > refalpha):
> -                return 1
> -            elif (selfalpha < refalpha):
> -                return -1
> -            ## now look for numeric version component
> -            (selfnum, selfversion) = re.match('([0-9]*)(.*)', selfversion).groups()
> -            (refnum, refversion) = re.match('([0-9]*)(.*)', refversion).groups()
> -            #print(('selfnum', selfnum, selfversion)
> -            #print(('refnum', refnum, refversion)
> -            if (selfnum != ''):
> -                selfnum = int(selfnum)
> -            else:
> -                selfnum = -1
> -            if (refnum != ''):
> -                refnum = int(refnum)
> -            else:
> -                refnum = -1
> -            if (selfnum > refnum):
> +
> +        value = list(selfversion)
> +        ref = list(refversion)
> +
> +        while value or ref:
> +            first_diff = 0
> +            # alphanumeric comparison
> +            while (value and not str.isdigit(value[0])) or (ref and not str.isdigit(ref[0])):
> +                vc = order(value.pop(0) if value else None)
> +                rc = order(ref.pop(0) if ref else None)
> +                if vc != rc:
> +                    return -1 if vc < rc else 1
> +
> +            # comparing numbers
> +            # start by skipping 0
> +            while value and value[0] == "0":
> +                value.pop(0)
> +            while ref and ref[0] == "0":
> +                ref.pop(0)
> +
> +            # actual number comparison
> +            while value and str.isdigit(value[0]) and ref and str.isdigit(ref[0]):
> +                if not first_diff:
> +                    first_diff = int(value.pop(0)) - int(ref.pop(0))
> +                else:
> +                    value.pop(0)
> +                    ref.pop(0)
> +
> +            # the one that has a value remaining was the highest number
> +            if value and str.isdigit(value[0]):
>                   return 1
> -            elif (selfnum < refnum):
> +            if ref and str.isdigit(ref[0]):
>                   return -1
> -            if selfversion == '' and refversion == '':
> -                return 0
> +            # in case of equal length numbers look at the first diff
> +            if first_diff:
> +                return 1 if first_diff > 0 else -1
> +        return 0
>   
>       def compare(self, ref):
> +
>           if (self.epoch > ref.epoch):
>               return 1
>           elif (self.epoch < ref.epoch):
> @@ -191,9 +218,6 @@ class Package(object):
>           if name == "md5":
>               self._computeFileMD5()
>               return self.md5
> -        elif name == "sha256":
> -            self._computeFileSHA256()
> -            return self.sha256

this seems unrelated to your patch, was it intentional to remove sha256 support?

>           elif name == 'size':
>               return self._get_file_size()
>           else:
> @@ -213,20 +237,6 @@ class Package(object):
>               f.close()
>               self.md5 = sum.hexdigest()
>   
> -    def _computeFileSHA256(self):
> -        # compute the SHA256.
> -        if not self.fn:
> -            self.sha256 = 'Unknown'
> -        else:
> -            f = open(self.fn, "rb")
> -            sum = hashlib.sha256()
> -            while True:
> -               data = f.read(1024)
> -               if not data: break
> -               sum.update(data)
> -            f.close()
> -            self.sha256 = sum.hexdigest()
> -

same here

>       def _get_file_size(self):
>           if not self.fn:
>               self.size = 0;
> @@ -259,8 +269,6 @@ class Package(object):
>                       self.size = int(value)
>                   elif name_lowercase == 'md5sum':
>                       self.md5 = value
> -                elif name_lowercase == 'sha256sum':
> -                    self.sha256 = value

and here

>                   elif name_lowercase in self.__dict__:
>                       self.__dict__[name_lowercase] = value
>                   elif all_fields:
> @@ -384,7 +392,6 @@ class Package(object):
>                   error = subprocess.CalledProcessError(retcode, cmd)
>                   error.output = output
>                   raise error
> -            output = output.decode("utf-8")
>               return output
>   
>           if not self.fn:
> @@ -408,12 +415,8 @@ class Package(object):
>               return []
>           f = open(self.fn, "rb")
>           ar = arfile.ArFile(f, self.fn)
> -        try:
> -            tarStream = ar.open("data.tar.gz")
> -            tarf = tarfile.open("data.tar.gz", "r", tarStream)
> -        except IOError:
> -            tarStream = ar.open("data.tar.xz")
> -            tarf = tarfile.open("data.tar.xz", "r:xz", tarStream)
> +        tarStream = ar.open("data.tar.gz")
> +        tarf = tarfile.open("data.tar.gz", "r", tarStream)

this also seem to revert a newer commit (add xz support)

>           self.file_list = tarf.getnames()
>           self.file_list = [["./", ""][a.startswith("./")] + a for a 
> in self.file_list]
>   
> @@ -488,7 +491,7 @@ class Package(object):
>               ref.parsed_version = parse_version(ref.version)
>           return self.parsed_version.compare(ref.parsed_version)
>   
> -    def print(self, checksum):
> +    def __str__(self):

this change also looks unrelated

>           out = ""
>   
>           # XXX - Some checks need to be made, and some exceptions @@ 
> -505,10 +508,7 @@ class Package(object):
>           if self.section: out = out + "Section: %s\n" % (self.section)
>           if self.architecture: out = out + "Architecture: %s\n" % (self.architecture)
>           if self.maintainer: out = out + "Maintainer: %s\n" % (self.maintainer)
> -        if 'md5' in checksum:
> -            if self.md5: out = out + "MD5Sum: %s\n" % (self.md5)
> -        if 'sha256' in checksum:
> -            if self.sha256: out = out + "SHA256sum: %s\n" % (self.sha256)
> +        if self.md5: out = out + "MD5Sum: %s\n" % (self.md5)
>           if self.size: out = out + "Size: %d\n" % int(self.size)
>           if self.installed_size: out = out + "InstalledSize: %d\n" % int(self.installed_size)
>           if self.filename: out = out + "Filename: %s\n" % 
> (self.filename) @@ -585,14 +585,14 @@ class Packages(object):
>       def __getitem__(self, key):
>           return self.packages[key]
>   
> -if __name__ == "__main__":
>   
> +if __name__ == "__main__":
>       assert Version(0, "1.2.2-r1").compare(Version(0, "1.2.3-r0")) == -1
>       assert Version(0, "1.2.2-r0").compare(Version(0, "1.2.2+cvs20070308-r0")) == -1
>       assert Version(0, "1.2.2+cvs20070308").compare(Version(0, "1.2.2-r0")) == 1
>       assert Version(0, "1.2.2-r0").compare(Version(0, "1.2.2-r0")) == 0
>       assert Version(0, "1.2.2-r5").compare(Version(0, "1.2.2-r0")) == 
> 1
> -
> +    assert Version(0, "1.1.2~r1").compare(Version(0, "1.1.2")) == -1
>       package = Package()
>   
>       package.set_package("FooBar")
> 

--
Cheers,

Alejandro

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [opkg-devel] [opkg-utils PATCH] Opkg.py : Support for tilde in version compare
  2020-03-30 20:31   ` Maarten van Megen
@ 2020-03-31 21:11     ` Alejandro del Castillo
  0 siblings, 0 replies; 3+ messages in thread
From: Alejandro del Castillo @ 2020-03-31 21:11 UTC (permalink / raw
  To: Maarten van Megen, opkg-devel@googlegroups.com,
	yocto@yoctoproject.org

Looks good, merged.

thanks!

On 3/30/20 3:31 PM, Maarten van Megen wrote:
> Hi Alejandro
> 
> Thanks for the review. Here is the revised version that addresses the issues with the original patch
> 
> Regards,
> Maarten
> 
> ---
>   opkg.py | 87 ++++++++++++++++++++++++++++++++++++++-------------------
>   1 file changed, 58 insertions(+), 29 deletions(-)
> diff --git a/opkg.py b/opkg.py
> index ba947c2..f1d1dcb 100644
> --- a/opkg.py
> +++ b/opkg.py
> @@ -48,6 +48,20 @@ import tarfile
>   import textwrap
>   import collections
>   
> +
> +def order(x):
> +    if not x:
> +        return 0
> +    if x == "~":
> +        return -1
> +    if str.isdigit(x):
> +        return 0
> +    if str.isalpha(x):
> +        return ord(x)
> +
> +    return 256 + ord(x)
> +
> +
>   class Version(object):
>       """A class for holding parsed package version information."""
>       def __init__(self, epoch, version):
> @@ -55,39 +69,53 @@ class Version(object):
>           self.version = version
>   
>       def _versioncompare(self, selfversion, refversion):
> +        """
> +                Implementation below is a copy of the opkg version comparison algorithm
> +                https://urldefense.com/v3/__http://git.yoctoproject.org/cgit/cgit.cgi/opkg/tree/libopkg/pkg.c*n933__;Iw!!FbZ0ZwI3Qg!8GdI9G-uXBcfGN71MZdAi9z1Yu4jriaG8UE8nU1yaOxpX0UJ1MhoOrviZ2da4OSknx3-BQ$
> +                it alternates between number and non number comparisons until a difference is found
> +                digits are compared by value. other characters are sorted lexically using the above method orderOfChar
> +
> +                One slight modification, the original version can return any value, whereas this one is limited to -1, 0, +1
> +                """
>           if not selfversion: selfversion = ""
>           if not refversion: refversion = ""
> -        while 1:
> -            ## first look for non-numeric version component
> -            selfm = re.match('([^0-9]*)(.*)', selfversion)
> -            #print(('selfm', selfm.groups()))
> -            (selfalpha, selfversion) = selfm.groups()
> -            refm = re.match('([^0-9]*)(.*)', refversion)
> -            #print(('refm', refm.groups())
> -            (refalpha, refversion) = refm.groups()
> -            if (selfalpha > refalpha):
> -                return 1
> -            elif (selfalpha < refalpha):
> -                return -1
> -            ## now look for numeric version component
> -            (selfnum, selfversion) = re.match('([0-9]*)(.*)', selfversion).groups()
> -            (refnum, refversion) = re.match('([0-9]*)(.*)', refversion).groups()
> -            #print(('selfnum', selfnum, selfversion)
> -            #print(('refnum', refnum, refversion)
> -            if (selfnum != ''):
> -                selfnum = int(selfnum)
> -            else:
> -                selfnum = -1
> -            if (refnum != ''):
> -                refnum = int(refnum)
> -            else:
> -                refnum = -1
> -            if (selfnum > refnum):
> +
> +        value = list(selfversion)
> +        ref = list(refversion)
> +
> +        while value or ref:
> +            first_diff = 0
> +            # alphanumeric comparison
> +            while (value and not str.isdigit(value[0])) or (ref and not str.isdigit(ref[0])):
> +                vc = order(value.pop(0) if value else None)
> +                rc = order(ref.pop(0) if ref else None)
> +                if vc != rc:
> +                    return -1 if vc < rc else 1
> +
> +            # comparing numbers
> +            # start by skipping 0
> +            while value and value[0] == "0":
> +                value.pop(0)
> +            while ref and ref[0] == "0":
> +                ref.pop(0)
> +
> +            # actual number comparison
> +            while value and str.isdigit(value[0]) and ref and str.isdigit(ref[0]):
> +                if not first_diff:
> +                    first_diff = int(value.pop(0)) - int(ref.pop(0))
> +                else:
> +                    value.pop(0)
> +                    ref.pop(0)
> +
> +            # the one that has a value remaining was the highest number
> +            if value and str.isdigit(value[0]):
>                   return 1
> -            elif (selfnum < refnum):
> +            if ref and str.isdigit(ref[0]):
>                   return -1
> -            if selfversion == '' and refversion == '':
> -                return 0
> +            # in case of equal length numbers look at the first diff
> +            if first_diff:
> +                return 1 if first_diff > 0 else -1
> +        return 0
>   
>       def compare(self, ref):
>           if (self.epoch > ref.epoch):
> @@ -592,6 +620,7 @@ if __name__ == "__main__":
>       assert Version(0, "1.2.2+cvs20070308").compare(Version(0, "1.2.2-r0")) == 1
>       assert Version(0, "1.2.2-r0").compare(Version(0, "1.2.2-r0")) == 0
>       assert Version(0, "1.2.2-r5").compare(Version(0, "1.2.2-r0")) == 1
> +    assert Version(0, "1.1.2~r1").compare(Version(0, "1.1.2")) == -1
>   
>       package = Package()
>   
> --
> 
> -----Original Message-----
> From: Alejandro del Castillo <alejandro.delcastillo@ni.com>
> Sent: Friday, March 27, 2020 9:34 PM
> To: opkg-devel@googlegroups.com; Maarten van Megen <maarten.vanmegen@nedap.com>; yocto@yoctoproject.org
> Subject: Re: [opkg-devel] [opkg-utils PATCH] Opkg.py : Support for tilde in version compare
> 
> Hi Marteen,
> 
> thanks for working on this, a couple of comments
> 
> On 3/19/20 6:26 AM, Maarten wrote:
>> Add support for the special tilde character. The current opkg version
>> allows specifying a version appended with a tilde e.g.
>> 1.2.2~releasecandidate which is lower in version than 1.2.2 itself.
>> With this commit, this is now also supported by the opkg.py script.
>>
>> Signed-off-by: Maarten <maarten.vanmegen@nedap.com>
>> ---
>>    opkg.py | 132 ++++++++++++++++++++++++++++----------------------------
>>    1 file changed, 66 insertions(+), 66 deletions(-)
>>
>> diff --git a/opkg.py b/opkg.py
>> index ba947c2..34af483 100644
>> --- a/opkg.py
>> +++ b/opkg.py
>> @@ -1,5 +1,4 @@
>> -#!/usr/bin/env python3
>> -# SPDX-License-Identifier: GPL-2.0-or-later
> 
> did you intentionally modified the shebang & removed the SPDX identifier?
> 
>> +#!/usr/bin/env python
>>    #   Copyright (C) 2001 Alexander S. Guy <a7r@andern.org>
>>    #                      Andern Research Labs
>>    #
>> @@ -38,58 +37,86 @@ from __future__ import print_function
>>    import tempfile
>>    import os
>>    import sys
>> -import glob
>>    import hashlib
>>    import re
>>    import subprocess
>>    from stat import ST_SIZE
>>    import arfile
>>    import tarfile
>> -import textwrap
>>    import collections
>>    
>> +
>> +def order(x):
>> +    if not x:
>> +        return 0
>> +    if x == "~":
>> +        return -1
>> +    if str.isdigit(x):
>> +        return 0
>> +    if str.isalpha(x):
>> +        return ord(x)
>> +
>> +    return 256 + ord(x)
>> +
>> +
>>    class Version(object):
>>        """A class for holding parsed package version information."""
>> +
>>        def __init__(self, epoch, version):
>>            self.epoch = epoch
>>            self.version = version
>>    
>>        def _versioncompare(self, selfversion, refversion):
>> +        """
>> +        Implementation below is a copy of the opkg version comparison algorithm
>> +        https://urldefense.com/v3/__http://git.yoctoproject.org/cgit/cgit.cgi/opkg/tree/libopkg/pkg.c*n933__;Iw!!FbZ0ZwI3Qg!5LDeOr__t_Ow1oYBFtvf1sVRXNIPxT_bNylibWik21KDrv8His3FsfSxBbFI88ZZzzTfeg$
>> +        it alternates between number and non number comparisons until a difference is found
>> +        digits are compared by value. other characters are sorted
>> + lexically using the above method orderOfChar
>> +
>> +        One slight modification, the original version can return any value, whereas this one is limited to -1, 0, +1
>> +        """
>>            if not selfversion: selfversion = ""
>>            if not refversion: refversion = ""
>> -        while 1:
>> -            ## first look for non-numeric version component
>> -            selfm = re.match('([^0-9]*)(.*)', selfversion)
>> -            #print(('selfm', selfm.groups()))
>> -            (selfalpha, selfversion) = selfm.groups()
>> -            refm = re.match('([^0-9]*)(.*)', refversion)
>> -            #print(('refm', refm.groups())
>> -            (refalpha, refversion) = refm.groups()
>> -            if (selfalpha > refalpha):
>> -                return 1
>> -            elif (selfalpha < refalpha):
>> -                return -1
>> -            ## now look for numeric version component
>> -            (selfnum, selfversion) = re.match('([0-9]*)(.*)', selfversion).groups()
>> -            (refnum, refversion) = re.match('([0-9]*)(.*)', refversion).groups()
>> -            #print(('selfnum', selfnum, selfversion)
>> -            #print(('refnum', refnum, refversion)
>> -            if (selfnum != ''):
>> -                selfnum = int(selfnum)
>> -            else:
>> -                selfnum = -1
>> -            if (refnum != ''):
>> -                refnum = int(refnum)
>> -            else:
>> -                refnum = -1
>> -            if (selfnum > refnum):
>> +
>> +        value = list(selfversion)
>> +        ref = list(refversion)
>> +
>> +        while value or ref:
>> +            first_diff = 0
>> +            # alphanumeric comparison
>> +            while (value and not str.isdigit(value[0])) or (ref and not str.isdigit(ref[0])):
>> +                vc = order(value.pop(0) if value else None)
>> +                rc = order(ref.pop(0) if ref else None)
>> +                if vc != rc:
>> +                    return -1 if vc < rc else 1
>> +
>> +            # comparing numbers
>> +            # start by skipping 0
>> +            while value and value[0] == "0":
>> +                value.pop(0)
>> +            while ref and ref[0] == "0":
>> +                ref.pop(0)
>> +
>> +            # actual number comparison
>> +            while value and str.isdigit(value[0]) and ref and str.isdigit(ref[0]):
>> +                if not first_diff:
>> +                    first_diff = int(value.pop(0)) - int(ref.pop(0))
>> +                else:
>> +                    value.pop(0)
>> +                    ref.pop(0)
>> +
>> +            # the one that has a value remaining was the highest number
>> +            if value and str.isdigit(value[0]):
>>                    return 1
>> -            elif (selfnum < refnum):
>> +            if ref and str.isdigit(ref[0]):
>>                    return -1
>> -            if selfversion == '' and refversion == '':
>> -                return 0
>> +            # in case of equal length numbers look at the first diff
>> +            if first_diff:
>> +                return 1 if first_diff > 0 else -1
>> +        return 0
>>    
>>        def compare(self, ref):
>> +
>>            if (self.epoch > ref.epoch):
>>                return 1
>>            elif (self.epoch < ref.epoch):
>> @@ -191,9 +218,6 @@ class Package(object):
>>            if name == "md5":
>>                self._computeFileMD5()
>>                return self.md5
>> -        elif name == "sha256":
>> -            self._computeFileSHA256()
>> -            return self.sha256
> 
> this seems unrelated to your patch, was it intentional to remove sha256 support?
> 
>>            elif name == 'size':
>>                return self._get_file_size()
>>            else:
>> @@ -213,20 +237,6 @@ class Package(object):
>>                f.close()
>>                self.md5 = sum.hexdigest()
>>    
>> -    def _computeFileSHA256(self):
>> -        # compute the SHA256.
>> -        if not self.fn:
>> -            self.sha256 = 'Unknown'
>> -        else:
>> -            f = open(self.fn, "rb")
>> -            sum = hashlib.sha256()
>> -            while True:
>> -               data = f.read(1024)
>> -               if not data: break
>> -               sum.update(data)
>> -            f.close()
>> -            self.sha256 = sum.hexdigest()
>> -
> 
> same here
> 
>>        def _get_file_size(self):
>>            if not self.fn:
>>                self.size = 0;
>> @@ -259,8 +269,6 @@ class Package(object):
>>                        self.size = int(value)
>>                    elif name_lowercase == 'md5sum':
>>                        self.md5 = value
>> -                elif name_lowercase == 'sha256sum':
>> -                    self.sha256 = value
> 
> and here
> 
>>                    elif name_lowercase in self.__dict__:
>>                        self.__dict__[name_lowercase] = value
>>                    elif all_fields:
>> @@ -384,7 +392,6 @@ class Package(object):
>>                    error = subprocess.CalledProcessError(retcode, cmd)
>>                    error.output = output
>>                    raise error
>> -            output = output.decode("utf-8")
>>                return output
>>    
>>            if not self.fn:
>> @@ -408,12 +415,8 @@ class Package(object):
>>                return []
>>            f = open(self.fn, "rb")
>>            ar = arfile.ArFile(f, self.fn)
>> -        try:
>> -            tarStream = ar.open("data.tar.gz")
>> -            tarf = tarfile.open("data.tar.gz", "r", tarStream)
>> -        except IOError:
>> -            tarStream = ar.open("data.tar.xz")
>> -            tarf = tarfile.open("data.tar.xz", "r:xz", tarStream)
>> +        tarStream = ar.open("data.tar.gz")
>> +        tarf = tarfile.open("data.tar.gz", "r", tarStream)
> 
> this also seem to revert a newer commit (add xz support)
> 
>>            self.file_list = tarf.getnames()
>>            self.file_list = [["./", ""][a.startswith("./")] + a for a
>> in self.file_list]
>>    
>> @@ -488,7 +491,7 @@ class Package(object):
>>                ref.parsed_version = parse_version(ref.version)
>>            return self.parsed_version.compare(ref.parsed_version)
>>    
>> -    def print(self, checksum):
>> +    def __str__(self):
> 
> this change also looks unrelated
> 
>>            out = ""
>>    
>>            # XXX - Some checks need to be made, and some exceptions @@
>> -505,10 +508,7 @@ class Package(object):
>>            if self.section: out = out + "Section: %s\n" % (self.section)
>>            if self.architecture: out = out + "Architecture: %s\n" % (self.architecture)
>>            if self.maintainer: out = out + "Maintainer: %s\n" % (self.maintainer)
>> -        if 'md5' in checksum:
>> -            if self.md5: out = out + "MD5Sum: %s\n" % (self.md5)
>> -        if 'sha256' in checksum:
>> -            if self.sha256: out = out + "SHA256sum: %s\n" % (self.sha256)
>> +        if self.md5: out = out + "MD5Sum: %s\n" % (self.md5)
>>            if self.size: out = out + "Size: %d\n" % int(self.size)
>>            if self.installed_size: out = out + "InstalledSize: %d\n" % int(self.installed_size)
>>            if self.filename: out = out + "Filename: %s\n" %
>> (self.filename) @@ -585,14 +585,14 @@ class Packages(object):
>>        def __getitem__(self, key):
>>            return self.packages[key]
>>    
>> -if __name__ == "__main__":
>>    
>> +if __name__ == "__main__":
>>        assert Version(0, "1.2.2-r1").compare(Version(0, "1.2.3-r0")) == -1
>>        assert Version(0, "1.2.2-r0").compare(Version(0, "1.2.2+cvs20070308-r0")) == -1
>>        assert Version(0, "1.2.2+cvs20070308").compare(Version(0, "1.2.2-r0")) == 1
>>        assert Version(0, "1.2.2-r0").compare(Version(0, "1.2.2-r0")) == 0
>>        assert Version(0, "1.2.2-r5").compare(Version(0, "1.2.2-r0")) ==
>> 1
>> -
>> +    assert Version(0, "1.1.2~r1").compare(Version(0, "1.1.2")) == -1
>>        package = Package()
>>    
>>        package.set_package("FooBar")
>>
> 
> --
> Cheers,
> 
> Alejandro
> 

-- 
Cheers,

Alejandro

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-03-31 21:11 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20200319112625.13804-1-maarten.vanmegen@nedap.com>
2020-03-27 20:34 ` [opkg-devel] [opkg-utils PATCH] Opkg.py : Support for tilde in version compare Alejandro del Castillo
2020-03-30 20:31   ` Maarten van Megen
2020-03-31 21:11     ` Alejandro del Castillo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.