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

Improve Spec literals, add Spec.from_dict() (#5151)

Browse Source 
* Simplified Spec.__init__ signature by removing the *dep_like argument.

The `*dep_like` argument of `Spec.__init__` is used only for tests. This
PR removes it from the call signature and introduces an equivalent
fixture to be used in tests.

* Refactored ``spec_from_dict`` to be a static method of ``Spec``

The fixture ``spec_from_dict`` has been refactored to be a static method
of ``Spec``. Test code has been updated accordingly. Added tests for
exceptional paths.

* Renamed argument `unique` to `normal` + added LazySpecCache class

As requested in the review the argument `unique` of `Spec.from_literal`
has been renamed to `normal`. To avoid eager evaluations of
`Spec(spec_like)` expressions a subclass of `collections.defaultdict`
has been introduced.

* Spec object can be keys of the dictionary for a spec literal.

Added back the possibility use a spec directly as a key. This permits
to build DAGs that are partially normalized.
This commit is contained in:
Massimiliano Culpo 2017-08-23 23:20:40 +02:00 committed by Todd Gamblin
parent fa1d0a8a4d
commit 5d7901b312
4 changed files with 468 additions and 195 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

183
lib/spack/spack/spec.py
View File

@ -971,7 +971,159 @@ def __init__(self, spec, name, query_parameters):
@key_ordering
class Spec(object):
def __init__(self, spec_like, *dep_like, **kwargs):
@staticmethod
def from_literal(spec_dict, normal=True):
"""Builds a Spec from a dictionary containing the spec literal.
The dictionary must have a single top level key, representing the root,
and as many secondary level keys as needed in the spec.
The keys can be either a string or a Spec or a tuple containing the
Spec and the dependency types.
Args:
spec_dict (dict): the dictionary containing the spec literal
normal (bool): if True the same key appearing at different levels
of the ``spec_dict`` will map to the same object in memory.
Examples:
A simple spec ``foo`` with no dependencies:
.. code-block:: python
{'foo': None}
A spec ``foo`` with a ``(build, link)`` dependency ``bar``:
.. code-block:: python
{'foo':
{'bar:build,link': None}}
A spec with a diamond dependency and various build types:
.. code-block:: python
{'dt-diamond': {
'dt-diamond-left:build,link': {
'dt-diamond-bottom:build': None
},
'dt-diamond-right:build,link': {
'dt-diamond-bottom:build,link,run': None
}
}}
The same spec with a double copy of ``dt-diamond-bottom`` and
no diamond structure:
.. code-block:: python
{'dt-diamond': {
'dt-diamond-left:build,link': {
'dt-diamond-bottom:build': None
},
'dt-diamond-right:build,link': {
'dt-diamond-bottom:build,link,run': None
}
}, normal=False}
Constructing a spec using a Spec object as key:
.. code-block:: python
mpich = Spec('mpich')
libelf = Spec('libelf@1.8.11')
expected_normalized = Spec.from_literal({
'mpileaks': {
'callpath': {
'dyninst': {
'libdwarf': {libelf: None},
libelf: None
},
mpich: None
},
mpich: None
},
})
"""
# Maps a literal to a Spec, to be sure we are reusing the same object
spec_cache = LazySpecCache()
def spec_builder(d):
# The invariant is that the top level dictionary must have
# only one key
assert len(d) == 1
# Construct the top-level spec
spec_like, dep_like = next(iter(d.items()))
# If the requirements was for unique nodes (default)
# then re-use keys from the local cache. Otherwise build
# a new node every time.
if not isinstance(spec_like, Spec):
spec = spec_cache[spec_like] if normal else Spec(spec_like)
else:
spec = spec_like
if dep_like is None:
return spec
def name_and_dependency_types(s):
"""Given a key in the dictionary containing the literal,
extracts the name of the spec and its dependency types.
Args:
s (str): key in the dictionary containing the literal
"""
t = s.split(':')
if len(t) > 2:
msg = 'more than one ":" separator in key "{0}"'
raise KeyError(msg.format(s))
n = t[0]
if len(t) == 2:
dtypes = tuple(dt.strip() for dt in t[1].split(','))
else:
dtypes = ()
return n, dtypes
def spec_and_dependency_types(s):
"""Given a non-string key in the literal, extracts the spec
and its dependency types.
Args:
s (spec or tuple): either a Spec object or a tuple
composed of a Spec object and a string with the
dependency types
"""
if isinstance(s, Spec):
return s, ()
spec_obj, dtypes = s
return spec_obj, tuple(dt.strip() for dt in dtypes.split(','))
# Recurse on dependencies
for s, s_dependencies in dep_like.items():
if isinstance(s, string_types):
dag_node, dependency_types = name_and_dependency_types(s)
else:
dag_node, dependency_types = spec_and_dependency_types(s)
dependency_spec = spec_builder({dag_node: s_dependencies})
spec._add_dependency(dependency_spec, dependency_types)
return spec
return spec_builder(spec_dict)
def __init__(self, spec_like, **kwargs):
# Copy if spec_like is a Spec.
if isinstance(spec_like, Spec):
self._dup(spec_like)
@ -1014,22 +1166,6 @@ def __init__(self, spec_like, *dep_like, **kwargs):
self.external_path = kwargs.get('external_path', None)
self.external_module = kwargs.get('external_module', None)
# This allows users to construct a spec DAG with literals.
# Note that given two specs a and b, Spec(a) copies a, but
# Spec(a, b) will copy a but just add b as a dep.
deptypes = ()
for dep in dep_like:
if isinstance(dep, (list, tuple)):
# Literals can be deptypes -- if there are tuples in the
# list, they will be used as deptypes for the following Spec.
deptypes = tuple(dep)
continue
spec = dep if isinstance(dep, Spec) else Spec(dep)
self._add_dependency(spec, deptypes)
deptypes = ()
@property
def external(self):
return bool(self.external_path) or bool(self.external_module)
@ -2944,6 +3080,19 @@ def __repr__(self):
return str(self)
class LazySpecCache(collections.defaultdict):
"""Cache for Specs that uses a spec_like as key, and computes lazily
the corresponding value ``Spec(spec_like``.
"""
def __init__(self):
super(LazySpecCache, self).__init__(Spec)
def __missing__(self, key):
value = self.default_factory(key)
self[key] = value
return value
#
# These are possible token types in the spec grammar.
#

107
lib/spack/spack/test/concretize.py
View File

@ -330,59 +330,94 @@ def test_external_and_virtual(self):
def test_find_spec_parents(self):
"""Tests the spec finding logic used by concretization. """
s = Spec('a +foo',
Spec('b +foo',
Spec('c'),
Spec('d +foo')),
Spec('e +foo'))
s = Spec.from_literal({
'a +foo': {
'b +foo': {
'c': None,
'd+foo': None
},
'e +foo': None
}
})
assert 'a' == find_spec(s['b'], lambda s: '+foo' in s).name
def test_find_spec_children(self):
s = Spec('a',
Spec('b +foo',
Spec('c'),
Spec('d +foo')),
Spec('e +foo'))
s = Spec.from_literal({
'a': {
'b +foo': {
'c': None,
'd+foo': None
},
'e +foo': None
}
})
assert 'd' == find_spec(s['b'], lambda s: '+foo' in s).name
s = Spec('a',
Spec('b +foo',
Spec('c +foo'),
Spec('d')),
Spec('e +foo'))
s = Spec.from_literal({
'a': {
'b +foo': {
'c+foo': None,
'd': None
},
'e +foo': None
}
})
assert 'c' == find_spec(s['b'], lambda s: '+foo' in s).name
def test_find_spec_sibling(self):
s = Spec('a',
Spec('b +foo',
Spec('c'),
Spec('d')),
Spec('e +foo'))
s = Spec.from_literal({
'a': {
'b +foo': {
'c': None,
'd': None
},
'e +foo': None
}
})
assert 'e' == find_spec(s['b'], lambda s: '+foo' in s).name
assert 'b' == find_spec(s['e'], lambda s: '+foo' in s).name
s = Spec('a',
Spec('b +foo',
Spec('c'),
Spec('d')),
Spec('e',
Spec('f +foo')))
s = Spec.from_literal({
'a': {
'b +foo': {
'c': None,
'd': None
},
'e': {
'f +foo': None
}
}
})
assert 'f' == find_spec(s['b'], lambda s: '+foo' in s).name
def test_find_spec_self(self):
s = Spec('a',
Spec('b +foo',
Spec('c'),
Spec('d')),
Spec('e'))
s = Spec.from_literal({
'a': {
'b +foo': {
'c': None,
'd': None
},
'e': None
}
})
assert 'b' == find_spec(s['b'], lambda s: '+foo' in s).name
def test_find_spec_none(self):
s = Spec('a',
Spec('b',
Spec('c'),
Spec('d')),
Spec('e'))
s = Spec.from_literal({
'a': {
'b': {
'c': None,
'd': None
},
'e': None
}
})
assert find_spec(s['b'], lambda s: '+foo' in s) is None
def test_compiler_child(self):

119
lib/spack/spack/test/optional_deps.py
View File

@ -29,65 +29,82 @@
@pytest.fixture(
params=[
# Normalize simple conditionals
('optional-dep-test', Spec('optional-dep-test')),
('optional-dep-test~a', Spec('optional-dep-test~a')),
('optional-dep-test+a', Spec('optional-dep-test+a', Spec('a'))),
('optional-dep-test a=true', Spec(
'optional-dep-test a=true', Spec('a')
)),
('optional-dep-test a=true', Spec('optional-dep-test+a', Spec('a'))),
('optional-dep-test@1.1', Spec('optional-dep-test@1.1', Spec('b'))),
('optional-dep-test%intel', Spec(
'optional-dep-test%intel', Spec('c')
)),
('optional-dep-test%intel@64.1', Spec(
'optional-dep-test%intel@64.1', Spec('c'), Spec('d')
)),
('optional-dep-test%intel@64.1.2', Spec(
'optional-dep-test%intel@64.1.2', Spec('c'), Spec('d')
)),
('optional-dep-test%clang@35', Spec(
'optional-dep-test%clang@35', Spec('e')
)),
('optional-dep-test', {'optional-dep-test': None}),
('optional-dep-test~a', {'optional-dep-test~a': None}),
('optional-dep-test+a', {'optional-dep-test+a': {'a': None}}),
('optional-dep-test a=true', {
'optional-dep-test a=true': {
'a': None
}}),
('optional-dep-test a=true', {
'optional-dep-test+a': {
'a': None
}}),
('optional-dep-test@1.1', {'optional-dep-test@1.1': {'b': None}}),
('optional-dep-test%intel', {'optional-dep-test%intel': {'c': None}}),
('optional-dep-test%intel@64.1', {
'optional-dep-test%intel@64.1': {
'c': None,
'd': None
}}),
('optional-dep-test%intel@64.1.2', {
'optional-dep-test%intel@64.1.2': {
'c': None,
'd': None
}}),
('optional-dep-test%clang@35', {
'optional-dep-test%clang@35': {
'e': None
}}),
# Normalize multiple conditionals
('optional-dep-test+a@1.1', Spec(
'optional-dep-test+a@1.1', Spec('a'), Spec('b')
)),
('optional-dep-test+a%intel', Spec(
'optional-dep-test+a%intel', Spec('a'), Spec('c')
)),
('optional-dep-test@1.1%intel', Spec(
'optional-dep-test@1.1%intel', Spec('b'), Spec('c')
)),
('optional-dep-test@1.1%intel@64.1.2+a', Spec(
'optional-dep-test@1.1%intel@64.1.2+a',
Spec('b'),
Spec('a'),
Spec('c'),
Spec('d')
)),
('optional-dep-test@1.1%clang@36.5+a', Spec(
'optional-dep-test@1.1%clang@36.5+a',
Spec('b'),
Spec('a'),
Spec('e')
)),
('optional-dep-test+a@1.1', {
'optional-dep-test+a@1.1': {
'a': None,
'b': None
}}),
('optional-dep-test+a%intel', {
'optional-dep-test+a%intel': {
'a': None,
'c': None
}}),
('optional-dep-test@1.1%intel', {
'optional-dep-test@1.1%intel': {
'b': None,
'c': None
}}),
('optional-dep-test@1.1%intel@64.1.2+a', {
'optional-dep-test@1.1%intel@64.1.2+a': {
'a': None,
'b': None,
'c': None,
'd': None
}}),
('optional-dep-test@1.1%clang@36.5+a', {
'optional-dep-test@1.1%clang@36.5+a': {
'b': None,
'a': None,
'e': None
}}),
# Chained MPI
('optional-dep-test-2+mpi', Spec(
'optional-dep-test-2+mpi',
Spec('optional-dep-test+mpi', Spec('mpi'))
)),
('optional-dep-test-2+mpi', {
'optional-dep-test-2+mpi': {
'optional-dep-test+mpi': {'mpi': None}
}}),
# Each of these dependencies comes from a conditional
# dependency on another. This requires iterating to evaluate
# the whole chain.
('optional-dep-test+f', Spec(
'optional-dep-test+f', Spec('f'), Spec('g'), Spec('mpi')
))
('optional-dep-test+f', {
'optional-dep-test+f': {
'f': None,
'g': None,
'mpi': None
}})
]
)
def spec_and_expected(request):
"""Parameters for te normalization test."""
return request.param
"""Parameters for the normalization test."""
spec, d = request.param
return spec, Spec.from_literal(d)
def test_normalize(spec_and_expected, config, builtin_mock):

254
lib/spack/spack/test/spec_dag.py
View File

@ -197,15 +197,19 @@ def test_normalize_a_lot(self):
spec.normalize()
spec.normalize()
def test_normalize_with_virtual_spec(self):
dag = Spec('mpileaks',
Spec('callpath',
Spec('dyninst',
Spec('libdwarf',
Spec('libelf')),
Spec('libelf')),
Spec('mpi')),
Spec('mpi'))
def test_normalize_with_virtual_spec(self, ):
dag = Spec.from_literal({
'mpileaks': {
'callpath': {
'dyninst': {
'libdwarf': {'libelf': None},
'libelf': None
},
'mpi': None
},
'mpi': None
}
})
dag.normalize()
# make sure nothing with the same name occurs twice
@ -219,14 +223,18 @@ def test_normalize_with_virtual_spec(self):
assert counts[name] == 1
def test_dependents_and_dependencies_are_correct(self):
spec = Spec('mpileaks',
Spec('callpath',
Spec('dyninst',
Spec('libdwarf',
Spec('libelf')),
Spec('libelf')),
Spec('mpi')),
Spec('mpi'))
spec = Spec.from_literal({
'mpileaks': {
'callpath': {
'dyninst': {
'libdwarf': {'libelf': None},
'libelf': None
},
'mpi': None
},
'mpi': None
}
})
check_links(spec)
spec.normalize()
@ -274,21 +282,45 @@ def test_invalid_dep(self):
def test_equal(self):
# Different spec structures to test for equality
flat = Spec('mpileaks ^callpath ^libelf ^libdwarf')
flat = Spec.from_literal(
{'mpileaks ^callpath ^libelf ^libdwarf': None}
)
flat_init = Spec(
'mpileaks', Spec('callpath'), Spec('libdwarf'), Spec('libelf'))
flat_init = Spec.from_literal({
'mpileaks': {
'callpath': None,
'libdwarf': None,
'libelf': None
}
})
flip_flat = Spec(
'mpileaks', Spec('libelf'), Spec('libdwarf'), Spec('callpath'))
flip_flat = Spec.from_literal({
'mpileaks': {
'libelf': None,
'libdwarf': None,
'callpath': None
}
})
dag = Spec('mpileaks', Spec('callpath',
Spec('libdwarf',
Spec('libelf'))))
dag = Spec.from_literal({
'mpileaks': {
'callpath': {
'libdwarf': {
'libelf': None
}
}
}
})
flip_dag = Spec('mpileaks', Spec('callpath',
Spec('libelf',
Spec('libdwarf'))))
flip_dag = Spec.from_literal({
'mpileaks': {
'callpath': {
'libelf': {
'libdwarf': None
}
}
}
})
# All these are equal to each other with regular ==
specs = (flat, flat_init, flip_flat, dag, flip_dag)
@ -311,39 +343,52 @@ def test_equal(self):
def test_normalize_mpileaks(self):
# Spec parsed in from a string
spec = Spec('mpileaks ^mpich ^callpath ^dyninst ^libelf@1.8.11'
' ^libdwarf')
spec = Spec.from_literal({
'mpileaks ^mpich ^callpath ^dyninst ^libelf@1.8.11 ^libdwarf': None
})
# What that spec should look like after parsing
expected_flat = Spec(
'mpileaks', Spec('mpich'), Spec('callpath'), Spec('dyninst'),
Spec('libelf@1.8.11'), Spec('libdwarf'))
expected_flat = Spec.from_literal({
'mpileaks': {
'mpich': None,
'callpath': None,
'dyninst': None,
'libelf@1.8.11': None,
'libdwarf': None
}
})
# What it should look like after normalization
mpich = Spec('mpich')
libelf = Spec('libelf@1.8.11')
expected_normalized = Spec(
'mpileaks',
Spec('callpath',
Spec('dyninst',
Spec('libdwarf',
libelf),
libelf),
mpich),
mpich)
expected_normalized = Spec.from_literal({
'mpileaks': {
'callpath': {
'dyninst': {
'libdwarf': {libelf: None},
libelf: None
},
mpich: None
},
mpich: None
},
})
# Similar to normalized spec, but now with copies of the same
# libelf node. Normalization should result in a single unique
# node for each package, so this is the wrong DAG.
non_unique_nodes = Spec(
'mpileaks',
Spec('callpath',
Spec('dyninst',
Spec('libdwarf',
Spec('libelf@1.8.11')),
Spec('libelf@1.8.11')),
mpich),
Spec('mpich'))
non_unique_nodes = Spec.from_literal({
'mpileaks': {
'callpath': {
'dyninst': {
'libdwarf': {'libelf@1.8.11': None},
'libelf@1.8.11': None
},
mpich: None
},
mpich: None
}
}, normal=False)
# All specs here should be equal under regular equality
specs = (spec, expected_flat, expected_normalized, non_unique_nodes)
@ -380,14 +425,18 @@ def test_normalize_with_virtual_package(self):
spec = Spec('mpileaks ^mpi ^libelf@1.8.11 ^libdwarf')
spec.normalize()
expected_normalized = Spec(
'mpileaks',
Spec('callpath',
Spec('dyninst',
Spec('libdwarf',
Spec('libelf@1.8.11')),
Spec('libelf@1.8.11')),
Spec('mpi')), Spec('mpi'))
expected_normalized = Spec.from_literal({
'mpileaks': {
'callpath': {
'dyninst': {
'libdwarf': {'libelf@1.8.11': None},
'libelf@1.8.11': None
},
'mpi': None
},
'mpi': None
}
})
assert str(spec) == str(expected_normalized)
@ -552,16 +601,18 @@ def test_hash_bits(self):
def test_traversal_directions(self):
"""Make sure child and parent traversals of specs work."""
# We'll use d for a diamond dependency
d = Spec('d')
# Mock spec.
spec = Spec('a',
Spec('b',
Spec('c', d),
Spec('e')),
Spec('f',
Spec('g', d)))
# Mock spec - d is used for a diamond dependency
spec = Spec.from_literal({
'a': {
'b': {
'c': {'d': None},
'e': None
},
'f': {
'g': {'d': None}
}
}
})
assert (
['a', 'b', 'c', 'd', 'e', 'f', 'g'] ==
@ -577,16 +628,18 @@ def test_traversal_directions(self):
def test_edge_traversals(self):
"""Make sure child and parent traversals of specs work."""
# We'll use d for a diamond dependency
d = Spec('d')
# Mock spec.
spec = Spec('a',
Spec('b',
Spec('c', d),
Spec('e')),
Spec('f',
Spec('g', d)))
# Mock spec - d is used for a diamond dependency
spec = Spec.from_literal({
'a': {
'b': {
'c': {'d': None},
'e': None
},
'f': {
'g': {'d': None}
}
}
})
assert (
['a', 'b', 'c', 'd', 'e', 'f', 'g'] ==
@ -610,12 +663,14 @@ def test_copy_dependencies(self):
def test_construct_spec_with_deptypes(self):
"""Ensure that it is possible to construct a spec with explicit
dependency types."""
s = Spec('a',
Spec('b',
['build'], Spec('c')),
Spec('d',
['build', 'link'], Spec('e',
['run'], Spec('f'))))
s = Spec.from_literal({
'a': {
'b': {'c:build': None},
'd': {
'e:build,link': {'f:run': None}
}
}
})
assert s['b']._dependencies['c'].deptypes == ('build',)
assert s['d']._dependencies['e'].deptypes == ('build', 'link')
@ -653,12 +708,19 @@ def check_diamond_deptypes(self, spec):
'dt-diamond-bottom'].deptypes == ('build', 'link', 'run')
def check_diamond_normalized_dag(self, spec):
bottom = Spec('dt-diamond-bottom')
dag = Spec('dt-diamond',
['build', 'link'], Spec('dt-diamond-left',
['build'], bottom),
['build', 'link'], Spec('dt-diamond-right',
['build', 'link', 'run'], bottom))
dag = Spec.from_literal({
'dt-diamond': {
'dt-diamond-left:build,link': {
'dt-diamond-bottom:build': None
},
'dt-diamond-right:build,link': {
'dt-diamond-bottom:build,link,run': None
},
}
})
assert spec.eq_dag(dag)
def test_normalize_diamond_deptypes(self):
@ -788,3 +850,13 @@ def test_canonical_deptype(self):
canonical_deptype(('foo', 'bar'))
with pytest.raises(ValueError):
canonical_deptype(('foo',))
def test_invalid_literal_spec(self):
# Can't give type 'build' to a top-level spec
with pytest.raises(spack.spec.SpecParseError):
Spec.from_literal({'foo:build': None})
# Can't use more than one ':' separator
with pytest.raises(KeyError):
Spec.from_literal({'foo': {'bar:build:link': None}})