Skip to content

Commit fe6f39b

Browse files
tgamblintgamblin
committed
Use key sorting instead of cmp()
- Get rid of pkgsort() usage for preferred variants. - Concretization is now entirely based on key-based sorting. - Remove PreferredPackages class and various spec cmp() methods. - Replace with PackagePrefs class that implements a key function for sorting according to packages.yaml. - Clear package pref caches on config test. - Explicit compare methods instead of total_ordering in Version. - Our total_ordering backport wasn't making Python 3 happy for some reason. - Python 3's functools.total_ordering and spelling the operators out fixes the problem. - Fix unicode issues with spec hashes, json, & YAML - Try to use str everywhere and avoid unicode objects in python 2.
1 parent 0cd6555 commit fe6f39b

19 files changed

+318
-396
lines changed

‎lib/spack/external/functools_backport.py‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,20 @@ def total_ordering(cls):
2828
opfunc.__doc__ = getattr(int, opname).__doc__
2929
setattr(cls, opname, opfunc)
3030
return cls
31+
32+
33+
@total_ordering
34+
class reverse_order(object):
35+
"""Helper for creating key functions.
36+
37+
This is a wrapper that inverts the sense of the natural
38+
comparisons on the object.
39+
"""
40+
def __init__(self, value):
41+
self.value = value
42+
43+
def __eq__(self, other):
44+
return other.value == self.value
45+
46+
def __lt__(self, other):
47+
return other.value < self.value

‎lib/spack/llnl/util/lang.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@
3333
ignore_modules = [r'^\.#', '~$']
3434

3535

36+
class classproperty(property):
37+
"""classproperty decorator: like property but for classmethods."""
38+
def __get__(self, cls, owner):
39+
return self.fget.__get__(None, owner)()
40+
41+
3642
def index_by(objects, *funcs):
3743
"""Create a hierarchy of dictionaries by splitting the supplied
3844
set of objects on unique values of the supplied functions.

‎lib/spack/spack/__init__.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,6 @@
7878
import spack.config
7979
import spack.fetch_strategy
8080
from spack.file_cache import FileCache
81-
from spack.package_prefs import PreferredPackages
8281
from spack.abi import ABI
8382
from spack.concretize import DefaultConcretizer
8483
from spack.version import Version

‎lib/spack/spack/concretize.py‎

Lines changed: 66 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -35,87 +35,77 @@
3535
"""
3636
from __future__ import print_function
3737
from six import iteritems
38+
from spack.version import *
39+
from itertools import chain
40+
from ordereddict_backport import OrderedDict
41+
from functools_backport import reverse_order
3842

3943
import spack
4044
import spack.spec
4145
import spack.compilers
4246
import spack.architecture
4347
import spack.error
44-
from spack.version import *
45-
from functools import partial
46-
from itertools import chain
4748
from spack.package_prefs import *
4849

4950

5051
class DefaultConcretizer(object):
51-
5252
"""This class doesn't have any state, it just provides some methods for
5353
concretization. You can subclass it to override just some of the
5454
default concretization strategies, or you can override all of them.
5555
"""
56-
5756
def _valid_virtuals_and_externals(self, spec):
5857
"""Returns a list of candidate virtual dep providers and external
59-
packages that coiuld be used to concretize a spec."""
58+
packages that coiuld be used to concretize a spec.
59+
60+
Preferred specs come first in the list.
61+
"""
6062
# First construct a list of concrete candidates to replace spec with.
6163
candidates = [spec]
64+
pref_key = lambda spec: 0 # no-op pref key
65+
6266
if spec.virtual:
63-
providers = spack.repo.providers_for(spec)
64-
if not providers:
65-
raise UnsatisfiableProviderSpecError(providers[0], spec)
66-
spec_w_preferred_providers = find_spec(
67-
spec,
68-
lambda x: pkgsort().spec_has_preferred_provider(
69-
x.name, spec.name))
70-
if not spec_w_preferred_providers:
71-
spec_w_preferred_providers = spec
72-
provider_cmp = partial(pkgsort().provider_compare,
73-
spec_w_preferred_providers.name,
74-
spec.name)
75-
candidates = sorted(providers, cmp=provider_cmp)
67+
candidates = spack.repo.providers_for(spec)
68+
if not candidates:
69+
raise UnsatisfiableProviderSpecError(candidates[0], spec)
70+
71+
# Find nearest spec in the DAG (up then down) that has prefs.
72+
spec_w_prefs = find_spec(
73+
spec, lambda p: PackagePrefs.has_preferred_providers(
74+
p.name, spec.name),
75+
spec) # default to spec itself.
76+
77+
# Create a key to sort candidates by the prefs we found
78+
pref_key = PackagePrefs(spec_w_prefs.name, 'providers', spec.name)
7679

7780
# For each candidate package, if it has externals, add those
7881
# to the usable list. if it's not buildable, then *only* add
7982
# the externals.
80-
usable = []
83+
#
84+
# Use an OrderedDict to avoid duplicates (use it like a set)
85+
usable = OrderedDict()
8186
for cspec in candidates:
8287
if is_spec_buildable(cspec):
83-
usable.append(cspec)
88+
usable[cspec] = True
89+
8490
externals = spec_externals(cspec)
8591
for ext in externals:
8692
if ext.satisfies(spec):
87-
usable.append(ext)
93+
usable[ext] = True
8894

8995
# If nothing is in the usable list now, it's because we aren't
9096
# allowed to build anything.
9197
if not usable:
9298
raise NoBuildError(spec)
9399

94-
def cmp_externals(a, b):
95-
if a.name != b.name and (not a.external or a.external_module and
96-
not b.external and b.external_module):
97-
# We're choosing between different providers, so
98-
# maintain order from provider sort
99-
index_of_a = next(i for i in range(0, len(candidates))
100-
if a.satisfies(candidates[i]))
101-
index_of_b = next(i for i in range(0, len(candidates))
102-
if b.satisfies(candidates[i]))
103-
return index_of_a - index_of_b
104-
105-
result = cmp_specs(a, b)
106-
if result != 0:
107-
return result
108-
109-
# prefer external packages to internal packages.
110-
if a.external is None or b.external is None:
111-
return -cmp(a.external, b.external)
112-
else:
113-
return cmp(a.external, b.external)
114-
115-
usable.sort(cmp=cmp_externals)
116-
return usable
100+
# Use a sort key to order the results
101+
return sorted(usable, key=lambda spec: (
102+
not (spec.external or spec.external_module), # prefer externals
103+
pref_key(spec), # respect prefs
104+
spec.name, # group by name
105+
reverse_order(spec.versions), # latest version
106+
spec # natural order
107+
))
117108

118-
# XXX(deptypes): Look here.
119109
def choose_virtual_or_external(self, spec):
120110
"""Given a list of candidate virtual and external packages, try to
121111
find one that is most ABI compatible.
@@ -126,25 +116,16 @@ def choose_virtual_or_external(self, spec):
126116

