config: allow user to add configuration scopes on the command line.

- Add command-line scope option to Spack

- Rework structure of main to allow configuration system to raise
  errors more naturally

Co-authored-by: Todd Gamblin <tgamblin@llnl.gov>

4 files changed, 194 insertions, 72 deletions

diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index 2e67f0e1da..eb00c2453c 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -22,6 +22,8 @@
 # License along with this program; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##############################################################################
+from __future__ import print_function
+
 """This module implements Spack's configuration file handling.
 
 This implements Spack's configuration system, which handles merging
@@ -206,6 +208,19 @@ class ConfigScope(object):
         return '<ConfigScope: %s: %s>' % (self.name, self.path)
 
 
+class ImmutableConfigScope(ConfigScope):
+    """A configuration scope that cannot be written to.
+
+    This is used for ConfigScopes passed on the command line.
+    """
+
+    def write_section(self, section):
+        raise ConfigError("Cannot write to immutable scope %s" % self)
+
+    def __repr__(self):
+        return '<ImmutableConfigScope: %s: %s>' % (self.name, self.path)
+
+
 class InternalConfigScope(ConfigScope):
     """An internal configuration scope that is not persisted to a file.
 
@@ -274,9 +289,8 @@ class Configuration(object):
 
     @property
     def file_scopes(self):
-        """List of scopes with an associated file (non-internal scopes)."""
-        return [s for s in self.scopes.values()
-                if not isinstance(s, InternalConfigScope)]
+        """List of writable scopes with an associated file."""
+        return [s for s in self.scopes.values() if type(s) == ConfigScope]
 
     def highest_precedence_scope(self):
         """Non-internal scope with highest precedence."""
@@ -455,17 +469,65 @@ class Configuration(object):
 
 
 @contextmanager
-def override(path, value):
-    """Simple way to override config settings within a context."""
-    overrides = InternalConfigScope('overrides')
+def override(path_or_scope, value=None):
+    """Simple way to override config settings within a context.
+
+    Arguments:
+        path_or_scope (ConfigScope or str): scope or single option to override
+        value (object, optional): value for the single option
+
+    Temporarily push a scope on the current configuration, then remove it
+    after the context completes. If a single option is provided, create
+    an internal config scope for it and push/pop that scope.
+
+    """
+    if isinstance(path_or_scope, ConfigScope):
+        config.push_scope(path_or_scope)
+        yield config
+        config.pop_scope(path_or_scope)
+
+    else:
+        overrides = InternalConfigScope('overrides')
+
+        config.push_scope(overrides)
+        config.set(path_or_scope, value, scope='overrides')
+
+        yield config
+
+        scope = config.pop_scope()
+        assert scope is overrides
+
+
+#: configuration scopes added on the command line
+#: set by ``spack.main.main()``.
+command_line_scopes = []
+
+
+def _add_platform_scope(cfg, scope_type, name, path):
+    """Add a platform-specific subdirectory for the current platform."""
+    platform = spack.architecture.platform().name
+    plat_name = '%s/%s' % (name, platform)
+    plat_path = os.path.join(path, platform)
+    cfg.push_scope(scope_type(plat_name, plat_path))
 
-    config.push_scope(overrides)
-    config.set(path, value, scope='overrides')
 
-    yield config
+def _add_command_line_scopes(cfg, command_line_scopes):
+    """Add additional scopes from the --config-scope argument.
+
+    Command line scopes are named after their position in the arg list.
+    """
+    for i, path in enumerate(command_line_scopes):
+        # We ensure that these scopes exist and are readable, as they are
+        # provided on the command line by the user.
+        if not os.path.isdir(path):
+            raise ConfigError("config scope is not a directory: '%s'" % path)
+        elif not os.access(path, os.R_OK):
+            raise ConfigError("config scope is not readable: '%s'" % path)
 
