Commit of compiler flags addition:

Flags are passed from the command line all the way through
build environments to environment variables.
      Flags are specified using +name=value and values are quoted
using escaped quotes when necessary.

Future work includes using the flags in the compiler wrapper script
and hopefully updating the parser for a gentler user experience of
the spec language.

lib/spack/spack/build_environment.py

@@ -36,6 +36,7 @@
 
 import spack
 import spack.compilers as compilers
+import spack.compiler as Compiler
 from spack.util.executable import Executable, which
 from spack.util.environment import *
 
@@ -57,7 +58,6 @@
 SPACK_SHORT_SPEC       = 'SPACK_SHORT_SPEC'
 SPACK_DEBUG_LOG_DIR    = 'SPACK_DEBUG_LOG_DIR'
 
-
 class MakeExecutable(Executable):
     """Special callable executable object for make so the user can
        specify parallel or not on a per-invocation basis.  Using
@@ -86,6 +86,7 @@ def __call__(self, *args, **kwargs):
 def set_compiler_environment_variables(pkg):
     assert(pkg.spec.concrete)
     compiler = pkg.compiler
+    flags = pkg.spec.compiler_flags
 
     # Set compiler variables used by CMake and autotools
     os.environ['CC']  = join_path(spack.build_env_path, 'cc')
@@ -103,16 +104,17 @@ def set_compiler_environment_variables(pkg):
     if compiler.fc:
         os.environ['SPACK_FC']  = compiler.fc
 
-    # Set SPACK compiler flags so our wrapper can add default flags
+    # Encorporate the compiler default flags into the set of flags
-    if compiler.cflags:
+    for flag in flags:
-        os.environ['SPACK_CFLAGS'] = compiler.cflags
+        if flag in compiler.flags:
-    if compiler.cxxflags:
+            compiler.flags[flag] += ' '+flags[flag]
-        os.environ['SPACK_CXXFLAGS'] = compiler.cxxflags
+        else:
-    if compiler.fflags:
+            compiler.flags[flag] = flags[flag]
-        os.environ['SPACK_FFLAGS'] = compiler.fflags
-    if compiler.ldflags:
-        os.environ['SPACK_LDFLAGS'] = compiler.ldflags
 
+    # Add every valid compiler flag to the environment, prefaced by "SPACK_"
+    for flag in Compiler.valid_compiler_flags():
+        if flag in compiler.flags:
+            os.environ['SPACK_'+flag.upper()] = compiler.flags[flag]
 
     os.environ['SPACK_COMPILER_SPEC']  = str(pkg.spec.compiler)
 

lib/spack/spack/cmd/__init__.py

@@ -88,6 +88,7 @@ def parse_specs(args, **kwargs):
     if isinstance(args, (python_list, tuple)):
         args = " ".join(args)
 
+
     try:
         specs = spack.spec.parse(args)
         for spec in specs:

lib/spack/spack/compiler.py

@@ -63,6 +63,10 @@ def dumpversion(compiler_path):
     """Simple default dumpversion method -- this is what gcc does."""
     return get_compiler_version(compiler_path, '-dumpversion')
 
+_valid_compiler_flags = ['cflags', 'cxxflags', 'fflags', 'ldflags', 'cppflags']
+def valid_compiler_flags():
+    return _valid_compiler_flags
+
 
 class Compiler(object):
     """This class encapsulates a Spack "compiler", which includes C,
@@ -98,7 +102,7 @@ class Compiler(object):
     cxx11_flag = "-std=c++11"
 
 
-    def __init__(self, cspec, cc, cxx, f77, fc, cflags=None, cxxflags=None, fflags=None, ldflags=None):
+    def __init__(self, cspec, cc, cxx, f77, fc, **kwargs):
         def check(exe):
             if exe is None:
                 return None
@@ -110,10 +114,13 @@ def check(exe):
         self.f77 = check(f77)
         self.fc  = check(fc)
 
-        self.cflags = cflags
+        #Unfortunately have to make sure these params are accepted in the same order the are returned
-        self.cxxflags = cxxflags
+        #by sorted(flags) in compilers/__init__.py
-        self.fflags = fflags
+        self.flags = {}
-        self.ldflags = ldflags
+        for flag in _valid_compiler_flags:
+            value = kwargs.get(flag, None)
+            if value is not None:
+                self.flags[flag] = value
 
         self.spec = cspec
 