127117
# Find the nearest spec in the dag that has a compiler. We'll
128118
# use that spec to calibrate compiler compatibility.
129-
abi_exemplar = find_spec(spec, lambda x: x.compiler)
130-
if not abi_exemplar:
131-
abi_exemplar = spec.root
132-
133-
# Make a list including ABI compatibility of specs with the exemplar.
134-
strict = [spack.abi.compatible(c, abi_exemplar) for c in candidates]
135-
loose = [spack.abi.compatible(c, abi_exemplar, loose=True)
136-
for c in candidates]
137-
keys = zip(strict, loose, candidates)
119+
abi_exemplar = find_spec(spec, lambda x: x.compiler, spec.root)
138120

139121
# Sort candidates from most to least compatibility.
140-
# Note:
141-
# 1. We reverse because True > False.
142-
# 2. Sort is stable, so c's keep their order.
143-
keys.sort(key=lambda k: k[:2], reverse=True)
144-
145-
# Pull the candidates back out and return them in order
146-
candidates = [c for s, l, c in keys]
147-
return candidates
122+
# We reverse because True > False.
123+
# Sort is stable, so candidates keep their order.
124+
return sorted(candidates,
125+
reverse=True,
126+
key=lambda spec: (
127+
spack.abi.compatible(spec, abi_exemplar, loose=True),
128+
spack.abi.compatible(spec, abi_exemplar)))
148129

149130
def concretize_version(self, spec):
150131
"""If the spec is already concrete, return. Otherwise take
@@ -164,39 +145,24 @@ def concretize_version(self, spec):
164145
if spec.versions.concrete:
165146
return False
166147

167-
# If there are known available versions, return the most recent
168-
# version that satisfies the spec
148+
# List of versions we could consider, in sorted order
169149
pkg = spec.package
150+
usable = [v for v in pkg.versions
151+
if any(v.satisfies(sv) for sv in spec.versions)]
170152

171-
# ---------- Produce prioritized list of versions
172-
# Get list of preferences from packages.yaml
173-
preferred = pkgsort()
174-
# NOTE: pkgsort() == spack.package_prefs.PreferredPackages()
175-
176-
yaml_specs = [
177-
x[0] for x in
178-
preferred._spec_for_pkgname(spec.name, 'version', None)]
179-
n = len(yaml_specs)
180-
yaml_index = dict(
181-
[(spc, n - index) for index, spc in enumerate(yaml_specs)])
182-
183-
# List of versions we could consider, in sorted order
184-
unsorted_versions = [
185-
v for v in pkg.versions
186-
if any(v.satisfies(sv) for sv in spec.versions)]
153+
yaml_prefs = PackagePrefs(spec.name, 'version')
187154

188155
# The keys below show the order of precedence of factors used
189156
# to select a version when concretizing. The item with
190157
# the "largest" key will be selected.
191158
#
192159
# NOTE: When COMPARING VERSIONS, the '@develop' version is always
193160
# larger than other versions. BUT when CONCRETIZING,
194-
# the largest NON-develop version is selected by
195-
# default.
196-
keys = [(
161+
# the largest NON-develop version is selected by default.
162+
keyfn = lambda v: (
197163
# ------- Special direction from the user
198164
# Respect order listed in packages.yaml
199-
yaml_index.get(v, -1),
165+
-yaml_prefs(v),
200166

201167
# The preferred=True flag (packages or packages.yaml or both?)
202168
pkg.versions.get(Version(v)).get('preferred', False),
@@ -211,15 +177,11 @@ def concretize_version(self, spec):
211177
# a) develop > everything (disabled by "not v.isdevelop() above)
212178
# b) numeric > non-numeric
213179
# c) Numeric or string comparison
214-
v) for v in unsorted_versions]
215-
keys.sort(reverse=True)
216-
217-
# List of versions in complete sorted order
218-
valid_versions = [x[-1] for x in keys]
219-
# --------------------------
180+
v)
181+
usable.sort(key=keyfn, reverse=True)
220182

221-
if valid_versions:
222-
spec.versions = ver([valid_versions[0]])
183+
if usable:
184+
spec.versions = ver([usable[0]])
223185
else:
224186
# We don't know of any SAFE versions that match the given
225187
# spec. Grab the spec's versions and grab the highest
@@ -278,16 +240,15 @@ def concretize_variants(self, spec):
278240
the package specification.
279241
"""
280242
changed = False
281-
preferred_variants = pkgsort().spec_preferred_variants(
282-
spec.package_class.name)
243+
preferred_variants = PackagePrefs.preferred_variants(spec.name)
283244
for name, variant in spec.package_class.variants.items():
284245
if name not in spec.variants:
285246
changed = True
286247
if name in preferred_variants:
287248
spec.variants[name] = preferred_variants.get(name)
288249
else:
289-
spec.variants[name] = \
290-
spack.spec.VariantSpec(name, variant.default)
250+
spec.variants[name] = spack.spec.VariantSpec(
251+
name, variant.default)
291252
return changed
292253

