Fixed target satisfaction and updated tests accordingly

5 files changed, 79 insertions, 64 deletions

diff --git a/lib/spack/spack/concretize.py b/lib/spack/spack/concretize.py
index ce86786004..43637ed468 100644
--- a/lib/spack/spack/concretize.py
+++ b/lib/spack/spack/concretize.py
@@ -209,37 +209,6 @@ class DefaultConcretizer(object):
 
         return True   # Things changed
 
-    def class_from_platform_name(self, platform_name):
-        file_path = join_path(spack.platform_path, platform_name)
-        platform_mod = imp.load_source('spack.platforms', file_path + '.py')
-        cls = getattr(platform_mod, mod_to_class(platform_name))
-        
-        return cls
-
-    def spec_add_target_from_string(self, spec, target):
-        """If only a target is provided, spack will assume the default architecture.
-        A platform-target pair can be input delimited by a '-'. If either portion of
-        a platform-target pair is empty, spack will supply a default, in the case of
-        a blank target the default will be dependent on the platform.
-        E.g. x86_64        -> 64 bit x86
-             bgq-          -> default bgq target (back end/powerpc)
-             cray-hawswell -> haswell target on cray platform
-        """
-        if '-' in target:
-            platform, target = target.split('-')
-        else:
-            platform = ''
-            
-        if platform != '':
-            cls = self.class_from_platform_name(platform)
-            platform = cls()
-        else:
-            platform = spack.architecture.sys_type()
-        if target != '':
-            spec.target = platform.target(target)
-        else:
-            spec.target = platform.target('default')
-
     def concretize_target(self, spec):
         """If the spec already has an target and it is a an target type,
         return. Otherwise, if it has a target that is a string type, generate a
@@ -251,14 +220,14 @@ class DefaultConcretizer(object):
             if isinstance(spec.target,spack.architecture.Target):
                 return False
             else:
-                self.spec_add_target_from_string(spec, spec.target)
+                spec.add_target_from_string(spec, spec.target)
                 return True #changed
 
         if spec.root.target:
             if isinstance(spec.root.target,spack.architecture.Target):
                 spec.target = spec.root.target
             else:
-                self.spec_add_target_from_string(spec, spec.root.target)
+                spec.add_target_from_string(spec, spec.root.target)
         else:
             platform = spack.architecture.sys_type()
             spec.target = platform.target('default')
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index 9e011bfb9f..8f90cc0d7f 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -1228,6 +1228,36 @@ class Spec(object):
             return parse_anonymous_spec(spec_like, self.name)
 
 
+    def add_target_from_string(self, target):
+        """If only a target is provided, spack will assume the default architecture.
+        A platform-target pair can be input delimited by a '-'. If either portion of
+        a platform-target pair is empty, spack will supply a default, in the case of
+        a blank target the default will be dependent on the platform.
+        E.g. x86_64        -> 64 bit x86
+             bgq-          -> default bgq target (back end/powerpc)
+             cray-hawswell -> haswell target on cray platform
+        """
+        if target is None:
+            return
+        if '-' in target:
+            platform, target = target.split('-')
+        else:
+            platform = ''
+            
+        if platform != '':
+            # Find the class for the platform name given
+            file_path = join_path(spack.platform_path, platform_name)
+            platform_mod = imp.load_source('spack.platforms', file_path + '.py')
+            cls = getattr(platform_mod, mod_to_class(platform_name))
+            platform = cls()
+        else:
+            platform = spack.architecture.sys_type()
+        if target != '':
+            self.target = platform.target(target)
+        else:
+            self.target = platform.target('default')
+
+
     def satisfies(self, other, deps=True, strict=False):
         """Determine if this spec satisfies all constraints of another.
 
@@ -1275,6 +1305,11 @@ class Spec(object):
 
         # Target satisfaction is currently just class equality.
         # If not strict, None means unconstrained.
+        if not isinstance(self.target, spack.architecture.Target):
+            self.add_target_from_string(self.target)
+        if not isinstance(other.target, spack.architecture.Target):
+            other.add_target_from_string(other.target)
+
         if self.target and other.target:
             if self.target != other.target:
                 return False
