Improve output of the external find command (#18017)

This commit adds output to the "spack external find"
command to inform users of the result of the operation.

It also fixes a bug introduced in #17804 due to the fact
that a function was not updated to conform to the new
packages.yaml format (_get_predefined_externals).

2 files changed, 38 insertions, 6 deletions

diff --git a/lib/spack/spack/cmd/external.py b/lib/spack/spack/cmd/external.py
index 5dcd6ebbd1..a5bbe54e1c 100644
--- a/lib/spack/spack/cmd/external.py
+++ b/lib/spack/spack/cmd/external.py
@@ -15,6 +15,7 @@ import llnl.util.tty as tty
 import llnl.util.tty.colify as colify
 import six
 import spack
+import spack.cmd
 import spack.error
 import spack.util.environment
 import spack.util.spack_yaml as syaml
@@ -146,7 +147,17 @@ def external_find(args):
         packages_to_check = spack.repo.path.all_packages()
 
     pkg_to_entries = _get_external_packages(packages_to_check)
-    _update_pkg_config(pkg_to_entries, args.not_buildable)
+    new_entries, write_scope = _update_pkg_config(
+        pkg_to_entries, args.not_buildable
+    )
+    if new_entries:
+        path = spack.config.config.get_config_filename(write_scope, 'packages')
+        msg = ('The following specs have been detected on this system '
+               'and added to {0}')
+        tty.msg(msg.format(path))
+        spack.cmd.display_specs(new_entries)
+    else:
+        tty.msg('No new external packages detected')
 
 
 def _group_by_prefix(paths):
@@ -188,23 +199,24 @@ def _get_predefined_externals():
     pkg_config = spack.config.get('packages')
     already_defined_specs = set()
     for pkg_name, per_pkg_cfg in pkg_config.items():
-        paths = per_pkg_cfg.get('paths', {})
-        already_defined_specs.update(spack.spec.Spec(k) for k in paths)
-        modules = per_pkg_cfg.get('modules', {})
-        already_defined_specs.update(spack.spec.Spec(k) for k in modules)
+        for item in per_pkg_cfg.get('externals', []):
+            already_defined_specs.add(spack.spec.Spec(item['spec']))
     return already_defined_specs
 
 
 def _update_pkg_config(pkg_to_entries, not_buildable):
     predefined_external_specs = _get_predefined_externals()
 
-    pkg_to_cfg = {}
+    pkg_to_cfg, all_new_specs = {}, []
     for pkg_name, ext_pkg_entries in pkg_to_entries.items():
         new_entries = list(
             e for e in ext_pkg_entries
             if (e.spec not in predefined_external_specs))
 
         pkg_config = _generate_pkg_config(new_entries)
+        all_new_specs.extend([
+            spack.spec.Spec(x['spec']) for x in pkg_config.get('externals', [])
+        ])
         if not_buildable:
             pkg_config['buildable'] = False
         pkg_to_cfg[pkg_name] = pkg_config
@@ -215,6 +227,8 @@ def _update_pkg_config(pkg_to_entries, not_buildable):
     spack.config.merge_yaml(pkgs_cfg, pkg_to_cfg)
     spack.config.set('packages', pkgs_cfg, scope=cfg_scope)
 
+    return all_new_specs, cfg_scope
+
 
 def _get_external_packages(packages_to_check, system_path_to_exe=None):
     if not system_path_to_exe:
diff --git a/lib/spack/spack/test/cmd/external.py b/lib/spack/spack/test/cmd/external.py
index 547d20de24..02db1652b7 100644
--- a/lib/spack/spack/test/cmd/external.py
+++ b/lib/spack/spack/test/cmd/external.py
@@ -222,3 +222,21 @@ def test_overriding_prefix(mock_executable, mutable_config, monkeypatch):
     externals = packages_yaml['gcc']['externals']
     assert len(externals) == 1
     assert externals[0]['prefix'] == '/opt/gcc/bin'
+
+
+def test_new_entries_are_reported_correctly(
+        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)
+
+    # The first run will find and add the external gcc
+    output = external('find', 'gcc')
+    assert 'The following specs have been' in output
+
+    # The second run should report that no new external
+    # has been found
+    output = external('find', 'gcc')
+    assert 'No new external packages detected' in output