293254
def concretize_compiler(self, spec):
@@ -329,12 +290,9 @@ def _proper_compiler_style(cspec, aspec):
329290
spec.compiler, spec.architecture)
330291
return False
331292

332-
# Find the another spec that has a compiler, or the root if none do
293+
# Find another spec that has a compiler, or the root if none do
333294
other_spec = spec if spec.compiler else find_spec(
334-
spec, lambda x: x.compiler)
335-
336-
if not other_spec:
337-
other_spec = spec.root
295+
spec, lambda x: x.compiler, spec.root)
338296
other_compiler = other_spec.compiler
339297
assert(other_spec)
340298

@@ -353,9 +311,9 @@ def _proper_compiler_style(cspec, aspec):
353311
if not compiler_list:
354312
# No compiler with a satisfactory spec was found
355313
raise UnavailableCompilerVersionError(other_compiler)
356-
cmp_compilers = partial(
357-
pkgsort().compiler_compare, other_spec.name)
358-
matches = sorted(compiler_list, cmp=cmp_compilers)
314+
315+
ppk = PackagePrefs(other_spec.name, 'compiler')
316+
matches = sorted(compiler_list, key=ppk)
359317

360318
# copy concrete version into other_compiler
361319
try:
@@ -420,7 +378,7 @@ def concretize_compiler_flags(self, spec):
420378
return ret
421379

422380

423-
def find_spec(spec, condition):
381+
def find_spec(spec, condition, default=None):
424382
"""Searches the dag from spec in an intelligent order and looks
425383
for a spec that matches a condition"""
426384
# First search parents, then search children
@@ -447,7 +405,7 @@ def find_spec(spec, condition):
447405
if condition(spec):
448406
return spec
449407

450-
return None # Nothing matched the condition.
408+
return default # Nothing matched the condition; return default.
451409

452410

453411
def _compiler_concretization_failure(compiler_spec, arch):
@@ -466,7 +424,7 @@ def _compiler_concretization_failure(compiler_spec, arch):
466424
class NoCompilersForArchError(spack.error.SpackError):
467425
def __init__(self, arch, available_os_targets):
468426
err_msg = ("No compilers found"
469-
" for operating system %s and target %s."
427+
" for operating system %s and target %s."
470428
"\nIf previous installations have succeeded, the"
471429
" operating system may have been updated." %
472430
(arch.platform_os, arch.target))
@@ -485,7 +443,6 @@ def __init__(self, arch, available_os_targets):
485443

486444

487445
class UnavailableCompilerVersionError(spack.error.SpackError):
488-
489446
"""Raised when there is no available compiler that satisfies a
490447
compiler spec."""
491448

@@ -500,7 +457,6 @@ def __init__(self, compiler_spec, arch=None):
500457

501458

502459
class NoValidVersionError(spack.error.SpackError):
503-
504460
"""Raised when there is no way to have a concrete version for a
505461
particular spec."""
506462

‎lib/spack/spack/fetch_strategy.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,6 @@ class FetchStrategy(with_metaclass(FSMeta, object)):
9090
enabled = False # Non-abstract subclasses should be enabled.
9191
required_attributes = None # Attributes required in version() args.
9292

93-
9493
def __init__(self):
9594
# The stage is initialized late, so that fetch strategies can be
9695
# constructed at package construction time. This is where things

0 commit comments

Comments
 (0)