From d7612e7aaace3b0fa483f17840e20cde5b28f430 Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Mon, 23 May 2016 16:54:41 -0700 Subject: [PATCH] Fixed errors caught by spec and concretize tests --- lib/spack/spack/architecture.py | 78 ++++++++++++++++++++----- lib/spack/spack/concretize.py | 23 ++++++-- lib/spack/spack/spec.py | 81 +++++++++++++++++++++----- lib/spack/spack/test/architecture.py | 2 + lib/spack/spack/test/concretize.py | 4 +- lib/spack/spack/test/spec_semantics.py | 14 ++--- lib/spack/spack/test/spec_syntax.py | 6 -- 7 files changed, 160 insertions(+), 48 deletions(-) diff --git a/lib/spack/spack/architecture.py b/lib/spack/spack/architecture.py index 41778795c3..e4b3dbf9c7 100644 --- a/lib/spack/spack/architecture.py +++ b/lib/spack/spack/architecture.py @@ -105,12 +105,25 @@ def __init__(self, name): self.operating_sys = {} self.name = name + def to_dict(self): + n = {} + n['targets'] = dict((name, target.to_dict()) for (name, target) in self.targets.items()) + n['operating_systems'] = dict((name, os.to_dict()) for (name, os) in self.operating_sys.items()) + n['priority'] = self.priority + n['default_front_end_target'] = self.front_end + n['default_back_end_target'] = self.back_end + n['default_target'] = self.default + n['default_front_end_os'] = self.front_os + n['default_back_end_os'] = self.back_os + n['default_os'] = self.default_os + return {self.name: n} + def add_target(self, name, target): """Used by the platform specific subclass to list available targets. Raises an error if the platform specifies a name that is reserved by spack as an alias. """ - if name in ['front_end', 'fe', 'back_end', 'be', 'default']: + if name in ['frontend', 'fe', 'backend', 'be', 'default_target']: raise ValueError( "%s is a spack reserved alias " "and cannot be the name of a target" % name) @@ -135,6 +148,10 @@ def add_operating_system(self, name, os_class): """ Add the operating_system class object into the platform.operating_sys dictionary """ + if name in ['frontend', 'fe', 'backend', 'be', 'default_os']: + raise ValueError( + "%s is a spack reserved alias " + "and cannot be the name of an OS" % name) self.operating_sys[name] = os_class def operating_system(self, name): @@ -284,15 +301,18 @@ def to_dict(self): class Arch(object): "Architecture is now a class to help with setting attributes" - def __init__(self, platform_os=None, target=None): - self.platform = sys_type() - if platform_os: - platform_os = self.platform.operating_system(platform_os) + def __init__(self, platform=None, platform_os=None, target=None): + self.platform = platform + if platform and platform_os: + platform_os = self.platform.operating_system(platform_os) self.platform_os = platform_os - if target: + if platform and target: target = self.platform.target(target) self.target = target + # Hooks for parser to use when platform is set after target or os + self.target_string = None + self.os_string = None @property def concrete(self): @@ -302,16 +322,19 @@ def concrete(self): def __str__(self): - if self.platform.name == 'darwin': - os_name = self.platform_os.name - else: - os_name = str(self.platform_os) + if self.platform or self.platform_os or self.target: + if self.platform.name == 'darwin': + os_name = self.platform_os.name + else: + os_name = str(self.platform_os) - return (str(self.platform) +"-"+ - os_name + "-" + str(self.target)) + return (str(self.platform) +"-"+ + os_name + "-" + str(self.target)) + else: + return '' def _cmp_key(self): - platform = self.platform.name + platform = self.platform.name if isinstance(self.platform, Platform) else self.platform os = self.platform_os.name if isinstance(self.platform_os, OperatingSystem) else self.platform_os target = self.target.name if isinstance(self.target, Target) else self.target return (platform, os, target) @@ -322,7 +345,7 @@ def to_dict(self): platform_os = self.platform_os target = self.target - d['platform'] = self.platform.name + d['platform'] = self.platform.to_dict() if self.platform else None d['platform_os'] = self.platform_os.to_dict() if self.platform_os else None d['target'] = self.target.to_dict() if self.target else None @@ -350,6 +373,27 @@ def _operating_system_from_dict(os_dict): operating_system.version = os_dict['version'] return operating_system +def _platform_from_dict(platform_dict): + """ Constructs a platform from a dictionary. """ + platform = Platform.__new__(Platform) + name, p_dict = platform_dict.items()[0] + platform.name = name + platform.targets = {} + for name, t_dict in p_dict['targets'].items(): + platform.add_target(name, _target_from_dict(t_dict)) + platform.operating_sys = {} + for name, o_dict in p_dict['operating_systems'].items(): + platform.add_operating_system(name, _operating_system_from_dict(o_dict)) + platform.priority = p_dict['priority'] + platform.front_end = p_dict['default_front_end_target'] + platform.back_end = p_dict['default_back_end_target'] + platform.default = p_dict['default_target'] + platform.front_os = p_dict['default_front_end_os'] + platform.back_os = p_dict['default_back_end_os'] + platform.default_os = p_dict['default_os'] + + return platform + def arch_from_dict(d): """ Uses _platform_from_dict, _operating_system_from_dict, _target_from_dict helper methods to recreate the arch tuple from the dictionary read from @@ -359,14 +403,20 @@ def arch_from_dict(d): if d is None: return None + platform_dict = d['platform'] os_dict = d['platform_os'] target_dict = d['target'] + platform = _platform_from_dict(platform_dict) if platform_dict else None target = _target_from_dict(target_dict) if os_dict else None platform_os = _operating_system_from_dict(os_dict) if os_dict else None + arch.platform = platform arch.target = target arch.platform_os = platform_os + arch.os_string = None + arch.target_string = None + return arch @memoized diff --git a/lib/spack/spack/concretize.py b/lib/spack/spack/concretize.py index f38afd38dc..1f5c07779e 100644 --- a/lib/spack/spack/concretize.py +++ b/lib/spack/spack/concretize.py @@ -198,7 +198,7 @@ def _concretize_operating_system(self, spec): spec.architecture.platform_os = spec.root.architecture.platform_os else: spec.architecture.platform_os = spec.architecture.platform.operating_system('default_os') - return True #changed + return True #changed def _concretize_target(self, spec): platform = spec.architecture.platform @@ -210,7 +210,19 @@ def _concretize_target(self, spec): spec.architecture.target = spec.root.architecture.target else: spec.architecture.target = spec.architecture.platform.target('default_target') - return True #changed + print spec.architecture, spec.architecture.platform, spec.architecture.platform_os, spec.architecture.target + return True #changed + + def _concretize_platform(self, spec): + if spec.architecture.platform is not None and isinstance( + spec.architecture.platform, spack.architecture.Platform): + return False + if spec.root.architecture and spec.root.architecture.platform: + if isinstance(spec.root.architecture.platform,spack.architecture.Platform): + spec.architecture.platform = spec.root.architecture.platform + else: + spec.architecture.platform = spack.architecture.sys_type() + return True #changed? def concretize_architecture(self, spec): """If the spec is empty provide the defaults of the platform. If the @@ -227,10 +239,11 @@ def concretize_architecture(self, spec): # Set the architecture to all defaults spec.architecture = spack.architecture.Arch() return True - + # Concretize the operating_system and target based of the spec - ret = any((self._concretize_operating_system(spec), - self._concretize_target(spec))) + ret = any((self._concretize_platform(spec), + self._concretize_operating_system(spec), + self._concretize_target(spec))) return ret diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index f88475d1c8..d15598405f 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -99,6 +99,7 @@ import itertools import hashlib import base64 +import imp from StringIO import StringIO from operator import attrgetter import yaml @@ -107,6 +108,7 @@ import llnl.util.tty as tty from llnl.util.lang import * from llnl.util.tty.color import * +from llnl.util.filesystem import join_path import spack import spack.architecture @@ -119,6 +121,7 @@ from spack.version import * from spack.util.string import * from spack.util.prefix import Prefix +from spack.util.naming import mod_to_class from spack.virtual import ProviderIndex from spack.build_environment import get_path_from_module, load_module @@ -535,10 +538,24 @@ def _add_flag(self, name, value): Known flags currently include "arch" """ valid_flags = FlagMap.valid_compiler_flags() - if name == 'os' or name == 'operating_system': - self._set_os(value) +# if name == 'arch' or name == 'architecture': +# platform, op_sys, target = value.split('-') +# print platform, op_sys, target, '+++++++' +# self._set_platform(platform) +# self._set_os(op_sys) +# self._set_target(target) + if name == 'platform': + self._set_platform(value) + elif name == 'os' or name == 'operating_system': + if self.architecture.platform: + self._set_os(value) + else: + self.architecture.os_string = value elif name == 'target': - self._set_target(value) + if self.architecture.platform: + self._set_target(value) + else: + self.architecture.target_string = value elif name in valid_flags: assert(self.compiler_flags is not None) self.compiler_flags[name] = value.split() @@ -551,6 +568,39 @@ def _set_compiler(self, compiler): "Spec for '%s' cannot have two compilers." % self.name) self.compiler = compiler + def _set_platform(self, value): + """Called by the parser to set the architecture platform""" + if isinstance(value, basestring): + mod_path = spack.platform_path + mod_string = 'spack.platformss' + names = list_modules(mod_path) + if value in names: + # Create a platform object from the name + mod_name = mod_string + value + path = join_path(mod_path, value) + '.py' + mod = imp.load_source(mod_name, path) + class_name = mod_to_class(value) + if not hasattr(mod, class_name): + tty.die('No class %s defined in %s' % (class_name, mod_name)) + cls = getattr(mod, class_name) + if not inspect.isclass(cls): + tty.die('%s.%s is not a class' % (mod_name, class_name)) + platform = cls() + else: + tty.die("No platform class %s defined." % value) + else: + # The value is a platform + platform = value + + self.architecture.platform = platform + + # Set os and target if we previously got strings for them + if self.architecture.os_string: + self._set_os(self.architecture.os_string) + self.architecture.os_string = None + if self.architecture.target_string: + self._set_target(self.architecture.target_string) + self.architecture.target_string = None def _set_os(self, value): """Called by the parser to set the architecture operating system""" @@ -1016,6 +1066,7 @@ def _expand_virtual_packages(self): changed = True spec.dependencies = DependencyMap() replacement.dependencies = DependencyMap() + replacement.architecture = self.architecture # TODO: could this and the stuff in _dup be cleaned up? def feq(cfield, sfield): @@ -1426,7 +1477,6 @@ def constrain(self, other, deps=True): other.variants[v]) # TODO: Check out the logic here - print self.architecture, other.architecture, "^^^^^^^^^^^^^^^^^^^^^^^" if self.architecture is not None and other.architecture is not None: if self.architecture.platform is not None and other.architecture.platform is not None: if self.architecture.platform != other.architecture.platform: @@ -1831,8 +1881,7 @@ def _cmp_node(self): self.variants, self.architecture, self.compiler, - self.compiler_flags, - self.dag_hash()) + self.compiler_flags) def eq_node(self, other): @@ -1946,7 +1995,7 @@ def write(s, c): if self.variants: write(fmt % str(self.variants), c) elif c == '=': - if self.architecture: + if self.architecture and str(self.architecture): write(fmt % (' arch' + c + str(self.architecture)), c) elif c == '#': out.write('-' + fmt % (self.dag_hash(7))) @@ -2004,7 +2053,7 @@ def write(s, c): if self.variants: write(fmt % str(self.variants), '+') elif named_str == 'ARCHITECTURE': - if self.architecture: + if self.architecture and str(self.architecture): write(fmt % str(self.architecture), ' arch=') elif named_str == 'SHA1': if self.dependencies: @@ -2054,13 +2103,13 @@ def __cmp__(self, other): self.variants, other.variants) #Target - if self.target != other.target: - return spack.pkgsort.target_compare(pkgname, - self.target, other.target) + if self.architecture != other.architecture: + return spack.pkgsort.architecture_compare(pkgname, + self.architecture, other.architecture) #Dependency is not configurable - if self.dep_hash() != other.dep_hash(): - return -1 if self.dep_hash() < other.dep_hash() else 1 + if self.dependencies != other.dependencies: + return -1 if self.dependencies < other.dependencies else 1 #Equal specs return 0 @@ -2181,6 +2230,11 @@ def do_parse(self): raise SpecParseError(e) + # If the spec has an os or a target and no platform, give it the default platform + for spec in specs: + for s in spec.traverse(): + if s.architecture.os_string or s.architecture.target_string: + s._set_platform(spack.architecture.sys_type()) return specs @@ -2401,7 +2455,6 @@ class SpecError(spack.error.SpackError): def __init__(self, message): super(SpecError, self).__init__(message) - class SpecParseError(SpecError): """Wrapper for ParseError for when we're parsing specs.""" def __init__(self, parse_error): diff --git a/lib/spack/spack/test/architecture.py b/lib/spack/spack/test/architecture.py index 2927e468a0..f5b1068435 100644 --- a/lib/spack/spack/test/architecture.py +++ b/lib/spack/spack/test/architecture.py @@ -26,12 +26,14 @@ def tearDown(self): def test_dict_functions_for_architecture(self): arch = Arch() + arch.platform = spack.architecture.sys_type() arch.platform_os = arch.platform.operating_system('default_os') arch.target = arch.platform.target('default_target') d = arch.to_dict() new_arch = spack.architecture.arch_from_dict(d) + self.assertEqual(arch, new_arch) self.assertTrue( isinstance(arch, Arch) ) diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index 49281b9a0c..963481054e 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -55,8 +55,8 @@ def check_spec(self, abstract, concrete): if abstract.compiler and abstract.compiler.concrete: self.assertEqual(abstract.compiler, concrete.compiler) - if abstract.architecture and abstract.architecture.target.concrete: - self.assertEqual(abstract.target, concrete.target) + if abstract.architecture and abstract.architecture.concrete: + self.assertEqual(abstract.architecture, concrete.architecture) def check_concretize(self, abstract_spec): diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py index 9bd32a3d10..45c89100d4 100644 --- a/lib/spack/spack/test/spec_semantics.py +++ b/lib/spack/spack/test/spec_semantics.py @@ -383,14 +383,14 @@ def test_constrain_compiler(self): def test_invalid_constraint(self): -# self.check_invalid_constraint('libelf@0:2.0', 'libelf@2.1:3') -# self.check_invalid_constraint('libelf@0:2.5%gcc@4.8:4.9', 'libelf@2.1:3%gcc@4.5:4.7') + self.check_invalid_constraint('libelf@0:2.0', 'libelf@2.1:3') + self.check_invalid_constraint('libelf@0:2.5%gcc@4.8:4.9', 'libelf@2.1:3%gcc@4.5:4.7') -# self.check_invalid_constraint('libelf+debug', 'libelf~debug') -# self.check_invalid_constraint('libelf+debug~foo', 'libelf+debug+foo') -# self.check_invalid_constraint('libelf debug=2', 'libelf debug=1') + self.check_invalid_constraint('libelf+debug', 'libelf~debug') + self.check_invalid_constraint('libelf+debug~foo', 'libelf+debug+foo') + self.check_invalid_constraint('libelf debug=2', 'libelf debug=1') -# self.check_invalid_constraint('libelf cppflags="-O3"', 'libelf cppflags="-O2"') + self.check_invalid_constraint('libelf cppflags="-O3"', 'libelf cppflags="-O2"') platform = spack.architecture.sys_type() if len(platform.operating_sys.keys()) > 1 or len(platform.targets.keys()) > 1: os1 = platform.operating_sys.keys()[0] @@ -439,9 +439,9 @@ def test_constrain_dependency_changed(self): self.check_constrain_changed('libelf^foo%gcc', 'libelf^foo%gcc@4.5') self.check_constrain_changed('libelf^foo', 'libelf^foo+debug') self.check_constrain_changed('libelf^foo', 'libelf^foo~debug') + platform = spack.architecture.sys_type() default_target = platform.target('default_target').name - print default_target self.check_constrain_changed('libelf^foo', 'libelf^foo target='+default_target) diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index ae19de177b..4a534d7b5c 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -71,12 +71,6 @@ def check_parse(self, expected, spec=None, remove_arch=True): spec = expected output = spack.spec.parse(spec) - # Remove architectures that get added by parser. - if remove_arch: - for spec in output: - for s in spec.traverse(): - s.architecture = None - parsed = (" ".join(str(spec) for spec in output)) self.assertEqual(expected, parsed)