add/remove/list working for new config format.

- mirrors.yaml now uses dict order for precedence, instead of lists of
  dicts.

- spack.cmd now specifies default scope for add/remove and for list
  with `default_modify_scope` and `default_list_scope`.
  - commands that only read or list default to all scopes (merged)
  - commands that modify configs modify user scope (highest
    precedence) by default
  - These vars are used in setup_paraser for mirror/repo/compiler.

- Spack's argparse supports aliases now.
  - added 'rm' alias for `spack [repo|compiler|mirror] remove`

8 files changed, 221 insertions, 109 deletions

diff --git a/lib/spack/spack/__init__.py b/lib/spack/spack/__init__.py
index 973ba64b96..6c4a15aaab 100644
--- a/lib/spack/spack/__init__.py
+++ b/lib/spack/spack/__init__.py
@@ -56,15 +56,12 @@ etc_path       = join_path(prefix, "etc")
 # Set up the default packages database.
 #
 import spack.repository
-_repo_paths = spack.config.get_repos_config()
-if not _repo_paths:
-    tty.die("Spack configuration contains no package repositories.")
-
 try:
-    repo = spack.repository.RepoPath(*_repo_paths)
+    repo = spack.repository.RepoPath()
     sys.meta_path.append(repo)
-except spack.repository.BadRepoError, e:
-    tty.die('Bad repository. %s' % e.message)
+except spack.repository.RepoError, e:
+    tty.error('while initializing Spack RepoPath:')
+    tty.die(e.message)
 
 #
 # Set up the installed packages database
diff --git a/lib/spack/spack/cmd/__init__.py b/lib/spack/spack/cmd/__init__.py
index 926e7ac14a..6c635a1e6c 100644
--- a/lib/spack/spack/cmd/__init__.py
+++ b/lib/spack/spack/cmd/__init__.py
@@ -31,6 +31,15 @@ from llnl.util.lang import attr_setdefault
 
 import spack
 import spack.spec
+import spack.config
+
+#
+# Settings for commands that modify configuration
+#
+# Commands that modify confguration By default modify the *highest* priority scope.
+default_modify_scope = spack.config.highest_precedence_scope().name
+# Commands that list confguration list *all* scopes by default.
+default_list_scope = None
 
 # cmd has a submodule called "list" so preserve the python list module
 python_list = list
diff --git a/lib/spack/spack/cmd/compiler.py b/lib/spack/spack/cmd/compiler.py
index a3860abf76..75b51f6b49 100644
--- a/lib/spack/spack/cmd/compiler.py
+++ b/lib/spack/spack/cmd/compiler.py
@@ -42,25 +42,31 @@ def setup_parser(subparser):
     sp = subparser.add_subparsers(
         metavar='SUBCOMMAND', dest='compiler_command')
 
+    scopes = spack.config.config_scopes
+
+    # Add
     add_parser = sp.add_parser('add', help='Add compilers to the Spack configuration.')
     add_parser.add_argument('add_paths', nargs=argparse.REMAINDER)
