Simplify the detection protocol for packages

Packages can implement “detect_version” to support detection
of external instances of a package. This is generally easier
than implementing “determine_spec_details”. The API for
determine_version is similar: for example you can return
“None” to indicate that an executable is not an instance
of a package.

Users may implement a “determine_variants” method for a package.
When doing external detection, executables are grouped by version
and each group results in a single invocation of “determine_variants”
for the associated spec. The method returns a string specifying
the variants for the package. The method may additionally return
a dictionary representing extra attributes for the package.

These will be stored in the spec yaml and can be retrieved
from self.spec.extra_attributes

The Spack GCC package has been updated with an implementation
of “determine_variants” which adds the following extra
attributes to the package: c, cxx, fortran

lib/spack/docs/packaging_guide.rst

@@ -4054,21 +4054,223 @@ File functions
 Making a package discoverable with ``spack external find``
 ----------------------------------------------------------
 
-To make a package discoverable with
+The simplest way to make a package discoverable with
-:ref:`spack external find <cmd-spack-external-find>` you must
+:ref:`spack external find <cmd-spack-external-find>` is to:
-define one or more executables associated with the package and must
-implement a method to generate a Spec when given an executable.
 
-The executables are specified as a package level ``executables``
+1. Define the executables associated with the package
-attribute which is a list of strings (see example below); each string
+2. Implement a method to determine the versions of these executables
-is treated as a regular expression (e.g. 'gcc' would match 'gcc', 'gcc-8.3',
-'my-weird-gcc', etc.).
 
-The method ``determine_spec_details`` has the following signature:
+^^^^^^^^^^^^^^^^^
+Minimal detection
+^^^^^^^^^^^^^^^^^
+
+The first step is fairly simple, as it requires only to
+specify a package level ``executables`` attribute:
 
 .. code-block:: python
 