@@ -151,7 +158,6 @@ def f77_version(cls, f77):
     def fc_version(cls, fc):
         return cls.default_version(fc)
 
-
     @classmethod
     def _find_matches_in_path(cls, compiler_names, detect_version, *path):
         """Finds compilers in the paths supplied.
@@ -259,24 +265,6 @@ def find(cls, *path):
 
         return list(compilers.values())
 
-    def update_flags(self,c=None,cxx=None,f=None,ld=None):
-        """Update any flag values provided. Cannot be used to erase values"""
-        if c:
-            self.cflags=c
-        if cxx:
-            self.cxxflags=cxx
-        if f:
-            self.fflags=f
-        if ld:
-            self.ldflags=ld
-
-    def erase_flags(self):
-        """Erase the flag settings"""
-        self.cflags=None
-        self.cxxflags=None
-        self.fflags=None
-        self.ldflags=None
-
 
     def __repr__(self):
         """Return a string represntation of the compiler toolchain."""

lib/spack/spack/compilers/__init__.py

@@ -37,6 +37,7 @@
 import spack.config
 
 from spack.util.multiproc import parmap
+import spack.compiler as Comp
 from spack.compiler import Compiler
 from spack.util.executable import which
 from spack.util.naming import mod_to_class
@@ -44,7 +45,6 @@
 
 _imported_compilers_module = 'spack.compilers'
 _required_instance_vars = ['cc', 'cxx', 'f77', 'fc']
-_optional_flag_vars = ['cflags', 'cxxflags', 'fflags', 'ldflags']
 
 _default_order = ['gcc', 'intel', 'pgi', 'clang', 'xlc']
 
@@ -182,20 +182,19 @@ def get_compiler(cspec):
             raise InvalidCompilerConfigurationError(cspec)
 
         cls  = class_for_compiler_name(cspec.name)
-        compiler_params = []
+        compiler_paths = []
         for c in _required_instance_vars:
             compiler_path = items[c]
             if compiler_path != "None":
-                compiler_params.append(compiler_path)
+                compiler_paths.append(compiler_path)
             else:
-                compiler_params.append(None)
+                compiler_paths.append(None)
 
-        for c in _optional_flag_vars:
+        flags = {}
-            if c not in items:
+        for f in Comp.valid_compiler_flags():
-                items[c]=None
+            if f in items:
-            compiler_params.append(items[c])
+                flags[f] = items[f]
-
+        return cls(cspec, *compiler_paths, **flags)
-        return cls(cspec, *compiler_params)
 
     matches = find(compiler_spec)
     return [get_compiler(cspec) for cspec in matches]

lib/spack/spack/concretize.py

@@ -175,6 +175,26 @@ def concretize_compiler(self, spec):
         return True  # things changed.
 
 
+    def concretize_compiler_flags(self, spec):
+        """
+        The compiler flags are updated to match those of the spec whose
+        compiler is used, defaulting to no compiler flags in the spec.
+        Default specs set at the compiler level will still be added later.
+        """
+        try:
+            nearest = next(p for p in spec.traverse(direction='parents')
+                           if p.compiler == spec.compiler and p is not spec)
+            if spec.compiler_flags == nearest.compiler_flags:
+                return False
+            spec.compiler_flags = nearest.compiler_flags.copy()
+
+        except StopIteration:
+            return False
+
+        return True  # things changed.
+
+
+
     def choose_provider(self, spec, providers):
         """This is invoked for virtual specs.  Given a spec with a virtual name,
            say "mpi", and a list of specs of possible providers of that spec,

lib/spack/spack/package.py

@@ -352,6 +352,7 @@ def __init__(self, spec):
         # Fix up self.url if this package fetches with a URLFetchStrategy.
         # This makes self.url behave sanely.
         if self.spec.versions.concrete:
+
             # TODO: this is a really roundabout way of determining the type
             # TODO: of fetch to do. figure out a more sane fetch strategy/package
             # TODO: init order (right now it's conflated with stage, package, and
