From b2e9696052dc76f9612051d7ad85094a762271c8 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Fri, 27 Dec 2019 23:17:34 -0800 Subject: argparse: lazily construct common arguments Continuing to shave small bits of time off startup -- `spack.cmd.common.arguments` constructs many `Args` objects at module scope, which has to be done for all commands that import it. Instead of doing this at load time, do it lazily. - [x] construct Args objects lazily - [x] remove the module-scoped argparse fixture - [x] make the mock config scope set dirty to False by default (like the regular scope) This *seems* to reduce load time slightly --- lib/spack/spack/cmd/common/arguments.py | 178 +++++++++++++++++++++----------- lib/spack/spack/test/cmd/install.py | 12 +-- lib/spack/spack/test/data/config.yaml | 2 +- 3 files changed, 120 insertions(+), 72 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/cmd/common/arguments.py b/lib/spack/spack/cmd/common/arguments.py index 8a38d09f03..66ed953d6e 100644 --- a/lib/spack/spack/cmd/common/arguments.py +++ b/lib/spack/spack/cmd/common/arguments.py @@ -18,9 +18,22 @@ from spack.util.pattern import Args __all__ = ['add_common_arguments'] +#: dictionary of argument-generating functions, keyed by name _arguments = {} +def arg(fn): + """Decorator for a function that generates a common argument. + + This ensures that argument bunches are created lazily. Decorate + argument-generating functions below with @arg so that + ``add_common_arguments()`` can find them. + + """ + _arguments[fn.__name__] = fn + return fn + + def add_common_arguments(parser, list_of_arguments): """Extend a parser with extra arguments @@ -32,7 +45,8 @@ def add_common_arguments(parser, list_of_arguments): if argument not in _arguments: message = 'Trying to add non existing argument "{0}" to a command' raise KeyError(message.format(argument)) - x = _arguments[argument] + + x = _arguments[argument]() parser.add_argument(*x.flags, **x.kwargs) @@ -118,64 +132,104 @@ class DeptypeAction(argparse.Action): setattr(namespace, self.dest, deptype) -_arguments['constraint'] = Args( - 'constraint', nargs=argparse.REMAINDER, action=ConstraintAction, - help='constraint to select a subset of installed packages') - -_arguments['yes_to_all'] = Args( - '-y', '--yes-to-all', action='store_true', dest='yes_to_all', - help='assume "yes" is the answer to every confirmation request') - -_arguments['recurse_dependencies'] = Args( - '-r', '--dependencies', action='store_true', dest='recurse_dependencies', - help='recursively traverse spec dependencies') - -_arguments['recurse_dependents'] = Args( - '-R', '--dependents', action='store_true', dest='dependents', - help='also uninstall any packages that depend on the ones given ' - 'via command line') - -_arguments['clean'] = Args( - '--clean', - action='store_false', - default=spack.config.get('config:dirty'), - dest='dirty', - help='unset harmful variables in the build environment (default)') - -_arguments['deptype'] = Args( - '--deptype', action=DeptypeAction, default=dep.all_deptypes, - help="comma-separated list of deptypes to traverse\ndefault=%s" - % ','.join(dep.all_deptypes)) - -_arguments['dirty'] = Args( - '--dirty', - action='store_true', - default=spack.config.get('config:dirty'), - dest='dirty', - help='preserve user environment in the spack build environment (danger!)') - -_arguments['long'] = Args( - '-l', '--long', action='store_true', - help='show dependency hashes as well as versions') - -_arguments['very_long'] = Args( - '-L', '--very-long', action='store_true', - help='show full dependency hashes as well as versions') - -_arguments['tags'] = Args( - '-t', '--tags', action='append', - help='filter a package query by tags') - -_arguments['jobs'] = Args( - '-j', '--jobs', action=SetParallelJobs, type=int, dest='jobs', - help='explicitly set number of parallel jobs') - -_arguments['install_status'] = Args( - '-I', '--install-status', action='store_true', default=False, - help='show install status of packages. packages can be: ' - 'installed [+], missing and needed by an installed package [-], ' - 'or not installed (no annotation)') - -_arguments['no_checksum'] = Args( - '-n', '--no-checksum', action='store_true', default=False, - help="do not use checksums to verify downloaded files (unsafe)") +@arg +def constraint(): + return Args( + 'constraint', nargs=argparse.REMAINDER, action=ConstraintAction, + help='constraint to select a subset of installed packages') + + +@arg +def yes_to_all(): + return Args( + '-y', '--yes-to-all', action='store_true', dest='yes_to_all', + help='assume "yes" is the answer to every confirmation request') + + +@arg +def recurse_dependencies(): + return Args( + '-r', '--dependencies', action='store_true', + dest='recurse_dependencies', + help='recursively traverse spec dependencies') + + +@arg +def recurse_dependents(): + return Args( + '-R', '--dependents', action='store_true', dest='dependents', + help='also uninstall any packages that depend on the ones given ' + 'via command line') + + +@arg +def clean(): + return Args( + '--clean', + action='store_false', + default=spack.config.get('config:dirty'), + dest='dirty', + help='unset harmful variables in the build environment (default)') + + +@arg +def deptype(): + return Args( + '--deptype', action=DeptypeAction, default=dep.all_deptypes, + help="comma-separated list of deptypes to traverse\ndefault=%s" + % ','.join(dep.all_deptypes)) + + +@arg +def dirty(): + return Args( + '--dirty', + action='store_true', + default=spack.config.get('config:dirty'), + dest='dirty', + help="preserve user environment in spack's build environment (danger!)" + ) + + +@arg +def long(): + return Args( + '-l', '--long', action='store_true', + help='show dependency hashes as well as versions') + + +@arg +def very_long(): + return Args( + '-L', '--very-long', action='store_true', + help='show full dependency hashes as well as versions') + + +@arg +def tags(): + return Args( + '-t', '--tags', action='append', + help='filter a package query by tags') + + +@arg +def jobs(): + return Args( + '-j', '--jobs', action=SetParallelJobs, type=int, dest='jobs', + help='explicitly set number of parallel jobs') + + +@arg +def install_status(): + return Args( + '-I', '--install-status', action='store_true', default=False, + help='show install status of packages. packages can be: ' + 'installed [+], missing and needed by an installed package [-], ' + 'or not installed (no annotation)') + + +@arg +def no_checksum(): + return Args( + '-n', '--no-checksum', action='store_true', default=False, + help="do not use checksums to verify downloaded files (unsafe)") diff --git a/lib/spack/spack/test/cmd/install.py b/lib/spack/spack/test/cmd/install.py index 0d3d5f5de1..e8240a87b1 100644 --- a/lib/spack/spack/test/cmd/install.py +++ b/lib/spack/spack/test/cmd/install.py @@ -30,14 +30,6 @@ env = SpackCommand('env') add = SpackCommand('add') -@pytest.fixture(scope='module') -def parser(): - """Returns the parser for the module command""" - parser = argparse.ArgumentParser() - spack.cmd.install.setup_parser(parser) - return parser - - @pytest.fixture() def noop_install(monkeypatch): def noop(*args, **kwargs): @@ -115,7 +107,9 @@ def test_install_package_already_installed( (['--clean'], False), (['--dirty'], True), ]) -def test_install_dirty_flag(parser, arguments, expected): +def test_install_dirty_flag(arguments, expected): + parser = argparse.ArgumentParser() + spack.cmd.install.setup_parser(parser) args = parser.parse_args(arguments) assert args.dirty == expected diff --git a/lib/spack/spack/test/data/config.yaml b/lib/spack/spack/test/data/config.yaml index 2d2c2e356a..0611251189 100644 --- a/lib/spack/spack/test/data/config.yaml +++ b/lib/spack/spack/test/data/config.yaml @@ -11,4 +11,4 @@ config: misc_cache: ~/.spack/cache verify_ssl: true checksum: true - dirty: True + dirty: false -- cgit v1.2.3-60-g2f50