Reworking of `lapack_shared_libs` and similar properties (#1682)

* Turned <provider>_libs into an iterable

Modifications :
- added class LibraryList + unit tests
- added convenience functions `find_libraries` and `dedupe`
- modifed non Intel blas/lapack providers
- modified packages using blas_shared_libs and similar functions

* atlas : added pthread variant

* intel packages : added lapack_libs and blas_libs

* find_library_path : removed unused function

* PR review : fixed last issues

* LibraryList : added test on __add__ return type

* LibraryList : added __radd__ fixed unit tests

fix : failing unit tests due to missing `self`

* cp2k and dependecies : fixed blas-lapack related statements in package.py

6 files changed, 303 insertions, 38 deletions

diff --git a/lib/spack/docs/packaging_guide.rst b/lib/spack/docs/packaging_guide.rst
index 6936b5e423..0294f32748 100644
--- a/lib/spack/docs/packaging_guide.rst
+++ b/lib/spack/docs/packaging_guide.rst
@@ -2090,12 +2090,11 @@ Blas and Lapack libraries
 
 Different packages provide implementation of ``Blas`` and ``Lapack`` routines.
 The names of the resulting static and/or shared libraries differ from package
-to package. In order to make the ``install()`` method indifferent to the
+to package. In order to make the ``install()`` method independent of the
 choice of ``Blas`` implementation, each package which provides it
-sets up ``self.spec.blas_shared_lib`` and ``self.spec.blas_static_lib`` to
-point to the shared and static ``Blas`` libraries, respectively. The same
-applies to packages which provide ``Lapack``. Package developers are advised to
-use these variables, for example ``spec['blas'].blas_shared_lib`` instead of
+sets up ``self.spec.blas_libs`` to point to the correct ``Blas`` libraries.
+The same applies to packages which provide ``Lapack``. Package developers are advised to
+use these variables, for example ``spec['blas'].blas_libs.joined()`` instead of
 hard-coding ``join_path(spec['blas'].prefix.lib, 'libopenblas.so')``.
 
 ^^^^^^^^^^^^^^^^^^^^^
diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py
index 22ca85abf9..c3ecfde4f4 100644
--- a/lib/spack/llnl/util/filesystem.py
+++ b/lib/spack/llnl/util/filesystem.py
@@ -22,18 +22,22 @@
 # 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 os
+import collections
+import errno
+import fileinput
+import getpass
 import glob
+import numbers
+import os
 import re
 import shutil
 import stat
-import errno
-import getpass
-from contextlib import contextmanager
 import subprocess
-import fileinput
+import sys
+from contextlib import contextmanager
 
 import llnl.util.tty as tty
+from llnl.util.lang import dedupe
 
 __all__ = ['set_install_permissions', 'install', 'install_tree',
            'traverse_tree',
@@ -42,8 +46,8 @@ __all__ = ['set_install_permissions', 'install', 'install_tree',
            'filter_file',
            'FileFilter', 'change_sed_delimiter', 'is_exe', 'force_symlink',
            'set_executable', 'copy_mode', 'unset_executable_mode',
-           'remove_dead_links', 'remove_linked_tree', 'find_library_path',
-           'fix_darwin_install_name', 'to_link_flags', 'to_lib_name']
+           'remove_dead_links', 'remove_linked_tree',
+           'fix_darwin_install_name', 'find_libraries', 'LibraryList']
 
 
 def filter_file(regex, repl, *filenames, **kwargs):
@@ -326,7 +330,7 @@ def traverse_tree(source_root, dest_root, rel_path='', **kwargs):
     follow_links = kwargs.get('follow_link', False)
 
     # Yield in pre or post order?
-    order  = kwargs.get('order', 'pre')
+    order = kwargs.get('order', 'pre')
     if order not in ('pre', 'post'):
         raise ValueError("Order must be 'pre' or 'post'.")
 
@@ -338,7 +342,7 @@ def traverse_tree(source_root, dest_root, rel_path='', **kwargs):
         return
 
     source_path = os.path.join(source_root, rel_path)
-    dest_path   = os.path.join(dest_root, rel_path)
+    dest_path = os.path.join(dest_root, rel_path)
 
     # preorder yields directories before children
     if order == 'pre':
@@ -346,8 +350,8 @@ def traverse_tree(source_root, dest_root, rel_path='', **kwargs):
 
     for f in os.listdir(source_path):
         source_child = os.path.join(source_path, f)
-        dest_child   = os.path.join(dest_path, f)
-        rel_child    = os.path.join(rel_path, f)
+        dest_child = os.path.join(dest_path, f)
+        rel_child = os.path.join(rel_path, f)
 
         # Treat as a directory
         if os.path.isdir(source_child) and (
@@ -440,35 +444,162 @@ def fix_darwin_install_name(path):
                         stdout=subprocess.PIPE).communicate()[0]
                     break
 
+# Utilities for libraries
+
 
-def to_lib_name(library):
-    """Transforms a path to the library /path/to/lib<name>.xyz into <name>
+class LibraryList(collections.Sequence):
+    """Sequence of absolute paths to libraries
+
+    Provides a few convenience methods to manipulate library paths and get
+    commonly used compiler flags or names
     """
-    # Assume libXYZ.suffix
-    return os.path.basename(library)[3:].split(".")[0]
 
+    def __init__(self, libraries):
+        self.libraries = list(libraries)
 
-def to_link_flags(library):
-    """Transforms a path to a <library> into linking flags -L<dir> -l<name>.
+    @property
+    def directories(self):
+        """Stable de-duplication of the directories where the libraries
+        reside
 
-    Return:
-      A string of linking flags.
-    """
-    dir  = os.path.dirname(library)
-    name = to_lib_name(library)
-    res = '-L%s -l%s' % (dir, name)
-    return res
+        >>> l = LibraryList(['/dir1/liba.a', '/dir2/libb.a', '/dir1/libc.a'])
+        >>> assert l.directories == ['/dir1', '/dir2']
+        """
+        return list(dedupe(
+            os.path.dirname(x) for x in self.libraries if os.path.dirname(x)
+        ))
+
+    @property
+    def basenames(self):
+        """Stable de-duplication of the base-names in the list
+
+        >>> l = LibraryList(['/dir1/liba.a', '/dir2/libb.a', '/dir3/liba.a'])
+        >>> assert l.basenames == ['liba.a', 'libb.a']
+        """
+        return list(dedupe(os.path.basename(x) for x in self.libraries))
+
+    @property
+    def names(self):
+        """Stable de-duplication of library names in the list
+
+        >>> l = LibraryList(['/dir1/liba.a', '/dir2/libb.a', '/dir3/liba.so'])
+        >>> assert l.names == ['a', 'b']
+        """
+        return list(dedupe(x.split('.')[0][3:] for x in self.basenames))
+
+    @property
+    def search_flags(self):
+        """Search flags for the libraries
+
+        >>> l = LibraryList(['/dir1/liba.a', '/dir2/libb.a', '/dir1/liba.so'])
+        >>> assert l.search_flags == '-L/dir1 -L/dir2'
+        """
+        return ' '.join(['-L' + x for x in self.directories])
+
+    @property
+    def link_flags(self):
+        """Link flags for the libraries
+
+        >>> l = LibraryList(['/dir1/liba.a', '/dir2/libb.a', '/dir1/liba.so'])
+        >>> assert l.search_flags == '-la -lb'
+        """
+        return ' '.join(['-l' + name for name in self.names])
 
+    @property
+    def ld_flags(self):
+        """Search flags + link flags
 
-def find_library_path(libname, *paths):
-    """Searches for a file called <libname> in each path.
+        >>> l = LibraryList(['/dir1/liba.a', '/dir2/libb.a', '/dir1/liba.so'])
+        >>> assert l.search_flags == '-L/dir1 -L/dir2 -la -lb'
+        """
+        return self.search_flags + ' ' + self.link_flags
 
-    Return:
-      directory where the library was found, if found.  None otherwise.
+    def __getitem__(self, item):
+        cls = type(self)
+        if isinstance(item, numbers.Integral):
+            return self.libraries[item]
+        return cls(self.libraries[item])
 
+    def __add__(self, other):
+        return LibraryList(dedupe(self.libraries + list(other)))
+
+    def __radd__(self, other):
+        return self.__add__(other)
+
+    def __eq__(self, other):
+        return self.libraries == other.libraries
+
+    def __len__(self):
+        return len(self.libraries)
+
+    def joined(self, separator=' '):
+        return separator.join(self.libraries)
+
+    def __repr__(self):
+        return self.__class__.__name__ + '(' + repr(self.libraries) + ')'
+
+    def __str__(self):
+        return self.joined()
+
+
+def find_libraries(args, root, shared=True, recurse=False):
+    """Returns an iterable object containing a list of full paths to
+    libraries if found.
+
+    Args:
+        args: iterable object containing a list of library names to \
+            search for (e.g. 'libhdf5')
+        root: root folder where to start searching
+        shared: if True searches for shared libraries, otherwise for static
+        recurse: if False search only root folder, if True descends top-down \
+            from the root
+
+    Returns:
+        list of full paths to the libraries that have been found
     """
-    for path in paths:
-        library = join_path(path, libname)
-        if os.path.exists(library):
-            return path
-    return None
+    if not isinstance(args, collections.Sequence) or isinstance(args, str):
+        message = '{0} expects a sequence of strings as first argument'
+        message += ' [got {1} instead]'
+        raise TypeError(message.format(find_libraries.__name__, type(args)))
+
+    # Construct the right suffix for the library
+    if shared is True:
+        suffix = 'dylib' if sys.platform == 'darwin' else 'so'
+    else:
+        suffix = 'a'
+    # List of libraries we are searching with suffixes
+    libraries = ['{0}.{1}'.format(lib, suffix) for lib in args]
+    # Search method
+    if recurse is False:
+        search_method = _find_libraries_non_recursive
+    else:
+        search_method = _find_libraries_recursive
+
+    return search_method(libraries, root)
+
+
+def _find_libraries_recursive(libraries, root):
+    library_dict = collections.defaultdict(list)
+    for path, _, files in os.walk(root):
+        for lib in libraries:
+            if lib in files:
+                library_dict[lib].append(
+                    join_path(path, lib)
+                )
+    answer = []
+    for lib in libraries:
+        answer.extend(library_dict[lib])
+    return LibraryList(answer)
+
+
+def _find_libraries_non_recursive(libraries, root):
+
+    def lib_or_none(lib):
+        library = join_path(root, lib)
+        if not os.path.exists(library):
+            return None
+        return library
+
+    return LibraryList(
+        [lib_or_none(lib) for lib in libraries if lib_or_none(lib) is not None]
+    )
diff --git a/lib/spack/llnl/util/lang.py b/lib/spack/llnl/util/lang.py
index df32012e2d..253334c416 100644
--- a/lib/spack/llnl/util/lang.py
+++ b/lib/spack/llnl/util/lang.py
@@ -374,6 +374,22 @@ def DictWrapper(dictionary):
     return wrapper()
 
 
+def dedupe(sequence):
+    """Yields a stable de-duplication of an hashable sequence
+
+    Args:
+        sequence: hashable sequence to be de-duplicated
+
+    Returns:
+        stable de-duplication of the sequence
+    """
+    seen = set()
+    for x in sequence:
+        if x not in seen:
+            yield x
+            seen.add(x)
+
+
 class RequiredAttributeError(ValueError):
 
     def __init__(self, message):
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index 8b0486c4da..158d76e568 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -527,6 +527,13 @@ class Spec(object):
             # XXX(deptype): default deptypes
             self._add_dependency(spec, ('build', 'link'))
 
+    def __getattr__(self, item):
+        """Delegate to self.package if the attribute is not in the spec"""
+        # This line is to avoid infinite recursion in case package is
+        # not present among self attributes
+        package = super(Spec, self).__getattribute__('package')
+        return getattr(package, item)
+
     def get_dependency(self, name):
         dep = self._dependencies.get(name)
         if dep is not None:
diff --git a/lib/spack/spack/test/__init__.py b/lib/spack/spack/test/__init__.py
index db683917b5..0a946ff2ff 100644
--- a/lib/spack/spack/test/__init__.py
+++ b/lib/spack/spack/test/__init__.py
@@ -53,6 +53,7 @@ test_names = [
     'git_fetch',
     'hg_fetch',
     'install',
+    'library_list',
     'link_tree',
     'lock',
     'make_executable',
diff --git a/lib/spack/spack/test/library_list.py b/lib/spack/spack/test/library_list.py
new file mode 100644
index 0000000000..7fc2fd222f
--- /dev/null
+++ b/lib/spack/spack/test/library_list.py
@@ -0,0 +1,111 @@
+##############################################################################
+# Copyright (c) 2013-2016, 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/llnl/spack
+# Please also see the LICENSE file 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 unittest
+
+from llnl.util.filesystem import LibraryList
+
+
+class LibraryListTest(unittest.TestCase):
+    def setUp(self):
+        l = [
+            '/dir1/liblapack.a',
+            '/dir2/libfoo.dylib',
+            '/dir1/libblas.a',
+            '/dir3/libbar.so',
+            'libbaz.so'
+        ]
+        self.liblist = LibraryList(l)
+
+    def test_repr(self):
+        x = eval(repr(self.liblist))
+        self.assertEqual(self.liblist, x)
+
+    def test_joined_and_str(self):
+        s1 = self.liblist.joined()
+        self.assertEqual(
+            s1,
+            '/dir1/liblapack.a /dir2/libfoo.dylib /dir1/libblas.a /dir3/libbar.so libbaz.so'  # NOQA: ignore=E501
+        )
+        s2 = str(self.liblist)
+        self.assertEqual(s1, s2)
+        s3 = self.liblist.joined(';')
+        self.assertEqual(
+            s3,
+            '/dir1/liblapack.a;/dir2/libfoo.dylib;/dir1/libblas.a;/dir3/libbar.so;libbaz.so'  # NOQA: ignore=E501
+        )
+
+    def test_flags(self):
+        search_flags = self.liblist.search_flags
+        self.assertTrue('-L/dir1' in search_flags)
+        self.assertTrue('-L/dir2' in search_flags)
+        self.assertTrue('-L/dir3' in search_flags)
+        self.assertTrue(isinstance(search_flags, str))
+
+        link_flags = self.liblist.link_flags
+        self.assertEqual(
+            link_flags,
+            '-llapack -lfoo -lblas -lbar -lbaz'
+        )
+
+        ld_flags = self.liblist.ld_flags
+        self.assertEqual(ld_flags, search_flags + ' ' + link_flags)
+
+    def test_paths_manipulation(self):
+        names = self.liblist.names
+        self.assertEqual(names, ['lapack', 'foo', 'blas', 'bar', 'baz'])
+
+        directories = self.liblist.directories
+        self.assertEqual(directories, ['/dir1', '/dir2', '/dir3'])
+
+    def test_get_item(self):
+        a = self.liblist[0]
+        self.assertEqual(a, '/dir1/liblapack.a')
+
+        b = self.liblist[:]
+        self.assertEqual(type(b), type(self.liblist))
+        self.assertEqual(self.liblist, b)
+        self.assertTrue(self.liblist is not b)
+
+    def test_add(self):
+        pylist = [
+            '/dir1/liblapack.a',  # removed from the final list
+            '/dir2/libbaz.so',
+            '/dir4/libnew.a'
+        ]
+        another = LibraryList(pylist)
+        l = self.liblist + another
+        self.assertEqual(len(l), 7)
+        # Invariant : l == l + l
+        self.assertEqual(l, l + l)
+        # Always produce an instance of LibraryList
+        self.assertEqual(
+            type(self.liblist),
+            type(self.liblist + pylist)
+        )
+        self.assertEqual(
+            type(pylist + self.liblist),
+            type(self.liblist)
+        )