@@ -587,6 +588,7 @@ def installed_dependents(self):
     @property
     def prefix(self):
         """Get the prefix into which this package should be installed."""
+#        print self.spec, self.spec.prefix
         return self.spec.prefix
 
 

lib/spack/spack/spec.py

@@ -107,6 +107,7 @@
 import spack.parse
 import spack.error
 import spack.compilers as compilers
+import spack.compiler as Compiler
 
 from spack.cmd.find import display_specs
 from spack.version import *
@@ -144,7 +145,6 @@
    every time we call str()"""
 _any_version = VersionList([':'])
 
-
 def index_specs(specs):
     """Take a list of specs and return a dict of lists.  Dict is
        keyed by spec name and lists include all specs with the
@@ -192,10 +192,12 @@ def __init__(self, *args):
                 c = SpecParser().parse_compiler(arg)
                 self.name = c.name
                 self.versions = c.versions
+#                self.flags = c.flags
 
             elif isinstance(arg, CompilerSpec):
                 self.name = arg.name
                 self.versions = arg.versions.copy()
+#                self.flags = arg.flags.copy()
 
             else:
                 raise TypeError(
@@ -207,10 +209,18 @@ def __init__(self, *args):
             self.name = name
             self.versions = VersionList()
             self.versions.add(ver(version))
+#            self.flags = {'cflags':None,'cxxflags':None,'fflags':None,'ldflags':None}
+
+#        elif nargs == 3:
+#            name, version, flags = args
+#            self.name = name
+#            self.versions = VersionList()
+#            self.versions.add(ver(version))
+#            self.flags = flags
 
         else:
             raise TypeError(
-                "__init__ takes 1 or 2 arguments. (%d given)" % nargs)
+                "__init__ takes 1, 2, or 3 arguments. (%d given)" % nargs)
 
 
     def _add_version(self, version):