-    scope = config.pop_scope()
-    assert scope is overrides
+        # name based on order on the command line
+        name = 'cmd_scope_%d' % i
+        cfg.push_scope(ImmutableConfigScope(name, path))
+        _add_platform_scope(cfg, ImmutableConfigScope, name, path)
 
 
 def _config():
@@ -485,16 +547,15 @@ def _config():
     defaults = InternalConfigScope('_builtin', config_defaults)
     cfg.push_scope(defaults)
 
-    # Each scope can have per-platfom overrides in subdirectories
-    platform = spack.architecture.platform().name
-
     # add each scope and its platform-specific directory
     for name, path in configuration_paths:
         cfg.push_scope(ConfigScope(name, path))
 
-        plat_name = '%s/%s' % (name, platform)
-        plat_path = os.path.join(path, platform)
-        cfg.push_scope(ConfigScope(plat_name, plat_path))
+        # Each scope can have per-platfom overrides in subdirectories
+        _add_platform_scope(cfg, ConfigScope, name, path)
+
+    # add command-line scopes
+    _add_command_line_scopes(cfg, command_line_scopes)
 
     # we make a special scope for spack commands so that they can
     # override configuration options.
diff --git a/lib/spack/spack/error.py b/lib/spack/spack/error.py
index 327b97b5d6..e179cb5876 100644
--- a/lib/spack/spack/error.py
+++ b/lib/spack/spack/error.py
@@ -30,6 +30,11 @@ import inspect
 import llnl.util.tty as tty
 
 
+#: whether we should write stack traces or short error messages
+#: this is module-scoped because it needs to be set very early
+debug = False
+
+
 class SpackError(Exception):
     """This is the superclass for all Spack errors.
        Subclasses can be found in the modules they have to do with.
@@ -72,8 +77,7 @@ class SpackError(Exception):
             sys.stderr.write('\n')
 
         # stack trace, etc. in debug mode.
-        import spack.config
-        if spack.config.get('config:debug'):
+        if debug:
             if self.traceback:
                 # exception came from a build child, already got
                 # traceback in child, so print it.
diff --git a/lib/spack/spack/main.py b/lib/spack/spack/main.py
index 324012d494..00084cfcf2 100644
--- a/lib/spack/spack/main.py
+++ b/lib/spack/spack/main.py
@@ -292,7 +292,9 @@ class SpackArgumentParser(argparse.ArgumentParser):
         subparser = self.subparsers.add_parser(
             cmd_name, help=module.description, description=module.description)
         module.setup_parser(subparser)
-        return module
+
+        # return the callable function for the command
+        return spack.cmd.get_command(cmd_name)
 
     def format_help(self, level='short'):
         if self.prog == 'spack':
@@ -329,6 +331,9 @@ def make_argument_parser(**kwargs):
         choices=('always', 'never', 'auto'),
         help="when to colorize output (default: auto)")
     parser.add_argument(
+        '-C', '--config-scope', dest='config_scopes', action='append',
+        metavar='DIRECTORY', help="use an additional configuration scope")
+    parser.add_argument(
         '-d', '--debug', action='store_true',
         help="write out debug logs during compile")
     parser.add_argument(
@@ -379,15 +384,18 @@ def setup_main_options(args):
     tty.set_debug(args.debug)
     tty.set_stacktrace(args.stacktrace)
 
+    # debug must be set first so that it can even affect behvaior of
+    # errors raised by spack.config.
+    if args.debug:
+        spack.error.debug = True
+        spack.util.debug.register_interrupt_handler()
+        spack.config.set('config:debug', True, scope='command_line')
+
     # override lock configuration if passed on command line
     if args.locks is not None:
         spack.util.lock.check_lock_safety(spack.paths.prefix)
         spack.config.set('config:locks', False, scope='command_line')
 
-    if args.debug:
-        spack.util.debug.register_interrupt_handler()
-        spack.config.set('config:debug', True, scope='command_line')
-
     if args.mock:
         rp = spack.repo.RepoPath(spack.paths.mock_packages_path)
         spack.repo.set_path(rp)
@@ -414,7 +422,7 @@ def allows_unknown_args(command):
     return (argcount == 3 and varnames[2] == 'unknown_args')
 
 
-def _invoke_spack_command(command, parser, args, unknown_args):
+def _invoke_command(command, parser, args, unknown_args):
     """Run a spack command *without* setting spack global options."""
     if allows_unknown_args(command):
         return_val = command(parser, args, unknown_args)
@@ -438,16 +446,15 @@ class SpackCommand(object):
     Use this to invoke Spack commands directly from Python and check
     their output.
     """