-    add_parser.add_argument('--scope', choices=spack.config.config_scopes,
+    add_parser.add_argument('--scope', choices=scopes, default=spack.cmd.default_modify_scope,
                             help="Configuration scope to modify.")
 
-    remove_parser = sp.add_parser('remove', help='Remove compiler by spec.')
+    # Remove
+    remove_parser = sp.add_parser('remove', aliases=['rm'], help='Remove compiler by spec.')
     remove_parser.add_argument(
         '-a', '--all', action='store_true', help='Remove ALL compilers that match spec.')
     remove_parser.add_argument('compiler_spec')
-    remove_parser.add_argument('--scope', choices=spack.config.config_scopes,
+    remove_parser.add_argument('--scope', choices=scopes, default=spack.cmd.default_modify_scope,
                                help="Configuration scope to modify.")
 
+    # List
     list_parser = sp.add_parser('list', help='list available compilers')
-    list_parser.add_argument('--scope', choices=spack.config.config_scopes,
+    list_parser.add_argument('--scope', choices=scopes, default=spack.cmd.default_list_scope,
                              help="Configuration scope to read from.")
 
+    # Info
     info_parser = sp.add_parser('info', help='Show compiler paths.')
     info_parser.add_argument('compiler_spec')
-    info_parser.add_argument('--scope', choices=spack.config.config_scopes,
+    info_parser.add_argument('--scope', choices=scopes, default=spack.cmd.default_list_scope,
                              help="Configuration scope to read from.")
 
 
@@ -132,6 +138,7 @@ def compiler_list(args):
 def compiler(parser, args):
     action = { 'add'    : compiler_add,
                'remove' : compiler_remove,
+               'rm'     : compiler_remove,
                'info'   : compiler_info,
                'list'   : compiler_list }
     action[args.compiler_command](args)
diff --git a/lib/spack/spack/cmd/mirror.py b/lib/spack/spack/cmd/mirror.py
index 946b50350b..885483a840 100644
--- a/lib/spack/spack/cmd/mirror.py
+++ b/lib/spack/spack/cmd/mirror.py
@@ -36,6 +36,7 @@ import spack.config
 import spack.mirror
 from spack.spec import Spec
 from spack.error import SpackError
+from spack.util.spack_yaml import syaml_dict
 
 description = "Manage mirrors."
 
@@ -47,6 +48,7 @@ def setup_parser(subparser):
     sp = subparser.add_subparsers(
         metavar='SUBCOMMAND', dest='mirror_command')
 
+    # Create
     create_parser = sp.add_parser('create', help=mirror_create.__doc__)
     create_parser.add_argument('-d', '--directory', default=None,
                                help="Directory in which to create mirror.")
@@ -60,22 +62,29 @@ def setup_parser(subparser):
         '-o', '--one-version-per-spec', action='store_const', const=1, default=0,
         help="Only fetch one 'preferred' version per spec, not all known versions.")
 
+    scopes = spack.config.config_scopes
 
+    # Add
     add_parser = sp.add_parser('add', help=mirror_add.__doc__)
     add_parser.add_argument('name', help="Mnemonic name for mirror.")
     add_parser.add_argument(
         'url', help="URL of mirror directory created by 'spack mirror create'.")
-    add_parser.add_argument('--scope', choices=spack.config.config_scopes,
-                            help="Configuration scope to modify.")
+    add_parser.add_argument(
+        '--scope', choices=scopes, default=spack.cmd.default_modify_scope,
+        help="Configuration scope to modify.")
 
-    remove_parser = sp.add_parser('remove', help=mirror_remove.__doc__)
+    # Remove
+    remove_parser = sp.add_parser('remove', aliases=['rm'], help=mirror_remove.__doc__)
     remove_parser.add_argument('name')
-    remove_parser.add_argument('--scope', choices=spack.config.config_scopes,
-                               help="Configuration scope to modify.")
+    remove_parser.add_argument(
+        '--scope', choices=scopes, default=spack.cmd.default_modify_scope,
+        help="Configuration scope to modify.")
 
+    # List
     list_parser = sp.add_parser('list', help=mirror_list.__doc__)
-    list_parser.add_argument('--scope', choices=spack.config.config_scopes,
-                             help="Configuration scope to read from.")
+    list_parser.add_argument(
+        '--scope', choices=scopes, default=spack.cmd.default_list_scope,
+        help="Configuration scope to read from.")
 
 
 def mirror_add(args):
@@ -86,17 +95,18 @@ def mirror_add(args):
 
     mirrors = spack.config.get_config('mirrors', scope=args.scope)
     if not mirrors:
-        mirrors = []
-
-    for m in mirrors:
-        for name, u in m.items():
-            if name == args.name:
-                tty.die("Mirror with name %s already exists." % name)
-            if u == url:
-                tty.die("Mirror with url %s already exists." % url)
-            # should only be one item per mirror dict.
-
-    mirrors.insert(0, { args.name : url })
+        mirrors = syaml_dict()
+
+    for name, u in mirrors.items():
+        if name == args.name:
+            tty.die("Mirror with name %s already exists." % name)
+        if u == url:
+            tty.die("Mirror with url %s already exists." % url)
+        # should only be one item per mirror dict.
+
+    items = [(n,u) for n,u in mirrors.items()]
+    items.insert(0, (args.name, url))
+    mirrors = syaml_dict(items)
     spack.config.update_config('mirrors', mirrors, scope=args.scope)
 
 
@@ -106,15 +116,14 @@ def mirror_remove(args):
 
     mirrors = spack.config.get_config('mirrors', scope=args.scope)
     if not mirrors:
-        mirrors = []
+        mirrors = syaml_dict()
 
-    names = [n for m in mirrors for n,u in m.items()]
-    if not name in names:
+    if not name in mirrors:
         tty.die("No mirror with name %s" % name)
 
-    old_mirror = mirrors.pop(names.index(name))
+    old_value = mirrors.pop(name)
     spack.config.update_config('mirrors', mirrors, scope=args.scope)
-    tty.msg("Removed mirror %s with url %s." % old_mirror.popitem())
+    tty.msg("Removed mirror %s with url %s." % (name, old_value))
 
 
 def mirror_list(args):
@@ -124,14 +133,11 @@ def mirror_list(args):
         tty.msg("No mirrors configured.")
         return
 
-    names = [n for m in mirrors for n,u in m.items()]
-    max_len = max(len(n) for n in names)
+    max_len = max(len(n) for n in mirrors.keys())
     fmt = "%%-%ds%%s" % (max_len + 4)
 
-    for m in mirrors:
-        for name, url in m.items():
-            print fmt % (name, url)
-            # should only be one item per mirror dict.
+    for name in mirrors:
+        print fmt % (name, mirrors[name])
 
 
 def _read_specs_from_file(filename):
@@ -205,6 +211,7 @@ def mirror(parser, args):
     action = { 'create' : mirror_create,
                'add'    : mirror_add,
                'remove' : mirror_remove,
+               'rm'     : mirror_remove,
                'list'   : mirror_list }
 
     action[args.mirror_command](args)
diff --git a/lib/spack/spack/cmd/repo.py b/lib/spack/spack/cmd/repo.py
index 991d306c04..ebe42d0138 100644
--- a/lib/spack/spack/cmd/repo.py
+++ b/lib/spack/spack/cmd/repo.py
@@ -1,5 +1,5 @@
 ##############################################################################
-# Copyright (c) 2013, Lawrence Livermore National Security, LLC.
+# Copyright (c) 2013-2015, Lawrence Livermore National Security, LLC.
 # Produced at the Lawrence Livermore National Laboratory.
 #
 # This file is part of Spack.
@@ -33,12 +33,13 @@ from llnl.util.filesystem import join_path, mkdirp
 import spack.spec
 import spack.config
 from spack.util.environment import get_path
-from spack.repository import packages_dir_name, repo_config_name, Repo
+from spack.repository import *
 
 description = "Manage package source repositories."
 
 def setup_parser(subparser):
     sp = subparser.add_subparsers(metavar='SUBCOMMAND', dest='repo_command')
+    scopes = spack.config.config_scopes
 
     # Create
     create_parser = sp.add_parser('create', help=repo_create.__doc__)
@@ -49,6 +50,25 @@ def setup_parser(subparser):
 
     # List
     list_parser = sp.add_parser('list', help=repo_list.__doc__)
+    list_parser.add_argument(
+        '--scope', choices=scopes, default=spack.cmd.default_list_scope,
+        help="Configuration scope to read from.")
+
+    # Add
+    add_parser = sp.add_parser('add', help=repo_add.__doc__)
+    add_parser.add_argument('path', help="Path to a Spack package repository directory.")
+    add_parser.add_argument(
+        '--scope', choices=scopes, default=spack.cmd.default_modify_scope,
+        help="Configuration scope to modify.")
+
+    # Remove
+    remove_parser = sp.add_parser('remove', help=repo_remove.__doc__, aliases=['rm'])
+    remove_parser.add_argument(
+        'path_or_namespace',
+        help="Path or namespace of a Spack package repository.")
+    remove_parser.add_argument(
+        '--scope', choices=scopes, default=spack.cmd.default_modify_scope,
+        help="Configuration scope to modify.")
 
 
 def repo_create(args):
@@ -104,25 +124,87 @@ def repo_create(args):
 
 
 def repo_add(args):
-    """Remove a package source from the Spack configuration"""
-    # FIXME: how to deal with this with the current config architecture?
-    # FIXME: Repos do not have mnemonics, which I assumed would be simpler... should they have them after all?
+    """Add a package source to the Spack configuration"""
+    path = args.path
+
+    # check if the path is relative to the spack directory.
+    real_path = path
+    if path.startswith('$spack'):
+        real_path = spack.repository.substitute_spack_prefix(path)
+    elif not os.path.isabs(real_path):
+        real_path = os.path.abspath(real_path)
+        path = real_path
+
+    # check if the path exists
+    if not os.path.exists(real_path):
+        tty.die("No such file or directory: '%s'." % path)
+
+    # Make sure the path is a directory.
+    if not os.path.isdir(real_path):
+        tty.die("Not a Spack repository: '%s'." % path)
+
+    # Make sure it's actually a spack repository by constructing it.
+    repo = Repo(real_path)
+
+    # If that succeeds, finally add it to the configuration.
+    repos = spack.config.get_config('repos', args.scope)
+    if not repos: repos = []
+
+    if repo.root in repos or path in repos:
+        tty.die("Repository is already registered with Spack: '%s'" % path)
+
+    repos.insert(0, path)
+    spack.config.update_config('repos', repos, args.scope)
+    tty.msg("Created repo with namespace '%s'." % repo.namespace)
 
 
 def repo_remove(args):
-    """Remove a package source from the Spack configuration"""
-    # FIXME: see above.
+    """Remove a repository from the Spack configuration."""
+    repos = spack.config.get_config('repos', args.scope)
+    path_or_namespace = args.path_or_namespace
+
+    # If the argument is a path, remove that repository from config.
+    path = os.path.abspath(path_or_namespace)
+    if path in repos:
+        repos.remove(path)
+        spack.config.update_config('repos', repos, args.scope)
+        tty.msg("Removed repository '%s'." % path)
+        return
+
+    # If it is a namespace, remove corresponding repo
+    for path in repos:
+        try:
+            repo = Repo(path)
+            if repo.namespace == path_or_namespace:
+                repos.remove(repo.root)
+                spack.config.update_config('repos', repos, args.scope)
+                tty.msg("Removed repository '%s' with namespace %s."
+                        % (repo.root, repo.namespace))
+                return
+        except RepoError as e:
+            continue
+
+    tty.die("No repository with path or namespace: '%s'"
+            % path_or_namespace)
 
 
 def repo_list(args):
     """List package sources and their mnemoics"""
-    roots = spack.config.get_repos_config()
-    repos = [Repo(r) for r in roots]
+    roots = spack.config.get_config('repos', args.scope)
+    repos = []
+    for r in roots:
+        try:
+            repos.append(Repo(r))
+        except RepoError as e:
+            continue
 
     msg = "%d package repositor" % len(repos)
     msg += "y." if len(repos) == 1 else "ies."
     tty.msg(msg)
 
+    if not repos:
+        return
+
     max_ns_len = max(len(r.namespace) for r in repos)
     for repo in repos:
         fmt = "%%-%ds%%s" % (max_ns_len + 4)
@@ -131,5 +213,8 @@ def repo_list(args):
 
 def repo(parser, args):
     action = { 'create' : repo_create,
-               'list'   : repo_list }
+               'list'   : repo_list,
+               'add'    : repo_add,
+               'remove' : repo_remove,
+               'rm'     : repo_remove}
     action[args.repo_command](args)
diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index d266d2e23f..c53dcbc405 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -204,6 +204,11 @@ ConfigScope('site', os.path.join(spack.etc_path, 'spack')),
 ConfigScope('user', os.path.expanduser('~/.spack'))
 
 
+def highest_precedence_scope():
+    """Get the scope with highest precedence (prefs will override others)."""
+    return config_scopes.values()[-1]
+
+
 def validate_scope(scope):
     """Ensure that scope is valid, and return a valid scope if it is None.
 
@@ -214,7 +219,7 @@ def validate_scope(scope):
     """
     if scope is None:
         # default to the scope with highest precedence.
-        return config_scopes.values()[-1]
+        return highest_precedence_scope()
 
     elif scope in config_scopes:
         return config_scopes[scope]
@@ -287,15 +292,10 @@ def _merge_yaml(dest, source):
         dest[:] = source + [x for x in dest if x not in seen]
         return dest
 
-    # Source dict is merged into dest. Extra ':' means overwrite.
+    # Source dict is merged into dest.
     elif they_are(dict):
         for sk, sv in source.iteritems():
-            # allow total override with, e.g., repos::
-            override = sk.endswith(':')
-            if override:
-                sk = sk.rstrip(':')
-
-            if override or not sk in dest:
+            if not sk in dest:
                 dest[sk] = copy.copy(sv)
             else:
                 dest[sk] = _merge_yaml(dest[sk], source[sk])
@@ -306,18 +306,13 @@ def _merge_yaml(dest, source):
         return copy.copy(source)
 
 
-def substitute_spack_prefix(path):
-    """Replaces instances of $spack with Spack's prefix."""
-    return path.replace('$spack', spack.prefix)
-
-
 def get_config(section, scope=None):
     """Get configuration settings for a section.
 
        Strips off the top-level section name from the YAML dict.
     """
     validate_section(section)
-    merged_section = {}
+    merged_section = syaml.syaml_dict()
 
     if scope is None:
         scopes = config_scopes.values()
@@ -327,37 +322,25 @@ def get_config(section, scope=None):
     for scope in scopes:
         # read potentially cached data from the scope.
         data = scope.get_section(section)
-        if not data or not section in data:
-            continue
 
-        # extract data under the section name header
-        data = data[section]
+        # Skip empty configs
+        if not data or not isinstance(data, dict):
+            continue
 
-        # ignore empty sections for easy commenting of single-line configs.
-        if not data:
+        # Allow complete override of site config with '<section>::'
+        override_key = section + ':'
+        if not (section in data or override_key in data):
+            tty.warn("Skipping bad configuration file: '%s'" % scope.path)
             continue
 
-        # merge config data from scopes.
-        merged_section = _merge_yaml(merged_section, data)
+        if override_key in data:
+            merged_section = data[override_key]
+        else:
+            merged_section = _merge_yaml(merged_section, data[section])
 
     return merged_section
 
 
-def get_repos_config():
-    repo_list = get_config('repos')
-    if repo_list is None:
-        return []
-
-    if not isinstance(repo_list, list):
-        tty.die("Bad repository configuration. 'repos' element does not contain a list.")
-
-    def expand_repo_path(path):
-        path = substitute_spack_prefix(path)
-        path = os.path.expanduser(path)
-        return path
-    return [expand_repo_path(repo) for repo in repo_list]
-
-
 def get_config_filename(scope, section):
     """For some scope and section, get the name of the configuration file"""
     scope = validate_scope(scope)
diff --git a/lib/spack/spack/repository.py b/lib/spack/spack/repository.py
index 4e91855db0..3367572ef5 100644
--- a/lib/spack/spack/repository.py
+++ b/lib/spack/spack/repository.py
@@ -36,6 +36,7 @@ import llnl.util.tty as tty
 from llnl.util.filesystem import join_path
 
 import spack.error
+import spack.config
 import spack.spec
 from spack.virtual import ProviderIndex
 from spack.util.naming import *
@@ -53,6 +54,7 @@ repo_config_name   = 'repo.yaml'   # Top-level filename for repo config.
 packages_dir_name  = 'packages'    # Top-level repo directory containing pkgs.
 package_file_name  = 'package.py'  # Filename for packages in a repository.
 
+
 def _autospec(function):
     """Decorator that automatically converts the argument of a single-arg
        function to a Spec."""
@@ -71,6 +73,11 @@ def _make_namespace_module(ns):
     return module
 
 
+def substitute_spack_prefix(path):
+    """Replaces instances of $spack with Spack's prefix."""
+    return path.replace('$spack', spack.prefix)
+
+
 class RepoPath(object):
     """A RepoPath is a list of repos that function as one.
 
@@ -89,10 +96,20 @@ class RepoPath(object):
         self._all_package_names = []
         self._provider_index = None
 
+        # If repo_dirs is empty, just use the configuration
+        if not repo_dirs:
+            repo_dirs = spack.config.get_config('repos')
+            if not repo_dirs:
+                raise NoRepoConfiguredError(
+                    "Spack configuration contains no package repositories.")
+
         # Add each repo to this path.
         for root in repo_dirs:
-            repo = Repo(root, self.super_namespace)
-            self.put_last(repo)
+            try:
+                repo = Repo(root, self.super_namespace)
+                self.put_last(repo)
+            except RepoError as e:
+                tty.warn("Failed to initialize repository at '%s'." % root, e.message)
 
 
     def swap(self, other):
@@ -121,12 +138,12 @@ class RepoPath(object):
 
         """
         if repo.root in self.by_path:
-            raise DuplicateRepoError("Package repos are the same",
-                                     repo, self.by_path[repo.root])
+            raise DuplicateRepoError("Duplicate repository: '%s'" % repo.root)
 
         if repo.namespace in self.by_namespace:
-            raise DuplicateRepoError("Package repos cannot provide the same namespace",
-                                     repo, self.by_namespace[repo.namespace])
+            raise DuplicateRepoError(
+                "Package repos '%s' and '%s' both provide namespace %s."
+                % (repo.root, self.by_namespace[repo.namespace].root, repo.namespace))
 
         # Add repo to the pkg indexes
         self.by_namespace[repo.full_namespace] = repo
@@ -292,7 +309,8 @@ class Repo(object):
 
         """
         # Root directory, containing _repo.yaml and package dirs
-        self.root = root
+        # Allow roots to by spack-relative by starting with '$spack'
+        self.root = substitute_spack_prefix(root)
 
         # super-namespace for all packages in the Repo
         self.super_namespace = namespace
@@ -629,13 +647,27 @@ class Repo(object):
         return self.exists(pkg_name)
 
 
-class BadRepoError(spack.error.SpackError):
+class RepoError(spack.error.SpackError):
+    """Superclass for repository-related errors."""
+
+
+class NoRepoConfiguredError(RepoError):
+    """Raised when there are no repositories configured."""
+
+
+class BadRepoError(RepoError):
     """Raised when repo layout is invalid."""
-    def __init__(self, msg):
-        super(BadRepoError, self).__init__(msg)
 
 
-class UnknownPackageError(spack.error.SpackError):
+class DuplicateRepoError(RepoError):
+    """Raised when duplicate repos are added to a RepoPath."""
+
+
+class PackageLoadError(spack.error.SpackError):
+    """Superclass for errors related to loading packages."""
+
+
+class UnknownPackageError(PackageLoadError):
     """Raised when we encounter a package spack doesn't have."""
     def __init__(self, name, repo=None):
         msg = None
@@ -647,14 +679,7 @@ class UnknownPackageError(spack.error.SpackError):
         self.name = name
 
 
-class DuplicateRepoError(spack.error.SpackError):
-    """Raised when duplicate repos are added to a RepoPath."""
-    def __init__(self, msg, repo1, repo2):
-        super(UnknownPackageError, self).__init__(
-            "%s: %s, %s" % (msg, repo1, repo2))
-
-
-class FailedConstructorError(spack.error.SpackError):
+class FailedConstructorError(PackageLoadError):
     """Raised when a package's class constructor fails."""
     def __init__(self, name, exc_type, exc_obj, exc_tb):
         super(FailedConstructorError, self).__init__(
diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py
index a9631d4b62..61f9846835 100644
--- a/lib/spack/spack/stage.py
+++ b/lib/spack/spack/stage.py
@@ -244,8 +244,7 @@ class Stage(object):
         # TODO: move mirror logic out of here and clean it up!
         if self.mirror_path:
             mirrors = spack.config.get_config('mirrors')
-            mirrors = [(n,u) for m in mirrors for n,u in m.items()]
-            urls = [urljoin(u, self.mirror_path) for name, u in mirrors]
+            urls = [urljoin(u, self.mirror_path) for name, u in mirrors.items()]
 
             digest = None
             if isinstance(self.fetcher, fs.URLFetchStrategy):