From ff27233e309e2f382d0d3c98a73ef72305a7d656 Mon Sep 17 00:00:00 2001
From: Todd Gamblin <tgamblin@llnl.gov>
Date: Fri, 31 Jul 2020 18:57:18 -0700
Subject: [PATCH] bugfix: fix spack buildcache list --allarch

`spack buildcache list` was trying to construct an `Arch` object and
compare it to `arch_for_spec(<spec>)`. for each spec in the buildcache.
`Arch` objects are only intended to be constructed for the machine they
describe. The `ArchSpec` object (part of the `Spec`) is the descriptor
that lets us talk about architectures anywhere.

- [x] Modify `spack buildcache list` and `spack buildcache install` to
      filter with `Spec` matching instead of using `Arch`.
---
 lib/spack/spack/binary_distribution.py | 12 +++------
 lib/spack/spack/cmd/buildcache.py      | 22 +++++++++++++---
 lib/spack/spack/test/cmd/buildcache.py | 36 ++++++++++++++++++++++----
 3 files changed, 53 insertions(+), 17 deletions(-)

diff --git a/lib/spack/spack/binary_distribution.py b/lib/spack/spack/binary_distribution.py
index f53501cbd6..220686f831 100644
--- a/lib/spack/spack/binary_distribution.py
+++ b/lib/spack/spack/binary_distribution.py
@@ -36,7 +36,6 @@
 from spack.spec import Spec
 from spack.stage import Stage
 from spack.util.gpg import Gpg
-import spack.architecture as architecture
 
 _build_cache_relative_path = 'build_cache'
 
@@ -856,13 +855,11 @@ def get_spec(spec=None, force=False):
     return try_download_specs(urls=urls, force=force)
 
 
-def get_specs(allarch=False):
+def get_specs():
     """
     Get spec.yaml's for build caches available on mirror
     """
     global _cached_specs
-    arch = architecture.Arch(architecture.platform(),
-                             'default_os', 'default_target')
 
     if not spack.mirror.MirrorCollection():
         tty.debug("No Spack mirrors are currently configured")
@@ -882,8 +879,7 @@ def get_specs(allarch=False):
                 index_url, 'application/json')
             index_object = codecs.getreader('utf-8')(file_stream).read()
         except (URLError, web_util.SpackWebError) as url_err:
-            tty.error('Failed to read index {0}'.format(index_url))
-            tty.debug(url_err)
+            tty.debug('Failed to read index {0}'.format(index_url), url_err, 1)
             # Continue on to the next mirror
             continue
 
@@ -900,9 +896,7 @@ def get_specs(allarch=False):
         spec_list = db.query_local(installed=False)
 
         for indexed_spec in spec_list:
-            spec_arch = architecture.arch_for_spec(indexed_spec.architecture)
-            if (allarch is True or spec_arch == arch):
-                _cached_specs.add(indexed_spec)
+            _cached_specs.add(indexed_spec)
 
     return _cached_specs
 
diff --git a/lib/spack/spack/cmd/buildcache.py b/lib/spack/spack/cmd/buildcache.py
index c28397b0aa..ef6c386703 100644
--- a/lib/spack/spack/cmd/buildcache.py
+++ b/lib/spack/spack/cmd/buildcache.py
@@ -8,6 +8,7 @@
 import sys
 
 import llnl.util.tty as tty
+import spack.architecture
 import spack.binary_distribution as bindist
 import spack.cmd
 import spack.cmd.common.arguments as arguments
@@ -25,6 +26,7 @@
 
 from spack.error import SpecError
 from spack.spec import Spec, save_dependency_spec_yamls
+from spack.util.string import plural
 
 from spack.cmd import display_specs
 
@@ -288,8 +290,12 @@ def match_downloaded_specs(pkgs, allow_multiple_matches=False, force=False,
     # List of specs that match expressions given via command line
     specs_from_cli = []
     has_errors = False
-    allarch = other_arch
-    specs = bindist.get_specs(allarch)
+
+    specs = bindist.get_specs()
+    if not other_arch:
+        arch = spack.architecture.default_arch().to_spec()
+        specs = [s for s in specs if s.satisfies(arch)]
+
     for pkg in pkgs:
         matches = []
         tty.msg("buildcache spec(s) matching %s \n" % pkg)
@@ -488,10 +494,20 @@ def install_tarball(spec, args):
 
 def listspecs(args):
     """list binary packages available from mirrors"""
-    specs = bindist.get_specs(args.allarch)
+    specs = bindist.get_specs()
+    if not args.allarch:
+        arch = spack.architecture.default_arch().to_spec()
+        specs = [s for s in specs if s.satisfies(arch)]
+
     if args.specs:
         constraints = set(args.specs)
         specs = [s for s in specs if any(s.satisfies(c) for c in constraints)]
+    if sys.stdout.isatty():
+        builds = len(specs)
+        tty.msg("%s." % plural(builds, 'cached build'))
+        if not builds and not args.allarch:
+            tty.msg("You can query all available architectures with:",
+                    "spack buildcache list --allarch")
     display_specs(specs, args, all_headers=True)
 
 
diff --git a/lib/spack/spack/test/cmd/buildcache.py b/lib/spack/spack/test/cmd/buildcache.py
index eb718a91da..6cd60db55f 100644
--- a/lib/spack/spack/test/cmd/buildcache.py
+++ b/lib/spack/spack/test/cmd/buildcache.py
@@ -12,6 +12,7 @@
 import spack.main
 import spack.binary_distribution
 import spack.environment as ev
+import spack.spec
 from spack.spec import Spec
 
 buildcache = spack.main.SpackCommand('buildcache')
@@ -24,7 +25,22 @@
 def mock_get_specs(database, monkeypatch):
     specs = database.query_local()
     monkeypatch.setattr(
-        spack.binary_distribution, 'get_specs', lambda x: specs
+        spack.binary_distribution, 'get_specs', lambda: specs
+    )
+
+
+@pytest.fixture()
+def mock_get_specs_multiarch(database, monkeypatch):
+    specs = [spec.copy() for spec in database.query_local()]
+
+    # make one spec that is NOT the test architecture
+    for spec in specs:
+        if spec.name == "mpileaks":
+            spec.architecture = spack.spec.ArchSpec('linux-rhel7-x86_64')
+            break
+
+    monkeypatch.setattr(
+        spack.binary_distribution, 'get_specs', lambda: specs
     )
 
 
@@ -37,10 +53,6 @@ def test_buildcache_preview_just_runs(database):
     buildcache('preview', 'mpileaks')
 
 
-@pytest.mark.skipif(
-    platform.system().lower() != 'linux',
-    reason='implementation for MacOS still missing'
-)
 @pytest.mark.db
 @pytest.mark.regression('13757')
 def test_buildcache_list_duplicates(mock_get_specs, capsys):
@@ -50,6 +62,20 @@ def test_buildcache_list_duplicates(mock_get_specs, capsys):
     assert output.count('mpileaks') == 3
 
 
+@pytest.mark.db
+@pytest.mark.regression('17827')
+def test_buildcache_list_allarch(database, mock_get_specs_multiarch, capsys):
+    with capsys.disabled():
+        output = buildcache('list', '--allarch')
+
+    assert output.count('mpileaks') == 3
+
+    with capsys.disabled():
+        output = buildcache('list')
+
+    assert output.count('mpileaks') == 2
+
+
 def tests_buildcache_create(
         install_mockery, mock_fetch, monkeypatch, tmpdir):
     """"Ensure that buildcache create creates output files"""
-- 
GitLab