Bugfix: '::' only worked on top-level key in config.

- generalized and fixed to work with any key in YAML file
- simplified schema writing, as well
- add more unit tests for the config system
- Rename test/yaml.py to test/spack_yaml.py
  - Add test/yaml.pyc to ignored pyc files.

12 files changed, 257 insertions, 152 deletions

diff --git a/bin/spack b/bin/spack
index 29991c070d..1f5dec0b3d 100755
--- a/bin/spack
+++ b/bin/spack
@@ -72,7 +72,8 @@ orphaned_pyc_files = [
     os.path.join(SPACK_LIB_PATH, 'spack', 'platforms', 'cray_xc.pyc'),
     os.path.join(SPACK_LIB_PATH, 'spack', 'cmd', 'package-list.pyc'),
     os.path.join(SPACK_LIB_PATH, 'spack', 'cmd', 'test-install.pyc'),
-    os.path.join(SPACK_LIB_PATH, 'spack', 'cmd', 'url-parse.pyc')
+    os.path.join(SPACK_LIB_PATH, 'spack', 'cmd', 'url-parse.pyc'),
+    os.path.join(SPACK_LIB_PATH, 'spack', 'test', 'yaml.pyc')
 ]
 
 for pyc_file in orphaned_pyc_files:
diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index c47eb68e09..989b3da169 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -266,7 +266,7 @@ def _read_config_file(filename, schema):
     try:
         tty.debug("Reading config file %s" % filename)
         with open(filename) as f:
-            data = syaml.load(f)
+            data = _mark_overrides(syaml.load(f))
 
         if data:
             validate_section(data, schema)
@@ -288,6 +288,34 @@ def clear_config_caches():
         scope.clear()
 
 
