summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorElizabeth Fischer <rpf2116@columbia.edu>2016-10-06 05:35:34 -0400
committerTodd Gamblin <tgamblin@llnl.gov>2016-10-06 02:35:34 -0700
commit208537f6f2620cba0520a5856df9387f80a82440 (patch)
treed1d3e83f2f19e1d815f14ce07c0d44d70be769ff
parenta648dc6b334e8a170beab7d983130debc2ea9db1 (diff)
downloadspack-208537f6f2620cba0520a5856df9387f80a82440.tar.gz
spack-208537f6f2620cba0520a5856df9387f80a82440.tar.bz2
spack-208537f6f2620cba0520a5856df9387f80a82440.tar.xz
spack-208537f6f2620cba0520a5856df9387f80a82440.zip
Fix Issues with non-numeric versions, as well as preferred=True (#1561)
* Fix bug in handling of precedence of preferred=True vs. versions given in packages.yaml (#1556) * Standardized comparison of versions: numeric versions are always greater than non-numeric versions; and non-numeric versions are sorted alphabetically. This is a) simple b) ensures that non-numeric versions (such as 'develop') in package.py are not chosen ahead of numeric versions, when nothing is specified in packages.yaml Fixes Issue #1557 * Removed debugging output * Fix variable shadowing bug * Ensure develop < numeric version. * Bug fix. * Passes all unit tests in versions.py * flake8 fixes * flake8 fixes * Changed type test to be more correct. See http://stackoverflow.com/questions/8203336/difference-between-int-and-numbers-integral-in-python
-rw-r--r--lib/spack/spack/concretize.py52
-rw-r--r--lib/spack/spack/test/versions.py3
-rw-r--r--lib/spack/spack/version.py97
3 files changed, 113 insertions, 39 deletions
diff --git a/lib/spack/spack/concretize.py b/lib/spack/spack/concretize.py
index 726dee62e3..cf8cf2965c 100644
--- a/lib/spack/spack/concretize.py
+++ b/lib/spack/spack/concretize.py
@@ -33,6 +33,7 @@ or user preferences.
TODO: make this customizable and allow users to configure
concretization policies.
"""
+from __future__ import print_function
import spack
import spack.spec
import spack.compilers
@@ -42,6 +43,7 @@ from spack.version import *
from functools import partial
from itertools import chain
from spack.config import *
+import spack.preferred_packages
class DefaultConcretizer(object):
@@ -160,23 +162,43 @@ class DefaultConcretizer(object):
# If there are known available versions, return the most recent
# version that satisfies the spec
pkg = spec.package
- cmp_versions = partial(spack.pkgsort.version_compare, spec.name)
- valid_versions = sorted(
- [v for v in pkg.versions
- if any(v.satisfies(sv) for sv in spec.versions)],
- cmp=cmp_versions)
- def prefer_key(v):
- return pkg.versions.get(Version(v)).get('preferred', False)
- valid_versions.sort(key=prefer_key, reverse=True)
+ # ---------- Produce prioritized list of versions
+ # Get list of preferences from packages.yaml
+ preferred = spack.pkgsort
+ # NOTE: spack.pkgsort == spack.preferred_packages.PreferredPackages()
+
+ yaml_specs = [
+ x[0] for x in
+ preferred._spec_for_pkgname(spec.name, 'version', None)]
+ n = len(yaml_specs)
+ yaml_index = dict(
+ [(spc, n - index) for index, spc in enumerate(yaml_specs)])
+
+ # List of versions we could consider, in sorted order
+ unsorted_versions = [
+ v for v in pkg.versions
+ if any(v.satisfies(sv) for sv in spec.versions)]
+
+ keys = [(
+ # Respect order listed in packages.yaml
+ yaml_index.get(v, -1),
+ # The preferred=True flag (packages or packages.yaml or both?)
+ pkg.versions.get(Version(v)).get('preferred', False),
+ # @develop special case
+ v.isdevelop(),
+ # Numeric versions more preferred than non-numeric
+ v.isnumeric(),
+ # Compare the version itself
+ v) for v in unsorted_versions]
+ keys.sort(reverse=True)
+
+ # List of versions in complete sorted order
+ valid_versions = [x[-1] for x in keys]
+ # --------------------------
if valid_versions:
- # Disregard @develop and take the next valid version
- if ver(valid_versions[0]) == ver('develop') and \
- len(valid_versions) > 1:
- spec.versions = ver([valid_versions[1]])
- else:
- spec.versions = ver([valid_versions[0]])
+ spec.versions = ver([valid_versions[0]])
else:
# We don't know of any SAFE versions that match the given
# spec. Grab the spec's versions and grab the highest
@@ -255,7 +277,7 @@ class DefaultConcretizer(object):
spec.architecture = spack.architecture.Arch()
return True
- # Concretize the operating_system and target based of the spec
+ # Concretize the operating_system and target based of the spec
ret = any((self._concretize_platform(spec),
self._concretize_operating_system(spec),
self._concretize_target(spec)))
diff --git a/lib/spack/spack/test/versions.py b/lib/spack/spack/test/versions.py
index 41d72e7c34..9b4dc29f35 100644
--- a/lib/spack/spack/test/versions.py
+++ b/lib/spack/spack/test/versions.py
@@ -428,3 +428,6 @@ class VersionsTest(unittest.TestCase):
self.assertEqual(str(b), '1_2-3')
# Raise TypeError on tuples
self.assertRaises(TypeError, b.__getitem__, 1, 2)
+
+if __name__ == '__main__':
+ unittest.main()
diff --git a/lib/spack/spack/version.py b/lib/spack/spack/version.py
index e3efa6c87a..67a22f4660 100644
--- a/lib/spack/spack/version.py
+++ b/lib/spack/spack/version.py
@@ -107,6 +107,10 @@ def coerced(method):
return coercing_method
+def _numeric_lt(self0, other):
+ """Compares two versions, knowing they're both numeric"""
+
+
@total_ordering
class Version(object):
"""Class to represent versions"""
@@ -154,6 +158,27 @@ 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'
+
@coerced
def satisfies(self, other):
"""A Version 'satisfies' another if it is at least as specific and has
@@ -225,6 +250,27 @@ class Version(object):
def concrete(self):
return self
+ def _numeric_lt(self, other):
+ """Compares two versions, knowing they're both numeric"""
+ # Standard comparison of two numeric versions
+ for a, b in zip(self.version, other.version):
+ if a == b:
+ continue
+ else:
+ # Numbers are always "newer" than letters.
+ # This is for consistency with RPM. See patch
+ # #60884 (and details) from bugzilla #50977 in
+ # the RPM project at rpm.org. Or look at
+ # rpmvercmp.c if you want to see how this is
+ # implemented there.
+ if type(a) != type(b):
+ 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
@@ -240,30 +286,33 @@ class Version(object):
if self.version == other.version:
return False
- # dev is __gt__ than anything but itself.
- if other.string == 'develop':
- return True
-
- # If lhs is dev then it can't be < than anything
- if self.string == 'develop':
- return False
-
- for a, b in zip(self.version, other.version):
- if a == b:
- continue
- else:
- # Numbers are always "newer" than letters. This is for
- # consistency with RPM. See patch #60884 (and details)
- # from bugzilla #50977 in the RPM project at rpm.org.
- # Or look at rpmvercmp.c if you want to see how this is
- # implemented there.
- if type(a) != type(b):
- 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)
+ # 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):