@@ -226,9 +236,21 @@ def _autospec(self, compiler_spec_like):
     def satisfies(self, other, strict=False):
         other = self._autospec(other)
         return (self.name == other.name and
-                self.versions.satisfies(other.versions, strict=strict))
+                self.versions.satisfies(other.versions, strict=strict))# and
+#                self.flags_satisfy(other, strict=strict))
 
 
+#    def flags_satisfy(self,other,strict = False):
+#        if strict:
+#            for flag in self.flags:
+#                if not self.flags[flag] == other.flags[flag]:
+#                    return False
+#        else:
+#            for flag in self.flags:
+#                if other.flags[flag] and (not self.flags[flag] or other.flags[flag] not in self.flags[flag]):
+#                    return False
+#        return True
+
     def constrain(self, other):
         """Intersect self's versions with other.
 
@@ -261,23 +283,25 @@ def copy(self):
         clone = CompilerSpec.__new__(CompilerSpec)
         clone.name = self.name
         clone.versions = self.versions.copy()
+#        clone.flags = self.flags.copy()
         return clone
 
 
     def _cmp_key(self):
-        return (self.name, self.versions)
+        return (self.name, self.versions)#, str(sorted(self.flags.items())))
 
 
     def to_dict(self):
         d = {'name' : self.name}
         d.update(self.versions.to_dict())
+#        d['flags'] = self.flags
         return { 'compiler' : d }
 
 
     @staticmethod
     def from_dict(d):
         d = d['compiler']
-        return CompilerSpec(d['name'], VersionList.from_dict(d))
+        return CompilerSpec(d['name'], VersionList.from_dict(d))#, d['flags'])
 
 
     def __str__(self):
@@ -285,6 +309,11 @@ def __str__(self):
         if self.versions and self.versions != _any_version:
             vlist = ",".join(str(v) for v in self.versions)
             out += "@%s" % vlist
+#        if self.flags:
+#            for flag, value in self.flags.items():
+#                if value is not None:
+#                    out += "+" + flag + "=" + value
+#                    print "outing"
         return out
 
     def __repr__(self):
@@ -372,6 +401,59 @@ def __str__(self):
         return ''.join(str(self[key]) for key in sorted_keys)
 
 
+class FlagMap(HashableMap):
+    def __init__(self, spec):
+        super(FlagMap, self).__init__()
+        self.spec = spec
+
+
+    def satisfies(self, other, strict=False):
+        #"strict" makes no sense if this works, but it matches how we need it. Maybe
+        if strict:
+            return all(k in self and self[k] == other[k]
+                       for k in other)
+        else:
+            return self == other
+
+    def constrain(self, other):
+        """Add all flags in other that aren't in self to self.
+
+        Return whether the spec changed.
+        """
+        changed = False
+        for k in other:
+            if k in self:
+                if self[k] != other[k]:
+                    #This will not properly recognize incompatible flags
+                    self[k] += other[k]
+                    changed = True
+            else:
+                self[k] = other[k]
+                changed = True
+        return changed
+
+
+    @property
+    def concrete(self):
+        return self.spec._concrete
+
+
+    def copy(self):
+        clone = FlagMap(None)
+        for name, value in self.items():
+            clone[name] = value
+        return clone
+
+
+    def _cmp_key(self):
+        return ''.join(str(key)+str(value) for key, value in sorted(self.items()))
+
+
+    def __str__(self):
+        sorted_keys = sorted(self.keys())
+        return '+' + '+'.join(str(key) + '=\"' + str(self[key]) + '\"' for key in sorted_keys)
+
+
 class DependencyMap(HashableMap):
     """Each spec has a DependencyMap containing specs for its dependencies.
        The DependencyMap is keyed by name. """
@@ -413,6 +495,7 @@ def __init__(self, spec_like, *dep_like, **kwargs):
         self.versions = other.versions
         self.architecture = other.architecture
         self.compiler = other.compiler
+        self.compiler_flags = other.compiler_flags
         self.dependencies = other.dependencies
         self.variants = other.variants
         self.variants.spec = self
@@ -446,14 +529,19 @@ def _add_variant(self, name, value):
                 "Cannot specify variant '%s' twice" % name)
         self.variants[name] = VariantSpec(name, value)
 
+
     def _add_flag(self, name, value):
         """Called by the parser to add a known flag.
         Known flags currently include "arch"
         """
+        valid_flags = Compiler.valid_compiler_flags()
         if name == 'arch':
             self._set_architecture(value)
+        elif name in valid_flags:
+            assert(self.compiler_flags is not None)
+            self.compiler_flags[name] = value
         else:
-            raise SpecError("Invalid flag specified")
+            self._add_variant(self,name,value)
 
     def _set_compiler(self, compiler):
         """Called by the parser to set the compiler."""
@@ -533,6 +621,7 @@ def concrete(self):
                               and self.variants.concrete
                               and self.architecture
                               and self.compiler and self.compiler.concrete
+#                              and self.compiler_flags.concrete
                               and self.dependencies.concrete)
         return self._concrete
 
@@ -667,7 +756,8 @@ def to_node_dict(self):
                 (name,v.value) for name, v in self.variants.items()),
             'arch' : self.architecture,
             'dependencies' : dict((d, self.dependencies[d].dag_hash())
-                                  for d in sorted(self.dependencies))
+                                  for d in sorted(self.dependencies)),
+            'compiler_flags' : dict((name, value) for name, value in self.compiler_flags.items())
         }
         if self.compiler:
             d.update(self.compiler.to_dict())
@@ -704,6 +794,9 @@ def from_node_dict(node):
         for name, value in node['variants'].items():
             spec.variants[name] = VariantSpec(name, value)
 
+        for name, value in node['compiler_flags'].items():
+            spec.compiler_flags[name] = value
+
         return spec
 
 
@@ -769,6 +862,7 @@ def _concretize_helper(self, presets=None, visited=None):
                 changed |= any(
                     (spack.concretizer.concretize_architecture(self),
                      spack.concretizer.concretize_compiler(self),
+                     spack.concretizer.concretize_compiler_flags(self),#has to be concretized after compiler
                      spack.concretizer.concretize_version(self),
                      spack.concretizer.concretize_variants(self)))
             presets[self.name] = self
@@ -843,9 +937,19 @@ def concretize(self):
         force = False
 
         while changed:
-            changes = (self.normalize(force=force),
+#debugging code
-                       self._expand_virtual_packages(),
+#            print self, "raw"
-                       self._concretize_helper())
+            a = self.normalize(force=force)
+#            print self, "normal"
+            b = self._expand_virtual_packages()
+#            print self, "expanded"
+            c = self._concretize_helper()
+#            print self, "concrete-ish"
+            changes = (a,b,c)
+#            print a, b, c
+#            changes = (self.normalize(force=force),
+#                       self._expand_virtual_packages(),
+#                       self._concretize_helper())
             changed = any(changes)
             force=True
 
@@ -1058,7 +1162,6 @@ def _normalize_helper(self, visited, spec_deps, provider_index):
             for dep_name in pkg.dependencies:
                 # Do we depend on dep_name?  If so pkg_dep is not None.
                 pkg_dep = self._evaluate_dependency_conditions(dep_name)
-
                 # If pkg_dep is a dependency, merge it.
                 if pkg_dep:
                     changed |= self._merge_dependency(
@@ -1177,6 +1280,8 @@ def constrain(self, other, deps=True):
         changed |= self.versions.intersect(other.versions)
         changed |= self.variants.constrain(other.variants)
 
+        changed |= self.compiler_flags.constrain(other.compiler_flags)
+
         old = self.architecture
         self.architecture = self.architecture or other.architecture
         changed |= (self.architecture != old)
@@ -1304,6 +1409,9 @@ def satisfies(self, other, deps=True, strict=False):
         elif strict and (other.architecture and not self.architecture):
             return False
 
+        if not self.compiler_flags.satisfies(other.compiler_flags, strict=strict):
+            return False
+
         # If we need to descend into dependencies, do it, otherwise we're done.
         if deps:
             deps_strict = strict
@@ -1378,6 +1486,7 @@ def _dup(self, other, **kwargs):
         self.versions = other.versions.copy()
         self.architecture = other.architecture
         self.compiler = other.compiler.copy() if other.compiler else None
+        self.compiler_flags = other.compiler_flags.copy()
         self.dependents = DependencyMap()
         self.dependencies = DependencyMap()
         self.variants = other.variants.copy()
@@ -1499,9 +1608,11 @@ def ne_dag(self, other):
 
     def _cmp_node(self):
         """Comparison key for just *this node* and not its deps."""
+#        if self.compiler:
+#            return (self.name, self.versions, self.variants,
+#                    self.architecture, self.compiler._cmp_key())
         return (self.name, self.versions, self.variants,
-                self.architecture, self.compiler)
+                self.architecture, self.compiler, self.compiler_flags)
-
 
     def eq_node(self, other):
         """Equality with another spec, not including dependencies."""
@@ -1518,7 +1629,7 @@ def _cmp_key(self):
            considering structure.  This is the default, as
            normalization will restore structure.
         """