+def override(string):
+    """Test if a spack YAML string is an override.
+
+    See ``spack_yaml`` for details.  Keys in Spack YAML can end in `::`,
+    and if they do, their values completely replace lower-precedence
+    configs instead of merging into them.
+
+    """
+    return hasattr(string, 'override') and string.override
+
+
+def _mark_overrides(data):
+    if isinstance(data, list):
+        return [_mark_overrides(elt) for elt in data]
+
+    elif isinstance(data, dict):
+        marked = {}
+        for key, val in data.iteritems():
+            if isinstance(key, basestring) and key.endswith(':'):
+                key = syaml.syaml_str(key[:-1])
+                key.override = True
+            marked[key] = _mark_overrides(val)
+        return marked
+
+    else:
+        return data
+
+
 def _merge_yaml(dest, source):
     """Merges source into dest; entries in source take precedence over dest.
 
@@ -320,9 +348,11 @@ def _merge_yaml(dest, source):
     # Source dict is merged into dest.
     elif they_are(dict):
         for sk, sv in source.iteritems():
-            if sk not in dest:
+            if override(sk) or sk not in dest:
+                # if sk ended with ::, or if it's new, completely override
                 dest[sk] = copy.copy(sv)
             else:
+                # otherwise, merge the YAML
                 dest[sk] = _merge_yaml(dest[sk], source[sk])
         return dest
 
@@ -371,18 +401,18 @@ def get_config(section, scope=None):
         if not data or not isinstance(data, dict):
             continue
 
-        # Allow complete override of site config with '<section>::'
-        override_key = section + ':'
-        if not (section in data or override_key in data):
+        if section not in data:
             tty.warn("Skipping bad configuration file: '%s'" % scope.path)
             continue
 
-        if override_key in data:
-            merged_section = data[override_key]
-        else:
-            merged_section = _merge_yaml(merged_section, data[section])
+        merged_section = _merge_yaml(merged_section, data)
+
+    # no config files -- empty config.
+    if section not in merged_section:
+        return {}
 
-    return merged_section
+    # take the top key off before returning.
+    return merged_section[section]
 
 
 def get_config_filename(scope, section):
diff --git a/lib/spack/spack/schema/compilers.py b/lib/spack/spack/schema/compilers.py
index 1c7894d675..ea1071729f 100644
--- a/lib/spack/spack/schema/compilers.py
+++ b/lib/spack/spack/schema/compilers.py
@@ -35,7 +35,7 @@ schema = {
     'type': 'object',
     'additionalProperties': False,
     'patternProperties': {
-        'compilers:?': {  # optional colon for overriding site config.
+        'compilers': {
             'type': 'array',
             'items': {
                 'compiler': {
diff --git a/lib/spack/spack/schema/config.py b/lib/spack/spack/schema/config.py
index 31d4b8a8a8..e51fa69afe 100644
--- a/lib/spack/spack/schema/config.py
+++ b/lib/spack/spack/schema/config.py
@@ -49,7 +49,6 @@ schema = {
                 },
                 'module_roots': {
                     'type': 'object',
-                    'default': {},
                     'additionalProperties': False,
                     'properties': {
                         'tcl': {'type': 'string'},
@@ -59,18 +58,9 @@ schema = {
                 },
                 'source_cache': {'type': 'string'},
                 'misc_cache': {'type': 'string'},
-                'verify_ssl': {
-                    'type': 'boolean',
-                    'default': True,
-                },
-                'checksum': {
-                    'type': 'boolean',
-                    'default': True,
-                },
-                'dirty': {
-                    'type': 'boolean',
-                    'default': False,
-                },
+                'verify_ssl': {'type': 'boolean'},
+                'checksum': {'type': 'boolean'},
+                'dirty': {'type': 'boolean'},
             }
         },
     },
diff --git a/lib/spack/spack/schema/mirrors.py b/lib/spack/spack/schema/mirrors.py
index 9aa3a0f747..60b865bb42 100644
--- a/lib/spack/spack/schema/mirrors.py
+++ b/lib/spack/spack/schema/mirrors.py
@@ -35,7 +35,7 @@ schema = {
     'type': 'object',
     'additionalProperties': False,
     'patternProperties': {
-        r'mirrors:?': {
+        r'mirrors': {
             'type': 'object',
             'default': {},
             'additionalProperties': False,
diff --git a/lib/spack/spack/schema/modules.py b/lib/spack/spack/schema/modules.py
index 2e776635d2..2059e14fa6 100644
--- a/lib/spack/spack/schema/modules.py
+++ b/lib/spack/spack/schema/modules.py
@@ -127,7 +127,7 @@ schema = {
         }
     },
     'patternProperties': {
-        r'modules:?': {
+        r'modules': {
             'type': 'object',
             'default': {},
             'additionalProperties': False,
diff --git a/lib/spack/spack/schema/packages.py b/lib/spack/spack/schema/packages.py
index 29c62150ef..bf5648b1b7 100644
--- a/lib/spack/spack/schema/packages.py
+++ b/lib/spack/spack/schema/packages.py
@@ -35,7 +35,7 @@ schema = {
     'type': 'object',
     'additionalProperties': False,
     'patternProperties': {
-        r'packages:?': {
+        r'packages': {
             'type': 'object',
             'default': {},
             'additionalProperties': False,
diff --git a/lib/spack/spack/schema/repos.py b/lib/spack/spack/schema/repos.py
index 74dcee7cdc..c7a3495ae1 100644
--- a/lib/spack/spack/schema/repos.py
+++ b/lib/spack/spack/schema/repos.py
@@ -35,7 +35,7 @@ schema = {
     'type': 'object',
     'additionalProperties': False,
     'patternProperties': {
-        r'repos:?': {
+        r'repos': {
             'type': 'array',
             'default': [],
             'items': {
diff --git a/lib/spack/spack/test/__init__.py b/lib/spack/spack/test/__init__.py
index 457e5db9dc..c0a4c7354f 100644
--- a/lib/spack/spack/test/__init__.py
+++ b/lib/spack/spack/test/__init__.py
@@ -78,7 +78,7 @@ test_names = [
     'url_substitution',
     'versions',
     'provider_index',
-    'yaml',
+    'spack_yaml',
     # This test needs to be last until global compiler cache is fixed.
     'cmd.test_compiler_cmd',
 ]
diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py
index d5e1791b40..adc0795916 100644
--- a/lib/spack/spack/test/config.py
+++ b/lib/spack/spack/test/config.py
@@ -25,6 +25,7 @@
 import os
 import shutil
 import getpass
+import yaml
 from tempfile import mkdtemp
 
 import spack
@@ -34,109 +35,136 @@ from ordereddict_backport import OrderedDict
 from spack.test.mock_packages_test import *
 
 # Some sample compiler config data
-a_comps = [
-    {'compiler': {
-        'paths': {
-            "cc": "/gcc473",
-            "cxx": "/g++473",
-            "f77": None,
-            "fc": None
-        },
-        'modules': None,
-        'spec': 'gcc@4.7.3',
-        'operating_system': 'CNL10'
-    }},
-    {'compiler': {
-        'paths': {
-            "cc": "/gcc450",
-            "cxx": "/g++450",
-            "f77": 'gfortran',
-            "fc": 'gfortran'
-        },
-        'modules': None,
-        'spec': 'gcc@4.5.0',
-        'operating_system': 'CNL10'
-    }},
-    {'compiler': {
-        'paths': {
-            "cc": "/gcc422",
-            "cxx": "/g++422",
-            "f77": 'gfortran',
-            "fc": 'gfortran'
-        },
-        'flags': {
-            "cppflags": "-O0 -fpic",
-            "fflags": "-f77",
-        },
-        'modules': None,
-        'spec': 'gcc@4.2.2',
-        'operating_system': 'CNL10'
-    }},
-    {'compiler': {
-        'paths': {
-            "cc": "<overwritten>",
-            "cxx": "<overwritten>",
-            "f77": '<overwritten>',
-            "fc": '<overwritten>'},
-        'modules': None,
-        'spec': 'clang@3.3',
-        'operating_system': 'CNL10'
-    }}
-]
-
-b_comps = [
-    {'compiler': {
-        'paths': {
-            "cc": "/icc100",
-            "cxx": "/icp100",
-            "f77": None,
-            "fc": None
-        },
-        'modules': None,
-        'spec': 'icc@10.0',
-        'operating_system': 'CNL10'
-    }},
-    {'compiler': {
-        'paths': {
-            "cc": "/icc111",
-            "cxx": "/icp111",
-            "f77": 'ifort',
-            "fc": 'ifort'
-        },
-        'modules': None,
-        'spec': 'icc@11.1',
-        'operating_system': 'CNL10'
-    }},
-    {'compiler': {
-        'paths': {
-            "cc": "/icc123",
-            "cxx": "/icp123",
-            "f77": 'ifort',
-            "fc": 'ifort'
-        },
-        'flags': {
-            "cppflags": "-O3",
-            "fflags": "-f77rtl",
-        },
-        'modules': None,
-        'spec': 'icc@12.3',
-        'operating_system': 'CNL10'
-    }},
-    {'compiler': {
-        'paths': {
-            "cc": "<overwritten>",
-            "cxx": "<overwritten>",
-            "f77": '<overwritten>',
-            "fc": '<overwritten>'},
-        'modules': None,
-        'spec': 'clang@3.3',
-        'operating_system': 'CNL10'
-    }}
-]
+a_comps = {
+    'compilers': [
+        {'compiler': {
+            'paths': {
+                "cc": "/gcc473",
+                "cxx": "/g++473",
+                "f77": None,
+                "fc": None
+            },
+            'modules': None,
+            'spec': 'gcc@4.7.3',
+            'operating_system': 'CNL10'
+        }},
+        {'compiler': {
+            'paths': {
+                "cc": "/gcc450",
+                "cxx": "/g++450",
+                "f77": 'gfortran',
+                "fc": 'gfortran'
+            },
+            'modules': None,
+            'spec': 'gcc@4.5.0',
+            'operating_system': 'CNL10'
+        }},
+        {'compiler': {
+            'paths': {
+                "cc": "/gcc422",
+                "cxx": "/g++422",
+                "f77": 'gfortran',
+                "fc": 'gfortran'
+            },
+            'flags': {
+                "cppflags": "-O0 -fpic",
+                "fflags": "-f77",
+            },
+            'modules': None,
+            'spec': 'gcc@4.2.2',
+            'operating_system': 'CNL10'
+        }},
+        {'compiler': {
+            'paths': {
+                "cc": "<overwritten>",
+                "cxx": "<overwritten>",
+                "f77": '<overwritten>',
+                "fc": '<overwritten>'},
+            'modules': None,
+            'spec': 'clang@3.3',
+            'operating_system': 'CNL10'
+        }}
+    ]
+}
+
+b_comps = {
+    'compilers': [
+        {'compiler': {
+            'paths': {
+                "cc": "/icc100",
+                "cxx": "/icp100",
+                "f77": None,
+                "fc": None
+            },
+            'modules': None,
+            'spec': 'icc@10.0',
+            'operating_system': 'CNL10'
+        }},
+        {'compiler': {
+            'paths': {
+                "cc": "/icc111",
+                "cxx": "/icp111",
+                "f77": 'ifort',
+                "fc": 'ifort'
+            },
+            'modules': None,
+            'spec': 'icc@11.1',
+            'operating_system': 'CNL10'
+        }},
+        {'compiler': {
+            'paths': {
+                "cc": "/icc123",
+                "cxx": "/icp123",
+                "f77": 'ifort',
+                "fc": 'ifort'
+            },
+            'flags': {
+                "cppflags": "-O3",
+                "fflags": "-f77rtl",
+            },
+            'modules': None,
+            'spec': 'icc@12.3',
+            'operating_system': 'CNL10'
+        }},
+        {'compiler': {
+            'paths': {
+                "cc": "<overwritten>",
+                "cxx": "<overwritten>",
+                "f77": '<overwritten>',
+                "fc": '<overwritten>'},
+            'modules': None,
+            'spec': 'clang@3.3',
+            'operating_system': 'CNL10'
+        }}
+    ]
+}
 
 # Some Sample repo data
-repos_low = ["/some/path"]
-repos_high = ["/some/other/path"]
+repos_low = {'repos': ["/some/path"]}
+repos_high = {'repos': ["/some/other/path"]}
+
+
+# sample config data
+config_low = {
+    'config': {
+        'install_tree': 'install_tree_path',
+        'build_stage': ['path1', 'path2', 'path3']}}
+
+config_override_all = {
+    'config:': {
+        'install_tree:': 'override_all'}}
+
+config_override_key = {
+    'config': {
+        'install_tree:': 'override_key'}}
+
+config_merge_list = {
+    'config': {
+        'build_stage': ['patha', 'pathb']}}
+
+config_override_list = {
+    'config': {
+        'build_stage:': ['patha', 'pathb']}}
 
 
 class ConfigTest(MockPackagesTest):
@@ -144,19 +172,30 @@ class ConfigTest(MockPackagesTest):
     def setUp(self):
         super(ConfigTest, self).setUp()
         self.tmp_dir = mkdtemp('.tmp', 'spack-config-test-')
-        self.a_comp_specs = [ac['compiler']['spec'] for ac in a_comps]
-        self.b_comp_specs = [bc['compiler']['spec'] for bc in b_comps]
+        self.a_comp_specs = [
+            ac['compiler']['spec'] for ac in a_comps['compilers']]
+        self.b_comp_specs = [
+            bc['compiler']['spec'] for bc in b_comps['compilers']]
 
         spack.config.config_scopes = OrderedDict()
         for priority in ['low', 'high']:
-            spack.config.ConfigScope('test_{0}_priority'.format(priority),
-                                     os.path.join(self.tmp_dir, priority))
+            scope_dir = os.path.join(self.tmp_dir, priority)
+            spack.config.ConfigScope(priority, scope_dir)
 
     def tearDown(self):
         super(ConfigTest, self).tearDown()
         shutil.rmtree(self.tmp_dir, True)
 
-    def check_config(self, comps, *compiler_names):
+    def write_config_file(self, config, data, scope):
+        scope_dir = os.path.join(self.tmp_dir, scope)
+        mkdirp(scope_dir)
+
+        path = os.path.join(scope_dir, config + '.yaml')
+        with open(path, 'w') as f:
+            print yaml
+            yaml.dump(data, f)
+
+    def check_compiler_config(self, comps, *compiler_names):
         """Check that named compilers in comps match Spack's config."""
         config = spack.config.get_config('compilers')
         compiler_list = ['cc', 'cxx', 'f77', 'fc']
