Better error message for spack providers (#7748)

* Better error message for spack providers

fixes #1355

`spack providers` now outputs a sensible error message if non-virtual
specs are provided as arguments:
```
$ spack providers mpi zlib petsc
==> Error: non-virtual specs cannot be part of the query [zlib, petsc]
```

Formatting of the output changed slightly.

* Calling 'spack providers' without arguments print the virtual pkg list

Also, the error message in case of a wrong parameter has been improved
to show the list of valid packages.

* Avoid printing headers if stdout is not a tty

* The provider list is formatted with colify if not in a tty

* Added a test to check the list of providers returned from the command

3 files changed, 116 insertions, 7 deletions

diff --git a/lib/spack/spack/cmd/__init__.py b/lib/spack/spack/cmd/__init__.py
index f0e62fbc56..3f713287e6 100644
--- a/lib/spack/spack/cmd/__init__.py
+++ b/lib/spack/spack/cmd/__init__.py
@@ -265,7 +265,11 @@ def display_specs(specs, args=None, **kwargs):
         header = "%s{%s} / %s{%s}" % (spack.spec.architecture_color,
                                       architecture, spack.spec.compiler_color,
                                       compiler)
-        tty.hline(colorize(header), char='-')
+        # Sometimes we want to display specs that are not yet concretized.
+        # If they don't have a compiler / architecture attached to them,
+        # then skip the header
+        if architecture is not None or compiler is not None:
+            tty.hline(colorize(header), char='-')
 
         specs = index[(architecture, compiler)]
         specs.sort()
diff --git a/lib/spack/spack/cmd/providers.py b/lib/spack/spack/cmd/providers.py
index 2944bdcb67..3b07036d48 100644
--- a/lib/spack/spack/cmd/providers.py
+++ b/lib/spack/spack/cmd/providers.py
@@ -22,9 +22,10 @@
 # License along with this program; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##############################################################################
-import argparse
+import six
+import sys
 
-from llnl.util.tty.colify import colify
+import llnl.util.tty.colify as colify
 
 import spack
 import spack.cmd
@@ -35,11 +36,46 @@ level = "long"
 
 
 def setup_parser(subparser):
+    subparser.epilog = 'If called without argument returns ' \
+                       'the list of all valid virtual packages'
     subparser.add_argument(
-        'vpkg_spec', metavar='VPACKAGE_SPEC', nargs=argparse.REMAINDER,
-        help='find packages that provide this virtual package')
+        'virtual_package',
+        nargs='*',
+        help='find packages that provide this virtual package'
+    )
 
 
 def providers(parser, args):
-    for spec in spack.cmd.parse_specs(args.vpkg_spec):
-        colify(sorted(spack.repo.providers_for(spec)), indent=4)
+    valid_virtuals = sorted(spack.repo.provider_index.providers.keys())
+
+    buffer = six.StringIO()
+    isatty = sys.stdout.isatty()
+    if isatty:
+        buffer.write('Virtual packages:\n')
+    colify.colify(valid_virtuals, output=buffer, tty=isatty, indent=4)
+    valid_virtuals_str = buffer.getvalue()
+
+    # If called without arguments, list all the virtual packages
+    if not args.virtual_package:
+        print(valid_virtuals_str)
+        return
+
+    # Otherwise, parse the specs from command line
+    specs = spack.cmd.parse_specs(args.virtual_package)
+
+    # Check prerequisites
+    non_virtual = [
+        str(s) for s in specs if not s.virtual or s.name not in valid_virtuals
+    ]
+    if non_virtual:
+        msg = 'non-virtual specs cannot be part of the query '
+        msg += '[{0}]\n'.format(', '.join(non_virtual))
+        msg += valid_virtuals_str
+        raise ValueError(msg)
+
+    # Display providers
+    for spec in specs:
+        if sys.stdout.isatty():
+            print("{0}:".format(spec))
+        spack.cmd.display_specs(sorted(spack.repo.providers_for(spec)))
+        print('')
diff --git a/lib/spack/spack/test/cmd/providers.py b/lib/spack/spack/test/cmd/providers.py
new file mode 100644
index 0000000000..20a4192573
--- /dev/null
+++ b/lib/spack/spack/test/cmd/providers.py
@@ -0,0 +1,69 @@
+##############################################################################
+# Copyright (c) 2013-2018, Lawrence Livermore National Security, LLC.
+# Produced at the Lawrence Livermore National Laboratory.
+#
+# This file is part of Spack.
+# Created by Todd Gamblin, tgamblin@llnl.gov, All rights reserved.
+# LLNL-CODE-647188
+#
+# For details, see https://github.com/spack/spack
+# Please also see the NOTICE and LICENSE files for our notice and the LGPL.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU Lesser General Public License (as
+# published by the Free Software Foundation) version 2.1, February 1999.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and
+# conditions of the GNU Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+##############################################################################
+
+import pytest
+
+from spack.main import SpackCommand
+
+providers = SpackCommand('providers')
+
+
+@pytest.mark.parametrize('pkg', [
+    ('mpi',),
+    ('mpi@2',),
+    ('mpi', 'lapack'),
+    ('',)  # Lists all the available virtual packages
+])
+def test_it_just_runs(pkg):
+    providers(*pkg)
+
+
+@pytest.mark.parametrize('vpkg,provider_list', [
+    (('mpi',), ['intel-mpi',
+                'intel-parallel-studio',
+                'mpich',
+                'mpich@1:',
+                'mpich@3:',
+                'mvapich2',
+                'openmpi',
+                'openmpi@1.6.5',
+                'openmpi@1.7.5:',
+                'openmpi@2.0.0:',
+                'spectrum-mpi']),
+    (('D', 'awk'), ['ldc', 'gawk', 'mawk'])  # Call 2 virtual packages at once
+])
+def test_provider_lists(vpkg, provider_list):
+    output = providers(*vpkg)
+    for item in provider_list:
+        assert item in output
+
+
+@pytest.mark.parametrize('pkg,error_cls', [
+    ('zlib', ValueError),
+    ('foo', ValueError)  # Trying to call with a package that does not exist
+])
+def test_it_just_fails(pkg, error_cls):
+    with pytest.raises(error_cls):
+        providers(pkg)