From 04a6a55cf8c0539ffe02f38162622a789fb8ad1f Mon Sep 17 00:00:00 2001
From: Todd Gamblin <tgamblin@llnl.gov>
Date: Thu, 23 Jan 2020 14:48:06 -0800
Subject: [PATCH] commands: add simple `spack commands --update-completion`
 argument (#14607)

Instead of another script, this adds a simple argument to `spack
commands` that updates the completion script.  Developers can now just
run:

    spack commands --update-completion

This should make it simpler for developers to remember to run this
*before* the tests fail.  Also, this version tab-completes.
---
 lib/spack/spack/cmd/commands.py             | 60 ++++++++++++++++++++-
 lib/spack/spack/test/cmd/commands.py        | 38 ++++++++++++-
 share/spack/qa/update-completion-scripts.sh | 23 --------
 share/spack/spack-completion.bash           |  2 +-
 4 files changed, 96 insertions(+), 27 deletions(-)
 delete mode 100755 share/spack/qa/update-completion-scripts.sh

diff --git a/lib/spack/spack/cmd/commands.py b/lib/spack/spack/cmd/commands.py
index 4966bd7858..3664fce477 100644
--- a/lib/spack/spack/cmd/commands.py
+++ b/lib/spack/spack/cmd/commands.py
@@ -11,6 +11,7 @@
 import re
 import sys
 
+import llnl.util.filesystem as fs
 import llnl.util.tty as tty
 from llnl.util.argparsewriter import (
     ArgparseWriter, ArgparseRstWriter, ArgparseCompletionWriter
@@ -19,6 +20,7 @@
 
 import spack.cmd
 import spack.main
+import spack.paths
 from spack.main import section_descriptions
 
 
@@ -31,6 +33,20 @@
 formatters = {}
 
 
+#: standard arguments for updating completion scripts
+#: we iterate through these when called with --update-completion
+update_completion_args = {
+    "bash":  {
+        "aliases": True,
+        "format": "bash",
+        "header": os.path.join(
+            spack.paths.share_path, "bash", "spack-completion.in"),
+        "update": os.path.join(
+            spack.paths.share_path, "spack-completion.bash"),
+    },
+}
+
+
 def formatter(func):
     """Decorator used to register formatters"""
     formatters[func.__name__] = func
@@ -39,7 +55,12 @@ def formatter(func):
 
 def setup_parser(subparser):
     subparser.add_argument(
-        '-a', '--aliases', action='store_true', help='include command aliases')
+        "--update-completion", action='store_true', default=False,
+        help="regenerate spack's tab completion scripts")
+
+    subparser.add_argument(
+        '-a', '--aliases', action='store_true', default=False,
+        help='include command aliases')
     subparser.add_argument(
         '--format', default='names', choices=formatters,
         help='format to be used to print the output (default: names)')
@@ -229,7 +250,11 @@ def prepend_header(args, out):
         out.write(header.read())
 
 
-def commands(parser, args):
+def _commands(parser, args):
+    """This is the 'regular' command, which can be called multiple times.
+
+    See ``commands()`` below for ``--update-completion`` handling.
+    """
     formatter = formatters[args.format]
 
     # check header first so we don't open out files unnecessarily
@@ -255,6 +280,37 @@ def commands(parser, args):
             prepend_header(args, f)
             formatter(args, f)
 
+        if args.update_completion:
+            fs.set_executable(args.update)
+
     else:
         prepend_header(args, sys.stdout)
         formatter(args, sys.stdout)
+
+
+def update_completion(parser, args):
+    """Iterate through the shells and update the standard completion files.
+
+    This is a convenience method to avoid calling this command many
+    times, and to simplify completion update for developers.
+
+    """
+    for shell, shell_args in update_completion_args.items():
+        for attr, value in shell_args.items():
+            setattr(args, attr, value)
+        _commands(parser, args)
+
+
+def commands(parser, args):
+    if args.update_completion:
+        if args.format != 'names' or any([
+                args.aliases, args.update, args.header
+        ]):
+            tty.die("--update-completion can only be specified alone.")
+
+        # this runs the command multiple times with different arguments
+        return update_completion(parser, args)
+
+    else:
+        # run commands normally
+        return _commands(parser, args)
diff --git a/lib/spack/spack/test/cmd/commands.py b/lib/spack/spack/test/cmd/commands.py
index 2fe62b9bba..409dea7c51 100644
--- a/lib/spack/spack/test/cmd/commands.py
+++ b/lib/spack/spack/test/cmd/commands.py
@@ -5,6 +5,7 @@
 
 import filecmp
 import os
+import shutil
 import subprocess
 
 import pytest
@@ -208,12 +209,47 @@ def test_bash_completion():
     assert '_spack_compiler_add() {' in out2
 
 
+def test_update_completion_arg(tmpdir, monkeypatch):
+    mock_infile = tmpdir.join("spack-completion.in")
+    mock_bashfile = tmpdir.join("spack-completion.bash")
+
+    mock_args = {
+        "bash":  {
+            "aliases": True,
+            "format": "bash",
+            "header": str(mock_infile),
+            "update": str(mock_bashfile),
+        },
+    }
+
+    # make a mock completion file missing the --update-completion argument
+    real_args = spack.cmd.commands.update_completion_args
+    shutil.copy(real_args['bash']['header'], mock_args['bash']['header'])
+    with open(real_args['bash']['update']) as old:
+        old_file = old.read()
+        with open(mock_args['bash']['update'], 'w') as mock:
+            mock.write(old_file.replace("--update-completion", ""))
+    mock_bashfile.setmtime(0)  # ensure mtime triggers update
+
+    monkeypatch.setattr(
+        spack.cmd.commands, 'update_completion_args', mock_args)
+
+    # ensure things fail if --update-completion isn't specified alone
+    with pytest.raises(spack.main.SpackCommandError):
+        commands("--update-completion", "-a")
+
+    # ensure arg is restored
+    assert "--update-completion" not in mock_bashfile.read()
+    commands("--update-completion")
+    assert "--update-completion" in mock_bashfile.read()
+
+
 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"
+           "    spack commands --update-completion\n\n"
            "and adding the changed files to your pull request.")
 
     for shell in ['bash']:  # 'zsh', 'fish']:
diff --git a/share/spack/qa/update-completion-scripts.sh b/share/spack/qa/update-completion-scripts.sh
deleted file mode 100755
index 8fcd321457..0000000000
--- a/share/spack/qa/update-completion-scripts.sh
+++ /dev/null
@@ -1,23 +0,0 @@
-#!/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
diff --git a/share/spack/spack-completion.bash b/share/spack/spack-completion.bash
index 79dcf8e559..9550137bff 100755
--- a/share/spack/spack-completion.bash
+++ b/share/spack/spack-completion.bash
@@ -507,7 +507,7 @@ _spack_clone() {
 _spack_commands() {
     if $list_options
     then
-        SPACK_COMPREPLY="-h --help -a --aliases --format --header --update"
+        SPACK_COMPREPLY="-h --help --update-completion -a --aliases --format --header --update"
     else
         SPACK_COMPREPLY=""
     fi
-- 
GitLab