@@ -182,43 +221,50 @@ class ConfigTest(MockPackagesTest):
                     self.assertEqual(expected, actual)
 
     def test_write_list_in_memory(self):
-        spack.config.update_config('repos', repos_low, 'test_low_priority')
-        spack.config.update_config('repos', repos_high, 'test_high_priority')
+        spack.config.update_config('repos', repos_low['repos'], scope='low')
+        spack.config.update_config('repos', repos_high['repos'], scope='high')
+
         config = spack.config.get_config('repos')
-        self.assertEqual(config, repos_high + repos_low)
+        self.assertEqual(config, repos_high['repos'] + repos_low['repos'])
 
     def test_write_key_in_memory(self):
         # Write b_comps "on top of" a_comps.
-        spack.config.update_config('compilers', a_comps, 'test_low_priority')
-        spack.config.update_config('compilers', b_comps, 'test_high_priority')
+        spack.config.update_config(
+            'compilers', a_comps['compilers'], scope='low')
+        spack.config.update_config(
+            'compilers', b_comps['compilers'], scope='high')
 
         # Make sure the config looks how we expect.
-        self.check_config(a_comps, *self.a_comp_specs)
-        self.check_config(b_comps, *self.b_comp_specs)
+        self.check_compiler_config(a_comps['compilers'], *self.a_comp_specs)
+        self.check_compiler_config(b_comps['compilers'], *self.b_comp_specs)
 
     def test_write_key_to_disk(self):
         # Write b_comps "on top of" a_comps.