-   def determine_spec_details(prefix, exes_in_prefix):
+   class Foo(Package):
+       # Each string provided here is treated as a regular expression, and
+       # would match for example 'foo', 'foobar', and 'bazfoo'.
+       executables = ['foo']
+
+This attribute must be a list of strings. Each string is a regular
+expression (e.g. 'gcc' would match 'gcc', 'gcc-8.3', 'my-weird-gcc', etc.) to
+determine a set of system executables that might be part or this package. Note
+that to match only executables named 'gcc' the regular expression ``'^gcc$'``
+must be used.
+
+Finally to determine the version of each executable the ``determine_version``
+method must be implemented:
+
+.. code-block:: python
+
+   @classmethod
+   def determine_version(cls, exe):
+       """Return either the version of the executable passed as argument
+       or ``None`` if the version cannot be determined.
+
+       Args:
+           exe (str): absolute path to the executable being examined
+       """
+
+This method receives as input the path to a single executable and must return
+as output its version as a string; if the user cannot determine the version
+or determines that the executable is not an instance of the package, they can
+return None and the exe will be discarded as a candidate.
+Implementing the two steps above is mandatory, and gives the package the
+basic ability to detect if a spec is present on the system at a given version.
+
+.. note::
+   Any executable for which the ``determine_version`` method returns ``None``
+   will be discarded and won't appear in later stages of the workflow described below.
+
+^^^^^^^^^^^^^^^^^^^^^^^^
+Additional functionality
+^^^^^^^^^^^^^^^^^^^^^^^^
+
+Besides the two mandatory steps described above, there are also optional
+methods that can be implemented to either increase the amount of details
+being detected or improve the robustness of the detection logic in a package.
+
+""""""""""""""""""""""""""""""
+Variants and custom attributes
+""""""""""""""""""""""""""""""
+
+The ``determine_variants`` method can be optionally implemented in a package
+to detect additional details of the spec:
+
+.. code-block:: python
+
+   @classmethod
+   def determine_variants(cls, exes, version_str):
+       """Return either a variant string, a tuple of a variant string
+       and a dictionary of extra attributes that will be recorded in
+       packages.yaml or a list of those items.
+
+       Args:
+           exes (list of str): list of executables (absolute paths) that
+               live in the same prefix and share the same version
+           version_str (str): version associated with the list of
+               executables, as detected by ``determine_version``
+       """
+
+This method takes as input a list of executables that live in the same prefix and
+share the same version string, and returns either:
+
+1. A variant string
+2. A tuple of a variant string and a dictionary of extra attributes
+3. A list of items matching either 1 or 2 (if multiple specs are detected
+   from the set of executables)
+
+If extra attributes are returned, they will be recorded in ``packages.yaml``
+and be available for later reuse. As an example, the ``gcc`` package will record
+by default the different compilers found and an entry in ``packages.yaml``
+would look like:
+
+.. code-block:: yaml
+
+   packages:
+     gcc:
+       externals:
+       - spec: 'gcc@9.0.1 languages=c,c++,fortran'
+         prefix: /usr
+         extra_attributes:
+           compilers:
+             c: /usr/bin/x86_64-linux-gnu-gcc-9
+             c++: /usr/bin/x86_64-linux-gnu-g++-9
+             fortran: /usr/bin/x86_64-linux-gnu-gfortran-9
+
+This allows us, for instance, to keep track of executables that would be named
+differently if built by Spack (e.g. ``x86_64-linux-gnu-gcc-9``
+instead of just ``gcc``).
+
+.. TODO: we need to gather some more experience on overriding 'prefix'
+   and other special keywords in extra attributes, but as soon as we are
+   confident that this is the way to go we should document the process.
+   See https://github.com/spack/spack/pull/16526#issuecomment-653783204
+
+"""""""""""""""""""""""""""
+Filter matching executables
+"""""""""""""""""""""""""""
+
+Sometimes defining the appropriate regex for the ``executables``
+attribute might prove to be difficult, especially if one has to
+deal with corner cases or exclude "red herrings". To help keeping
+the regular expressions as simple as possible, each package can
+optionally implement a ``filter_executables`` method:
+
+.. code-block:: python
+
+    @classmethod
+    def filter_detected_exes(cls, prefix, exes_in_prefix):
+        """Return a filtered list of the executables in prefix"""
+
+which takes as input a prefix and a list of matching executables and
+returns a filtered list of said executables.
+
+Using this method has the advantage of allowing custom logic for
+filtering, and does not restrict the user to regular expressions
+only.  Consider the case of detecting the GNU C++ compiler. If we
+try to search for executables that match ``g++``, that would have
+the unwanted side effect of selecting also ``clang++`` - which is
+a C++ compiler provided by another package - if present on the system.
+Trying to select executables that contain ``g++`` but not ``clang``
+would be quite complicated to do using regex only. Employing the
+``filter_detected_exes`` method it becomes:
+
+.. code-block:: python
+
+   class Gcc(Package):
+      executables = ['g++']
+
+      def filter_detected_exes(cls, prefix, exes_in_prefix):
+         return [x for x in exes_in_prefix if 'clang' not in x]
+
+Another possibility that this method opens is to apply certain
+filtering logic when specific conditions are met (e.g. take some
+decisions on an OS and not on another).
+
+^^^^^^^^^^^^^^^^^^
+Validate detection
+^^^^^^^^^^^^^^^^^^
+
+To increase detection robustness, packagers may also implement a method
+to validate the detected Spec objects:
+
+.. code-block:: python
+
+   @classmethod
+   def validate_detected_spec(cls, spec, extra_attributes):
+       """Validate a detected spec. Raise an exception if validation fails."""
+
+This method receives a detected spec along with its extra attributes and can be
+used to check that certain conditions are met by the spec. Packagers can either
+use assertions or raise an ``InvalidSpecDetected`` exception when the check fails.
+In case the conditions are not honored the spec will be discarded and any message
+associated with the assertion or the exception will be logged as the reason for
+discarding it.
+
+As an example, a package that wants to check that the ``compilers`` attribute is
+in the extra attributes can implement this method like this:
+
+.. code-block:: python
+
+   @classmethod
+   def validate_detected_spec(cls, spec, extra_attributes):
+       """Check that 'compilers' is in the extra attributes."""
+       msg = ('the extra attribute "compilers" must be set for '
+              'the detected spec "{0}"'.format(spec))
+       assert 'compilers' in extra_attributes, msg
+
+or like this:
+
+.. code-block:: python
+
+   @classmethod
+   def validate_detected_spec(cls, spec, extra_attributes):
+       """Check that 'compilers' is in the extra attributes."""
+       if 'compilers' not in extra_attributes:
+           msg = ('the extra attribute "compilers" must be set for '
+                  'the detected spec "{0}"'.format(spec))
+           raise InvalidSpecDetected(msg)
+
+.. _determine_spec_details:
+
+^^^^^^^^^^^^^^^^^^^^^^^^^
+Custom detection workflow
+^^^^^^^^^^^^^^^^^^^^^^^^^
+
+In the rare case when the mechanisms described so far don't fit the
+detection of a package, the implementation of all the methods above
+can be disregarded and instead a custom ``determine_spec_details``
+method can be implemented directly in the package class (note that
+the definition of the ``executables`` attribute is still required):
+
+.. code-block:: python
+
+   @classmethod
+   def determine_spec_details(cls, prefix, exes_in_prefix):
        # exes_in_prefix = a set of paths, each path is an executable
        # prefix = a prefix that is common to each path in exes_in_prefix
 
