Unit tests for cflags PR

5 files changed, 105 insertions, 20 deletions

diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index 7ab8102935..a702030d1c 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -385,18 +385,12 @@ class FlagMap(HashableMap):
 
 
     def satisfies(self, other, strict=False):
-        #"strict" makes no sense if this works, but it matches how we need it. Maybe
-        #strict=True
-        if strict:
-#            if other.spec and other.spec.concrete:
-            return all(f in self and set(self[f]) == set(other[f])
+        if strict or (self.spec and self.spec._concrete):
+            return all(f in self and set(self[f]) <= set(other[f])
                        for f in other)
-#            else:
-#                return all(f in self and set(self[f]) >= set(other[f])
-#                           for f in other)
         else:
-            return all(f in self and set(self[f]) == set(other[f])
-                       for f in other if other[f] != [])
+            return all(set(self[f]) <= set(other[f])
+                   for f in other if (other[f] != [] and f in self))
 
 
     def constrain(self, other):
@@ -404,14 +398,16 @@ class FlagMap(HashableMap):
 
         Return whether the spec changed.
         """
-        changed = False
+        if other.spec and other.spec._concrete:
+            for k in self:
+                if k not in other:
+                    raise UnsatisfiableCompilerFlagSpecError(self[k], '<absent>')
 
-        # Others_set removes flags set to '' from the comparison
-        others_set = (k for k in other if other[k] != [])
-        for k in others_set:
-            if k in self and not set(self[k]) >= set(other[k]):
-                self[k] = list(set(self[k]) | set(other[k]))
-                changed = True
+        changed = False
+        for k in other:
+            if k in self and not set(self[k]) <= set(other[k]):
+                raise UnsatisfiableCompilerFlagSpecError(
+                    ' '.join(f for f in self[k]), ' '.join( f for f in other[k]))
             elif k not in self:
                 self[k] = other[k]
                 changed = True
@@ -485,6 +481,7 @@ class Spec(object):
         self.architecture = other.architecture
         self.compiler = other.compiler
         self.compiler_flags = other.compiler_flags
+        self.compiler_flags.spec = self
         self.dependencies = other.dependencies
         self.variants = other.variants
         self.variants.spec = self
@@ -520,6 +517,10 @@ class Spec(object):
         """Called by the parser to add a variant."""
         if name in self.variants: raise DuplicateVariantError(
                 "Cannot specify variant '%s' twice" % name)
+        if isinstance(value, basestring) and value.upper() == 'TRUE':
+            value = True
+        elif isinstance(value, basestring) and value.upper() == 'FALSE':
+            value = False
         self.variants[name] = VariantSpec(name, value)
 
 
@@ -2416,6 +2417,11 @@ class UnsatisfiableVariantSpecError(UnsatisfiableSpecError):
         super(UnsatisfiableVariantSpecError, self).__init__(
             provided, required, "variant")
 
+class UnsatisfiableCompilerFlagSpecError(UnsatisfiableSpecError):
+    """Raised when a spec variant conflicts with package constraints."""
+    def __init__(self, provided, required):
+        super(UnsatisfiableCompilerFlagSpecError, self).__init__(
+            provided, required, "compiler_flags")
 
 class UnsatisfiableArchitectureSpecError(UnsatisfiableSpecError):
     """Raised when a spec architecture conflicts with package constraints."""
diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py
index f75d1d2b7c..22b9754f04 100644
--- a/lib/spack/spack/test/concretize.py
+++ b/lib/spack/spack/test/concretize.py
@@ -40,9 +40,18 @@ class ConcretizeTest(MockPackagesTest):
                 cvariant = concrete.variants[name]
                 self.assertEqual(avariant.value, cvariant.value)
 
+        if abstract.compiler_flags:
+            for flag in abstract.compiler_flags:
+                aflag = abstract.compiler_flags[flag]
+                cflag = concrete.compiler_flags[flag]
+                self.assertTrue(set(aflag) <= set(cflag))
+
         for name in abstract.package.variants:
             self.assertTrue(name in concrete.variants)
 
+        for flag in concrete.compiler_flags.valid_compiler_flags():
+            self.assertTrue(flag in concrete.compiler_flags)
+
         if abstract.compiler and abstract.compiler.concrete:
             self.assertEqual(abstract.compiler, concrete.compiler)
 
@@ -75,9 +84,14 @@ class ConcretizeTest(MockPackagesTest):
     def test_concretize_variant(self):
         self.check_concretize('mpich+debug')
         self.check_concretize('mpich~debug')
+        self.check_concretize('mpich debug=2')
         self.check_concretize('mpich')
 
 
+    def test_conretize_compiler_flags(self):
+        self.check_concretize('mpich cppflags="-O3"')
+
+
     def test_concretize_preferred_version(self):
         spec = self.check_concretize('python')
         self.assertEqual(spec.versions, ver('2.7.11'))
diff --git a/lib/spack/spack/test/optional_deps.py b/lib/spack/spack/test/optional_deps.py
index 55f35ea4c9..8debde73d6 100644
--- a/lib/spack/spack/test/optional_deps.py
+++ b/lib/spack/spack/test/optional_deps.py
@@ -42,6 +42,13 @@ class ConcretizeTest(MockPackagesTest):
         self.check_normalize('optional-dep-test+a',
                              Spec('optional-dep-test+a', Spec('a')))
 
+        self.check_normalize('optional-dep-test a=true',
+                             Spec('optional-dep-test a=true', Spec('a')))
+
+
+        self.check_normalize('optional-dep-test a=true',
+                             Spec('optional-dep-test+a', Spec('a')))
+
         self.check_normalize('optional-dep-test@1.1',
                              Spec('optional-dep-test@1.1', Spec('b')))
 
diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py
index 7875d45f2a..f21dee9f27 100644
--- a/lib/spack/spack/test/spec_semantics.py
+++ b/lib/spack/spack/test/spec_semantics.py
@@ -191,12 +191,20 @@ class SpecSematicsTest(MockPackagesTest):
     def test_satisfies_matching_variant(self):
         self.check_satisfies('mpich+foo', 'mpich+foo')
         self.check_satisfies('mpich~foo', 'mpich~foo')
+        self.check_satisfies('mpich foo=1', 'mpich foo=1')
+
+        #confirm that synonymous syntax works correctly
+        self.check_satisfies('mpich+foo', 'mpich foo=True')
+        self.check_satisfies('mpich foo=true', 'mpich+foo')
+        self.check_satisfies('mpich~foo', 'mpich foo=FALSE')
+        self.check_satisfies('mpich foo=False', 'mpich~foo')
 
 
     def test_satisfies_unconstrained_variant(self):
         # only asked for mpich, no constraints.  Either will do.
         self.check_satisfies('mpich+foo', 'mpich')
         self.check_satisfies('mpich~foo', 'mpich')
+        self.check_satisfies('mpich foo=1', 'mpich')
 
 
     def test_unsatisfiable_variants(self):
@@ -205,16 +213,44 @@ class SpecSematicsTest(MockPackagesTest):
         # 'mpich' is not concrete:
         self.check_satisfies('mpich', 'mpich+foo', False)
         self.check_satisfies('mpich', 'mpich~foo', False)
+        self.check_satisfies('mpich', 'mpich foo=1', False)
 
         # 'mpich' is concrete:
         self.check_unsatisfiable('mpich', 'mpich+foo', True)
         self.check_unsatisfiable('mpich', 'mpich~foo', True)
+        self.check_unsatisfiable('mpich', 'mpich foo=1', True)
 
 
     def test_unsatisfiable_variant_mismatch(self):
         # No matchi in specs
         self.check_unsatisfiable('mpich~foo', 'mpich+foo')
         self.check_unsatisfiable('mpich+foo', 'mpich~foo')
+        self.check_unsatisfiable('mpich foo=1', 'mpich foo=2')
+
+
+    def test_satisfies_matching_compiler_flag(self):
+        self.check_satisfies('mpich cppflags="-O3"', 'mpich cppflags="-O3"')
+        self.check_satisfies('mpich cppflags="-O3 -Wall"', 'mpich cppflags="-O3 -Wall"')
+
+
+    def test_satisfies_unconstrained_compiler_flag(self):
+        # only asked for mpich, no constraints.  Any will do.
+        self.check_satisfies('mpich cppflags="-O3"', 'mpich')
+
+
+    def test_unsatisfiable_compiler_flag(self):
+        # This case is different depending on whether the specs are concrete.
+
+        # 'mpich' is not concrete:
+        self.check_satisfies('mpich', 'mpich cppflags="-O3"', False)
+
+        # 'mpich' is concrete:
+        self.check_unsatisfiable('mpich', 'mpich cppflags="-O3"', True)
+
+
+    def test_unsatisfiable_compiler_flag_mismatch(self):
+        # No matchi in specs
+        self.check_unsatisfiable('mpich cppflags="-O3"', 'mpich cppflags="-O2"')
 
 
     def test_satisfies_virtual(self):
@@ -302,18 +338,26 @@ class SpecSematicsTest(MockPackagesTest):
         self.check_constrain('libelf+debug+foo', 'libelf+debug', 'libelf+foo')
         self.check_constrain('libelf+debug+foo', 'libelf+debug', 'libelf+debug+foo')
 
+        self.check_constrain('libelf debug=2 foo=1', 'libelf debug=2', 'libelf foo=1')
+        self.check_constrain('libelf debug=2 foo=1', 'libelf debug=2', 'libelf debug=2 foo=1')
+
         self.check_constrain('libelf+debug~foo', 'libelf+debug', 'libelf~foo')
         self.check_constrain('libelf+debug~foo', 'libelf+debug', 'libelf+debug~foo')
 
 
+    def test_constrain_compiler_flags(self):
+        self.check_constrain('libelf cflags="-O3" cppflags="-Wall"', 'libelf cflags="-O3"', 'libelf cppflags="-Wall"')
+        self.check_constrain('libelf cflags="-O3" cppflags="-Wall"', 'libelf cflags="-O3"', 'libelf cflags="-O3" cppflags="-Wall"')
+
+
     def test_constrain_arch(self):
         self.check_constrain('libelf arch=bgqos_0', 'libelf arch=bgqos_0', 'libelf arch=bgqos_0')
         self.check_constrain('libelf arch=bgqos_0', 'libelf', 'libelf arch=bgqos_0')
 
 
     def test_constrain_compiler(self):
-        self.check_constrain('libelf arch=bgqos_0', 'libelf arch=bgqos_0', 'libelf arch=bgqos_0')
-        self.check_constrain('libelf arch=bgqos_0', 'libelf', 'libelf arch=bgqos_0')
+        self.check_constrain('libelf %gcc@4.4.7', 'libelf %gcc@4.4.7', 'libelf %gcc@4.4.7')
+        self.check_constrain('libelf %gcc@4.4.7', 'libelf', 'libelf %gcc@4.4.7')
 
 
     def test_invalid_constraint(self):
@@ -322,6 +366,9 @@ 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 debug=2', 'libelf debug=1')
+
+        self.check_invalid_constraint('libelf cppflags="-O3"', 'libelf cppflags="-O2"')
 
         self.check_invalid_constraint('libelf arch=bgqos_0', 'libelf arch=x86_54')
 
@@ -333,6 +380,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', 'debug=2')
+        self.check_constrain_changed('libelf', 'cppflags="-O3"')
         self.check_constrain_changed('libelf', ' arch=bgqos_0')
 
 
@@ -344,6 +393,8 @@ 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 debug=2', 'debug=2')
+        self.check_constrain_not_changed('libelf cppflags="-O3"', 'cppflags="-O3"')
         self.check_constrain_not_changed('libelf arch=bgqos_0', ' arch=bgqos_0')
         self.check_constrain_not_changed('libelf^foo', 'libelf^foo')
         self.check_constrain_not_changed('libelf^foo^bar', 'libelf^foo^bar')
@@ -356,6 +407,7 @@ 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 cppflags="-O3"')
         self.check_constrain_changed('libelf^foo', 'libelf^foo arch=bgqos_0')
 
 
@@ -366,5 +418,6 @@ 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 cppflags="-O3"', 'libelf^foo cppflags="-O3"')
         self.check_constrain_not_changed('libelf^foo arch=bgqos_0', 'libelf^foo arch=bgqos_0')
 
diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py
index 6e08e30e13..bda218c840 100644
--- a/lib/spack/spack/test/spec_syntax.py
+++ b/lib/spack/spack/test/spec_syntax.py
@@ -104,6 +104,8 @@ class SpecSyntaxTest(unittest.TestCase):
 
     def test_full_specs(self):
         self.check_parse("mvapich_foo^_openmpi@1.2:1.4,1.6%intel@12.1+debug~qt_4^stackwalker@8.1_1e")
+        self.check_parse("mvapich_foo^_openmpi@1.2:1.4,1.6%intel@12.1 debug=2~qt_4^stackwalker@8.1_1e")
+        self.check_parse('mvapich_foo^_openmpi@1.2:1.4,1.6%intel@12.1 cppflags="-O3"+debug~qt_4^stackwalker@8.1_1e')
 
     def test_canonicalize(self):
         self.check_parse(
@@ -128,7 +130,10 @@ class SpecSyntaxTest(unittest.TestCase):
 
     def test_duplicate_variant(self):
         self.assertRaises(DuplicateVariantError, self.check_parse, "x@1.2+debug+debug")
-        self.assertRaises(DuplicateVariantError, self.check_parse, "x ^y@1.2+debug+debug")
+        self.assertRaises(DuplicateVariantError, self.check_parse, "x ^y@1.2+debug debug=true")
+        self.assertRaises(DuplicateVariantError, self.check_parse, "x ^y@1.2 debug=false debug=true")
+        self.assertRaises(DuplicateVariantError, self.check_parse, "x ^y@1.2 debug=false~debug")
+
 
     def test_duplicate_depdendence(self):
         self.assertRaises(DuplicateDependencyError, self.check_parse, "x ^y ^y")