-        spack.config.update_config('compilers', a_comps, 'test_low_priority')
-        spack.config.update_config('compilers', b_comps, 'test_high_priority')
+        spack.config.update_config(
+            'compilers', a_comps['compilers'], scope='low')
+        spack.config.update_config(
+            'compilers', b_comps['compilers'], scope='high')
 
         # Clear caches so we're forced to read from disk.
         spack.config.clear_config_caches()
 
         # Same check again, to ensure consistency.
-        self.check_config(a_comps, *self.a_comp_specs)
-        self.check_config(b_comps, *self.b_comp_specs)
+        self.check_compiler_config(a_comps['compilers'], *self.a_comp_specs)
+        self.check_compiler_config(b_comps['compilers'], *self.b_comp_specs)
 
     def test_write_to_same_priority_file(self):
         # Write b_comps in the same file as a_comps.
-        spack.config.update_config('compilers', a_comps, 'test_low_priority')
-        spack.config.update_config('compilers', b_comps, 'test_low_priority')
+        spack.config.update_config(
+            'compilers', a_comps['compilers'], scope='low')
+        spack.config.update_config(
+            'compilers', b_comps['compilers'], scope='low')
 
         # Clear caches so we're forced to read from disk.
         spack.config.clear_config_caches()
 
         # Same check again, to ensure consistency.
