Merge pull request #4691 from Flamefire/looseversion

make `LooseVersion('1.0') == LooseVersion('1')`

easybuild/framework/easyconfig/format/format.py

@@ -433,7 +433,7 @@ def _squash(self, vt_tuple, processed, sanity):
         # walk over dictionary of parsed sections, and check for marker conflicts (using .add())
         for key, value in processed.items():
             if isinstance(value, NestedDict):
-                tmp = self._squash_netsed_dict(key, value, squashed, sanity, vt_tuple)
+                tmp = self._squash_nested_dict(key, value, squashed, sanity, vt_tuple)
                 res_sections.update(tmp)
             elif key in self.VERSION_OPERATOR_VALUE_TYPES:
                 self.log.debug("Found VERSION_OPERATOR_VALUE_TYPES entry (%s)" % key)
@@ -453,7 +453,7 @@ def _squash(self, vt_tuple, processed, sanity):
                        (processed, squashed.versions, squashed.result))
         return squashed
 
-    def _squash_netsed_dict(self, key, nested_dict, squashed, sanity, vt_tuple):
+    def _squash_nested_dict(self, key, nested_dict, squashed, sanity, vt_tuple):
         """
         Squash NestedDict instance, returns dict with already squashed data
             from possible higher sections

easybuild/framework/easyconfig/format/version.py

@@ -45,7 +45,16 @@
 
 
 class EasyVersion(LooseVersion):
-    """Exact LooseVersion. No modifications needed (yet)"""
+    """Represent a version"""
+
+    def __init__(self, vstring, is_default=False):
+        super().__init__(vstring)
+        self._is_default = is_default
+
+    @property
+    def is_default(self):
+        """Return whether this is the default version used when no explicit version is specified"""
+        return self._is_default
 
     def __len__(self):
         """Determine length of this EasyVersion instance."""
@@ -74,7 +83,7 @@ class VersionOperator(object):
     OPERATOR_FAMILIES = [['>', '>='], ['<', '<=']]  # similar operators
 
     # default version and operator when version is undefined
-    DEFAULT_UNDEFINED_VERSION = EasyVersion('0.0.0')
+    DEFAULT_UNDEFINED_VERSION = EasyVersion('0.0', is_default=True)
     DEFAULT_UNDEFINED_VERSION_OPERATOR = OPERATOR_MAP['>']
     # default operator when operator is undefined (but version is)
     DEFAULT_UNDEFINED_OPERATOR = OPERATOR_MAP['==']
@@ -256,7 +265,7 @@ def _convert_operator(self, operator_str, version=None):
         """Return the operator"""
         operator = None
         if operator_str is None:
-            if version == self.DEFAULT_UNDEFINED_VERSION or version is None:
+            if version is None or version.is_default:
                 operator = self.DEFAULT_UNDEFINED_VERSION_OPERATOR
             else:
                 operator = self.DEFAULT_UNDEFINED_OPERATOR

easybuild/tools/loose_version.py

@@ -1,13 +1,15 @@
-# This file contains the LooseVersion class based on the class with the same name
-# as present in Python 3.7.4 distutils.
-# The original class is licensed under the Python Software Foundation License Version 2.
-# It was slightly simplified as needed to make it shorter and easier to read.
-# In particular the following changes were made:
-# - Subclass object directly instead of abstract Version class
-# - Fully init the class in the constructor removing the parse method
-# - Always set self.vstring and self.version
-# - Shorten the comparison operators as the NotImplemented case doesn't apply anymore
-# - Changes to documentation and formatting
+"""
+This file contains the LooseVersion class based on the class with the same name
+as present in Python 3.7.4 distutils.
+The original class is licensed under the Python Software Foundation License Version 2.
+It was slightly simplified as needed to make it shorter and easier to read.
+In particular the following changes were made:
+- Subclass object directly instead of abstract Version class
+- Fully init the class in the constructor removing the parse method
+- Always set self.vstring and self.version
+- Shorten the comparison operators as the NotImplemented case doesn't apply anymore
+- Changes to documentation and formatting
+"""
 
 import re
 from itertools import zip_longest
@@ -75,17 +77,19 @@ def _cmp(self, other):
         if isinstance(other, str):
             other = LooseVersion(other)
 
-        # Modified: Behave the same in Python 2 & 3 when parts are of different types
-        # Taken from https://bugs.python.org/issue14894
-        for i, j in zip_longest(self.version, other.version, fillvalue=''):
-            if not type(i) is type(j):
+        # Modified: Use string comparison for different types and fill with zeroes/empty strings
+        # Based on https://bugs.python.org/issue14894
+        for i, j in zip_longest(self.version, other.version):
+            if i is None:
+                i = 0 if isinstance(j, int) else ''
+            elif j is None:
+                j = 0 if isinstance(i, int) else ''
+            elif not type(i) is type(j):
                 i = str(i)
                 j = str(j)
-            if i == j:
-                continue
-            elif i < j:
+            if i < j:
                 return -1
-            else:  # i > j
+            if i > j:
                 return 1
         return 0
 

test/framework/ebconfigobj.py

@@ -116,15 +116,32 @@ def test_squash_simple(self):
                 res = cov.squash(version, tc['name'], tc['version'])
                 self.assertEqual(res, {})  # very simple
 
+        # Ensure that a version of '0' with trailing '.0's is matched against '0.0' but not anything higher
+        # This is for testing the DEFAULT_UNDEFINED_VERSION detection
+        for num_zeroes in range(1, 6):
+            tc = tc_first
+            zero_version = '.'.join(['0'] * num_zeroes)
+            txt = [
+                '[SUPPORTED]',
+                'versions = ' + zero_version,
+                'toolchains = ' + tc_tmpl % tc,
+                '[DEFAULT]',
+                'y=a',
+            ]
+            co = ConfigObj(txt)
+            cov = EBConfigObj(co)
+            self.assertEqual(cov.squash('0.0', tc['name'], tc['version']), {'y': 'a'})
+            self.assertEqual(cov.squash('0.1', tc['name'], tc['version']), {})
+
     def test_squash_invalid(self):
         """Try to squash invalid files. Should trigger error"""
         tc_first = {'version': '10', 'name': self.tc_first}
         tc_last = {'version': '100', 'name': self.tc_last}
 
         tc_tmpl = '%(name)s == %(version)s'
 
-        default_version = '1.0'
-        all_wrong_versions = [default_version, '>= 0.0', '< 1.0']
+        default_version = '1.1'
+        all_wrong_versions = [default_version, '>= 0.0', '< 1.1']
 
         # all txt should have default version and first toolchain unmodified
 

test/framework/utilities_test.py

@@ -140,15 +140,22 @@ def test_LooseVersion(self):
         self.assertLess(LooseVersion('2.1.5'), LooseVersion('2.2'))
         self.assertLess(LooseVersion('2.1.3'), LooseVersion('3'))
         self.assertLessEqual(LooseVersion('2.1.0'), LooseVersion('2.2'))
-        # Careful here: 1.0 > 1 !!!
-        self.assertGreater(LooseVersion('1.0'), LooseVersion('1'))
-        self.assertLess(LooseVersion('1'), LooseVersion('1.0'))
-        # checking prereleases
-        self.assertGreater(LooseVersion('4.0.0-beta'), LooseVersion('4.0.0'))
-        self.assertEqual(LooseVersion('4.0.0-beta').is_prerelease('4.0.0', ['-beta']), True)
-        self.assertEqual(LooseVersion('4.0.0-beta').is_prerelease('4.0.0', ['rc']), False)
+        # Missing components are either empty strings or zeroes
+        self.assertEqual(LooseVersion('1.0'), LooseVersion('1'))
+        self.assertEqual(LooseVersion('1'), LooseVersion('1.0'))
+        self.assertEqual(LooseVersion('1.0'), LooseVersion('1.'))
+        self.assertGreater(LooseVersion('2.1.a'), LooseVersion('2.1'))
+        self.assertGreater(LooseVersion('2.a'), LooseVersion('2'))
 
-        # The following test is taken from Python distutils tests
+        # checking prereleases
+        version_4beta = LooseVersion('4.0.0-beta')
+        self.assertGreater(version_4beta, LooseVersion('4.0.0'))
+        self.assertTrue(version_4beta.is_prerelease('4.0.0', ['-beta']))
+        self.assertTrue(version_4beta.is_prerelease(LooseVersion('4.0.0'), ['-beta']))
+        self.assertFalse(version_4beta.is_prerelease('4.0.0', ['rc']))
+        self.assertFalse(version_4beta.is_prerelease('4.0.0', ['rc, -beta']))
+
+        # The following test is based on the Python distutils tests
         # licensed under the Python Software Foundation License Version 2
         versions = (('1.5.1', '1.5.2b2', -1),
                     ('161', '3.10a', 1),
@@ -158,16 +165,21 @@ def test_LooseVersion(self):
                     ('2g6', '11g', -1),
                     ('0.960923', '2.2beta29', -1),
                     ('1.13++', '5.5.kw', -1),
-                    # Added from https://bugs.python.org/issue14894
                     ('a.12.b.c', 'a.b.3', -1),
-                    ('1.0', '1', 1),
-                    ('1', '1.0', -1))
+                    ('1.0', '1', 0),
+                    ('1.a', '1', 1),
+                    )
 
         for v1, v2, wanted in versions:
             res = LooseVersion(v1)._cmp(LooseVersion(v2))
             self.assertEqual(res, wanted,
                              'cmp(%s, %s) should be %s, got %s' %
                              (v1, v2, wanted, res))
+            # Test the inverse
+            res = LooseVersion(v2)._cmp(LooseVersion(v1))
+            self.assertEqual(res, -wanted,
+                             'cmp(%s, %s) should be %s, got %s' %
+                             (v2, v1, -wanted, res))
             # vstring is the unparsed version
             self.assertEqual(LooseVersion(v1).vstring, v1)