diff --git a/lib/spack/spack/test/multimethod.py b/lib/spack/spack/test/multimethod.py
index 741dc96072..c651b7ad4a 100644
--- a/lib/spack/spack/test/multimethod.py
+++ b/lib/spack/spack/test/multimethod.py
@@ -92,22 +92,16 @@ class MultiMethodTest(MockPackagesTest):
 
 
     def test_target_match(self):
-        pkg = spack.db.get('multimethod=x86_64')
-        self.assertEqual(pkg.different_by_target(), 'x86_64')
-
-        pkg = spack.db.get('multimethod=ppc64')
-        self.assertEqual(pkg.different_by_target(), 'ppc64')
-
-        pkg = spack.db.get('multimethod=ppc32')
-        self.assertEqual(pkg.different_by_target(), 'ppc32')
-
-        pkg = spack.db.get('multimethod=arm64')
-        self.assertEqual(pkg.different_by_target(), 'arm64')
-
-        pkg = spack.db.get('multimethod=macos')
+        platform = spack.architecture.sys_type()
+        targets = platform.targets.values()
+        for target in targets[:-1]:
+            print target
+            pkg = spack.db.get('multimethod='+target.name)
+            self.assertEqual(pkg.different_by_target(), target.name)
+
+        pkg = spack.db.get('multimethod='+targets[-1].name)
         self.assertRaises(NoSuchMethodError, pkg.different_by_target)
 
-
     def test_dependency_match(self):
         pkg = spack.db.get('multimethod^zmpi')
         self.assertEqual(pkg.different_by_dep(), 'zmpi')
diff --git a/lib/spack/spack/test/spec_dag.py b/lib/spack/spack/test/spec_dag.py
index 6a2dd6140f..d116454d5c 100644
--- a/lib/spack/spack/test/spec_dag.py
+++ b/lib/spack/spack/test/spec_dag.py
@@ -241,9 +241,13 @@ class SpecDagTest(MockPackagesTest):
 
 
     def test_unsatisfiable_target(self):
-        set_pkg_dep('mpileaks', 'mpich=bgqos_0')
-        spec = Spec('mpileaks ^mpich=sles_10_ppc64 ^callpath ^dyninst ^libelf ^libdwarf')
-        self.assertRaises(spack.spec.UnsatisfiableTargetSpecError, spec.normalize)
+        platform = spack.architecture.sys_type()
+        if len(platform.targets) > 1:
+            first = platform.targets.values()[0].name
+            second = platform.targets.values()[1].name
+            set_pkg_dep('mpileaks', 'mpich='+first)
+            spec = Spec('mpileaks ^mpich='+ second +' ^callpath ^dyninst ^libelf ^libdwarf')
+            self.assertRaises(spack.spec.UnsatisfiableTargetSpecError, spec.normalize)
 
 
     def test_invalid_dep(self):
diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py
index b8b4fb951c..ef4db3fe65 100644
--- a/lib/spack/spack/test/spec_semantics.py
+++ b/lib/spack/spack/test/spec_semantics.py
@@ -111,12 +111,13 @@ class SpecSematicsTest(MockPackagesTest):
 
 
     def test_satisfies_target(self):
-        self.check_satisfies('foo=chaos_5_x86_64_ib', '=chaos_5_x86_64_ib')
-        self.check_satisfies('foo=bgqos_0', '=bgqos_0')
-
-        self.check_unsatisfiable('foo=bgqos_0', '=chaos_5_x86_64_ib')
-        self.check_unsatisfiable('foo=chaos_5_x86_64_ib', '=bgqos_0')
+        platform = spack.architecture.sys_type()
+        targets = platform.targets.values()
+        for target in targets:
+            self.check_satisfies('foo='+target.name, '='+target.name)
 
+        for i in range(1,len(targets)):
+            self.check_unsatisfiable('foo='+targets[i-1].name, '='+targets[i].name)
 
     def test_satisfies_dependencies(self):
         self.check_satisfies('mpileaks^mpich', '^mpich')