-        self.check_config(a_comps, *self.a_comp_specs)
-        self.check_config(b_comps, *self.b_comp_specs)
+        self.check_compiler_config(a_comps['compilers'], *self.a_comp_specs)
+        self.check_compiler_config(b_comps['compilers'], *self.b_comp_specs)
 
     def check_canonical(self, var, expected):
         """Ensure that <expected> is substituted properly for <var> in strings
@@ -270,3 +316,39 @@ class ConfigTest(MockPackagesTest):
         self.assertEqual(tempdir, canonicalize_path('$tempdir'))
         self.assertEqual(tempdir + '/foo/bar/baz',
                          canonicalize_path('$tempdir/foo/bar/baz'))
+
+    def test_read_config(self):
+        self.write_config_file('config', config_low, 'low')
+        self.assertEqual(spack.config.get_config('config'),
+                         config_low['config'])
+
+    def test_read_config_override_all(self):
+        self.write_config_file('config', config_low, 'low')
+        self.write_config_file('config', config_override_all, 'high')
+        self.assertEqual(spack.config.get_config('config'), {
+            'install_tree': 'override_all'
+        })
+
+    def test_read_config_override_key(self):
+        self.write_config_file('config', config_low, 'low')
+        self.write_config_file('config', config_override_key, 'high')
+        self.assertEqual(spack.config.get_config('config'), {
+            'install_tree': 'override_key',
+            'build_stage': ['path1', 'path2', 'path3']
+        })
+
+    def test_read_config_merge_list(self):
+        self.write_config_file('config', config_low, 'low')
+        self.write_config_file('config', config_merge_list, 'high')
+        self.assertEqual(spack.config.get_config('config'), {
+            'install_tree': 'install_tree_path',
+            'build_stage': ['patha', 'pathb', 'path1', 'path2', 'path3']
+        })
+
+    def test_read_config_override_list(self):
+        self.write_config_file('config', config_low, 'low')
+        self.write_config_file('config', config_override_list, 'high')
+        self.assertEqual(spack.config.get_config('config'), {
+            'install_tree': 'install_tree_path',
+            'build_stage': ['patha', 'pathb']
+        })
diff --git a/lib/spack/spack/test/yaml.py b/lib/spack/spack/test/spack_yaml.py
index dedbd15d10..30ed1672e2 100644
--- a/lib/spack/spack/test/yaml.py
+++ b/lib/spack/spack/test/spack_yaml.py
@@ -56,7 +56,7 @@ test_data = {
     ])}
 
 
-class YamlTest(unittest.TestCase):
+class SpackYamlTest(unittest.TestCase):
 
     def setUp(self):
         self.data = syaml.load(test_file)
diff --git a/lib/spack/spack/util/spack_yaml.py b/lib/spack/spack/util/spack_yaml.py
index 506f56633a..674c79bca1 100644
--- a/lib/spack/spack/util/spack_yaml.py
+++ b/lib/spack/spack/util/spack_yaml.py
@@ -91,7 +91,9 @@ class OrderedLineLoader(Loader):
             value = value.encode('ascii')
         except UnicodeEncodeError:
             pass
+
         value = syaml_str(value)
+
         mark(value, node)
         return value