find has been changed to accept glob expressions

Following the discussion with Todd and Adam, find has been modified to
accept glob expressions. This should not affect performance as every
glob implementation I inspected has 3 cases (no wildcard, wildcard but
no directories involved, wildcard and directories involved) and uses
fnmatch underneath.

Mixins have been changed to do by default a non-recursive search (but
a recursive search can still be triggered using the recursive keyword).

7 files changed, 79 insertions, 44 deletions

diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py
index f8d77e2727..6b92a374c7 100644
--- a/lib/spack/llnl/util/filesystem.py
+++ b/lib/spack/llnl/util/filesystem.py
@@ -26,7 +26,6 @@ import collections
 import errno
 import hashlib
 import fileinput
-import fnmatch
 import glob
 import numbers
 import os
@@ -606,7 +605,7 @@ def fix_darwin_install_name(path):
                     break
 
 
-def find(root, files, recurse=True):
+def find(root, files, recursive=True):
     """Search for ``files`` starting from the ``root`` directory.
 
     Like GNU/BSD find but written entirely in Python.
@@ -627,7 +626,7 @@ def find(root, files, recurse=True):
 
     is equivalent to:
 
-    >>> find('/usr/local/bin', 'python', recurse=False)
+    >>> find('/usr/local/bin', 'python', recursive=False)
 
     Accepts any glob characters accepted by fnmatch:
 
@@ -652,7 +651,7 @@ def find(root, files, recurse=True):
     if isinstance(files, six.string_types):
         files = [files]
 
-    if recurse:
+    if recursive:
         return _find_recursive(root, files)
     else:
         return _find_non_recursive(root, files)
@@ -666,11 +665,14 @@ def _find_recursive(root, search_files):
     # found in a key, and reconstructing the stable order later.
     found_files = collections.defaultdict(list)
 
+    # Make the path absolute to have os.walk also return an absolute path
+    root = os.path.abspath(root)
+
     for path, _, list_files in os.walk(root):
         for search_file in search_files:
-            for list_file in list_files:
-                if fnmatch.fnmatch(list_file, search_file):
-                    found_files[search_file].append(join_path(path, list_file))
+            matches = glob.glob(os.path.join(path, search_file))
+            matches = [os.path.join(path, x) for x in matches]
+            found_files[search_file].extend(matches)
 
     answer = []
     for search_file in search_files:
@@ -684,10 +686,13 @@ def _find_non_recursive(root, search_files):
     # can return files in any order (does not preserve stability)
     found_files = collections.defaultdict(list)
 
-    for list_file in os.listdir(root):
-        for search_file in search_files:
-            if fnmatch.fnmatch(list_file, search_file):
-                found_files[search_file].append(join_path(root, list_file))
+    # Make the path absolute to have absolute path returned
+    root = os.path.abspath(root)
+
+    for search_file in search_files:
+        matches = glob.glob(os.path.join(root, search_file))
+        matches = [os.path.join(root, x) for x in matches]
+        found_files[search_file].extend(matches)
 
     answer = []
     for search_file in search_files:
@@ -878,7 +883,7 @@ class HeaderList(FileList):
         self._macro_definitions.append(macro)
 
 
-def find_headers(headers, root, recurse=False):
+def find_headers(headers, root, recursive=False):
     """Returns an iterable object containing a list of full paths to
     headers if found.
 
@@ -896,7 +901,7 @@ def find_headers(headers, root, recurse=False):
     Parameters:
         headers (str or list of str): Header name(s) to search for
         root (str): The root directory to start searching from
-        recurses (bool, optional): if False search only root folder,
+        recursive (bool, optional): if False search only root folder,
             if True descends top-down from the root. Defaults to False.
 
     Returns:
@@ -916,7 +921,7 @@ def find_headers(headers, root, recurse=False):
     # List of headers we are searching with suffixes
     headers = ['{0}.{1}'.format(header, suffix) for header in headers]
 
-    return HeaderList(find(root, headers, recurse))
+    return HeaderList(find(root, headers, recursive))
 
 
 class LibraryList(FileList):
@@ -1057,7 +1062,7 @@ def find_system_libraries(libraries, shared=True):
 
     for library in libraries:
         for root in search_locations:
-            result = find_libraries(library, root, shared, recurse=True)
+            result = find_libraries(library, root, shared, recursive=True)
             if result:
                 libraries_found += result
                 break
@@ -1065,7 +1070,7 @@ def find_system_libraries(libraries, shared=True):
     return libraries_found
 
 