@@ -4076,14 +4278,13 @@ The method ``determine_spec_details`` has the following signature:
        # the package. Return one or more Specs for each instance of the
        # package which is thought to be installed in the provided prefix
 
-``determine_spec_details`` takes as parameters a set of discovered
+This method takes as input a set of discovered executables (which match
-executables (which match those specified by the user) as well as a
+those specified by the user) as well as a common prefix shared by all
-common prefix shared by all of those executables. The function must
+of those executables. The function must return one or more :py:class:`spack.spec.Spec` associated
-return one or more Specs associated with the executables (it can also
+with the executables (it can also return ``None`` to indicate that no
-return ``None`` to indicate that no provided executables are associated
+provided executables are associated with the package).
-with the package).
 
-Say for example we have a package called ``foo-package`` which
+As an example, consider a made-up package called ``foo-package`` which
 builds an executable called ``foo``. ``FooPackage`` would appear as
 follows:
 
@@ -4110,7 +4311,9 @@ follows:
            exe = spack.util.executable.Executable(exe_path)
            output = exe('--version')
            version_str = ...  # parse output for version string
-           return Spec('foo-package@{0}'.format(version_str))
+           return Spec.from_detection(
+               'foo-package@{0}'.format(version_str)
+           )
 
 .. _package-lifecycle:
 

lib/spack/spack/cmd/external.py

@@ -2,22 +2,24 @@
 # Spack Project Developers. See the top-level COPYRIGHT file for details.
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
-
 from __future__ import print_function
-from collections import defaultdict, namedtuple
+
 import argparse
 import os
 import re
-import six
+import sys
+from collections import defaultdict, namedtuple
 
+import llnl.util.filesystem
+import llnl.util.tty as tty
+import llnl.util.tty.colify as colify
+import six
 import spack
 import spack.error
-import llnl.util.tty as tty
-import spack.util.spack_yaml as syaml
 import spack.util.environment
-import llnl.util.filesystem
+import spack.util.spack_yaml as syaml
 
-description = "add external packages to Spack configuration"
+description = "manage external packages in Spack configuration"
 section = "config"
 level = "short"
 
@@ -26,12 +28,18 @@ def setup_parser(subparser):
     sp = subparser.add_subparsers(
         metavar='SUBCOMMAND', dest='external_command')
 
-    find_parser = sp.add_parser('find', help=external_find.__doc__)
+    find_parser = sp.add_parser(
+        'find', help='add external packages to packages.yaml'
+    )
     find_parser.add_argument(
         '--not-buildable', action='store_true', default=False,
         help="packages with detected externals won't be built with Spack")
     find_parser.add_argument('packages', nargs=argparse.REMAINDER)
 
+    sp.add_parser(
+        'list', help='list detectable packages, by repository and name'
+    )
+
 
 def is_executable(path):
     return os.path.isfile(path) and os.access(path, os.X_OK)
@@ -92,7 +100,16 @@ def _generate_pkg_config(external_pkg_entries):
             continue
 
         external_items = [('spec', str(e.spec)), ('prefix', e.base_dir)]
-        external_items.extend(e.spec.extra_attributes.items())
+        if e.spec.external_modules:
+            external_items.append(('modules', e.spec.external_modules))
+
+        if e.spec.extra_attributes:
+            external_items.append(
+                ('extra_attributes',
+                 syaml.syaml_dict(e.spec.extra_attributes.items()))
+            )
+
+        # external_items.extend(e.spec.extra_attributes.items())
         pkg_dict['externals'].append(
             syaml.syaml_dict(external_items)
         )
@@ -272,17 +289,29 @@ def _get_external_packages(packages_to_check, system_path_to_exe=None):
                     spec.validate_detection()
                 except Exception as e:
                     msg = ('"{0}" has been detected on the system but will '
-                           'not be added to packages.yaml [{1}]')
+                           'not be added to packages.yaml [reason={1}]')
                     tty.warn(msg.format(spec, str(e)))
                     continue
 
+                if spec.external_path:
+                    pkg_prefix = spec.external_path
+
                 pkg_to_entries[pkg.name].append(
                     ExternalPackageEntry(spec=spec, base_dir=pkg_prefix))
 
     return pkg_to_entries
 
 
