zhangyiss/spack
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix Issues with non-numeric versions, as well as preferred=True (#1561)

Browse Source 
* 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
This commit is contained in:
Elizabeth Fischer 2016-10-06 05:35:34 -04:00 committed by Todd Gamblin
parent a648dc6b33
commit 208537f6f2
3 changed files with 110 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
lib/spack/spack/concretize.py
View File

@ -33,6 +33,7 @@
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 functools import partial
from itertools import chain
from spack.config import *
import spack.preferred_packages
class DefaultConcretizer(object):
@ -160,23 +162,43 @@ def concretize_version(self, spec):
# 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 @@ def concretize_architecture(self, spec):
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)))

3
lib/spack/spack/test/versions.py
View File

@ -428,3 +428,6 @@ def test_get_item(self):
self.assertEqual(str(b), '1_2-3')
# Raise TypeError on tuples
self.assertRaises(TypeError, b.__getitem__, 1, 2)
if __name__ == '__main__':
unittest.main()

91
lib/spack/spack/version.py
View File

@ -107,6 +107,10 @@ def coercing_method(a, b, *args, **kwargs):
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 @@ def lowest(self):
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 @@ def __str__(self):
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 @@ def __lt__(self, other):
if self.version == other.version:
return False
# dev is __gt__ than anything but itself.
if other.string == 'develop':
return True
# First priority: anything < develop
sdev = self.isdevelop()
if sdev:
return False # source = develop, it can't be < anything
# If lhs is dev then it can't be < than anything
if self.string == 'develop':
return False
# Now we know !sdev
odev = other.isdevelop()
if odev:
return True # src < dst
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
# now we know neither self nor other isdevelop().
# If the common prefix is equal, the one with more segments is bigger.
return len(self.version) < len(other.version)
# 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):