-        return self._cmp_node() + (self.sorted_deps(),)
+        return self._cmp_node() + (self.sorted_deps())
 
 
     def colorized(self):
@@ -1533,7 +1644,7 @@ def format(self, format_string='$_$@$%@$+$=', **kwargs):
                $@   Version
                $%   Compiler
                $%@  Compiler & compiler version
-               $+   Options
+               $+   Options & compiler flags
                $=   Architecture
                $#   7-char prefix of DAG hash
                $$   $
@@ -1587,9 +1698,11 @@ def write(s, c):
                 elif c == '+':
                     if self.variants:
                         write(fmt % str(self.variants), c)
+                    if self.compiler_flags:
+                        write(fmt % str(self.compiler_flags), '%')
                 elif c == '=':
                     if self.architecture:
-                        write(fmt % (c + str(self.architecture)), c)
+                        write(fmt % ('+arch' + c + str(self.architecture)), c)
                 elif c == '#':
                     out.write('-' + fmt % (self.dag_hash(7)))
                 elif c == '$':
@@ -1669,7 +1782,7 @@ def __repr__(self):
 #
 # These are possible token types in the spec grammar.
 #
-HASH, DEP, AT, COLON, COMMA, ON, OFF, PCT, EQ, ID = range(10)
+HASH, DEP, AT, COLON, COMMA, ON, OFF, PCT, EQ, QT, ID = range(11)
 
 class SpecLexer(spack.parse.Lexer):
     """Parses tokens that make up spack specs."""