-def external(parser, args):
+def external_list(args):
-    action = {'find': external_find}
+    # Trigger a read of all packages, might take a long time.
+    list(spack.repo.path.all_packages())
+    # Print all the detectable packages
+    tty.msg("Detectable packages per repository")
+    for namespace, pkgs in sorted(spack.package.detectable_packages.items()):
+        print("Repository:", namespace)
+        colify.colify(pkgs, indent=4, output=sys.stdout)
 
+
+def external(parser, args):
+    action = {'find': external_find, 'list': external_list}
     action[args.external_command](args)

lib/spack/spack/package.py

@@ -11,6 +11,7 @@
 
 """
 import base64
+import collections
 import contextlib
 import copy
 import functools
@@ -23,19 +24,14 @@
 import textwrap
 import time
 import traceback
-from six import StringIO
+
-from six import string_types
+import six
-from six import with_metaclass
-from ordereddict_backport import OrderedDict
 
 import llnl.util.tty as tty
-
-import spack.config
-import spack.paths
-import spack.store
 import spack.compilers
-import spack.directives
+import spack.config
 import spack.dependency
+import spack.directives
 import spack.directory_layout
 import spack.error
 import spack.fetch_strategy as fs
@@ -43,15 +39,19 @@
 import spack.mirror
 import spack.mixins
 import spack.multimethod
+import spack.paths
 import spack.repo
+import spack.store
 import spack.url
 import spack.util.environment
 import spack.util.web
-import spack.multimethod
-
 from llnl.util.filesystem import mkdirp, touch, working_dir
 from llnl.util.lang import memoized
 from llnl.util.link_tree import LinkTree
+from ordereddict_backport import OrderedDict
+from six import StringIO
+from six import string_types
+from six import with_metaclass
 from spack.filesystem_view import YamlFilesystemView
 from spack.installer import \
     install_args_docstring, PackageInstaller, InstallError
@@ -141,7 +141,104 @@ def copy(self):
             return other
 
 
+#: Registers which are the detectable packages, by repo and package name
+#: Need a pass of package repositories to be filled.
+detectable_packages = collections.defaultdict(list)
+
+
+class DetectablePackageMeta(object):
+    """Check if a package is detectable and add default implementations
+    for the detection function.
+    """
+    def __init__(cls, name, bases, attr_dict):
+        # If a package has the executables attribute then it's
+        # assumed to be detectable
+        if hasattr(cls, 'executables'):
+            @classmethod
+            def determine_spec_details(cls, prefix, exes_in_prefix):
+                """Allow ``spack external find ...`` to locate installations.
+
+                Args:
+                    prefix (str): the directory containing the executables
+                    exes_in_prefix (set): the executables that match the regex
+
+                Returns:
+                    The list of detected specs for this package
+                """
+                exes_by_version = collections.defaultdict(list)
+                # The default filter function is the identity function for the
+                # list of executables
+                filter_fn = getattr(cls, 'filter_detected_exes',
+                                    lambda x, exes: exes)
+                exes_in_prefix = filter_fn(prefix, exes_in_prefix)
+                for exe in exes_in_prefix:
+                    try:
+                        version_str = cls.determine_version(exe)
+                        if version_str:
+                            exes_by_version[version_str].append(exe)
+                    except Exception as e:
+                        msg = ('An error occurred when trying to detect '
+                               'the version of "{0}" [{1}]')
+                        tty.debug(msg.format(exe, str(e)))
+
+                specs = []
+                for version_str, exes in exes_by_version.items():
+                    variants = cls.determine_variants(exes, version_str)
+                    # Normalize output to list
+                    if not isinstance(variants, list):
+                        variants = [variants]
+
+                    for variant in variants:
+                        if isinstance(variant, six.string_types):
+                            variant = (variant, {})
+                        variant_str, extra_attributes = variant
+                        spec_str = '{0}@{1} {2}'.format(
+                            cls.name, version_str, variant_str
+                        )
+
+                        # Pop a few reserved keys from extra attributes, since
+                        # they have a different semantics
+                        external_path = extra_attributes.pop('prefix', None)
+                        external_modules = extra_attributes.pop(
+                            'modules', None
+                        )
+                        spec = spack.spec.Spec(
+                            spec_str,
+                            external_path=external_path,
+                            external_modules=external_modules
+                        )
+                        specs.append(spack.spec.Spec.from_detection(
+                            spec, extra_attributes=extra_attributes
+                        ))
+
+                return sorted(specs)
+
+            @classmethod
+            def determine_variants(cls, exes, version_str):
+                return ''
+
+            # Register the class as a detectable package
+            detectable_packages[cls.namespace].append(cls.name)
+
+            # Attach function implementations to the detectable class
+            default = False
+            if not hasattr(cls, 'determine_spec_details'):
+                default = True
+                cls.determine_spec_details = determine_spec_details
+
+            if default and not hasattr(cls, 'determine_version'):
+                msg = ('the package "{0}" in the "{1}" repo needs to define'
+                       ' the "determine_version" method to be detectable')
+                NotImplementedError(msg.format(cls.name, cls.namespace))
+
+            if default and not hasattr(cls, 'determine_variants'):
+                cls.determine_variants = determine_variants
+
+        super(DetectablePackageMeta, cls).__init__(name, bases, attr_dict)
+
+
 class PackageMeta(
+    DetectablePackageMeta,
     spack.directives.DirectiveMeta,
     spack.mixins.PackageMixinsMeta,
     spack.multimethod.MultiMethodMeta

lib/spack/spack/pkgkit.py

@@ -39,7 +39,7 @@
 
 from spack.version import Version, ver
 
-from spack.spec import Spec
+from spack.spec import Spec, InvalidSpecDetected
 
 from spack.dependency import all_deptypes
 

lib/spack/spack/schema/packages.py

@@ -117,7 +117,7 @@
 
 
 def update(data):
-    """Update in-place the data to remove deprecated properties.
+    """Update the data in place to remove deprecated properties.
 
     Args:
         data (dict): dictionary to be updated

lib/spack/spack/spec.py

@@ -4571,3 +4571,7 @@ class SpecDependencyNotFoundError(spack.error.SpecError):
 
 class SpecDeprecatedError(spack.error.SpecError):
     """Raised when a spec concretizes to a deprecated spec or dependency."""
+
+
+class InvalidSpecDetected(spack.error.SpecError):
+    """Raised when a detected spec doesn't pass validation checks."""

