From 52fbbdf5a14b555a7f1c13107f266e6be4238fee Mon Sep 17 00:00:00 2001 From: Elizabeth Fischer Date: Thu, 29 Dec 2016 20:06:21 -0500 Subject: [PATCH] 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 --- lib/spack/spack/config.py | 95 ++++++++++++++++++++++----- lib/spack/spack/error.py | 8 ++- lib/spack/spack/main.py | 113 +++++++++++++++++---------------- lib/spack/spack/test/config.py | 50 +++++++++++++++ 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 @@ def __repr__(self): return '' % (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 '' % (self.name, self.path) + + class InternalConfigScope(ConfigScope): """An internal configuration scope that is not persisted to a file. @@ -274,9 +289,8 @@ def pop_scope(self): @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 @@ def print_section(self, section, blame=False): @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. - config.push_scope(overrides) - config.set(path, value, scope='overrides') + Arguments: + path_or_scope (ConfigScope or str): scope or single option to override + value (object, optional): value for the single option - yield config + 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. - scope = config.pop_scope() - assert scope is overrides + """ + 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)) + + +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) + + # 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 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 @@ def print_context(self): 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 @@ def add_command(self, cmd_name): 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': @@ -328,6 +330,9 @@ def make_argument_parser(**kwargs): '--color', action='store', default='auto', 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") @@ -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 @@ def __call__(self, *argv, **kwargs): 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 @@ def __call__(self, *argv, **kwargs): 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 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')