-    def __init__(self, command):
-        """Create a new SpackCommand that invokes ``command`` when called.
+    def __init__(self, command_name):
+        """Create a new SpackCommand that invokes ``command_name`` when called.
 
         Args:
-            command (str): name of the command to invoke
+            command_name (str): name of the command to invoke
         """
         self.parser = make_argument_parser()
-        self.parser.add_command(command)
-        self.command_name = command
-        self.command = spack.cmd.get_command(command)
+        self.command = self.parser.add_command(command_name)
+        self.command_name = command_name
 
     def __call__(self, *argv, **kwargs):
         """Invoke this SpackCommand.
@@ -477,7 +484,7 @@ class SpackCommand(object):
         out = StringIO()
         try:
             with log_output(out):
-                self.returncode = _invoke_spack_command(
+                self.returncode = _invoke_command(
                     self.command, self.parser, args, unknown)
 
         except SystemExit as e:
@@ -497,30 +504,6 @@ class SpackCommand(object):
         return out.getvalue()
 
 
-def _main(command, parser, args, unknown_args):
-    """Run a spack command *and* set spack globaloptions."""
-    # many operations will fail without a working directory.
-    set_working_dir()
-
-    # only setup main options in here, after the real parse (we'll get it
-    # wrong if we do it after the initial, partial parse)
-    setup_main_options(args)
-    spack.hooks.pre_run()
-
-    # Now actually execute the command
-    try:
-        return _invoke_spack_command(command, parser, args, unknown_args)
-    except SpackError as e:
-        e.die()  # gracefully die on any SpackErrors
-    except Exception as e:
-        if spack.config.get('config:debug'):
-            raise
-        tty.die(str(e))
-    except KeyboardInterrupt:
-        sys.stderr.write('\n')
-        tty.die("Keyboard interrupt.")
-
-
 def _profile_wrapper(command, parser, args, unknown_args):
     import cProfile
 
@@ -543,7 +526,7 @@ def _profile_wrapper(command, parser, args, unknown_args):
         # make a profiler and run the code.
         pr = cProfile.Profile()
         pr.enable()
-        return _main(command, parser, args, unknown_args)
+        return _invoke_command(command, parser, args, unknown_args)
 
     finally:
         pr.disable()
@@ -609,6 +592,10 @@ def main(argv=None):
     parser.add_argument('command', nargs=argparse.REMAINDER)
     args, unknown = parser.parse_known_args(argv)
 
+    # make spack.config aware of any command line configuration scopes
+    if args.config_scopes:
+        spack.config.command_line_scopes = args.config_scopes
+
     if args.print_shell_vars:
         print_setup_info(*args.print_shell_vars.split(','))
         return 0
@@ -631,31 +618,51 @@ def main(argv=None):
         parser.print_help()
         return 1
 
-    # Try to load the particular command the caller asked for.  If there
-    # is no module for it, just die.
-    cmd_name = args.command[0]
     try:
-        parser.add_command(cmd_name)
-    except ImportError:
-        if spack.config.get('config:debug'):
-            raise
-        tty.die("Unknown command: %s" % args.command[0])
+        # ensure options on spack command come before everything
+        setup_main_options(args)
 
-    # Re-parse with the proper sub-parser added.
-    args, unknown = parser.parse_known_args()
+        # Try to load the particular command the caller asked for.  If there
+        # is no module for it, just die.
+        cmd_name = args.command[0]
+        try:
+            command = parser.add_command(cmd_name)
+        except ImportError:
+            if spack.config.get('config:debug'):
+                raise
+            tty.die("Unknown command: %s" % args.command[0])
 
-    # now we can actually execute the command.
-    command = spack.cmd.get_command(cmd_name)
-    try:
+        # Re-parse with the proper sub-parser added.
+        args, unknown = parser.parse_known_args()
+
+        # many operations will fail without a working directory.
+        set_working_dir()
+
+        # pre-run hooks happen after we know we have a valid working dir
+        spack.hooks.pre_run()
+
+        # now we can actually execute the command.
         if args.spack_profile or args.sorted_profile:
             _profile_wrapper(command, parser, args, unknown)
         elif args.pdb:
             import pdb
-            pdb.runctx('_main(command, parser, args, unknown)',
+            pdb.runctx('_invoke_command(command, parser, args, unknown)',
                        globals(), locals())
             return 0
         else:
-            return _main(command, parser, args, unknown)
+            return _invoke_command(command, parser, args, unknown)
+
+    except SpackError as e:
+        e.die()  # gracefully die on any SpackErrors
+
+    except Exception as e:
+        if spack.config.get('config:debug'):
+            raise
+        tty.die(str(e))
+
+    except KeyboardInterrupt:
+        sys.stderr.write('\n')
+        tty.die("Keyboard interrupt.")
 
     except SystemExit as e:
         return e.code
diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py
index 95cd941547..e2aa3d7c66 100644
--- a/lib/spack/spack/test/config.py
+++ b/lib/spack/spack/test/config.py
@@ -27,6 +27,8 @@ import collections
 import getpass
 import tempfile
 
+from llnl.util.filesystem import touch, mkdirp
+
 import pytest
 import yaml
 
@@ -614,3 +616,51 @@ def test_bad_config_section(config):
 
     with pytest.raises(spack.config.ConfigSectionError):
         spack.config.get('foobar')
+
+
+def test_bad_command_line_scopes(tmpdir, config):
+    cfg = spack.config.Configuration()
+
+    with tmpdir.as_cwd():
+        with pytest.raises(spack.config.ConfigError):
+            spack.config._add_command_line_scopes(cfg, ['bad_path'])
+
+        touch('unreadable_file')
+        with pytest.raises(spack.config.ConfigError):
+            spack.config._add_command_line_scopes(cfg, ['unreadable_file'])
+
+        mkdirp('unreadable_dir')
+        with pytest.raises(spack.config.ConfigError):
+            try:
+                os.chmod('unreadable_dir', 0)
+                spack.config._add_command_line_scopes(cfg, ['unreadable_dir'])
+            finally:
+                os.chmod('unreadable_dir', 0o700)  # so tmpdir can be removed
+
+
+def test_add_command_line_scopes(tmpdir, mutable_config):
+    config_yaml = str(tmpdir.join('config.yaml'))
+    with open(config_yaml, 'w') as f:
+            f.write("""\
+config:
+    verify_ssh: False
+    dirty: False
+"""'')
+
+    spack.config._add_command_line_scopes(mutable_config, [str(tmpdir)])
+
+
+def test_immuntable_scope(tmpdir):
+    config_yaml = str(tmpdir.join('config.yaml'))
+    with open(config_yaml, 'w') as f:
+        f.write("""\
+config:
+    install_tree: dummy_tree_value
+"""'')
+    scope = spack.config.ImmutableConfigScope('test', str(tmpdir))
+
+    data = scope.get_section('config')
+    assert data['config']['install_tree'] == 'dummy_tree_value'
+
+    with pytest.raises(spack.config.ConfigError):
+        scope.write_section('config')