lib/spack/spack/test/cmd/external.py

@@ -2,10 +2,8 @@
 # Spack Project Developers. See the top-level COPYRIGHT file for details.
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
-
-import pytest
 import os
-import stat
+import os.path
 
 import spack
 from spack.spec import Spec
@@ -13,30 +11,10 @@
 from spack.main import SpackCommand
 
 
-@pytest.fixture()
+def test_find_external_single_package(mock_executable):
-def create_exe(tmpdir_factory):
-    def _create_exe(exe_name, content):
-        base_prefix = tmpdir_factory.mktemp('base-prefix')
-        base_prefix.ensure('bin', dir=True)
-        exe_path = str(base_prefix.join('bin', exe_name))
-        with open(exe_path, 'w') as f:
-            f.write("""\
-#!/bin/bash
-
-echo "{0}"
-""".format(content))
-
-        st = os.stat(exe_path)
-        os.chmod(exe_path, st.st_mode | stat.S_IEXEC)
-        return exe_path
-
-    yield _create_exe
-
-
-def test_find_external_single_package(create_exe):
     pkgs_to_check = [spack.repo.get('cmake')]
 
-    cmake_path = create_exe("cmake", "cmake version 1.foo")
+    cmake_path = mock_executable("cmake", output='echo "cmake version 1.foo"')
     system_path_to_exe = {cmake_path: 'cmake'}
 
     pkg_to_entries = spack.cmd.external._get_external_packages(
@@ -48,12 +26,16 @@ def test_find_external_single_package(create_exe):
     assert single_entry.spec == Spec('cmake@1.foo')
 
 
-def test_find_external_two_instances_same_package(create_exe):
+def test_find_external_two_instances_same_package(mock_executable):
     pkgs_to_check = [spack.repo.get('cmake')]
 
     # Each of these cmake instances is created in a different prefix
-    cmake_path1 = create_exe("cmake", "cmake version 1.foo")
+    cmake_path1 = mock_executable(
-    cmake_path2 = create_exe("cmake", "cmake version 3.17.2")
+        "cmake", output='echo "cmake version 1.foo"', subdir=('base1', 'bin')
+    )
+    cmake_path2 = mock_executable(
+        "cmake", output='echo "cmake version 3.17.2"', subdir=('base2', 'bin')
+    )
     system_path_to_exe = {
         cmake_path1: 'cmake',
         cmake_path2: 'cmake'}
@@ -86,8 +68,8 @@ def test_find_external_update_config(mutable_config):
     assert {'spec': 'cmake@3.17.2', 'prefix': '/x/y2/'} in cmake_externals
 
 
-def test_get_executables(working_env, create_exe):
+def test_get_executables(working_env, mock_executable):
-    cmake_path1 = create_exe("cmake", "cmake version 1.foo")
+    cmake_path1 = mock_executable("cmake", output="echo cmake version 1.foo")
 
     os.environ['PATH'] = ':'.join([os.path.dirname(cmake_path1)])
     path_to_exe = spack.cmd.external._get_system_executables()
@@ -97,11 +79,11 @@ def test_get_executables(working_env, create_exe):
 external = SpackCommand('external')
 
 
-def test_find_external_cmd(mutable_config, working_env, create_exe):
+def test_find_external_cmd(mutable_config, working_env, mock_executable):
     """Test invoking 'spack external find' with additional package arguments,
     which restricts the set of packages that Spack looks for.
     """
-    cmake_path1 = create_exe("cmake", "cmake version 1.foo")
+    cmake_path1 = mock_executable("cmake", output="echo cmake version 1.foo")
     prefix = os.path.dirname(os.path.dirname(cmake_path1))
 
     os.environ['PATH'] = ':'.join([os.path.dirname(cmake_path1)])
@@ -115,12 +97,12 @@ def test_find_external_cmd(mutable_config, working_env, create_exe):
 
 
 def test_find_external_cmd_not_buildable(
-        mutable_config, working_env, create_exe):
+        mutable_config, working_env, mock_executable):
     """When the user invokes 'spack external find --not-buildable', the config
     for any package where Spack finds an external version should be marked as
     not buildable.
     """