@@ -267,13 +268,15 @@ class SpecSematicsTest(MockPackagesTest):
 
 
     def test_constrain_target(self):
-        self.check_constrain('libelf=bgqos_0', 'libelf=bgqos_0', 'libelf=bgqos_0')
-        self.check_constrain('libelf=bgqos_0', 'libelf', 'libelf=bgqos_0')
+        platform = spack.architecture.sys_type()
+        target = platform.target('default').name
+        self.check_constrain('libelf='+target, 'libelf='+target, 'libelf='+target)
+        self.check_constrain('libelf='+target, 'libelf', 'libelf='+target)
 
 
     def test_constrain_compiler(self):
-        self.check_constrain('libelf=bgqos_0', 'libelf=bgqos_0', 'libelf=bgqos_0')
-        self.check_constrain('libelf=bgqos_0', 'libelf', 'libelf=bgqos_0')
+        self.check_constrain('libelf%intel', 'libelf%intel', 'libelf%intel')
+        self.check_constrain('libelf%intel', 'libelf', 'libelf%intel')
 
 
     def test_invalid_constraint(self):
@@ -283,7 +286,10 @@ class SpecSematicsTest(MockPackagesTest):
         self.check_invalid_constraint('libelf+debug', 'libelf~debug')
         self.check_invalid_constraint('libelf+debug~foo', 'libelf+debug+foo')
 
-        self.check_invalid_constraint('libelf=bgqos_0', 'libelf=x86_54')
+        platform = spack.architecture.sys_type()
+        targets = platform.targets.values()
+        if len(targets) > 1:
+            self.check_invalid_constraint('libelf='+targets[0].name, 'libelf='+targets[1].name)
 
 
     def test_constrain_changed(self):
@@ -293,7 +299,8 @@ class SpecSematicsTest(MockPackagesTest):
         self.check_constrain_changed('libelf%gcc', '%gcc@4.5')
         self.check_constrain_changed('libelf', '+debug')
         self.check_constrain_changed('libelf', '~debug')
-        self.check_constrain_changed('libelf', '=bgqos_0')
+        platform = spack.architecture.sys_type()
+        self.check_constrain_changed('libelf', '='+platform.target('default').name)
 
 
     def test_constrain_not_changed(self):
@@ -304,7 +311,9 @@ class SpecSematicsTest(MockPackagesTest):
         self.check_constrain_not_changed('libelf%gcc@4.5', '%gcc@4.5')
         self.check_constrain_not_changed('libelf+debug', '+debug')
         self.check_constrain_not_changed('libelf~debug', '~debug')
-        self.check_constrain_not_changed('libelf=bgqos_0', '=bgqos_0')
+        platform = spack.architecture.sys_type()
+        default = platform.target('default').name
+        self.check_constrain_not_changed('libelf='+default, '='+default)
         self.check_constrain_not_changed('libelf^foo', 'libelf^foo')
         self.check_constrain_not_changed('libelf^foo^bar', 'libelf^foo^bar')
 
@@ -316,7 +325,9 @@ class SpecSematicsTest(MockPackagesTest):
         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')
-        self.check_constrain_changed('libelf^foo', 'libelf^foo=bgqos_0')
+        platform = spack.architecture.sys_type()
+        default = platform.target('default').name
+        self.check_constrain_changed('libelf^foo', 'libelf^foo='+default)
 
 
     def test_constrain_dependency_not_changed(self):
@@ -326,5 +337,7 @@ class SpecSematicsTest(MockPackagesTest):
         self.check_constrain_not_changed('libelf^foo%gcc@4.5', 'libelf^foo%gcc@4.5')
         self.check_constrain_not_changed('libelf^foo+debug', 'libelf^foo+debug')
         self.check_constrain_not_changed('libelf^foo~debug', 'libelf^foo~debug')
-        self.check_constrain_not_changed('libelf^foo=bgqos_0', 'libelf^foo=bgqos_0')
+        platform = spack.architecture.sys_type()
+        default = platform.target('default').name
+        self.check_constrain_not_changed('libelf^foo='+default, 'libelf^foo='+default)