@@ -1687,6 +1800,8 @@ def __init__(self):
             (r'\=',        lambda scanner, val: self.token(EQ,    val)),
             # This is more liberal than identifier_re (see above).
             # Checked by check_identifier() for better error messages.
+            (r'([\"\'])(?:(?=(\\?))\2.)*?\1',lambda scanner, val: self.token(QT, val)),
+#            (r'([\"\'])([^\1]+?)(\1)',lambda scanner, val: self.token(QT, val)),
             (r'\w[\w.-]*', lambda scanner, val: self.token(ID,    val)),
             (r'\s+',       lambda scanner, val: None)])
 
@@ -1726,7 +1841,10 @@ def do_parse(self):
                     self.check_identifier()
                     name = self.token.value
                     if self.accept(EQ):
-                        self.expect(ID)
+                        if self.accept(QT):
+                            self.token.value = self.token.value[1:-1]
+                        else:
+                            self.expect(ID)
                         specs[-1]._add_flag(name,self.token.value)
                     else:
                         specs[-1]._add_variant(self.variant(name),True)
@@ -1741,10 +1859,10 @@ def do_parse(self):
         except spack.parse.ParseError, e:
             raise SpecParseError(e)
 
-        for top_spec in specs:
+#        for top_spec in specs:
-            for spec in top_spec.traverse():
+#            for spec in top_spec.traverse():
-                if 'arch' in spec.variants:
+#                if 'arch' in spec.variants:
-                    spec.architecture = spec.variants['arch']
+#                    spec.architecture = spec.variants['arch']
         return specs
 
 
@@ -1783,6 +1901,7 @@ def empty_spec(self):
         spec.variants = VariantMap(spec)
         spec.architecture = None
         spec.compiler = None
+        spec.compiler_flags = FlagMap(spec)
         spec.dependents   = DependencyMap()
         spec.dependencies = DependencyMap()
 
@@ -1793,7 +1912,10 @@ def empty_spec(self):
                 self.check_identifier()
                 name = self.token.value
                 if self.accept(EQ):
-                    self.expect(ID)
+                    if self.accept(QT):
+                        self.token.value = self.token.value[1:-1]
+                    else:
+                        self.expect(ID)
                     spec._add_flag(name,self.token.value)
                 else:
                     spec._add_variant(self.variant(name),True)
@@ -1821,6 +1943,7 @@ def spec(self):
         spec.variants = VariantMap(spec)
         spec.architecture = None
         spec.compiler = None
+        spec.compiler_flags = FlagMap(spec)
         spec.dependents   = DependencyMap()
         spec.dependencies = DependencyMap()
 
@@ -1844,7 +1967,10 @@ def spec(self):
                 self.check_identifier()
                 name = self.token.value
                 if self.accept(EQ):
-                    self.expect(ID)
+                    if self.accept(QT):
+                        self.token.value = self.token.value[1:-1]
+                    else:
+                        self.expect(ID)
                     spec._add_flag(name,self.token.value)
                 else:
                     spec._add_variant(self.variant(name),True)
@@ -1916,6 +2042,7 @@ def compiler(self):
         compiler = CompilerSpec.__new__(CompilerSpec)
         compiler.name = self.token.value
         compiler.versions = VersionList()
+#        compiler.flags = {'cflags':None,'cxxflags':None,'fflags':None,'ldflags':None}
         if self.accept(AT):
             vlist = self.version_list()
             for version in vlist:

lib/spack/spack/test/spec_dag.py

@@ -426,7 +426,6 @@ def test_copy_concretized(self):
         orig.concretize()
         copy = orig.copy()
 
-        print orig
         self.check_links(copy)
 
         self.assertEqual(orig, copy)

lib/spack/spack/test/spec_syntax.py

@@ -70,7 +70,6 @@ def check_parse(self, expected, spec=None):
             spec = expected
         output = spack.spec.parse(spec)
         parsed = (" ".join(str(spec) for spec in output))
-        print output, parsed
         self.assertEqual(expected, parsed)