-    cmake_path1 = create_exe("cmake", "cmake version 1.foo")
+    cmake_path1 = mock_executable("cmake", output="echo cmake version 1.foo")
     os.environ['PATH'] = ':'.join([os.path.dirname(cmake_path1)])
     external('find', '--not-buildable', 'cmake')
     pkgs_cfg = spack.config.get('packages')
@@ -128,13 +110,13 @@ def test_find_external_cmd_not_buildable(
 
 
 def test_find_external_cmd_full_repo(
-        mutable_config, working_env, create_exe, mutable_mock_repo):
+        mutable_config, working_env, mock_executable, mutable_mock_repo):
     """Test invoking 'spack external find' with no additional arguments, which
     iterates through each package in the repository.
     """
 
-    exe_path1 = create_exe(
+    exe_path1 = mock_executable(
-        "find-externals1-exe", "find-externals1 version 1.foo"
+        "find-externals1-exe", output="echo find-externals1 version 1.foo"
     )
     prefix = os.path.dirname(os.path.dirname(exe_path1))
 
@@ -182,3 +164,61 @@ def test_find_external_merge(mutable_config, mutable_mock_repo):
             'prefix': '/preexisting-prefix/'} in pkg_externals
     assert {'spec': 'find-externals1@1.2',
             'prefix': '/x/y2/'} in pkg_externals
+
+
+def test_list_detectable_packages(mutable_config, mutable_mock_repo):
+    external("list")
+    assert external.returncode == 0
+
+
+def test_packages_yaml_format(mock_executable, mutable_config, monkeypatch):
+    # Prepare an environment to detect a fake gcc
+    gcc_exe = mock_executable('gcc', output="echo 4.2.1")
+    prefix = os.path.dirname(gcc_exe)
+    monkeypatch.setenv('PATH', prefix)
+
+    # Find the external spec
+    external('find', 'gcc')
+
+    # Check entries in 'packages.yaml'
+    packages_yaml = spack.config.get('packages')
+    assert 'gcc' in packages_yaml
+    assert 'externals' in packages_yaml['gcc']
+    externals = packages_yaml['gcc']['externals']
+    assert len(externals) == 1
+    external_gcc = externals[0]
+    assert external_gcc['spec'] == 'gcc@4.2.1 languages=c'
+    assert external_gcc['prefix'] == os.path.dirname(prefix)
+    assert 'extra_attributes' in external_gcc
+    extra_attributes = external_gcc['extra_attributes']
+    assert 'prefix' not in extra_attributes
+    assert extra_attributes['compilers']['c'] == gcc_exe
+
+
+def test_overriding_prefix(mock_executable, mutable_config, monkeypatch):
+    # Prepare an environment to detect a fake gcc that
+    # override its external prefix
+    gcc_exe = mock_executable('gcc', output="echo 4.2.1")
+    prefix = os.path.dirname(gcc_exe)
+    monkeypatch.setenv('PATH', prefix)
+
+    @classmethod
+    def _determine_variants(cls, exes, version_str):
+        return 'languages=c', {
+            'prefix': '/opt/gcc/bin',
+            'compilers': {'c': exes[0]}
+        }
+
+    gcc_cls = spack.repo.path.get_pkg_class('gcc')
+    monkeypatch.setattr(gcc_cls, 'determine_variants', _determine_variants)
+
+    # Find the external spec
+    external('find', 'gcc')
+
+    # Check entries in 'packages.yaml'
+    packages_yaml = spack.config.get('packages')
+    assert 'gcc' in packages_yaml
+    assert 'externals' in packages_yaml['gcc']
+    externals = packages_yaml['gcc']['externals']
+    assert len(externals) == 1
+    assert externals[0]['prefix'] == '/opt/gcc/bin'

