Use spack commands --format=bash to generate shell completion (#14393)

This PR adds a `--format=bash` option to `spack commands` to
auto-generate the Bash programmable tab completion script. It can be
extended to work for other shells.

Progress:

- [x] Fix bug in superclass initialization in `ArgparseWriter`
- [x] Refactor `ArgparseWriter` (see below)
- [x] Ensure that output of old `--format` options remains the same
- [x] Add `ArgparseCompletionWriter` and `BashCompletionWriter`
- [x] Add `--aliases` option to add command aliases
- [x] Standardize positional argument names
- [x] Tests for `spack commands --format=bash` coverage
- [x] Tests to make sure `spack-completion.bash` stays up-to-date
- [x] Tests for `spack-completion.bash` coverage
- [x] Speed up `spack-completion.bash` by caching subroutine calls

This PR also necessitates a significant refactoring of
`ArgparseWriter`. Previously, `ArgparseWriter` was mostly a single
`_write` method which handled everything from extracting the information
we care about from the parser to formatting the output. Now, `_write`
only handles recursion, while the information extraction is split into a
separate `parse` method, and the formatting is handled by `format`. This
allows subclasses to completely redefine how the format will appear
without overriding all of `_write`.

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

.codecov.yml

@@ -12,6 +12,5 @@ ignore:
   - lib/spack/docs/.*
   - lib/spack/external/.*
   - share/spack/qa/.*
-  - share/spack/spack-completion.bash
 
 comment: off

.coveragerc

@@ -9,6 +9,7 @@ omit =
      lib/spack/spack/test/*
      lib/spack/docs/*
      lib/spack/external/*
+     share/spack/qa/*
 
 [report]
 # Regexes for lines to exclude from consideration

lib/spack/llnl/util/argparsewriter.py

@@ -4,134 +4,162 @@
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
 from __future__ import print_function
+
 import re
 import argparse
 import errno
 import sys
 
+from six import StringIO
 
+
+class Command(object):
+    """Parsed representation of a command from argparse.
+
+    This is a single command from an argparse parser. ``ArgparseWriter``
+    creates these and returns them from ``parse()``, and it passes one of
+    these to each call to ``format()`` so that we can take an action for
+    a single command.
+
+    Parts of a Command:
+      - prog: command name (str)
+      - description: command description (str)
+      - usage: command usage (str)
+      - positionals: list of positional arguments (list)
+      - optionals: list of optional arguments (list)
+      - subcommands: list of subcommand parsers (list)
+    """
+    def __init__(self, prog, description, usage,
+                 positionals, optionals, subcommands):
+        self.prog = prog
+        self.description = description
+        self.usage = usage
+        self.positionals = positionals
+        self.optionals = optionals
+        self.subcommands = subcommands
+
+
+# NOTE: The only reason we subclass argparse.HelpFormatter is to get access
+# to self._expand_help(), ArgparseWriter is not intended to be used as a
+# formatter_class.
 class ArgparseWriter(argparse.HelpFormatter):
     """Analyzes an argparse ArgumentParser for easy generation of help."""
-    def __init__(self, out=sys.stdout):
+
-        super(ArgparseWriter, self).__init__(out)
+    def __init__(self, prog, out=sys.stdout, aliases=False):
+        """Initializes a new ArgparseWriter instance.
+
+        Parameters:
+            prog (str): the program name
+            out (file object): the file to write to
+            aliases (bool): whether or not to include subparsers for aliases
+        """
+        super(ArgparseWriter, self).__init__(prog)
         self.level = 0
+        self.prog = prog
         self.out = out
+        self.aliases = aliases
 
-    def _write(self, parser, root=True, level=0):
+    def parse(self, parser, prog):
+        """Parses the parser object and returns the relavent components.
+
+        Parameters:
+            parser (argparse.ArgumentParser): the parser
+            prog (str): the command name
+
+        Returns:
+            (Command) information about the command from the parser
+        """
         self.parser = parser
-        self.level = level
+
+        split_prog = parser.prog.split(' ')
+        split_prog[-1] = prog
+        prog = ' '.join(split_prog)
+        description = parser.description
+
+        fmt = parser._get_formatter()
         actions = parser._actions
+        groups = parser._mutually_exclusive_groups
+        usage = fmt._format_usage(None, actions, groups, '').strip()
 
-        # allow root level to be flattened with rest of commands
+        # Go through actions and split them into optionals, positionals,
-        if type(root) == int:
-            self.level = root
-            root = True
-
-        # go through actions and split them into optionals, positionals,
         # and subcommands
         optionals = []
         positionals = []
         subcommands = []
         for action in actions:
             if action.option_strings:
-                optionals.append(action)
+                flags = action.option_strings
+                dest_flags = fmt._format_action_invocation(action)
+                help = self._expand_help(action) if action.help else ''
+                help = help.replace('\n', ' ')
+                optionals.append((flags, dest_flags, help))
             elif isinstance(action, argparse._SubParsersAction):
                 for subaction in action._choices_actions:
                     subparser = action._name_parser_map[subaction.dest]
-                    subcommands.append(subparser)
+                    subcommands.append((subparser, subaction.dest))
+
+                    # Look for aliases of the form 'name (alias, ...)'
+                    if self.aliases:
+                        match = re.match(r'(.*) \((.*)\)', subaction.metavar)
+                        if match:
+                            aliases = match.group(2).split(', ')
+                            for alias in aliases:
+                                subparser = action._name_parser_map[alias]
+                                subcommands.append((subparser, alias))
             else:
-                positionals.append(action)
+                args = fmt._format_action_invocation(action)
-
-        groups = parser._mutually_exclusive_groups
-        fmt = parser._get_formatter()
-        description = parser.description
-
-        def action_group(function, actions):
-            for action in actions:
-                arg = fmt._format_action_invocation(action)
                 help = self._expand_help(action) if action.help else ''
-                function(arg, re.sub('\n', ' ', help))
+                help = help.replace('\n', ' ')
+                positionals.append((args, help))
 
-        if root:
+        return Command(
-            self.begin_command(parser.prog)
+            prog, description, usage, positionals, optionals, subcommands)
 
-            if description:
+    def format(self, cmd):
-                self.description(parser.description)
+        """Returns the string representation of a single node in the
+        parser tree.
 
-            usage = fmt._format_usage(None, actions, groups, '').strip()
+        Override this in subclasses to define how each subcommand
-            self.usage(usage)
+        should be displayed.
 
-            if positionals:
+        Parameters:
-                self.begin_positionals()
+            (Command): parsed information about a command or subcommand
-                action_group(self.positional, positionals)
-                self.end_positionals()
 
-            if optionals:
+        Returns:
-                self.begin_optionals()
+            str: the string representation of this subcommand
-                action_group(self.optional, optionals)
+        """
-                self.end_optionals()
+        raise NotImplementedError
 
-        if subcommands:
+    def _write(self, parser, prog, level=0):
-            self.begin_subcommands(subcommands)
+        """Recursively writes a parser.
-            for subparser in subcommands:
-                self._write(subparser, root=True, level=level + 1)
-            self.end_subcommands(subcommands)
 
-        if root:
+        Parameters:
-            self.end_command(parser.prog)
+            parser (argparse.ArgumentParser): the parser
+            prog (str): the command name
+            level (int): the current level
+        """
+        self.level = level
 
-    def write(self, parser, root=True):
+        cmd = self.parse(parser, prog)
+        self.out.write(self.format(cmd))
+
+        for subparser, prog in cmd.subcommands:
+            self._write(subparser, prog, level=level + 1)
+
+    def write(self, parser):
         """Write out details about an ArgumentParser.
 
         Args:
-            parser (ArgumentParser): an ``argparse`` parser
+            parser (argparse.ArgumentParser): the parser
-            root (bool or int): if bool, whether to include the root parser;
-                or ``1`` to flatten the root parser with first-level
-                subcommands
         """
         try:
-            self._write(parser, root, level=0)
+            self._write(parser, self.prog)
         except IOError as e:
-            # swallow pipe errors
+            # Swallow pipe errors
+            # Raises IOError in Python 2 and BrokenPipeError in Python 3
             if e.errno != errno.EPIPE:
                 raise
 
-    def begin_command(self, prog):
-        pass
-
-    def end_command(self, prog):
-        pass
-
-    def description(self, description):
-        pass
-
-    def usage(self, usage):
-        pass
-
-    def begin_positionals(self):
-        pass
-
-    def positional(self, name, help):
-        pass
-
-    def end_positionals(self):
-        pass
-
-    def begin_optionals(self):
-        pass
-
-    def optional(self, option, help):
-        pass
-
-    def end_optionals(self):
-        pass
-
-    def begin_subcommands(self, subcommands):
-        pass
-
-    def end_subcommands(self, subcommands):
-        pass
-
 
 _rst_levels = ['=', '-', '^', '~', ':', '`']
 
@@ -139,66 +167,213 @@ def end_subcommands(self, subcommands):
 class ArgparseRstWriter(ArgparseWriter):
     """Write argparse output as rst sections."""
 
-    def __init__(self, out=sys.stdout, rst_levels=_rst_levels,
+    def __init__(self, prog, out=sys.stdout, aliases=False,
-                 strip_root_prog=True):
+                 rst_levels=_rst_levels):
         """Create a new ArgparseRstWriter.
 
-        Args:
+        Parameters:
+            prog (str): program name
             out (file object): file to write to
+            aliases (bool): whether or not to include subparsers for aliases
             rst_levels (list of str): list of characters
                 for rst section headings
-            strip_root_prog (bool): if ``True``, strip the base command name
-                from subcommands in output
         """
-        super(ArgparseRstWriter, self).__init__(out)
+        super(ArgparseRstWriter, self).__init__(prog, out, aliases)
         self.rst_levels = rst_levels
-        self.strip_root_prog = strip_root_prog
 
-    def line(self, string=''):
+    def format(self, cmd):
-        self.out.write('%s\n' % string)
+        string = StringIO()
+        string.write(self.begin_command(cmd.prog))
+
+        if cmd.description:
+            string.write(self.description(cmd.description))
+
+        string.write(self.usage(cmd.usage))
+
+        if cmd.positionals:
+            string.write(self.begin_positionals())
+            for args, help in cmd.positionals:
+                string.write(self.positional(args, help))
+            string.write(self.end_positionals())
+
+        if cmd.optionals:
+            string.write(self.begin_optionals())
+            for flags, dest_flags, help in cmd.optionals:
+                string.write(self.optional(dest_flags, help))
+            string.write(self.end_optionals())
+
+        if cmd.subcommands:
+            string.write(self.begin_subcommands(cmd.subcommands))
+
+        return string.getvalue()
 
     def begin_command(self, prog):
-        self.line()
+        return """
-        self.line('----')
+----
-        self.line()
+
-        self.line('.. _%s:\n' % prog.replace(' ', '-'))
+.. _{0}:
-        self.line('%s' % prog)
+
-        self.line(self.rst_levels[self.level] * len(prog) + '\n')
+{1}
+{2}
+
+""".format(prog.replace(' ', '-'), prog,
+           self.rst_levels[self.level] * len(prog))
 
     def description(self, description):
-        self.line('%s\n' % description)
+        return description + '\n\n'
 
     def usage(self, usage):
-        self.line('.. code-block:: console\n')
+        return """\
-        self.line('    %s\n' % usage)
+.. code-block:: console
+
+    {0}
+
+""".format(usage)
 
     def begin_positionals(self):
-        self.line()
+        return '\n**Positional arguments**\n\n'
-        self.line('**Positional arguments**\n')
 
     def positional(self, name, help):
-        self.line(name)
+        return """\
-        self.line('  %s\n' % help)
+{0}
+  {1}
+
+""".format(name, help)
+
+    def end_positionals(self):
+        return ''
 
     def begin_optionals(self):
-        self.line()
+        return '\n**Optional arguments**\n\n'
-        self.line('**Optional arguments**\n')
 
     def optional(self, opts, help):
-        self.line('``%s``' % opts)
+        return """\
-        self.line('  %s\n' % help)
+``{0}``
+  {1}
+
+""".format(opts, help)
+
+    def end_optionals(self):
+        return ''
 
     def begin_subcommands(self, subcommands):
-        self.line()
+        string = """
-        self.line('**Subcommands**\n')
+**Subcommands**
-        self.line('.. hlist::')
-        self.line('   :columns: 4\n')
 
-        for cmd in subcommands:
+.. hlist::
-            prog = cmd.prog
+   :columns: 4
-            if self.strip_root_prog:
-                prog = re.sub(r'^[^ ]* ', '', prog)
 
-            self.line('   * :ref:`%s <%s>`'
+"""
-                      % (prog, cmd.prog.replace(' ', '-')))
+
-        self.line()
+        for cmd, _ in subcommands:
+            prog = re.sub(r'^[^ ]* ', '', cmd.prog)
+            string += '   * :ref:`{0} <{1}>`\n'.format(
+                prog, cmd.prog.replace(' ', '-'))
+
+        return string + '\n'
+
+
+class ArgparseCompletionWriter(ArgparseWriter):
+    """Write argparse output as shell programmable tab completion functions."""
+
+    def format(self, cmd):
+        """Returns the string representation of a single node in the
+        parser tree.
+
+        Override this in subclasses to define how each subcommand
+        should be displayed.
+
+        Parameters:
+            (Command): parsed information about a command or subcommand
+
+        Returns:
+            str: the string representation of this subcommand
+        """
+
+        assert cmd.optionals  # we should always at least have -h, --help
+        assert not (cmd.positionals and cmd.subcommands)  # one or the other
+
+        # We only care about the arguments/flags, not the help messages
+        positionals = []
+        if cmd.positionals:
+            positionals, _ = zip(*cmd.positionals)
+        optionals, _, _ = zip(*cmd.optionals)
+        subcommands = []
+        if cmd.subcommands:
+            _, subcommands = zip(*cmd.subcommands)
+
+        # Flatten lists of lists
+        optionals = [x for xx in optionals for x in xx]
+
+        return (self.start_function(cmd.prog) +
+                self.body(positionals, optionals, subcommands) +
+                self.end_function(cmd.prog))
+
+    def start_function(self, prog):
+        """Returns the syntax needed to begin a function definition.
+
+        Parameters:
+            prog (str): the command name
+
+        Returns:
+            str: the function definition beginning
+        """
+        name = prog.replace('-', '_').replace(' ', '_')
+        return '\n_{0}() {{'.format(name)
+
+    def end_function(self, prog=None):
+        """Returns the syntax needed to end a function definition.
+
+        Parameters:
+            prog (str, optional): the command name
+
+        Returns:
+            str: the function definition ending
+        """
+        return '}\n'
+
+    def body(self, positionals, optionals, subcommands):
+        """Returns the body of the function.
+
+        Parameters:
+            positionals (list): list of positional arguments
+            optionals (list): list of optional arguments
+            subcommands (list): list of subcommand parsers
+
+        Returns:
+            str: the function body
+        """
+        return ''
+
+    def positionals(self, positionals):
+        """Returns the syntax for reporting positional arguments.
+
+        Parameters:
+            positionals (list): list of positional arguments
+
+        Returns:
+            str: the syntax for positional arguments
+        """
+        return ''
+
+    def optionals(self, optionals):
+        """Returns the syntax for reporting optional flags.
+
+        Parameters:
+            optionals (list): list of optional arguments
+
+        Returns:
+            str: the syntax for optional flags
+        """
+        return ''
+
+    def subcommands(self, subcommands):
+        """Returns the syntax for reporting subcommands.
+
+        Parameters:
+            subcommands (list): list of subcommand parsers
+
+        Returns:
+            str: the syntax for subcommand parsers
+        """
+        return ''

lib/spack/spack/cmd/activate.py

@@ -3,11 +3,10 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
-import argparse
-
 import llnl.util.tty as tty
 
 import spack.cmd
+import spack.cmd.common.arguments as arguments
 import spack.environment as ev
 from spack.filesystem_view import YamlFilesystemView
 
@@ -23,9 +22,7 @@ def setup_parser(subparser):
     subparser.add_argument(
         '-v', '--view', metavar='VIEW', type=str,
         help="the view to operate on")
-    subparser.add_argument(
+    arguments.add_common_arguments(subparser, ['installed_spec'])
-        'spec', nargs=argparse.REMAINDER,
-        help="spec of package extension to activate")
 
 
 def activate(parser, args):

lib/spack/spack/cmd/add.py

@@ -3,11 +3,10 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
-import argparse
-
 import llnl.util.tty as tty
 
 import spack.cmd
+import spack.cmd.common.arguments as arguments
 import spack.environment as ev
 
 
@@ -20,8 +19,7 @@ def setup_parser(subparser):
     subparser.add_argument('-l', '--list-name',
                            dest='list_name', default='specs',
                            help="name of the list to add specs to")
-    subparser.add_argument(
+    arguments.add_common_arguments(subparser, ['specs'])
-        'specs', nargs=argparse.REMAINDER, help="specs of packages to add")
 
 
 def add(parser, args):

lib/spack/spack/cmd/blame.py

@@ -35,7 +35,7 @@ def setup_parser(subparser):
         help='show git blame output instead of summary')
 
     subparser.add_argument(
-        'package_name', help='name of package to show contributions for, '
+        'package_or_file', help='name of package to show contributions for, '
         'or path to a file in the spack repo')
 
 
@@ -47,13 +47,13 @@ def blame(parser, args):
 
     # Get name of file to blame
     blame_file = None
-    if os.path.isfile(args.package_name):
+    if os.path.isfile(args.package_or_file):
-        path = os.path.realpath(args.package_name)
+        path = os.path.realpath(args.package_or_file)
         if path.startswith(spack.paths.prefix):
             blame_file = path
 
     if not blame_file:
-        pkg = spack.repo.get(args.package_name)
+        pkg = spack.repo.get(args.package_or_file)
         blame_file = pkg.module.__file__.rstrip('c')  # .pyc -> .py
 
     # get git blame for the package

lib/spack/spack/cmd/build_env.py

@@ -33,7 +33,7 @@ def setup_parser(subparser):
     subparser.add_argument(
         'spec', nargs=argparse.REMAINDER,
         metavar='spec [--] [cmd]...',
-        help="specs of package environment to emulate")
+        help="spec of package environment to emulate")
     subparser.epilog\
         = 'If a command is not specified, the environment will be printed ' \
         'to standard output (cf /usr/bin/env) unless --dump and/or --pickle ' \

lib/spack/spack/cmd/buildcache.py

@@ -3,7 +3,6 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
-import argparse
 import os
 import shutil
 import sys
@@ -61,11 +60,9 @@ def setup_parser(subparser):
                                             "building package(s)")
     create.add_argument('-y', '--spec-yaml', default=None,
                         help='Create buildcache entry for spec from yaml file')
-    create.add_argument(
-        'packages', nargs=argparse.REMAINDER,
-        help="specs of packages to create buildcache for")
     create.add_argument('--no-deps', action='store_true', default='false',
                         help='Create buildcache entry wo/ dependencies')
+    arguments.add_common_arguments(create, ['specs'])
     create.set_defaults(func=createtarball)
 
     install = subparsers.add_parser('install', help=installtarball.__doc__)
@@ -79,9 +76,7 @@ def setup_parser(subparser):
     install.add_argument('-u', '--unsigned', action='store_true',
                          help="install unsigned buildcache" +
                               " tarballs for testing")
-    install.add_argument(
+    arguments.add_common_arguments(install, ['specs'])
-        'packages', nargs=argparse.REMAINDER,
-        help="specs of packages to install buildcache for")
     install.set_defaults(func=installtarball)
 
     listcache = subparsers.add_parser('list', help=listspecs.__doc__)
@@ -92,9 +87,7 @@ def setup_parser(subparser):
                            help='show variants in output (can be long)')
     listcache.add_argument('-f', '--force', action='store_true',
                            help="force new download of specs")
-    listcache.add_argument(
+    arguments.add_common_arguments(listcache, ['specs'])
-        'packages', nargs=argparse.REMAINDER,
-        help="specs of packages to search for")
     listcache.set_defaults(func=listspecs)
 
     dlkeys = subparsers.add_parser('keys', help=getkeys.__doc__)
@@ -113,10 +106,9 @@ def setup_parser(subparser):
         help='analyzes an installed spec and reports whether '
              'executables and libraries are relocatable'
     )
-    preview_parser.add_argument(
+    arguments.add_common_arguments(preview_parser, ['installed_specs'])
-        'packages', nargs='+', help='list of installed packages'
-    )
     preview_parser.set_defaults(func=preview)
+
     # Check if binaries need to be rebuilt on remote mirror
     check = subparsers.add_parser('check', help=check_binaries.__doc__)
     check.add_argument(
@@ -313,8 +305,10 @@ def _createtarball(env, spec_yaml, packages, directory, key, no_deps, force,
             tty.debug(yaml_text)
             s = Spec.from_yaml(yaml_text)
             packages.add('/{0}'.format(s.dag_hash()))
+
     elif packages:
         packages = packages
+
     else:
         tty.die("build cache file creation requires at least one" +
                 " installed package argument or else path to a" +
@@ -378,17 +372,17 @@ def createtarball(args):
     # restrict matching to current environment if one is active
     env = ev.get_env(args, 'buildcache create')
 
-    _createtarball(env, args.spec_yaml, args.packages, args.directory,
+    _createtarball(env, args.spec_yaml, args.specs, args.directory,
                    args.key, args.no_deps, args.force, args.rel, args.unsigned,
                    args.allow_root, args.no_rebuild_index)
 
 
 def installtarball(args):
     """install from a binary package"""
-    if not args.packages:
+    if not args.specs:
         tty.die("build cache file installation requires" +
                 " at least one package spec argument")
-    pkgs = set(args.packages)
+    pkgs = set(args.specs)
     matches = match_downloaded_specs(pkgs, args.multiple, args.force)
 
     for match in matches:
@@ -422,8 +416,8 @@ def install_tarball(spec, args):
 def listspecs(args):
     """list binary packages available from mirrors"""
     specs = bindist.get_specs(args.force)
-    if args.packages:
+    if args.specs:
-        constraints = set(args.packages)
+        constraints = set(args.specs)
         specs = [s for s in specs if any(s.satisfies(c) for c in constraints)]
     display_specs(specs, args, all_headers=True)
 
@@ -440,7 +434,7 @@ def preview(args):
     Args:
         args: command line arguments
     """
-    specs = find_matching_specs(args.packages, allow_multiple_matches=True)
+    specs = find_matching_specs(args.specs, allow_multiple_matches=True)
 
     # Cycle over the specs that match
     for spec in specs:

lib/spack/spack/cmd/checksum.py

@@ -10,6 +10,7 @@
 import llnl.util.tty as tty
 
 import spack.cmd
+import spack.cmd.common.arguments as arguments
 import spack.repo
 import spack.stage
 import spack.util.crypto
@@ -22,12 +23,10 @@
 
 
 def setup_parser(subparser):
-    subparser.add_argument(
-        'package',
-        help='package to checksum versions for')
     subparser.add_argument(
         '--keep-stage', action='store_true',
         help="don't clean up staging area when command completes")
+    arguments.add_common_arguments(subparser, ['package'])
     subparser.add_argument(
         'versions', nargs=argparse.REMAINDER,
         help='versions to generate checksums for')

lib/spack/spack/cmd/clean.py

@@ -11,6 +11,7 @@
 
 import spack.caches
 import spack.cmd
+import spack.cmd.common.arguments as arguments
 import spack.repo
 import spack.stage
 from spack.paths import lib_path, var_path
@@ -43,11 +44,7 @@ def setup_parser(subparser):
     subparser.add_argument(
         '-a', '--all', action=AllClean, help="equivalent to -sdmp", nargs=0
     )
-    subparser.add_argument(
+    arguments.add_common_arguments(subparser, ['specs'])
-        'specs',
-        nargs=argparse.REMAINDER,
-        help="removes the build stages and tarballs for specs"
-    )
 
 
 def clean(parser, args):

lib/spack/spack/cmd/commands.py

@@ -5,13 +5,16 @@
 
 from __future__ import print_function
 
-import sys
+import argparse
+import copy
 import os
 import re
-import argparse
+import sys
 
 import llnl.util.tty as tty
-from llnl.util.argparsewriter import ArgparseWriter, ArgparseRstWriter
+from llnl.util.argparsewriter import (
+    ArgparseWriter, ArgparseRstWriter, ArgparseCompletionWriter
+)
 from llnl.util.tty.colify import colify
 
 import spack.cmd
@@ -35,6 +38,8 @@ def formatter(func):
 
 
 def setup_parser(subparser):
+    subparser.add_argument(
+        '-a', '--aliases', action='store_true', help='include command aliases')
     subparser.add_argument(
         '--format', default='names', choices=formatters,
         help='format to be used to print the output (default: names)')
@@ -52,29 +57,97 @@ def setup_parser(subparser):
 class SpackArgparseRstWriter(ArgparseRstWriter):
     """RST writer tailored for spack documentation."""
 
-    def __init__(self, documented_commands, out=sys.stdout):
+    def __init__(self, prog, out=sys.stdout, aliases=False,
-        super(SpackArgparseRstWriter, self).__init__(out)
+                 documented_commands=[],
-        self.documented = documented_commands if documented_commands else []
+                 rst_levels=['-', '-', '^', '~', ':', '`']):
+        super(SpackArgparseRstWriter, self).__init__(
+            prog, out, aliases, rst_levels)
+        self.documented = documented_commands
 
     def usage(self, *args):
-        super(SpackArgparseRstWriter, self).usage(*args)
+        string = super(SpackArgparseRstWriter, self).usage(*args)
-        cmd = re.sub(' ', '-', self.parser.prog)
+
+        cmd = self.parser.prog.replace(' ', '-')
         if cmd in self.documented:
-            self.line()
+            string += '\n:ref:`More documentation <cmd-{0}>`\n'.format(cmd)
-            self.line(':ref:`More documentation <cmd-%s>`' % cmd)
+
+        return string
 
 
 class SubcommandWriter(ArgparseWriter):
-    def begin_command(self, prog):
+    def format(self, cmd):
-        self.out.write('    ' * self.level + prog)
+        return '    ' * self.level + cmd.prog + '\n'
-        self.out.write('\n')
+
+
+_positional_to_subroutine = {
+    'package': '_all_packages',
+    'spec': '_all_packages',
+    'filter': '_all_packages',
+    'installed': '_installed_packages',
+    'compiler': '_installed_compilers',
+    'section': '_config_sections',
+    'env': '_environments',
+    'extendable': '_extensions',
+    'keys': '_keys',
+    'help_command': '_subcommands',
+    'mirror': '_mirrors',
+    'virtual': '_providers',
+    'namespace': '_repos',
+    'hash': '_all_resource_hashes',
+    'pytest': '_tests',
+}
+
+
+class BashCompletionWriter(ArgparseCompletionWriter):
+    """Write argparse output as bash programmable tab completion."""
+
+    def body(self, positionals, optionals, subcommands):
+        if positionals:
+            return """
+    if $list_options
+    then
+        {0}
+    else
+        {1}
+    fi
+""".format(self.optionals(optionals), self.positionals(positionals))
+        elif subcommands:
+            return """
+    if $list_options
+    then
+        {0}
+    else
+        {1}
+    fi
+""".format(self.optionals(optionals), self.subcommands(subcommands))
+        else:
+            return """
+    {0}
+""".format(self.optionals(optionals))
+
+    def positionals(self, positionals):
+        # If match found, return function name
+        for positional in positionals:
+            for key, value in _positional_to_subroutine.items():
+                if positional.startswith(key):
+                    return value
+
+        # If no matches found, return empty list
+        return 'SPACK_COMPREPLY=""'
+
+    def optionals(self, optionals):
+        return 'SPACK_COMPREPLY="{0}"'.format(' '.join(optionals))
+
+    def subcommands(self, subcommands):
+        return 'SPACK_COMPREPLY="{0}"'.format(' '.join(subcommands))
 
 
 @formatter
 def subcommands(args, out):
     parser = spack.main.make_argument_parser()
     spack.main.add_all_commands(parser)
-    SubcommandWriter(out).write(parser)
+    writer = SubcommandWriter(parser.prog, out, args.aliases)
+    writer.write(parser)
 
 
 def rst_index(out):
@@ -124,12 +197,28 @@ def rst(args, out):
     out.write('\n')
 
     # print sections for each command and subcommand
-    SpackArgparseRstWriter(documented_commands, out).write(parser, root=1)
+    writer = SpackArgparseRstWriter(
+        parser.prog, out, args.aliases, documented_commands)
+    writer.write(parser)
 
 
 @formatter
 def names(args, out):
-    colify(spack.cmd.all_commands(), output=out)
+    commands = copy.copy(spack.cmd.all_commands())
+
+    if args.aliases:
+        commands.extend(spack.main.aliases.keys())
+
+    colify(commands, output=out)
+
+
+@formatter
+def bash(args, out):
+    parser = spack.main.make_argument_parser()
+    spack.main.add_all_commands(parser)
+
+    writer = BashCompletionWriter(parser.prog, out, args.aliases)
+    writer.write(parser)
 
 
 def prepend_header(args, out):
@@ -148,12 +237,14 @@ def commands(parser, args):
         tty.die("No such file: '%s'" % args.header)
 
     # if we're updating an existing file, only write output if a command
-    # is newer than the file.
+    # or the header is newer than the file.
     if args.update:
         if os.path.exists(args.update):
             files = [
                 spack.cmd.get_module(command).__file__.rstrip('c')  # pyc -> py
                 for command in spack.cmd.all_commands()]
+            if args.header:
+                files.append(args.header)
             last_update = os.path.getmtime(args.update)
             if not any(os.path.getmtime(f) > last_update for f in files):
                 tty.msg('File is up to date: %s' % args.update)

lib/spack/spack/cmd/common/arguments.py

@@ -120,7 +120,7 @@ def default(self, value):
 
 
 class DeptypeAction(argparse.Action):
-    """Creates a tuple of valid dependency tpyes from a deptype argument."""
+    """Creates a tuple of valid dependency types from a deptype argument."""
     def __call__(self, parser, namespace, values, option_string=None):
         deptype = dep.all_deptypes
         if values:
@@ -132,11 +132,53 @@ def __call__(self, parser, namespace, values, option_string=None):
         setattr(namespace, self.dest, deptype)
 
 
+# TODO: merge constraint and installed_specs
 @arg
 def constraint():
     return Args(
         'constraint', nargs=argparse.REMAINDER, action=ConstraintAction,
-        help='constraint to select a subset of installed packages')
+        help='constraint to select a subset of installed packages',
+        metavar='installed_specs')
+
+
+@arg
+def package():
+    return Args('package', help='package name')
+
+
+@arg
+def packages():
+    return Args(
+        'packages', nargs='+', help='one or more package names',
+        metavar='package')
+
+
+# Specs must use `nargs=argparse.REMAINDER` because a single spec can
+# contain spaces, and contain variants like '-mpi' that argparse thinks
+# are a collection of optional flags.
+@arg
+def spec():
+    return Args('spec', nargs=argparse.REMAINDER, help='package spec')
+
+
+@arg
+def specs():
+    return Args(
+        'specs', nargs=argparse.REMAINDER, help='one or more package specs')
+
+
+@arg
+def installed_spec():
+    return Args(
+        'spec', nargs=argparse.REMAINDER, help='installed package spec',
+        metavar='installed_spec')
+
+
+@arg
+def installed_specs():
+    return Args(
+        'specs', nargs=argparse.REMAINDER,
+        help='one or more installed package specs', metavar='installed_specs')
 
 
 @arg

lib/spack/spack/cmd/config.py

@@ -34,7 +34,7 @@ def setup_parser(subparser):
                             help="configuration section to print. "
                                  "options: %(choices)s",
                             nargs='?',
-                            metavar='SECTION',
+                            metavar='section',
                             choices=spack.config.section_schemas)
 
     blame_parser = sp.add_parser(
@@ -42,14 +42,14 @@ def setup_parser(subparser):
     blame_parser.add_argument('section',
                               help="configuration section to print. "
                               "options: %(choices)s",
-                              metavar='SECTION',
+                              metavar='section',
                               choices=spack.config.section_schemas)
 
     edit_parser = sp.add_parser('edit', help='edit configuration file')
     edit_parser.add_argument('section',
                              help="configuration section to edit. "
                                   "options: %(choices)s",
-                             metavar='SECTION',
+                             metavar='section',
                              nargs='?',
                              choices=spack.config.section_schemas)
     edit_parser.add_argument(

lib/spack/spack/cmd/configure.py

@@ -7,6 +7,7 @@
 
 import llnl.util.tty as tty
 import spack.cmd
+import spack.cmd.common.arguments as arguments
 import spack.cmd.install as inst
 
 from spack.build_systems.autotools import AutotoolsPackage
@@ -36,16 +37,12 @@
 
 
 def setup_parser(subparser):
-    subparser.add_argument(
-        'package',
-        nargs=argparse.REMAINDER,
-        help="spec of the package to install"
-    )
     subparser.add_argument(
         '-v', '--verbose',
         action='store_true',
         help="print additional output during builds"
     )
+    arguments.add_common_arguments(subparser, ['spec'])
 
 
 def _stop_at_phase_during_install(args, calling_fn, phase_mapping):
@@ -64,15 +61,15 @@ def _stop_at_phase_during_install(args, calling_fn, phase_mapping):
         # Install package dependencies if needed
         parser = argparse.ArgumentParser()
         inst.setup_parser(parser)
-        tty.msg('Checking dependencies for {0}'.format(args.package[0]))
+        tty.msg('Checking dependencies for {0}'.format(args.spec[0]))
         cli_args = ['-v'] if args.verbose else []
         install_args = parser.parse_args(cli_args + ['--only=dependencies'])
-        install_args.package = args.package
+        install_args.spec = args.spec
         inst.install(parser, install_args)
         # Install package and stop at the given phase
         cli_args = ['-v'] if args.verbose else []
         install_args = parser.parse_args(cli_args + ['--only=package'])
-        install_args.package = args.package
+        install_args.spec = args.spec
         inst.install(parser, install_args, stop_at=phase)
     except IndexError:
         tty.error(

lib/spack/spack/cmd/deactivate.py

@@ -3,10 +3,10 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
-import argparse
 import llnl.util.tty as tty
 
 import spack.cmd
+import spack.cmd.common.arguments as arguments
 import spack.environment as ev
 import spack.store
 from spack.filesystem_view import YamlFilesystemView
@@ -28,9 +28,7 @@ def setup_parser(subparser):
         '-a', '--all', action='store_true',
         help="deactivate all extensions of an extendable package, or "
         "deactivate an extension AND its dependencies")
-    subparser.add_argument(
+    arguments.add_common_arguments(subparser, ['installed_spec'])
-        'spec', nargs=argparse.REMAINDER,
-        help="spec of package extension to deactivate")
 
 
 def deactivate(parser, args):

lib/spack/spack/cmd/dependencies.py

@@ -3,8 +3,6 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
-import argparse
-
 import llnl.util.tty as tty
 from llnl.util.tty.colify import colify
 
@@ -31,8 +29,7 @@ def setup_parser(subparser):
     subparser.add_argument(
         '-V', '--no-expand-virtuals', action='store_false', default=True,
         dest="expand_virtuals", help="do not expand virtual dependencies")
-    subparser.add_argument(
+    arguments.add_common_arguments(subparser, ['spec'])
-        'spec', nargs=argparse.REMAINDER, help="spec or package name")
 
 
 def dependencies(parser, args):

lib/spack/spack/cmd/dependents.py

@@ -3,15 +3,14 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
-import argparse
-
 import llnl.util.tty as tty
 from llnl.util.tty.colify import colify
 
+import spack.cmd
+import spack.cmd.common.arguments as arguments
 import spack.environment as ev
 import spack.repo
 import spack.store
-import spack.cmd
 
 description = "show packages that depend on another"
 section = "basic"
@@ -26,8 +25,7 @@ def setup_parser(subparser):
     subparser.add_argument(
         '-t', '--transitive', action='store_true', default=False,
         help="Show all transitive dependents.")
-    subparser.add_argument(
+    arguments.add_common_arguments(subparser, ['spec'])
-        'spec', nargs=argparse.REMAINDER, help="spec or package name")
 
 
 def inverted_dependencies():

lib/spack/spack/cmd/dev_build.py

@@ -5,14 +5,13 @@
 
 import sys
 import os
-import argparse
 
 import llnl.util.tty as tty
 
 import spack.config
 import spack.cmd
-import spack.repo
 import spack.cmd.common.arguments as arguments
+import spack.repo
 from spack.stage import DIYStage
 
 description = "developer build: build from code in current working directory"
@@ -41,9 +40,7 @@ def setup_parser(subparser):
     subparser.add_argument(
         '-u', '--until', type=str, dest='until', default=None,
         help="phase to stop after when installing (default None)")
-    subparser.add_argument(
+    arguments.add_common_arguments(subparser, ['spec'])
-        'spec', nargs=argparse.REMAINDER,
-        help="specs to use for install. must contain package AND version")
 
     cd_group = subparser.add_mutually_exclusive_group()
     arguments.add_common_arguments(cd_group, ['clean', 'dirty'])

lib/spack/spack/cmd/edit.py

@@ -84,12 +84,11 @@ def setup_parser(subparser):
         help="namespace of package to edit")
 
     subparser.add_argument(
-        'name', nargs='?', default=None,
+        'package', nargs='?', default=None, help="package name")
-        help="name of package to edit")
 
 
 def edit(parser, args):
-    name = args.name
+    name = args.package
 
     # By default, edit package files
     path = spack.paths.packages_path

lib/spack/spack/cmd/env.py

@@ -157,7 +157,7 @@ def env_deactivate(args):
 def env_create_setup_parser(subparser):
     """create a new environment"""
     subparser.add_argument(
-        'create_env', metavar='ENV', help='name of environment to create')
+        'create_env', metavar='env', help='name of environment to create')
     subparser.add_argument(
         '-d', '--dir', action='store_true',
         help='create an environment in a specific directory')
@@ -221,7 +221,7 @@ def _env_create(name_or_path, init_file=None, dir=False, with_view=None):
 def env_remove_setup_parser(subparser):
     """remove an existing environment"""
     subparser.add_argument(
-        'rm_env', metavar='ENV', nargs='+',
+        'rm_env', metavar='env', nargs='+',
         help='environment(s) to remove')
     arguments.add_common_arguments(subparser, ['yes_to_all'])
 

lib/spack/spack/cmd/extensions.py

@@ -37,7 +37,7 @@ def setup_parser(subparser):
 
     subparser.add_argument(
         'spec', nargs=argparse.REMAINDER,
-        help='spec of package to list extensions for')
+        help='spec of package to list extensions for', metavar='extendable')
 
 
 def extensions(parser, args):

lib/spack/spack/cmd/fetch.py

@@ -3,14 +3,12 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
-import argparse
-
 import llnl.util.tty as tty
 
 import spack.cmd
+import spack.cmd.common.arguments as arguments
 import spack.config
 import spack.repo
-import spack.cmd.common.arguments as arguments
 
 description = "fetch archives for packages"
 section = "build"
@@ -25,19 +23,17 @@ def setup_parser(subparser):
     subparser.add_argument(
         '-D', '--dependencies', action='store_true',
         help="also fetch all dependencies")
-    subparser.add_argument(
+    arguments.add_common_arguments(subparser, ['specs'])
-        'packages', nargs=argparse.REMAINDER,
-        help="specs of packages to fetch")
 
 
 def fetch(parser, args):
-    if not args.packages:
+    if not args.specs:
         tty.die("fetch requires at least one package argument")
 
     if args.no_checksum:
         spack.config.set('config:checksum', False, scope='command_line')
 
-    specs = spack.cmd.parse_specs(args.packages, concretize=True)
+    specs = spack.cmd.parse_specs(args.specs, concretize=True)
     for spec in specs:
         if args.missing or args.dependencies:
             for s in spec.traverse():

lib/spack/spack/cmd/gpg.py

@@ -6,6 +6,7 @@
 import os
 import argparse
 
+import spack.cmd.common.arguments as arguments
 import spack.paths
 from spack.util.gpg import Gpg
 
@@ -19,8 +20,7 @@ def setup_parser(subparser):
     subparsers = subparser.add_subparsers(help='GPG sub-commands')
 
     verify = subparsers.add_parser('verify', help=gpg_verify.__doc__)
-    verify.add_argument('package', type=str,
+    arguments.add_common_arguments(verify, ['installed_spec'])
-                        help='the package to verify')
     verify.add_argument('signature', type=str, nargs='?',
                         help='the signature file')
     verify.set_defaults(func=gpg_verify)
@@ -44,8 +44,7 @@ def setup_parser(subparser):
                       help='the key to use for signing')
     sign.add_argument('--clearsign', action='store_true',
                       help='if specified, create a clearsign signature')
-    sign.add_argument('package', type=str,
+    arguments.add_common_arguments(sign, ['installed_spec'])
-                      help='the package to sign')
     sign.set_defaults(func=gpg_sign)
 
     create = subparsers.add_parser('create', help=gpg_create.__doc__)
@@ -122,9 +121,9 @@ def gpg_sign(args):
                                'please choose one')
     output = args.output
     if not output:
-        output = args.package + '.asc'
+        output = args.spec[0] + '.asc'
     # TODO: Support the package format Spack creates.
-    Gpg.sign(key, args.package, output, args.clearsign)
+    Gpg.sign(key, ' '.join(args.spec), output, args.clearsign)
 
 
 def gpg_trust(args):
@@ -155,8 +154,8 @@ def gpg_verify(args):
     # TODO: Support the package format Spack creates.
     signature = args.signature
     if signature is None:
-        signature = args.package + '.asc'
+        signature = args.spec[0] + '.asc'
-    Gpg.verify(signature, args.package)
+    Gpg.verify(signature, ' '.join(args.spec))
 
 
 def gpg(parser, args):

lib/spack/spack/cmd/graph.py

@@ -5,7 +5,6 @@
 
 from __future__ import print_function
 
-import argparse
 import llnl.util.tty as tty
 
 import spack.cmd
@@ -38,11 +37,7 @@ def setup_parser(subparser):
         '-i', '--installed', action='store_true',
         help="graph all installed specs in dot format (implies --dot)")
 
-    arguments.add_common_arguments(subparser, ['deptype'])
+    arguments.add_common_arguments(subparser, ['deptype', 'specs'])
-
-    subparser.add_argument(
-        'specs', nargs=argparse.REMAINDER,
-        help="specs of packages to graph")
 
 
 def graph(parser, args):

lib/spack/spack/cmd/info.py

@@ -11,6 +11,7 @@
 import llnl.util.tty.color as color
 from llnl.util.tty.colify import colify
 
+import spack.cmd.common.arguments as arguments
 import spack.repo
 import spack.spec
 import spack.fetch_strategy as fs
@@ -36,8 +37,7 @@ def pad(string):
 
 
 def setup_parser(subparser):
-    subparser.add_argument(
+    arguments.add_common_arguments(subparser, ['package'])
-        'name', metavar='PACKAGE', help='name of package to get info for')
 
 
 def section_title(s):
@@ -237,5 +237,5 @@ def print_text_info(pkg):
 
 
 def info(parser, args):
-    pkg = spack.repo.get(args.name)
+    pkg = spack.repo.get(args.package)
     print_text_info(pkg)

lib/spack/spack/cmd/install.py

@@ -122,11 +122,6 @@ def setup_parser(subparser):
     cd_group = subparser.add_mutually_exclusive_group()
     arguments.add_common_arguments(cd_group, ['clean', 'dirty'])
 
-    subparser.add_argument(
-        'package',
-        nargs=argparse.REMAINDER,
-        help="spec of the package to install"
-    )
     testing = subparser.add_mutually_exclusive_group()
     testing.add_argument(
         '--test', default=None,
@@ -157,7 +152,7 @@ def setup_parser(subparser):
         help="Show usage instructions for CDash reporting"
     )
     add_cdash_args(subparser, False)
-    arguments.add_common_arguments(subparser, ['yes_to_all'])
+    arguments.add_common_arguments(subparser, ['yes_to_all', 'spec'])
 
 
 def add_cdash_args(subparser, add_help):
@@ -258,7 +253,7 @@ def install(parser, args, **kwargs):
         parser.print_help()
         return
 
-    if not args.package and not args.specfiles:
+    if not args.spec and not args.specfiles:
         # if there are no args but an active environment or spack.yaml file
         # then install the packages from it.
         env = ev.get_env(args, 'install')
@@ -293,7 +288,7 @@ def install(parser, args, **kwargs):
     if args.log_file:
         reporter.filename = args.log_file
 
-    abstract_specs = spack.cmd.parse_specs(args.package)
+    abstract_specs = spack.cmd.parse_specs(args.spec)
     tests = False
     if args.test == 'all' or args.run_tests:
         tests = True
@@ -303,7 +298,7 @@ def install(parser, args, **kwargs):
 
     try:
         specs = spack.cmd.parse_specs(
-            args.package, concretize=True, tests=tests)
+            args.spec, concretize=True, tests=tests)
     except SpackError as e:
         tty.debug(e)
         reporter.concretization_report(e.message)

lib/spack/spack/cmd/load.py

@@ -3,7 +3,6 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
-import argparse
 from spack.cmd.common import print_module_placeholder_help, arguments
 
 description = "add package to environment using `module load`"
@@ -14,11 +13,8 @@
 def setup_parser(subparser):
     """Parser is only constructed so that this prints a nice help
        message with -h. """
-    subparser.add_argument(
+    arguments.add_common_arguments(
-        'spec', nargs=argparse.REMAINDER,
+        subparser, ['recurse_dependencies', 'installed_spec'])
-        help="spec of package to load with modules "
-    )
-    arguments.add_common_arguments(subparser, ['recurse_dependencies'])
 
 
 def load(parser, args):

lib/spack/spack/cmd/location.py

@@ -6,11 +6,11 @@
 from __future__ import print_function
 
 import os
-import argparse
 import llnl.util.tty as tty
 
 import spack.environment as ev
 import spack.cmd
+import spack.cmd.common.arguments as arguments
 import spack.environment
 import spack.paths
 import spack.repo
@@ -55,9 +55,7 @@ def setup_parser(subparser):
         '-e', '--env', action='store',
         help="location of an environment managed by spack")
 
-    subparser.add_argument(
+    arguments.add_common_arguments(subparser, ['spec'])
-        'spec', nargs=argparse.REMAINDER,
-        help="spec of package to fetch directory for")
 
 
 def location(parser, args):

lib/spack/spack/cmd/maintainers.py

@@ -40,7 +40,7 @@ def setup_parser(subparser):
 
     # options for commands that take package arguments
     subparser.add_argument(
-        'pkg_or_user', nargs=argparse.REMAINDER,
+        'package_or_user', nargs=argparse.REMAINDER,
         help='names of packages or users to get info for')
 
 
@@ -104,31 +104,31 @@ def maintainers(parser, args):
 
     if args.all:
         if args.by_user:
-            maintainers = maintainers_to_packages(args.pkg_or_user)
+            maintainers = maintainers_to_packages(args.package_or_user)
             for user, packages in sorted(maintainers.items()):
                 color.cprint('@c{%s}: %s'
                              % (user, ', '.join(sorted(packages))))
             return 0 if maintainers else 1
 
         else:
-            packages = packages_to_maintainers(args.pkg_or_user)
+            packages = packages_to_maintainers(args.package_or_user)
             for pkg, maintainers in sorted(packages.items()):
                 color.cprint('@c{%s}: %s'
                              % (pkg, ', '.join(sorted(maintainers))))
             return 0 if packages else 1
 
     if args.by_user:
-        if not args.pkg_or_user:
+        if not args.package_or_user:
             tty.die('spack maintainers --by-user requires a user or --all')
 
-        packages = union_values(maintainers_to_packages(args.pkg_or_user))
+        packages = union_values(maintainers_to_packages(args.package_or_user))
         colify(packages)
         return 0 if packages else 1
 
     else:
-        if not args.pkg_or_user:
+        if not args.package_or_user:
             tty.die('spack maintainers requires a package or --all')
 
-        users = union_values(packages_to_maintainers(args.pkg_or_user))
+        users = union_values(packages_to_maintainers(args.package_or_user))
         colify(users)
         return 0 if users else 1

lib/spack/spack/cmd/mirror.py

@@ -5,7 +5,6 @@
 
 import sys
 
-import argparse
 import llnl.util.tty as tty
 from llnl.util.tty.colify import colify
 
@@ -39,9 +38,6 @@ def setup_parser(subparser):
     create_parser.add_argument('-d', '--directory', default=None,
                                help="directory in which to create mirror")
 
-    create_parser.add_argument(
-        'specs', nargs=argparse.REMAINDER,
-        help="specs of packages to put in mirror")
     create_parser.add_argument(
         '-a', '--all', action='store_true',
         help="mirror all versions of all packages in Spack, or all packages"
@@ -57,6 +53,7 @@ def setup_parser(subparser):
         '-n', '--versions-per-spec',
         help="the number of versions to fetch for each spec, choose 'all' to"
              " retrieve all versions of each package")
+    arguments.add_common_arguments(create_parser, ['specs'])
 
     # used to construct scope arguments below
     scopes = spack.config.scopes()
@@ -64,7 +61,8 @@ def setup_parser(subparser):
 
     # 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(
+        'name', help="mnemonic name for mirror", metavar="mirror")
     add_parser.add_argument(
         'url', help="url of mirror directory from 'spack mirror create'")
     add_parser.add_argument(
@@ -75,7 +73,8 @@ def setup_parser(subparser):
     # Remove
     remove_parser = sp.add_parser('remove', aliases=['rm'],
                                   help=mirror_remove.__doc__)
-    remove_parser.add_argument('name')
+    remove_parser.add_argument(
+        'name', help="mnemonic name for mirror", metavar="mirror")
     remove_parser.add_argument(
         '--scope', choices=scopes, metavar=scopes_metavar,
         default=spack.config.default_modify_scope(),
@@ -83,7 +82,8 @@ def setup_parser(subparser):
 
     # Set-Url
     set_url_parser = sp.add_parser('set-url', help=mirror_set_url.__doc__)
-    set_url_parser.add_argument('name', help="mnemonic name for mirror")
+    set_url_parser.add_argument(
+        'name', help="mnemonic name for mirror", metavar="mirror")
     set_url_parser.add_argument(
         'url', help="url of mirror directory from 'spack mirror create'")
     set_url_parser.add_argument(

lib/spack/spack/cmd/patch.py

@@ -3,8 +3,6 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
-import argparse
-
 import llnl.util.tty as tty
 
 import spack.repo
@@ -18,20 +16,17 @@
 
 
 def setup_parser(subparser):
-    arguments.add_common_arguments(subparser, ['no_checksum'])
+    arguments.add_common_arguments(subparser, ['no_checksum', 'specs'])
-    subparser.add_argument(
-        'packages', nargs=argparse.REMAINDER,
-        help="specs of packages to stage")
 
 
 def patch(parser, args):
-    if not args.packages:
+    if not args.specs:
-        tty.die("patch requires at least one package argument")
+        tty.die("patch requires at least one spec argument")
 
     if args.no_checksum:
         spack.config.set('config:checksum', False, scope='command_line')
 
-    specs = spack.cmd.parse_specs(args.packages, concretize=True)
+    specs = spack.cmd.parse_specs(args.specs, concretize=True)
     for spec in specs:
         package = spack.repo.get(spec)
         package.do_patch()

lib/spack/spack/cmd/pkg.py

@@ -6,7 +6,6 @@
 from __future__ import print_function
 
 import os
-import argparse
 import re
 
 import llnl.util.tty as tty
@@ -14,6 +13,7 @@
 from llnl.util.filesystem import working_dir
 
 import spack.cmd
+import spack.cmd.common.arguments as arguments
 import spack.paths
 import spack.repo
 from spack.util.executable import which
@@ -28,8 +28,7 @@ def setup_parser(subparser):
         metavar='SUBCOMMAND', dest='pkg_command')
 
     add_parser = sp.add_parser('add', help=pkg_add.__doc__)
-    add_parser.add_argument('packages', nargs=argparse.REMAINDER,
+    arguments.add_common_arguments(add_parser, ['packages'])
-                            help="names of packages to add to git repo")
 
     list_parser = sp.add_parser('list', help=pkg_list.__doc__)
     list_parser.add_argument('rev', default='HEAD', nargs='?',

lib/spack/spack/cmd/remove.py

@@ -3,11 +3,10 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
-import argparse
-
 import llnl.util.tty as tty
 
 import spack.cmd
+import spack.cmd.common.arguments as arguments
 import spack.environment as ev
 
 
@@ -26,8 +25,7 @@ def setup_parser(subparser):
     subparser.add_argument(
         '-f', '--force', action='store_true',
         help="remove concretized spec (if any) immediately")
-    subparser.add_argument(
+    arguments.add_common_arguments(subparser, ['specs'])
-        'specs', nargs=argparse.REMAINDER, help="specs to be removed")
 
 
 def remove(parser, args):

lib/spack/spack/cmd/repo.py

@@ -51,8 +51,8 @@ def setup_parser(subparser):
     remove_parser = sp.add_parser(
         'remove', help=repo_remove.__doc__, aliases=['rm'])
     remove_parser.add_argument(
-        'path_or_namespace',
+        'namespace_or_path',
-        help="path or namespace of a Spack package repository")
+        help="namespace or path of a Spack package repository")
     remove_parser.add_argument(
         '--scope', choices=scopes, metavar=scopes_metavar,
         default=spack.config.default_modify_scope(),
@@ -101,10 +101,10 @@ def repo_add(args):
 def repo_remove(args):
     """Remove a repository from Spack's configuration."""
     repos = spack.config.get('repos', scope=args.scope)
-    path_or_namespace = args.path_or_namespace
+    namespace_or_path = args.namespace_or_path
 
     # If the argument is a path, remove that repository from config.
-    canon_path = canonicalize_path(path_or_namespace)
+    canon_path = canonicalize_path(namespace_or_path)
     for repo_path in repos:
         repo_canon_path = canonicalize_path(repo_path)
         if canon_path == repo_canon_path:
@@ -117,7 +117,7 @@ def repo_remove(args):
     for path in repos:
         try:
             repo = Repo(path)
-            if repo.namespace == path_or_namespace:
+            if repo.namespace == namespace_or_path:
                 repos.remove(path)
                 spack.config.set('repos', repos, args.scope)
                 tty.msg("Removed repository %s with namespace '%s'."
@@ -127,7 +127,7 @@ def repo_remove(args):
             continue
 
     tty.die("No repository with path or namespace: %s"
-            % path_or_namespace)
+            % namespace_or_path)
 
 
 def repo_list(args):

lib/spack/spack/cmd/restage.py

@@ -3,11 +3,10 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
-import argparse
-
 import llnl.util.tty as tty
 
 import spack.cmd
+import spack.cmd.common.arguments as arguments
 import spack.repo
 
 description = "revert checked out package source code"
@@ -16,15 +15,14 @@
 
 
 def setup_parser(subparser):
-    subparser.add_argument('packages', nargs=argparse.REMAINDER,
+    arguments.add_common_arguments(subparser, ['specs'])
-                           help="specs of packages to restage")
 
 
 def restage(parser, args):
-    if not args.packages:
+    if not args.specs:
         tty.die("spack restage requires at least one package spec.")
 
-    specs = spack.cmd.parse_specs(args.packages, concretize=True)
+    specs = spack.cmd.parse_specs(args.specs, concretize=True)
     for spec in specs:
         package = spack.repo.get(spec)
         package.do_restage()

lib/spack/spack/cmd/setup.py

@@ -30,13 +30,10 @@ def setup_parser(subparser):
     subparser.add_argument(
         '-i', '--ignore-dependencies', action='store_true', dest='ignore_deps',
         help="do not try to install dependencies of requested packages")
-    arguments.add_common_arguments(subparser, ['no_checksum'])
+    arguments.add_common_arguments(subparser, ['no_checksum', 'spec'])
     subparser.add_argument(
         '-v', '--verbose', action='store_true', dest='verbose',
         help="display verbose build output while installing")
-    subparser.add_argument(
-        'spec', nargs=argparse.REMAINDER,
-        help="specs to use for install. must contain package AND version")
 
     cd_group = subparser.add_mutually_exclusive_group()
     arguments.add_common_arguments(cd_group, ['clean', 'dirty'])

lib/spack/spack/cmd/spec.py

@@ -5,7 +5,6 @@
 
 from __future__ import print_function
 
-import argparse
 import contextlib
 import sys
 
@@ -47,8 +46,7 @@ def setup_parser(subparser):
     subparser.add_argument(
         '-t', '--types', action='store_true', default=False,
         help='show dependency types')
-    subparser.add_argument(
+    arguments.add_common_arguments(subparser, ['specs'])
-        'specs', nargs=argparse.REMAINDER, help="specs of packages")
 
 
 @contextlib.contextmanager

lib/spack/spack/cmd/stage.py

@@ -3,8 +3,6 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
-import argparse
-
 import llnl.util.tty as tty
 
 import spack.environment as ev
@@ -18,14 +16,11 @@
 
 
 def setup_parser(subparser):
-    arguments.add_common_arguments(subparser, ['no_checksum'])
+    arguments.add_common_arguments(subparser, ['no_checksum', 'specs'])
     subparser.add_argument(
         '-p', '--path', dest='path',
         help="path to stage package, does not add to spack tree")
 
-    subparser.add_argument(
-        'specs', nargs=argparse.REMAINDER, help="specs of packages to stage")
-
 
 def stage(parser, args):
     if not args.specs:

lib/spack/spack/cmd/uninstall.py

@@ -5,7 +5,6 @@
 
 from __future__ import print_function
 
-import argparse
 import sys
 
 import spack.cmd
@@ -38,17 +37,13 @@
 }
 
 
-def add_common_arguments(subparser):
+def setup_parser(subparser):
     subparser.add_argument(
         '-f', '--force', action='store_true', dest='force',
         help="remove regardless of whether other packages or environments "
         "depend on this one")
     arguments.add_common_arguments(
-        subparser, ['recurse_dependents', 'yes_to_all'])
+        subparser, ['recurse_dependents', 'yes_to_all', 'installed_specs'])
-
-
-def setup_parser(subparser):
-    add_common_arguments(subparser)
     subparser.add_argument(
         '-a', '--all', action='store_true', dest='all',
         help="USE CAREFULLY. Remove ALL installed packages that match each "
@@ -58,11 +53,6 @@ def setup_parser(subparser):
         "If used in an environment, all packages in the environment "
         "will be uninstalled.")
 
-    subparser.add_argument(
-        'packages',
-        nargs=argparse.REMAINDER,
-        help="specs of packages to uninstall")
-
 
 def find_matching_specs(env, specs, allow_multiple_matches=False, force=False):
     """Returns a list of specs matching the not necessarily
@@ -351,10 +341,10 @@ def confirm_removal(specs):
 
 
 def uninstall(parser, args):
-    if not args.packages and not args.all:
+    if not args.specs and not args.all:
         tty.die('uninstall requires at least one package argument.',
                 '  Use `spack uninstall --all` to uninstall ALL packages.')
 
     # [any] here handles the --all case by forcing all specs to be returned
-    specs = spack.cmd.parse_specs(args.packages) if args.packages else [any]
+    specs = spack.cmd.parse_specs(args.specs) if args.specs else [any]
     uninstall_specs(args, specs)

lib/spack/spack/cmd/unload.py

@@ -3,8 +3,7 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
-import argparse
+from spack.cmd.common import print_module_placeholder_help, arguments
-from spack.cmd.common import print_module_placeholder_help
 
 description = "remove package from environment using `module unload`"
 section = "modules"
@@ -14,9 +13,7 @@
 def setup_parser(subparser):
     """Parser is only constructed so that this prints a nice help
        message with -h. """
-    subparser.add_argument(
+    arguments.add_common_arguments(subparser, ['installed_spec'])
-        'spec', nargs=argparse.REMAINDER,
-        help='spec of package to unload with modules')
 
 
 def unload(parser, args):

lib/spack/spack/cmd/verify.py

@@ -25,8 +25,8 @@ def setup_parser(subparser):
                            help="Ouptut json-formatted errors")
     subparser.add_argument('-a', '--all', action='store_true',
                            help="Verify all packages")
-    subparser.add_argument('files_or_specs', nargs=argparse.REMAINDER,
+    subparser.add_argument('specs_or_files', nargs=argparse.REMAINDER,
-                           help="Files or specs to verify")
+                           help="Specs or files to verify")
 
     type = subparser.add_mutually_exclusive_group()
     type.add_argument(
@@ -47,7 +47,7 @@ def verify(parser, args):
             setup_parser.parser.print_help()
             return 1
 
-        for file in args.files_or_specs:
+        for file in args.specs_or_files:
             results = spack.verify.check_file_manifest(file)
             if results.has_errors():
                 if args.json:
@@ -57,21 +57,21 @@ def verify(parser, args):
 
         return 0
     else:
-        spec_args = spack.cmd.parse_specs(args.files_or_specs)
+        spec_args = spack.cmd.parse_specs(args.specs_or_files)
 
     if args.all:
         query = spack.store.db.query_local if local else spack.store.db.query
 
         # construct spec list
         if spec_args:
-            spec_list = spack.cmd.parse_specs(args.files_or_specs)
+            spec_list = spack.cmd.parse_specs(args.specs_or_files)
             specs = []
             for spec in spec_list:
                 specs += query(spec, installed=True)
         else:
             specs = query(installed=True)
 
-    elif args.files_or_specs:
+    elif args.specs_or_files:
         # construct disambiguated spec list
         env = ev.get_env(args, 'verify')
         specs = list(map(lambda x: spack.cmd.disambiguate_spec(x, env,

lib/spack/spack/cmd/versions.py

@@ -5,11 +5,13 @@
 
 from __future__ import print_function
 
+import sys
+
 from llnl.util.tty.colify import colify
 import llnl.util.tty as tty
 
+import spack.cmd.common.arguments as arguments
 import spack.repo
-import sys
 
 description = "list available versions of a package"
 section = "packaging"
@@ -17,10 +19,9 @@
 
 
 def setup_parser(subparser):
-    subparser.add_argument('package', metavar='PACKAGE',
-                           help='package to list versions for')
     subparser.add_argument('-s', '--safe-only', action='store_true',
                            help='only list safe versions of the package')
+    arguments.add_common_arguments(subparser, ['package'])
 
 
 def versions(parser, args):

lib/spack/spack/reporters/cdash.py

@@ -72,8 +72,8 @@ def __init__(self, args):
             tty.verbose("Using CDash auth token from environment")
             self.authtoken = os.environ.get('SPACK_CDASH_AUTH_TOKEN')
 
-        if args.package:
+        if args.spec:
-            packages = args.package
+            packages = args.spec
         else:
             packages = []
             for file in args.specfiles:

lib/spack/spack/test/cmd/commands.py

@@ -3,13 +3,17 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
-import re
+import filecmp
+import os
+import subprocess
+
 import pytest
 
-from llnl.util.argparsewriter import ArgparseWriter
-
 import spack.cmd
+from spack.cmd.commands import _positional_to_subroutine
 import spack.main
+import spack.paths
+
 
 commands = spack.main.SpackCommand('commands')
 
@@ -17,38 +21,64 @@
 spack.main.add_all_commands(parser)
 
 
-def test_commands_by_name():
+def test_names():
     """Test default output of spack commands."""
-    out = commands()
+    out1 = commands().strip().split('\n')
-    assert out.strip().split('\n') == sorted(spack.cmd.all_commands())
+    assert out1 == spack.cmd.all_commands()
+    assert 'rm' not in out1
+
+    out2 = commands('--aliases').strip().split('\n')
+    assert out1 != out2
+    assert 'rm' in out2
+
+    out3 = commands('--format=names').strip().split('\n')
+    assert out1 == out3
 
 
 def test_subcommands():
     """Test subcommand traversal."""
-    out = commands('--format=subcommands')
+    out1 = commands('--format=subcommands')
-    assert 'spack mirror create' in out
+    assert 'spack mirror create' in out1
-    assert 'spack buildcache list' in out
+    assert 'spack buildcache list' in out1
-    assert 'spack repo add' in out
+    assert 'spack repo add' in out1
-    assert 'spack pkg diff' in out
+    assert 'spack pkg diff' in out1
-    assert 'spack url parse' in out
+    assert 'spack url parse' in out1
-    assert 'spack view symlink' in out
+    assert 'spack view symlink' in out1
+    assert 'spack rm' not in out1
+    assert 'spack compiler add' not in out1
 
-    class Subcommands(ArgparseWriter):
+    out2 = commands('--aliases', '--format=subcommands')
-        def begin_command(self, prog):
+    assert 'spack mirror create' in out2
-            assert prog in out
+    assert 'spack buildcache list' in out2
-
+    assert 'spack repo add' in out2
-    Subcommands().write(parser)
+    assert 'spack pkg diff' in out2
+    assert 'spack url parse' in out2
+    assert 'spack view symlink' in out2
+    assert 'spack rm' in out2
+    assert 'spack compiler add' in out2
 
 
 def test_rst():
     """Do some simple sanity checks of the rst writer."""
-    out = commands('--format=rst')
+    out1 = commands('--format=rst')
+    assert 'spack mirror create' in out1
+    assert 'spack buildcache list' in out1
+    assert 'spack repo add' in out1
+    assert 'spack pkg diff' in out1
+    assert 'spack url parse' in out1
+    assert 'spack view symlink' in out1
+    assert 'spack rm' not in out1
+    assert 'spack compiler add' not in out1
 
-    class Subcommands(ArgparseWriter):
+    out2 = commands('--aliases', '--format=rst')
-        def begin_command(self, prog):
+    assert 'spack mirror create' in out2
-            assert prog in out
+    assert 'spack buildcache list' in out2
-            assert re.sub(r' ', '-', prog) in out
+    assert 'spack repo add' in out2
-    Subcommands().write(parser)
+    assert 'spack pkg diff' in out2
+    assert 'spack url parse' in out2
+    assert 'spack view symlink' in out2
+    assert 'spack rm' in out2
+    assert 'spack compiler add' in out2
 
 
 def test_rst_with_input_files(tmpdir):
@@ -109,3 +139,91 @@ def test_rst_update(tmpdir):
     assert update_file.exists()
     with update_file.open() as f:
         assert f.read() == 'empty\n'
+
+
+def test_update_with_header(tmpdir):
+    update_file = tmpdir.join('output')
+
+    # not yet created when commands is run
+    commands('--update', str(update_file))
+    assert update_file.exists()
+    with update_file.open() as f:
+        assert f.read()
+    fake_header = 'this is a header!\n\n'
+
+    filename = tmpdir.join('header.txt')
+    with filename.open('w') as f:
+        f.write(fake_header)
+
+    # created, newer than commands, but older than header
+    commands('--update', str(update_file), '--header', str(filename))
+
+    # newer than commands and header
+    commands('--update', str(update_file), '--header', str(filename))
+
+
+@pytest.mark.xfail
+def test_no_pipe_error():
+    """Make sure we don't see any pipe errors when piping output."""
+
+    proc = subprocess.Popen(
+        ['spack', 'commands', '--format=rst'],
+        stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+
+    # Call close() on stdout to cause a broken pipe
+    proc.stdout.close()
+    proc.wait()
+    stderr = proc.stderr.read().decode('utf-8')
+
+    assert 'Broken pipe' not in stderr
+
+
+def test_bash_completion():
+    """Test the bash completion writer."""
+    out1 = commands('--format=bash')
+
+    # Make sure header not included
+    assert '_bash_completion_spack() {' not in out1
+    assert '_all_packages() {' not in out1
+
+    # Make sure subcommands appear
+    assert '_spack_remove() {' in out1
+    assert '_spack_compiler_find() {' in out1
+
+    # Make sure aliases don't appear
+    assert '_spack_rm() {' not in out1
+    assert '_spack_compiler_add() {' not in out1
+
+    # Make sure options appear
+    assert '-h --help' in out1
+
+    # Make sure subcommands are called
+    for function in _positional_to_subroutine.values():
+        assert function in out1
+
+    out2 = commands('--aliases', '--format=bash')
+
+    # Make sure aliases appear
+    assert '_spack_rm() {' in out2
+    assert '_spack_compiler_add() {' in out2
+
+
+def test_updated_completion_scripts(tmpdir):
+    """Make sure our shell tab completion scripts remain up-to-date."""
+
+    msg = ("It looks like Spack's command-line interface has been modified. "
+           "Please update Spack's shell tab completion scripts by running:\n\n"
+           "    share/spack/qa/update-completion-scripts.sh\n\n"
+           "and adding the changed files to your pull request.")
+
+    for shell in ['bash']:  # 'zsh', 'fish']:
+        header = os.path.join(
+            spack.paths.share_path, shell, 'spack-completion.in')
+        script = 'spack-completion.{0}'.format(shell)
+        old_script = os.path.join(spack.paths.share_path, script)
+        new_script = str(tmpdir.join(script))
+
+        commands('--aliases', '--format', shell,
+                 '--header', header, '--update', new_script)
+
+        assert filecmp.cmp(old_script, new_script), msg

lib/spack/spack/test/llnl/util/argparsewriter.py

@@ -0,0 +1,37 @@
+# Copyright 2013-2020 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+"""Tests for ``llnl/util/argparsewriter.py``
+
+These tests are fairly minimal, and ArgparseWriter is more extensively
+tested in ``cmd/commands.py``.
+"""
+
+import pytest
+
+import llnl.util.argparsewriter as aw
+
+import spack.main
+
+
+parser = spack.main.make_argument_parser()
+spack.main.add_all_commands(parser)
+
+
+def test_format_not_overridden():
+    writer = aw.ArgparseWriter('spack')
+
+    with pytest.raises(NotImplementedError):
+        writer.write(parser)
+
+
+def test_completion_format_not_overridden():
+    writer = aw.ArgparseCompletionWriter('spack')
+
+    assert writer.positionals([]) == ''
+    assert writer.optionals([]) == ''
+    assert writer.subcommands([]) == ''
+
+    writer.write(parser)

share/spack/bash/spack-completion.in

@@ -0,0 +1,309 @@
+# Copyright 2013-2020 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+
+# NOTE: spack-completion.bash is auto-generated by:
+#
+#   $ spack commands --aliases --format=bash
+#       --header=bash/spack-completion.in --update=spack-completion.bash
+#
+# Please do not manually modify this file.
+
+
+# The following global variables are set by Bash programmable completion:
+#
+#     COMP_CWORD:      An index into ${COMP_WORDS} of the word containing the
+#                      current cursor position
+#     COMP_KEY:        The key (or final key of a key sequence) used to invoke
+#                      the current completion function
+#     COMP_LINE:       The current command line
+#     COMP_POINT:      The index of the current cursor position relative to the
+#                      beginning of the current command
+#     COMP_TYPE:       Set to an integer value corresponding to the type of
+#                      completion attempted that caused a completion function
+#                      to be called
+#     COMP_WORDBREAKS: The set of characters that the readline library treats
+#                      as word separators when performing word completion
+#     COMP_WORDS:      An array variable consisting of the individual words in
+#                      the current command line
+#
+# The following global variable is used by Bash programmable completion:
+#
+#     COMPREPLY:       An array variable from which bash reads the possible
+#                      completions generated by a shell function invoked by the
+#                      programmable completion facility
+#
+# See `man bash` for more details.
+
+# Bash programmable completion for Spack
+_bash_completion_spack() {
+    # In all following examples, let the cursor be denoted by brackets, i.e. []
+
+    # For our purposes, flags should not affect tab completion. For instance,
+    # `spack install []` and `spack -d install --jobs 8 []` should both give the same
+    # possible completions. Therefore, we need to ignore any flags in COMP_WORDS.
+    local COMP_WORDS_NO_FLAGS=()
+    local index=0
+    while [[ "$index" -lt "$COMP_CWORD" ]]
+    do
+        if [[ "${COMP_WORDS[$index]}" == [a-z]* ]]
+        then
+            COMP_WORDS_NO_FLAGS+=("${COMP_WORDS[$index]}")
+        fi
+        let index++
+    done
+
+    # Options will be listed by a subfunction named after non-flag arguments.
+    # For example, `spack -d install []` will call _spack_install
+    # and `spack compiler add []` will call _spack_compiler_add
+    local subfunction=$(IFS='_'; echo "_${COMP_WORDS_NO_FLAGS[*]}")
+
+    # Translate dashes to underscores, as dashes are not permitted in
+    # compatibility mode. See https://github.com/spack/spack/pull/4079
+    subfunction=${subfunction//-/_}
+
+    # However, the word containing the current cursor position needs to be
+    # added regardless of whether or not it is a flag. This allows us to
+    # complete something like `spack install --keep-st[]`
+    COMP_WORDS_NO_FLAGS+=("${COMP_WORDS[$COMP_CWORD]}")
+
+    # Since we have removed all words after COMP_CWORD, we can safely assume
+    # that COMP_CWORD_NO_FLAGS is simply the index of the last element
+    local COMP_CWORD_NO_FLAGS=$((${#COMP_WORDS_NO_FLAGS[@]} - 1))
+
+    # There is no guarantee that the cursor is at the end of the command line
+    # when tab completion is envoked. For example, in the following situation:
+    #     `spack -d [] install`
+    # if the user presses the TAB key, a list of valid flags should be listed.
+    # Note that we cannot simply ignore everything after the cursor. In the
+    # previous scenario, the user should expect to see a list of flags, but
+    # not of other subcommands. Obviously, `spack -d list install` would be
+    # invalid syntax. To accomplish this, we use the variable list_options
+    # which is true if the current word starts with '-' or if the cursor is
+    # not at the end of the line.
+    local list_options=false
+    if [[ "${COMP_WORDS[$COMP_CWORD]}" == -* || "$COMP_POINT" -ne "${#COMP_LINE}" ]]
+    then
+        list_options=true
+    fi
+
+    # In general, when envoking tab completion, the user is not expecting to
+    # see optional flags mixed in with subcommands or package names. Tab
+    # completion is used by those who are either lazy or just bad at spelling.
+    # If someone doesn't remember what flag to use, seeing single letter flags
+    # in their results won't help them, and they should instead consult the
+    # documentation. However, if the user explicitly declares that they are
+    # looking for a flag, we can certainly help them out.
+    #     `spack install -[]`
+    # and
+    #     `spack install --[]`
+    # should list all flags and long flags, respectively. Furthermore, if a
+    # subcommand has no non-flag completions, such as `spack arch []`, it
+    # should list flag completions.
+
+    local cur=${COMP_WORDS_NO_FLAGS[$COMP_CWORD_NO_FLAGS]}
+
+    # If the cursor is in the middle of the line, like:
+    #     `spack -d [] install`
+    # COMP_WORDS will not contain the empty character, so we have to add it.
+    if [[ "${COMP_LINE:$COMP_POINT:1}" == " " ]]
+    then
+        cur=""
+    fi
+
+    # Uncomment this line to enable logging
+    #_test_vars >> temp
+
+    # Make sure function exists before calling it
+    if [[ "$(type -t $subfunction)" == "function" ]]
+    then
+        $subfunction
+        COMPREPLY=($(compgen -W "$SPACK_COMPREPLY" -- "$cur"))
+    fi
+}
+
+# Helper functions for subcommands
+# Results of each query are cached via environment variables
+
+_subcommands() {
+    if [[ -z "${SPACK_SUBCOMMANDS:-}" ]]
+    then
+        SPACK_SUBCOMMANDS="$(spack commands)"
+    fi
+    SPACK_COMPREPLY="$SPACK_SUBCOMMANDS"
+}
+
+_all_packages() {
+    if [[ -z "${SPACK_ALL_PACKAGES:-}" ]]
+    then
+        SPACK_ALL_PACKAGES="$(spack list)"
+    fi
+    SPACK_COMPREPLY="$SPACK_ALL_PACKAGES"
+}
+
+_all_resource_hashes() {
+    if [[ -z "${SPACK_ALL_RESOURCES_HASHES:-}" ]]
+    then
+        SPACK_ALL_RESOURCE_HASHES="$(spack resource list --only-hashes)"
+    fi
+    SPACK_COMPREPLY="$SPACK_ALL_RESOURCE_HASHES"
+}
+
+_installed_packages() {
+    if [[ -z "${SPACK_INSTALLED_PACKAGES:-}" ]]
+    then
+        SPACK_INSTALLED_PACKAGES="$(spack --color=never find --no-groups)"
+    fi
+    SPACK_COMPREPLY="$SPACK_INSTALLED_PACKAGES"
+}
+
+_installed_compilers() {
+    if [[ -z "${SPACK_INSTALLED_COMPILERS:-}" ]]
+    then
+        SPACK_INSTALLED_COMPILERS="$(spack compilers | egrep -v "^(-|=)")"
+    fi
+    SPACK_COMPREPLY="$SPACK_INSTALLED_COMPILERS"
+}
+
+_providers() {
+    if [[ -z "${SPACK_PROVIDERS:-}" ]]
+    then
+        SPACK_PROVIDERS="$(spack providers)"
+    fi
+    SPACK_COMPREPLY="$SPACK_PROVIDERS"
+}
+
+_mirrors() {
+    if [[ -z "${SPACK_MIRRORS:-}" ]]
+    then
+        SPACK_MIRRORS="$(spack mirror list | awk '{print $1}')"
+    fi
+    SPACK_COMPREPLY="$SPACK_MIRRORS"
+}
+
+_repos() {
+    if [[ -z "${SPACK_REPOS:-}" ]]
+    then
+        SPACK_REPOS="$(spack repo list | awk '{print $1}')"
+    fi
+    SPACK_COMPREPLY="$SPACK_REPOS"
+}
+
+_tests() {
+    if [[ -z "${SPACK_TESTS:-}" ]]
+    then
+        SPACK_TESTS="$(spack test -l)"
+    fi
+    SPACK_COMPREPLY="$SPACK_TESTS"
+}
+
+_environments() {
+    if [[ -z "${SPACK_ENVIRONMENTS:-}" ]]
+    then
+        SPACK_ENVIRONMENTS="$(spack env list)"
+    fi
+    SPACK_COMPREPLY="$SPACK_ENVIRONMENTS"
+}
+
+_keys() {
+    if [[ -z "${SPACK_KEYS:-}" ]]
+    then
+        SPACK_KEYS="$(spack gpg list)"
+    fi
+    SPACK_COMPREPLY="$SPACK_KEYS"
+}
+
+_config_sections() {
+    if [[ -z "${SPACK_CONFIG_SECTIONS:-}" ]]
+    then
+        SPACK_CONFIG_SECTIONS="compilers mirrors repos packages modules config upstreams"
+    fi
+    SPACK_COMPREPLY="$SPACK_CONFIG_SECTIONS"
+}
+
+_extensions() {
+    if [[ -z "${SPACK_EXTENSIONS:-}" ]]
+    then
+        SPACK_EXTENSIONS="aspell go-bootstrap go icedtea jdk kim-api lua matlab mofem-cephas octave openjdk perl python r ruby rust tcl yorick"
+    fi
+    SPACK_COMPREPLY="$SPACK_EXTENSIONS"
+}
+
+# Testing functions
+
+# Function for unit testing tab completion
+# Syntax: _spack_completions spack install py-
+_spack_completions() {
+    local COMP_CWORD COMP_KEY COMP_LINE COMP_POINT COMP_TYPE COMP_WORDS COMPREPLY
+
+    # Set each variable the way bash would
+    COMP_LINE="$*"
+    COMP_POINT=${#COMP_LINE}
+    COMP_WORDS=("$@")
+    if [[ ${COMP_LINE: -1} == ' ' ]]
+    then
+        COMP_WORDS+=('')
+    fi
+    COMP_CWORD=$((${#COMP_WORDS[@]} - 1))
+    COMP_KEY=9    # ASCII 09: Horizontal Tab
+    COMP_TYPE=64  # ASCII 64: '@', to list completions if the word is not unmodified
+
+    # Run Spack's tab completion function
+    _bash_completion_spack
+
+    # Return the result
+    echo "${COMPREPLY[@]:-}"
+}
+
+# Log the environment variables used
+# Syntax: _test_vars >> temp
+_test_vars() {
+    echo "-----------------------------------------------------"
+    echo "Variables set by bash:"
+    echo
+    echo "COMP_LINE:                '$COMP_LINE'"
+    echo "# COMP_LINE:              '${#COMP_LINE}'"
+    echo "COMP_WORDS:               $(_pretty_print COMP_WORDS[@])"
+    echo "# COMP_WORDS:             '${#COMP_WORDS[@]}'"
+    echo "COMP_CWORD:               '$COMP_CWORD'"
+    echo "COMP_KEY:                 '$COMP_KEY'"
+    echo "COMP_POINT:               '$COMP_POINT'"
+    echo "COMP_TYPE:                '$COMP_TYPE'"
+    echo "COMP_WORDBREAKS:          '$COMP_WORDBREAKS'"
+    echo
+    echo "Intermediate variables:"
+    echo
+    echo "COMP_WORDS_NO_FLAGS:      $(_pretty_print COMP_WORDS_NO_FLAGS[@])"
+    echo "# COMP_WORDS_NO_FLAGS:    '${#COMP_WORDS_NO_FLAGS[@]}'"
+    echo "COMP_CWORD_NO_FLAGS:      '$COMP_CWORD_NO_FLAGS'"
+    echo
+    echo "Subfunction:              '$subfunction'"
+    if $list_options
+    then
+        echo "List options:             'True'"
+    else
+        echo "List options:             'False'"
+    fi
+    echo "Current word:             '$cur'"
+}
+
+# Pretty-prints one or more arrays
+# Syntax: _pretty_print array1[@] ...
+_pretty_print() {
+    for arg in $@
+    do
+        local array=("${!arg}")
+        printf "$arg: ["
+        printf   "'%s'" "${array[0]}"
+        printf ", '%s'" "${array[@]:1}"
+        echo "]"
+    done
+}
+
+complete -o bashdefault -o default -F _bash_completion_spack spack
+
+# Spack commands
+#
+# Everything below here is auto-generated.

share/spack/qa/completion-test.sh

@@ -0,0 +1,89 @@
+#!/bin/sh
+#
+# Copyright 2013-2020 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+#
+# This script tests that Spack's tab completion scripts work.
+#
+# The tests are portable to bash, zsh, and bourne shell, and can be run
+# in any of these shells.
+#
+
+export QA_DIR=$(dirname "$0")
+export SHARE_DIR=$(cd "$QA_DIR/.." && pwd)
+export SPACK_ROOT=$(cd "$QA_DIR/../../.." && pwd)
+
+. "$QA_DIR/test-framework.sh"
+
+# Fail on undefined variables
+set -u
+
+# Source setup-env.sh before tests
+. "$SHARE_DIR/setup-env.sh"
+. "$SHARE_DIR/spack-completion.$_sp_shell"
+
+title "Testing spack-completion.$_sp_shell with $_sp_shell"
+
+# Spack command is now available
+succeeds which spack
+
+title 'Testing all subcommands'
+while IFS= read -r line
+do
+    # Test that completion with no args works
+    succeeds _spack_completions ${line[*]} ''
+
+    # Test that completion with flags works
+    contains '-h --help' _spack_completions ${line[*]} -
+done <<- EOF
+    $(spack commands --aliases --format=subcommands)
+EOF
+
+title 'Testing for correct output'
+contains 'compiler' _spack_completions spack ''
+contains 'install' _spack_completions spack inst
+contains 'find' _spack_completions spack help ''
+contains 'hdf5' _spack_completions spack list ''
+contains 'py-numpy' _spack_completions spack list py-
+contains 'mpi' _spack_completions spack providers ''
+contains 'builtin' _spack_completions spack repo remove ''
+contains 'packages' _spack_completions spack config edit ''
+contains 'python' _spack_completions spack extensions ''
+contains 'hdf5' _spack_completions spack -d install --jobs 8 ''
+contains 'hdf5' _spack_completions spack install -v ''
+
+# XFAIL: Fails for Python 2.6 because pkg_resources not found?
+#contains 'compilers.py' _spack_completions spack test ''
+
+title 'Testing debugging functions'
+
+# This is a particularly tricky case that involves the following situation:
+#     `spack -d [] install `
+# Here, [] represents the cursor, which is in the middle of the line.
+# We should tab-complete optional flags for `spack`, not optional flags for
+# `spack install` or package names.
+COMP_LINE='spack -d  install '
+COMP_POINT=9
+COMP_WORDS=(spack -d install)
+COMP_CWORD=2
+COMP_KEY=9
+COMP_TYPE=64
+
+_bash_completion_spack
+contains "--all-help" echo "${COMPREPLY[@]}"
+
+contains "['spack', '-d', 'install', '']" _pretty_print COMP_WORDS[@]
+
+# Set the rest of the intermediate variables manually
+COMP_WORDS_NO_FLAGS=(spack install)
+COMP_CWORD_NO_FLAGS=1
+subfunction=_spack
+cur=
+
+list_options=true
+contains "'True'" _test_vars
+list_options=false
+contains "'False'" _test_vars

share/spack/qa/run-unit-tests

@@ -23,7 +23,7 @@
 ORIGINAL_PATH="$PATH"
 
 . "$(dirname $0)/setup.sh"
-check_dependencies ${coverage} git hg svn
+check_dependencies $coverage git hg svn
 
 # Move to root directory of Spack
 # Allows script to be run from anywhere
@@ -46,7 +46,7 @@ extra_args=""
 if [[ -n "$@" ]]; then
     extra_args="-k $@"
 fi
-${coverage_run} bin/spack test -x --verbose "$extra_args"
+$coverage_run bin/spack test -x --verbose "$extra_args"
 
 #-----------------------------------------------------------
 # Run tests for setup-env.sh
@@ -57,15 +57,18 @@ export PATH="$ORIGINAL_PATH"
 unset spack
 
 # start in the spack root directory
-cd $SPACK_ROOT
+cd "$SPACK_ROOT"
 
 # Run bash tests with coverage enabled, but pipe output to /dev/null
 # because it seems that kcov seems to undo the script's redirection
 if [ "$BASH_COVERAGE" = true ]; then
-    ${QA_DIR}/bashcov ${QA_DIR}/setup-env-test.sh &> /dev/null
+    "$QA_DIR/bashcov" "$QA_DIR/setup-env-test.sh" &> /dev/null
+    "$QA_DIR/bashcov" "$QA_DIR/completion-test.sh" &> /dev/null
 fi
 
 # run the test scripts for their output (these will print nicely)
-bash ${QA_DIR}/setup-env-test.sh
+bash "$QA_DIR/setup-env-test.sh"
-zsh  ${QA_DIR}/setup-env-test.sh
+zsh  "$QA_DIR/setup-env-test.sh"
-dash ${QA_DIR}/setup-env-test.sh
+dash "$QA_DIR/setup-env-test.sh"
+
+bash "$QA_DIR/completion-test.sh"

share/spack/qa/setup-env-test.sh

@@ -12,159 +12,11 @@
 # in any of these shells.
 #
 
-# ------------------------------------------------------------------------
+export QA_DIR=$(dirname "$0")
-# Functions for color output.
+export SHARE_DIR=$(cd "$QA_DIR/.." && pwd)
-# ------------------------------------------------------------------------
+export SPACK_ROOT=$(cd "$QA_DIR/../../.." && pwd)
-
-# Colors for output
-red='\033[1;31m'
-cyan='\033[1;36m'
-green='\033[1;32m'
-reset='\033[0m'
-
-echo_red() {
-    printf "${red}$*${reset}\n"
-}
-
-echo_green() {
-    printf "${green}$*${reset}\n"
-}
-
-echo_msg() {
-    printf "${cyan}$*${reset}\n"
-}
-
-# ------------------------------------------------------------------------
-# Generic functions for testing shell code.
-# ------------------------------------------------------------------------
-
-# counts of test successes and failures.
-success=0
-errors=0
-
-# Print out a header for a group of tests.
-title() {
-    echo
-    echo_msg "$@"
-    echo_msg "---------------------------------"
-}
-
-# echo FAIL in red text; increment failures
-fail() {
-    echo_red FAIL
-    errors=$((errors+1))
-}
-
-#
-# Echo SUCCESS in green; increment successes
-#
-pass() {
-    echo_green SUCCESS
-    success=$((success+1))
-}
-
-#
-# Run a command and suppress output unless it fails.
-# On failure, echo the exit code and output.
-#
-succeeds() {
-    printf "'%s' succeeds ... " "$*"
-    output=$($* 2>&1)
-    err="$?"
-
-    if [ "$err" != 0 ]; then
-        fail
-        echo_red "Command failed with error $err."
-        if [ -n "$output" ]; then
-            echo_msg "Output:"
-            echo "$output"
-        else
-            echo_msg "No output."
-        fi
-    else
-        pass
-    fi
-}
-
-#
-# Run a command and suppress output unless it succeeds.
-# If the command succeeds, echo the output.
-#
-fails() {
-    printf "'%s' fails ... " "$*"
-    output=$("$@" 2>&1)
-    err="$?"
-
-    if [ "$err" = 0 ]; then
-        fail
-        echo_red "Command failed with error $err."
-        if [ -n "$output" ]; then
-            echo_msg "Output:"
-            echo "$output"
-        else
-            echo_msg "No output."
-        fi
-    else
-        pass
-    fi
-}
-
-#
-# Ensure that a string is in the output of a command.
-# Suppresses output on success.
-# On failure, echo the exit code and output.
-#
-contains() {
-    string="$1"
-    shift
-
-    printf "'%s' output contains '$string' ... " "$*"
-    output=$("$@" 2>&1)
-    err="$?"
-
-    if [ "${output#*$string}" = "${output}" ]; then
-        fail
-        echo_red "Command exited with error $err."
-        echo_red "'$string' was not in output."
-        if [ -n "$output" ]; then
-            echo_msg "Output:"
-            echo "$output"
-        else
-            echo_msg "No output."
-        fi
-    else
-        pass
-    fi
-}
-
-#
-# Ensure that a variable is set.
-#
-is_set() {
-    printf "'%s' is set ... " "$1"
-    if eval "[ -z \${${1:-}+x} ]"; then
-        fail
-        echo_msg "$1 was not set!"
-    else
-        pass
-    fi
-}
-
-#
-# Ensure that a variable is not set.
-# Fails and prints the value of the variable if it is set.
-#
-is_not_set() {
-    printf "'%s' is not set ... " "$1"
-    if eval "[ ! -z \${${1:-}+x} ]"; then
-        fail
-        echo_msg "$1 was set:"
-        echo "    $1"
-    else
-        pass
-    fi
-}
 
+. "$QA_DIR/test-framework.sh"
 
 # -----------------------------------------------------------------------
 # Instead of invoking the module commands, we print the
@@ -184,28 +36,28 @@ module() {
 # Make sure no environment is active
 unset SPACK_ENV
 
-# fail on undefined variables
+# Fail on undefined variables
 set -u
 
 # Source setup-env.sh before tests
-.  share/spack/setup-env.sh
+. "$SHARE_DIR/setup-env.sh"
 
-# bash should expand aliases even when non-interactive
+# Bash should expand aliases even when non-interactive
 if [ -n "${BASH:-}" ]; then
     shopt -s expand_aliases
 fi
 
 title "Testing setup-env.sh with $_sp_shell"
 
-# spack command is now avaialble
+# Spack command is now available
 succeeds which spack
 
-# mock cd command (intentionally define only AFTER setup-env.sh)
+# Mock cd command (intentionally define only AFTER setup-env.sh)
 cd() {
     echo cd "$@"
 }
 
-# create a fake mock package install and store its location for later
+# Create a fake mock package install and store its location for later
 title "Setup"
 echo "Creating a mock package installation"
 spack -m install --fake a
@@ -215,19 +67,13 @@ a_module=$(spack -m module tcl find a)
 b_install=$(spack location -i b)
 b_module=$(spack -m module tcl find b)
 
-# create a test environment for tesitng environment commands
+# Create a test environment for testing environment commands
 echo "Creating a mock environment"
 spack env create spack_test_env
 test_env_location=$(spack location -e spack_test_env)
 
-# ensure that we uninstall b on exit
+# Ensure that we uninstall b on exit
 cleanup() {
-    if [ "$?" != 0 ]; then
-        trapped_error=true
-    else
-        trapped_error=false
-    fi
-
     echo "Removing test environment before exiting."
     spack env deactivate 2>&1 > /dev/null
     spack env rm -y spack_test_env
@@ -235,24 +81,7 @@ cleanup() {
     title "Cleanup"
     echo "Removing test packages before exiting."
     spack -m uninstall -yf b a
-
-    echo
-    echo "$success tests succeeded."
-    echo "$errors tests failed."
-
-    if [ "$trapped_error" = true ]; then
-        echo "Exited due to an error."
-    fi
-
-    if [ "$errors" = 0 ] && [ "$trapped_error" = false ]; then
-        pass
-        exit 0
-    else
-        fail
-        exit 1
-    fi
 }
-trap cleanup EXIT
 
 # -----------------------------------------------------------------------
 # Test all spack commands with special env support

share/spack/qa/test-framework.sh

@@ -0,0 +1,195 @@
+# Copyright 2013-2020 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+#
+# A testing framework for any POSIX-compatible shell.
+#
+
+# ------------------------------------------------------------------------
+# Functions for color output.
+# ------------------------------------------------------------------------
+
+# Colors for output
+red='\033[1;31m'
+cyan='\033[1;36m'
+green='\033[1;32m'
+reset='\033[0m'
+
+echo_red() {
+    printf "${red}$*${reset}\n"
+}
+
+echo_green() {
+    printf "${green}$*${reset}\n"
+}
+
+echo_msg() {
+    printf "${cyan}$*${reset}\n"
+}
+
+# ------------------------------------------------------------------------
+# Generic functions for testing shell code.
+# ------------------------------------------------------------------------
+
+# counts of test successes and failures.
+success=0
+errors=0
+
+# Print out a header for a group of tests.
+title() {
+    echo
+    echo_msg "$@"
+    echo_msg "---------------------------------"
+}
+
+# echo FAIL in red text; increment failures
+fail() {
+    echo_red FAIL
+    errors=$((errors+1))
+}
+
+#
+# Echo SUCCESS in green; increment successes
+#
+pass() {
+    echo_green SUCCESS
+    success=$((success+1))
+}
+
+#
+# Run a command and suppress output unless it fails.
+# On failure, echo the exit code and output.
+#
+succeeds() {
+    printf "'%s' succeeds ... " "$*"
+    output=$("$@" 2>&1)
+    err="$?"
+
+    if [ "$err" != 0 ]; then
+        fail
+        echo_red "Command failed with error $err."
+        if [ -n "$output" ]; then
+            echo_msg "Output:"
+            echo "$output"
+        else
+            echo_msg "No output."
+        fi
+    else
+        pass
+    fi
+}
+
+#
+# Run a command and suppress output unless it succeeds.
+# If the command succeeds, echo the output.
+#
+fails() {
+    printf "'%s' fails ... " "$*"
+    output=$("$@" 2>&1)
+    err="$?"
+
+    if [ "$err" = 0 ]; then
+        fail
+        echo_red "Command failed with error $err."
+        if [ -n "$output" ]; then
+            echo_msg "Output:"
+            echo "$output"
+        else
+            echo_msg "No output."
+        fi
+    else
+        pass
+    fi
+}
+
+#
+# Ensure that a string is in the output of a command.
+# Suppresses output on success.
+# On failure, echo the exit code and output.
+#
+contains() {
+    string="$1"
+    shift
+
+    printf "'%s' output contains '$string' ... " "$*"
+    output=$("$@" 2>&1)
+    err="$?"
+
+    if [ "${output#*$string}" = "${output}" ]; then
+        fail
+        echo_red "Command exited with error $err."
+        echo_red "'$string' was not in output."
+        if [ -n "$output" ]; then
+            echo_msg "Output:"
+            echo "$output"
+        else
+            echo_msg "No output."
+        fi
+    else
+        pass
+    fi
+}
+
+#
+# Ensure that a variable is set.
+#
+is_set() {
+    printf "'%s' is set ... " "$1"
+    if eval "[ -z \${${1:-}+x} ]"; then
+        fail
+        echo_msg "$1 was not set!"
+    else
+        pass
+    fi
+}
+
+#
+# Ensure that a variable is not set.
+# Fails and prints the value of the variable if it is set.
+#
+is_not_set() {
+    printf "'%s' is not set ... " "$1"
+    if eval "[ ! -z \${${1:-}+x} ]"; then
+        fail
+        echo_msg "$1 was set:"
+        echo "    $1"
+    else
+        pass
+    fi
+}
+
+#
+# Report the number of tests that succeeded and failed on exit.
+#
+teardown() {
+    if [ "$?" != 0 ]; then
+        trapped_error=true
+    else
+        trapped_error=false
+    fi
+
+    if type cleanup &> /dev/null
+    then
+        cleanup
+    fi
+
+    echo
+    echo "$success tests succeeded."
+    echo "$errors tests failed."
+
+    if [ "$trapped_error" = true ]; then
+        echo "Exited due to an error."
+    fi
+
+    if [ "$errors" = 0 ] && [ "$trapped_error" = false ]; then
+        pass
+        exit 0
+    else
+        fail
+        exit 1
+    fi
+}
+
+trap teardown EXIT

share/spack/qa/update-completion-scripts.sh

@@ -0,0 +1,23 @@
+#!/usr/bin/env bash
+#
+# Copyright 2013-2020 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+# Updates Spack's shell tab completion scripts
+
+# Switch to parent directory
+QA_DIR="$(dirname "${BASH_SOURCE[0]}")"
+cd "$QA_DIR/.."
+
+# Update each shell
+for shell in bash # zsh fish
+do
+    header=$shell/spack-completion.in
+    script=spack-completion.$shell
+
+    rm -f $script
+    spack commands --aliases --format=$shell --header=$header --update=$script
+    chmod +x $script
+done