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 <[email protected]>
Sent: Friday, March 27, 2020 9:34 PM
To: [email protected]; Maarten van Megen 
<[email protected]>; [email protected]
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 <[email protected]>
---
   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 <[email protected]>
   #                      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
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#49005): https://lists.yoctoproject.org/g/yocto/message/49005
Mute This Topic: https://lists.yoctoproject.org/mt/72597747/21656
Group Owner: [email protected]
Unsubscribe: https://lists.yoctoproject.org/g/yocto/unsub  
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to