share/spack/spack-completion.bash

@@ -853,7 +853,7 @@ _spack_external() {
     then
         SPACK_COMPREPLY="-h --help"
     else
-        SPACK_COMPREPLY="find"
+        SPACK_COMPREPLY="find list"
     fi
 }
 
@@ -866,6 +866,10 @@ _spack_external_find() {
     fi
 }
 
+_spack_external_list() {
+    SPACK_COMPREPLY="-h --help"
+}
+
 _spack_fetch() {
     if $list_options
     then

var/spack/repos/builtin/packages/automake/package.py

@@ -2,10 +2,6 @@
 # Spack Project Developers. See the top-level COPYRIGHT file for details.
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
-
-from spack import *
-
-import os
 import re
 
 
@@ -28,23 +24,13 @@ class Automake(AutotoolsPackage, GNUMirrorPackage):
 
     build_directory = 'spack-build'
 
-    executables = ['automake']
+    executables = ['^automake$']
 
     @classmethod
-    def determine_spec_details(cls, prefix, exes_in_prefix):
+    def determine_version(cls, exe):
-        exe_to_path = dict(
+        output = Executable(exe)('--version', output=str)
-            (os.path.basename(p), p) for p in exes_in_prefix
+        match = re.search(r'GNU automake\)\s+(\S+)', output)
-        )
+        return match.group(1) if match else None
-        if 'automake' not in exe_to_path:
-            return None
-
-        exe = spack.util.executable.Executable(exe_to_path['automake'])
-        output = exe('--version', output=str)
-        if output:
-            match = re.search(r'GNU automake\)\s+(\S+)', output)
-            if match:
-                version_str = match.group(1)
-                return Spec('automake@{0}'.format(version_str))
 
     def patch(self):
         # The full perl shebang might be too long

var/spack/repos/builtin/packages/cmake/package.py

@@ -2,21 +2,18 @@
 # Spack Project Developers. See the top-level COPYRIGHT file for details.
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
-
-from spack import *
-
 import re
-import os
 
 
 class Cmake(Package):
     """A cross-platform, open-source build system. CMake is a family of
-       tools designed to build, test and package software."""
+    tools designed to build, test and package software.
+    """
     homepage = 'https://www.cmake.org'
-    url      = 'https://github.com/Kitware/CMake/releases/download/v3.15.5/cmake-3.15.5.tar.gz'
+    url = 'https://github.com/Kitware/CMake/releases/download/v3.15.5/cmake-3.15.5.tar.gz'
     maintainers = ['chuckatkins']
 
-    executables = ['cmake']
+    executables = ['^cmake$']
 
     version('3.18.1',   sha256='c0e3338bd37e67155b9d1e9526fec326b5c541f74857771b7ffed0c46ad62508')
     version('3.18.0',   sha256='83b4ffcb9482a73961521d2bafe4a16df0168f03f56e6624c419c461e5317e29')
@@ -163,20 +160,10 @@ class Cmake(Package):
     phases = ['bootstrap', 'build', 'install']
 
     @classmethod
-    def determine_spec_details(cls, prefix, exes_in_prefix):
+    def determine_version(cls, exe):
-        exe_to_path = dict(
+        output = Executable(exe)('--version', output=str)
-            (os.path.basename(p), p) for p in exes_in_prefix
+        match = re.search(r'cmake.*version\s+(\S+)', output)
-        )
+        return match.group(1) if match else None
-        if 'cmake' not in exe_to_path:
-            return None
-
-        cmake = spack.util.executable.Executable(exe_to_path['cmake'])
-        output = cmake('--version', output=str)
-        if output:
-            match = re.search(r'cmake.*version\s+(\S+)', output)
-            if match:
-                version_str = match.group(1)
-                return Spec('cmake@{0}'.format(version_str))
 
     def flag_handler(self, name, flags):
         if name == 'cxxflags' and self.compiler.name == 'fj':

var/spack/repos/builtin/packages/gcc/package.py

@@ -2,16 +2,17 @@
 # Spack Project Developers. See the top-level COPYRIGHT file for details.
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
-
-from spack import *
-from spack.operating_systems.mac_os import macos_version, macos_sdk_path
-from llnl.util import tty
-
 import glob
 import itertools
 import os
+import re
 import sys
 
+import llnl.util.tty as tty
+import spack.util.executable
+
+from spack.operating_systems.mac_os import macos_version, macos_sdk_path
+
 
 class Gcc(AutotoolsPackage, GNUMirrorPackage):
     """The GNU Compiler Collection includes front ends for C, C++, Objective-C,
@@ -269,6 +270,105 @@ class Gcc(AutotoolsPackage, GNUMirrorPackage):
 
     build_directory = 'spack-build'
 
+    @property
+    def executables(self):
+        names = [r'gcc', r'[^\w]?g\+\+', r'gfortran']
+        suffixes = [r'', r'-mp-\d+\.\d', r'-\d+\.\d', r'-\d+', r'\d\d']
+        return [r''.join(x) for x in itertools.product(names, suffixes)]
+
+    @classmethod
+    def filter_detected_exes(cls, prefix, exes_in_prefix):
+        result = []
+        for exe in exes_in_prefix:
+            # clang++ matches g++ -> clan[g++]
+            if any(x in exe for x in ('clang', 'ranlib')):
+                continue
+            # Filter out links in favor of real executables
+            if os.path.islink(exe):
+                continue
+            result.append(exe)
+        return result
+
+    @classmethod
+    def determine_version(cls, exe):
+        version_regex = re.compile(r'([\d\.]+)')
+        for vargs in ('-dumpfullversion', '-dumpversion'):
+            try:
+                output = spack.compiler.get_compiler_version_output(exe, vargs)
+                match = version_regex.search(output)
+                if match:
+                    return match.group(1)
+            except spack.util.executable.ProcessError:
+                pass
+            except Exception as e:
+                tty.debug(e)
+
+        return None
+
+    @classmethod
+    def determine_variants(cls, exes, version_str):
+        languages, compilers = set(), {}
+        for exe in exes:
+            basename = os.path.basename(exe)
+            if 'gcc' in basename:
+                languages.add('c')
+                compilers['c'] = exe
+            elif 'g++' in basename:
+                languages.add('c++')
+                compilers['cxx'] = exe
+            elif 'gfortran' in basename:
+                languages.add('fortran')
+                compilers['fortran'] = exe
+        variant_str = 'languages={0}'.format(','.join(languages))
+        return variant_str, {'compilers': compilers}
+
+    @classmethod
+    def validate_detected_spec(cls, spec, extra_attributes):
+        # For GCC 'compilers' is a mandatory attribute
+        msg = ('the extra attribute "compilers" must be set for '
+               'the detected spec "{0}"'.format(spec))
+        assert 'compilers' in extra_attributes, msg
+
+        compilers = extra_attributes['compilers']
+        for constraint, key in {
+            'languages=c': 'c',
+            'languages=c++': 'cxx',
+            'languages=fortran': 'fortran'
+        }.items():
+            if spec.satisfies(constraint, strict=True):
+                msg = '{0} not in {1}'
+                assert key in compilers, msg.format(key, spec)
+
+    @property
+    def cc(self):
+        msg = "cannot retrieve C compiler [spec is not concrete]"
+        assert self.spec.concrete, msg
+        if self.spec.external:
+            return self.spec.extra_attributes['compilers'].get('c', None)
+        return self.spec.prefix.bin.gcc if 'languages=c' in self.spec else None
+
+    @property
+    def cxx(self):
+        msg = "cannot retrieve C++ compiler [spec is not concrete]"
+        assert self.spec.concrete, msg
+        if self.spec.external:
+            return self.spec.extra_attributes['compilers'].get('cxx', None)
+        result = None
+        if 'languages=c++' in self.spec:
+            result = os.path.join(self.spec.prefix.bin, 'g++')
+        return result
+
+    @property
+    def fortran(self):
+        msg = "cannot retrieve Fortran compiler [spec is not concrete]"
+        assert self.spec.concrete, msg
+        if self.spec.external:
+            return self.spec.extra_attributes['compilers'].get('fortran', None)
+        result = None
+        if 'languages=fortran' in self.spec:
+            result = self.spec.prefix.bin.gfortran
+        return result
+
     def url_for_version(self, version):
         # This function will be called when trying to fetch from url, before
         # mirrors are tried. It takes care of modifying the suffix of gnu