-def find_libraries(libraries, root, shared=True, recurse=False):
+def find_libraries(libraries, root, shared=True, recursive=False):
     """Returns an iterable of full paths to libraries found in a root dir.
 
     Accepts any glob characters accepted by fnmatch:
@@ -1084,7 +1089,7 @@ def find_libraries(libraries, root, shared=True, recurse=False):
         root (str): The root directory to start searching from
         shared (bool, optional): if True searches for shared libraries,
             otherwise for static. Defaults to True.
-        recurse (bool, optional): if False search only root folder,
+        recursive (bool, optional): if False search only root folder,
             if True descends top-down from the root. Defaults to False.
 
     Returns:
@@ -1106,4 +1111,4 @@ def find_libraries(libraries, root, shared=True, recurse=False):
     # List of libraries we are searching with suffixes
     libraries = ['{0}.{1}'.format(lib, suffix) for lib in libraries]
 
-    return LibraryList(find(root, libraries, recurse))
+    return LibraryList(find(root, libraries, recursive))
diff --git a/lib/spack/spack/mixins.py b/lib/spack/spack/mixins.py
index 0a43e76961..d16f572a4e 100644
--- a/lib/spack/spack/mixins.py
+++ b/lib/spack/spack/mixins.py
@@ -163,6 +163,11 @@ def filter_compiler_wrappers(*files, **kwargs):
                 these two keyword arguments, if present, will be forwarded
                 to ``filter_file`` (see its documentation for more information
                 on their behavior)
+
+            recursive
+                this keyword argument, if present, will be forwarded to
+                ``find`` (see its documentation for more information on the
+                behavior)
     """
     after = kwargs.get('after', 'install')
     relative_root = kwargs.get('relative_root', None)
@@ -173,6 +178,10 @@ def filter_compiler_wrappers(*files, **kwargs):
         'string': True
     }
 
