From 5b82bf47af78a95ff38d0abc4e24255962914639 Mon Sep 17 00:00:00 2001 From: Denis Davydov Date: Thu, 2 May 2019 20:32:40 +0200 Subject: extend Version class so that 2.0 > 1.develop > 1.1 and develop > master > head > trunk > 9999 (#1983) * extend Version class so that 2.0 > 1.develop > 1.1 * add concretization tests, with preferences and preferred version. * add master, head, trunk as develop-like versions, develop > master > head > trunk * update documentation on version comparison --- lib/spack/docs/packaging_guide.rst | 14 +-- lib/spack/spack/test/concretize_preferences.py | 32 ++++++- lib/spack/spack/test/versions.py | 49 ++++++++++ lib/spack/spack/version.py | 100 ++++++++------------- .../builtin.mock/packages/develop-test2/package.py | 18 ++++ .../packages/preferred-test/package.py | 20 +++++ 6 files changed, 161 insertions(+), 72 deletions(-) create mode 100644 var/spack/repos/builtin.mock/packages/develop-test2/package.py create mode 100644 var/spack/repos/builtin.mock/packages/preferred-test/package.py diff --git a/lib/spack/docs/packaging_guide.rst b/lib/spack/docs/packaging_guide.rst index 641bb74fea..32fc484eb5 100644 --- a/lib/spack/docs/packaging_guide.rst +++ b/lib/spack/docs/packaging_guide.rst @@ -590,13 +590,15 @@ with `RPM `_. Spack versions may also be arbitrary non-numeric strings; any string here will suffice; for example, ``@develop``, ``@master``, ``@local``. -The following rules determine the sort order of numeric -vs. non-numeric versions: +Versions are compared as follows. First, a version string is split into +multiple fields based on delimiters such as ``.``, ``-`` etc. Then +matching fields are compared using the rules below: -#. The non-numeric version ``@develop`` is considered greatest (newest). +#. The following develop-like strings are greater (newer) than all + numbers and are ordered as ``develop > master > head > trunk``. -#. Numeric versions are all less than ``@develop`` version, and are - sorted numerically. +#. Numbers are all less than the chosen develop-like strings above, + and are sorted numerically. #. All other non-numeric versions are less than numeric versions, and are sorted alphabetically. @@ -610,7 +612,7 @@ The logic behind this sort order is two-fold: #. The most-recent development version of a package will usually be newer than any released numeric versions. This allows the - ``develop`` version to satisfy dependencies like ``depends_on(abc, + ``@develop`` version to satisfy dependencies like ``depends_on(abc, when="@x.y.z:")`` ^^^^^^^^^^^^^^^^^ diff --git a/lib/spack/spack/test/concretize_preferences.py b/lib/spack/spack/test/concretize_preferences.py index eb441e006e..e7c8727371 100644 --- a/lib/spack/spack/test/concretize_preferences.py +++ b/lib/spack/spack/test/concretize_preferences.py @@ -124,11 +124,39 @@ class TestConcretizePreferences(object): spec = concretize('mpileaks') assert 'zmpi' in spec + def test_preferred(self): + """"Test packages with some version marked as preferred=True""" + spec = Spec('preferred-test') + spec.concretize() + assert spec.version == spack.spec.Version('0.2.15') + + # now add packages.yaml with versions other than preferred + # ensure that once config is in place, non-preferred version is used + update_packages('preferred-test', 'version', ['0.2.16']) + spec = Spec('preferred-test') + spec.concretize() + assert spec.version == spack.spec.Version('0.2.16') + def test_develop(self): - """Test concretization with develop version""" - spec = Spec('builtin.mock.develop-test') + """Test concretization with develop-like versions""" + spec = Spec('develop-test') spec.concretize() assert spec.version == spack.spec.Version('0.2.15') + spec = Spec('develop-test2') + spec.concretize() + assert spec.version == spack.spec.Version('0.2.15') + + # now add packages.yaml with develop-like versions + # ensure that once config is in place, develop-like version is used + update_packages('develop-test', 'version', ['develop']) + spec = Spec('develop-test') + spec.concretize() + assert spec.version == spack.spec.Version('develop') + + update_packages('develop-test2', 'version', ['0.2.15.develop']) + spec = Spec('develop-test2') + spec.concretize() + assert spec.version == spack.spec.Version('0.2.15.develop') def test_no_virtuals_in_packages_yaml(self): """Verify that virtuals are not allowed in packages.yaml.""" diff --git a/lib/spack/spack/test/versions.py b/lib/spack/spack/test/versions.py index bf060c2f83..11da83df8c 100644 --- a/lib/spack/spack/test/versions.py +++ b/lib/spack/spack/test/versions.py @@ -90,13 +90,62 @@ def check_union(expected, a, b): assert ver(expected) == ver(a).union(ver(b)) +def test_string_prefix(): + assert_ver_eq('xsdk-0.2.0', 'xsdk-0.2.0') + assert_ver_lt('xsdk-0.2.0', 'xsdk-0.3') + assert_ver_gt('xsdk-0.3', 'xsdk-0.2.0') + + def test_two_segments(): assert_ver_eq('1.0', '1.0') assert_ver_lt('1.0', '2.0') assert_ver_gt('2.0', '1.0') + + +def test_develop(): assert_ver_eq('develop', 'develop') + assert_ver_eq('develop.local', 'develop.local') assert_ver_lt('1.0', 'develop') assert_ver_gt('develop', '1.0') + assert_ver_eq('1.develop', '1.develop') + assert_ver_lt('1.1', '1.develop') + assert_ver_gt('1.develop', '1.0') + assert_ver_gt('0.5.develop', '0.5') + assert_ver_lt('0.5', '0.5.develop') + assert_ver_lt('1.develop', '2.1') + assert_ver_gt('2.1', '1.develop') + assert_ver_lt('1.develop.1', '1.develop.2') + assert_ver_gt('1.develop.2', '1.develop.1') + assert_ver_lt('develop.1', 'develop.2') + assert_ver_gt('develop.2', 'develop.1') + # other +infinity versions + assert_ver_gt('master', '9.0') + assert_ver_gt('head', '9.0') + assert_ver_gt('trunk', '9.0') + assert_ver_gt('develop', '9.0') + # hierarchical develop-like versions + assert_ver_gt('develop', 'master') + assert_ver_gt('master', 'head') + assert_ver_gt('head', 'trunk') + assert_ver_gt('9.0', 'system') + # not develop + assert_ver_lt('mydevelopmentnightmare', '1.1') + assert_ver_lt('1.mydevelopmentnightmare', '1.1') + assert_ver_gt('1.1', '1.mydevelopmentnightmare') + + +def test_isdevelop(): + assert ver('develop').isdevelop() + assert ver('develop.1').isdevelop() + assert ver('develop.local').isdevelop() + assert ver('master').isdevelop() + assert ver('head').isdevelop() + assert ver('trunk').isdevelop() + assert ver('1.develop').isdevelop() + assert ver('1.develop.2').isdevelop() + assert not ver('1.1').isdevelop() + assert not ver('1.mydevelopmentnightmare.3').isdevelop() + assert not ver('mydevelopmentnightmare.3').isdevelop() def test_three_segments(): diff --git a/lib/spack/spack/version.py b/lib/spack/spack/version.py index a73b61d347..5abe6b84e2 100644 --- a/lib/spack/spack/version.py +++ b/lib/spack/spack/version.py @@ -38,6 +38,9 @@ __all__ = ['Version', 'VersionRange', 'VersionList', 'ver'] # Valid version characters VALID_VERSION = r'[A-Za-z0-9_.-]' +# Infinity-like versions. The order in the list implies the comparision rules +infinity_versions = ['develop', 'master', 'head', 'trunk'] + def int_if_int(string): """Convert a string to int if possible. Otherwise, return a string.""" @@ -199,26 +202,14 @@ class Version(object): def highest(self): return self - def isnumeric(self): - """Tells if this version is numeric (vs. a non-numeric version). A - version will be numeric as long as the first section of it is, - even if it contains non-numerica portions. - - Some numeric versions: - 1 - 1.1 - 1.1a - 1.a.1b - Some non-numeric versions: - develop - system - myfavoritebranch - """ - return isinstance(self.version[0], numbers.Integral) - def isdevelop(self): - """Triggers on the special case of the `@develop` version.""" - return self.string == 'develop' + """Triggers on the special case of the `@develop-like` version.""" + for inf in infinity_versions: + for v in self.version: + if v == inf: + return True + + return False @coerced def satisfies(self, other): @@ -272,13 +263,36 @@ class Version(object): def concrete(self): return self - def _numeric_lt(self, other): - """Compares two versions, knowing they're both numeric""" + @coerced + def __lt__(self, other): + """Version comparison is designed for consistency with the way RPM + does things. If you need more complicated versions in installed + packages, you should override your package's version string to + express it more sensibly. + """ + if other is None: + return False + + # Coerce if other is not a Version + # simple equality test first. + if self.version == other.version: + return False + # Standard comparison of two numeric versions for a, b in zip(self.version, other.version): if a == b: continue else: + if a in infinity_versions: + if b in infinity_versions: + return (infinity_versions.index(a) > + infinity_versions.index(b)) + else: + return False + if b in infinity_versions: + return True + + # Neither a nor b is infinity # Numbers are always "newer" than letters. # This is for consistency with RPM. See patch # #60884 (and details) from bugzilla #50977 in @@ -289,53 +303,11 @@ class Version(object): return type(b) == int else: return a < b + # If the common prefix is equal, the one # with more segments is bigger. return len(self.version) < len(other.version) - @coerced - def __lt__(self, other): - """Version comparison is designed for consistency with the way RPM - does things. If you need more complicated versions in installed - packages, you should override your package's version string to - express it more sensibly. - """ - if other is None: - return False - - # Coerce if other is not a Version - # simple equality test first. - if self.version == other.version: - return False - - # First priority: anything < develop - sdev = self.isdevelop() - if sdev: - return False # source = develop, it can't be < anything - - # Now we know !sdev - odev = other.isdevelop() - if odev: - return True # src < dst - - # now we know neither self nor other isdevelop(). - - # Principle: Non-numeric is less than numeric - # (so numeric will always be preferred by default) - if self.isnumeric(): - if other.isnumeric(): - return self._numeric_lt(other) - else: # self = numeric; other = non-numeric - # Numeric > Non-numeric (always) - return False - else: - if other.isnumeric(): # self = non-numeric, other = numeric - # non-numeric < numeric (always) - return True - else: # Both non-numeric - # Maybe consider other ways to compare here... - return self.string < other.string - @coerced def __eq__(self, other): return (other is not None and diff --git a/var/spack/repos/builtin.mock/packages/develop-test2/package.py b/var/spack/repos/builtin.mock/packages/develop-test2/package.py new file mode 100644 index 0000000000..762a82f838 --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/develop-test2/package.py @@ -0,0 +1,18 @@ +# Copyright 2013-2019 Lawrence Livermore National Security, LLC and other +# Spack Project Developers. See the top-level COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) + +from spack import * + + +class DevelopTest2(Package): + """Dummy package with develop version""" + homepage = "http://www.openblas.net" + url = "http://github.com/xianyi/OpenBLAS/archive/v0.2.15.tar.gz" + + version('0.2.15.develop', git='https://github.com/dummy/repo.git') + version('0.2.15', 'b1190f3d3471685f17cfd1ec1d252ac9') + + def install(self, spec, prefix): + pass diff --git a/var/spack/repos/builtin.mock/packages/preferred-test/package.py b/var/spack/repos/builtin.mock/packages/preferred-test/package.py new file mode 100644 index 0000000000..29da42242d --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/preferred-test/package.py @@ -0,0 +1,20 @@ +# Copyright 2013-2019 Lawrence Livermore National Security, LLC and other +# Spack Project Developers. See the top-level COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) + +from spack import * + + +class PreferredTest(Package): + """Dummy package with develop version and preffered version""" + homepage = "http://www.openblas.net" + url = "http://github.com/xianyi/OpenBLAS/archive/v0.2.15.tar.gz" + + version('develop', git='https://github.com/dummy/repo.git') + version('0.2.16', 'b1190f3d3471685f17cfd1ec1d252ac9') + version('0.2.15', 'b1190f3d3471685f17cfd1ec1d252ac9', preferred=True) + version('0.2.14', 'b1190f3d3471685f17cfd1ec1d252ac9') + + def install(self, spec, prefix): + pass -- cgit v1.2.3-60-g2f50