package_hash: add test to ensure that every package in Spack can be unparsed

- [x] add option to canonical source to *not* filter multimethods
- [x] add test to unparse every package in builtin

3 files changed, 30 insertions, 7 deletions

diff --git a/lib/spack/spack/test/package_sanity.py b/lib/spack/spack/test/package_sanity.py
index dd2f4b425f..0ae3313c8a 100644
--- a/lib/spack/spack/test/package_sanity.py
+++ b/lib/spack/spack/test/package_sanity.py
@@ -20,6 +20,7 @@ import spack.paths
 import spack.repo
 import spack.util.crypto as crypto
 import spack.util.executable as executable
+import spack.util.package_hash as ph
 import spack.variant
 
 
@@ -56,6 +57,21 @@ def test_packages_are_pickleable():
             pickle.dumps(pkg)
 
 
+def test_packages_are_unparseable():
+    failed_to_unparse = list()
+    for name in spack.repo.all_package_names():
+        try:
+            ph.canonical_source(name, filter_multimethods=False)
+        except Exception:
+            failed_to_unparse.append(name)
+
+    if failed_to_unparse:
+        tty.msg('The following packages failed to unparse: ' +
+                ', '.join(failed_to_unparse))
+
+    assert len(failed_to_unparse) == 0
+
+
 def test_repo_getpkg_names_and_classes():
     """Ensure that all_packages/names/classes are consistent."""
     names = spack.repo.path.all_package_names()
diff --git a/lib/spack/spack/util/package_hash.py b/lib/spack/spack/util/package_hash.py
index cabb5f8613..14642026ba 100644
--- a/lib/spack/spack/util/package_hash.py
+++ b/lib/spack/spack/util/package_hash.py
@@ -221,8 +221,11 @@ def package_content(spec):
     return ast.dump(package_ast(spec))
 
 
-def canonical_source(spec, filename=None):
-    return unparse(package_ast(spec, filename=filename), py_ver_consistent=True)
+def canonical_source(spec, filename=None, filter_multimethods=True):
+    return unparse(
+        package_ast(spec, filename, filter_multimethods),
+        py_ver_consistent=True,
+    )
 
 
 def canonical_source_hash(spec, filename=None):
@@ -236,7 +239,7 @@ def package_hash(spec, content=None):
     return hashlib.sha256(content.encode('utf-8')).digest().lower()
 
 
-def package_ast(spec, filename=None):
+def package_ast(spec, filename=None, filter_multimethods=True):
     spec = spack.spec.Spec(spec)
 
     if not filename:
@@ -250,10 +253,11 @@ def package_ast(spec, filename=None):
 
     RemoveDirectives(spec).visit(root)
 
-    tagger = TagMultiMethods(spec)
-    tagger.visit(root)
+    if filter_multimethods:
+        tagger = TagMultiMethods(spec)
+        tagger.visit(root)
+        root = ResolveMultiMethods(tagger.methods).visit(root)
 
-    root = ResolveMultiMethods(tagger.methods).visit(root)
     return root
 
 
diff --git a/var/spack/repos/builtin/packages/bash-completion/package.py b/var/spack/repos/builtin/packages/bash-completion/package.py
index c03dabf74f..c2bab1248c 100644
--- a/var/spack/repos/builtin/packages/bash-completion/package.py
+++ b/var/spack/repos/builtin/packages/bash-completion/package.py
@@ -39,7 +39,10 @@ class BashCompletion(AutotoolsPackage):
         print('include the following lines in your ~/.bash_profile :')
         print('')
         print('# Use bash-completion, if available')
-        print('[[ $PS1 && -f %s/share/bash-completion/bash_completion ]] && \ ' % prefix)  # NOQA: ignore=E501
+        print(
+            '[[ $PS1 && -f %s/share/bash-completion/bash_completion ]] && \\'
+            % prefix
+        )
         print('    . %s/share/bash-completion/bash_completion' % prefix)
         print('')
         print('=====================================================')