Use a patched argparse only in Python 2.X (#25376)

Spack is internally using a patched version of `argparse` mainly to backport Python 3 functionality
into Python 2. This PR makes it such that for the supported Python 3 versions we use `argparse`
from the standard Python library. This PR has been extracted from #25371 where it was needed
to be able to use recent versions of `pytest`.

* Fixed formatting issues when using a pristine argparse.py
* Fix error message for Python 3.X when missing positional arguments
* Account for the change of API in Python 3.7
* Layout multi-valued args into columns in error messages
* Seamless transition in develop if argparse.pyc is in external
* Be more defensive in case we can't remove the file.

3 files changed, 49 insertions, 3 deletions

diff --git a/bin/spack b/bin/spack
index a4c442227d..dffcbd2026 100755
--- a/bin/spack
+++ b/bin/spack
@@ -28,6 +28,7 @@ exit 1
 from __future__ import print_function
 
 import os
+import os.path
 import sys
 
 min_python3 = (3, 5)
@@ -70,6 +71,28 @@ if "ruamel.yaml" in sys.modules:
 if "ruamel" in sys.modules:
     del sys.modules["ruamel"]
 
+# The following code is here to avoid failures when updating
+# the develop version, due to spurious argparse.pyc files remaining
+# in the libs/spack/external directory, see:
+# https://github.com/spack/spack/pull/25376
+# TODO: Remove in v0.18.0 or later
+try:
+    import argparse
+except ImportError:
+    argparse_pyc = os.path.join(spack_external_libs, 'argparse.pyc')
+    if not os.path.exists(argparse_pyc):
+        raise
+    try:
+        os.remove(argparse_pyc)
+        import argparse  # noqa
+    except Exception:
+        msg = ('The file\n\n\t{0}\n\nis corrupted and cannot be deleted by Spack. '
+               'Either delete it manually or ask some administrator to '
+               'delete it for you.')
+        print(msg.format(argparse_pyc))
+        sys.exit(1)
+
+
 import spack.main  # noqa
 
 # Once we've set up the system path, run the spack main method
diff --git a/lib/spack/external/argparse.py b/lib/spack/external/py2/argparse.py
index d2d232d51e..d2d232d51e 100644
--- a/lib/spack/external/argparse.py
+++ b/lib/spack/external/py2/argparse.py
diff --git a/lib/spack/spack/main.py b/lib/spack/spack/main.py
index d237db8904..d29988627e 100644
--- a/lib/spack/spack/main.py
+++ b/lib/spack/spack/main.py
@@ -27,6 +27,7 @@ import archspec.cpu
 
 import llnl.util.filesystem as fs
 import llnl.util.tty as tty
+import llnl.util.tty.colify
 import llnl.util.tty.color as color
 from llnl.util.tty.log import log_output
 
@@ -173,14 +174,16 @@ class SpackHelpFormatter(argparse.RawTextHelpFormatter):
         usage = super(
             SpackHelpFormatter, self)._format_actions_usage(actions, groups)
 
+        # Eliminate any occurrence of two or more consecutive spaces
+        usage = re.sub(r'[ ]{2,}', ' ', usage)
+
         # compress single-character flags that are not mutually exclusive
         # at the beginning of the usage string
         chars = ''.join(re.findall(r'\[-(.)\]', usage))
         usage = re.sub(r'\[-.\] ?', '', usage)
         if chars:
-            return '[-%s] %s' % (chars, usage)
-        else:
-            return usage
+            usage = '[-%s] %s' % (chars, usage)
+        return usage.strip()
 
 
 class SpackArgumentParser(argparse.ArgumentParser):
@@ -293,7 +296,18 @@ class SpackArgumentParser(argparse.ArgumentParser):
     def add_subparsers(self, **kwargs):
         """Ensure that sensible defaults are propagated to subparsers"""
         kwargs.setdefault('metavar', 'SUBCOMMAND')
+
+        # From Python 3.7 we can require a subparser, earlier versions
+        # of argparse will error because required=True is unknown
+        if sys.version_info[:2] > (3, 6):
+            kwargs.setdefault('required', True)
+
         sp = super(SpackArgumentParser, self).add_subparsers(**kwargs)
+        # This monkey patching is needed for Python 3.5 and 3.6, which support
+        # having a required subparser but don't expose the API used above
+        if sys.version_info[:2] == (3, 5) or sys.version_info[:2] == (3, 6):
+            sp.required = True
+
         old_add_parser = sp.add_parser
 
         def add_parser(name, **kwargs):
@@ -336,6 +350,15 @@ class SpackArgumentParser(argparse.ArgumentParser):
             # in subparsers, self.prog is, e.g., 'spack install'
             return super(SpackArgumentParser, self).format_help()
 
+    def _check_value(self, action, value):
+        # converted value must be one of the choices (if specified)
+        if action.choices is not None and value not in action.choices:
+            cols = llnl.util.tty.colify.colified(
+                sorted(action.choices), indent=4, tty=True
+            )
+            msg = 'invalid choice: %r choose from:\n%s' % (value, cols)
+            raise argparse.ArgumentError(action, msg)
+
 
 def make_argument_parser(**kwargs):
     """Create an basic argument parser without any subcommands added."""