+    find_kwargs = {
+        'recursive': kwargs.get('recursive', False)
+    }
+
     def _filter_compiler_wrappers_impl(self):
         # Compute the absolute path of the search root
         root = os.path.join(
@@ -181,7 +190,7 @@ def filter_compiler_wrappers(*files, **kwargs):
 
         # Compute the absolute path of the files to be filtered and
         # remove links from the list.
-        abs_files = llnl.util.filesystem.find(root, files)
+        abs_files = llnl.util.filesystem.find(root, files, **find_kwargs)
         abs_files = [x for x in abs_files if not os.path.islink(x)]
 
         x = llnl.util.filesystem.FileFilter(*abs_files)
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index 61f127a66c..341e266493 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -689,7 +689,7 @@ def _headers_default_handler(descriptor, spec, cls):
     Raises:
         RuntimeError: If no headers are found
     """
-    headers = find_headers('*', root=spec.prefix.include, recurse=True)
+    headers = find_headers('*', root=spec.prefix.include, recursive=True)
 
     if headers:
         return headers
@@ -731,22 +731,22 @@ def _libs_default_handler(descriptor, spec, cls):
 
     if '+shared' in spec:
         libs = find_libraries(
-            name, root=spec.prefix, shared=True, recurse=True
+            name, root=spec.prefix, shared=True, recursive=True
         )
     elif '~shared' in spec:
         libs = find_libraries(
-            name, root=spec.prefix, shared=False, recurse=True
+            name, root=spec.prefix, shared=False, recursive=True
         )
     else:
         # Prefer shared
         libs = find_libraries(
-            name, root=spec.prefix, shared=True, recurse=True
+            name, root=spec.prefix, shared=True, recursive=True
         )
         if libs:
             return libs
 
         libs = find_libraries(
-            name, root=spec.prefix, shared=False, recurse=True
+            name, root=spec.prefix, shared=False, recursive=True
         )
 
     if libs:
diff --git a/lib/spack/spack/test/data/directory_search/a/foobar.txt b/lib/spack/spack/test/data/directory_search/a/foobar.txt
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/lib/spack/spack/test/data/directory_search/a/foobar.txt
diff --git a/lib/spack/spack/test/data/directory_search/b/bar.txp b/lib/spack/spack/test/data/directory_search/b/bar.txp
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/lib/spack/spack/test/data/directory_search/b/bar.txp
diff --git a/lib/spack/spack/test/data/directory_search/c/bar.txt b/lib/spack/spack/test/data/directory_search/c/bar.txt
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/lib/spack/spack/test/data/directory_search/c/bar.txt
diff --git a/lib/spack/spack/test/llnl/util/file_list.py b/lib/spack/spack/test/llnl/util/file_list.py
index f7d5dde4d3..d92f15c5e1 100644
--- a/lib/spack/spack/test/llnl/util/file_list.py
+++ b/lib/spack/spack/test/llnl/util/file_list.py
@@ -30,7 +30,7 @@ import pytest
 import six
 import spack
 from llnl.util.filesystem import LibraryList, HeaderList
-from llnl.util.filesystem import find_libraries, find_headers
+from llnl.util.filesystem import find_libraries, find_headers, find
 
 
 @pytest.fixture()
@@ -215,36 +215,40 @@ search_dir = os.path.join(spack.test_path, 'data', 'directory_search')
 
 
 @pytest.mark.parametrize('search_fn,search_list,root,kwargs', [
-    (find_libraries, 'liba', search_dir, {'recurse': True}),
-    (find_libraries, ['liba'], search_dir, {'recurse': True}),
-    (find_libraries, 'libb', search_dir, {'recurse': True}),
-    (find_libraries, ['libc'], search_dir, {'recurse': True}),
-    (find_libraries, ['libc', 'liba'], search_dir, {'recurse': True}),
-    (find_libraries, ['liba', 'libc'], search_dir, {'recurse': True}),
-    (find_libraries, ['libc', 'libb', 'liba'], search_dir, {'recurse': True}),
-    (find_libraries, ['liba', 'libc'], search_dir, {'recurse': True}),
+    (find_libraries, 'liba', search_dir, {'recursive': True}),
+    (find_libraries, ['liba'], search_dir, {'recursive': True}),
+    (find_libraries, 'libb', search_dir, {'recursive': True}),
+    (find_libraries, ['libc'], search_dir, {'recursive': True}),
+    (find_libraries, ['libc', 'liba'], search_dir, {'recursive': True}),
+    (find_libraries, ['liba', 'libc'], search_dir, {'recursive': True}),
     (find_libraries,
      ['libc', 'libb', 'liba'],
      search_dir,
-     {'recurse': True, 'shared': False}
+     {'recursive': True}
      ),
-    (find_headers, 'a', search_dir, {'recurse': True}),
-    (find_headers, ['a'], search_dir, {'recurse': True}),
-    (find_headers, 'b', search_dir, {'recurse': True}),
-    (find_headers, ['c'], search_dir, {'recurse': True}),
-    (find_headers, ['c', 'a'], search_dir, {'recurse': True}),
-    (find_headers, ['a', 'c'], search_dir, {'recurse': True}),
-    (find_headers, ['c', 'b', 'a'], search_dir, {'recurse': True}),
-    (find_headers, ['a', 'c'], search_dir, {'recurse': True}),
+    (find_libraries, ['liba', 'libc'], search_dir, {'recursive': True}),
+    (find_libraries,
+     ['libc', 'libb', 'liba'],
+     search_dir,
+     {'recursive': True, 'shared': False}
+     ),
+    (find_headers, 'a', search_dir, {'recursive': True}),
+    (find_headers, ['a'], search_dir, {'recursive': True}),
+    (find_headers, 'b', search_dir, {'recursive': True}),
+    (find_headers, ['c'], search_dir, {'recursive': True}),
+    (find_headers, ['c', 'a'], search_dir, {'recursive': True}),
+    (find_headers, ['a', 'c'], search_dir, {'recursive': True}),
+    (find_headers, ['c', 'b', 'a'], search_dir, {'recursive': True}),
+    (find_headers, ['a', 'c'], search_dir, {'recursive': True}),
     (find_libraries,
      ['liba', 'libd'],
      os.path.join(search_dir, 'b'),
-     {'recurse': False}
+     {'recursive': False}
      ),
     (find_headers,
      ['b', 'd'],
      os.path.join(search_dir, 'b'),
-     {'recurse': False}
+     {'recursive': False}
      ),
 ])
 def test_searching_order(search_fn, search_list, root, kwargs):
@@ -275,3 +279,20 @@ def test_searching_order(search_fn, search_list, root, kwargs):
 
     # List should be empty here
     assert len(L) == 0
+
+
+@pytest.mark.parametrize('root,search_list,kwargs,expected', [
+    (search_dir, '*/*bar.tx?', {'recursive': False}, [
+        os.path.join(search_dir, 'a/foobar.txt'),
+        os.path.join(search_dir, 'b/bar.txp'),
+        os.path.join(search_dir, 'c/bar.txt'),
+    ]),
+    (search_dir, '*/*bar.tx?', {'recursive': True}, [
+        os.path.join(search_dir, 'a/foobar.txt'),
+        os.path.join(search_dir, 'b/bar.txp'),
+        os.path.join(search_dir, 'c/bar.txt'),
+    ])
+])
+def test_find_with_globbing(root, search_list, kwargs, expected):
+    matches = find(root, search_list, **kwargs)
+    assert matches == expected