YamlDirectoryLayout now working.

6 files changed, 114 insertions, 134 deletions

diff --git a/lib/spack/spack/__init__.py b/lib/spack/spack/__init__.py
index eb891e3d57..80fc0390f9 100644
--- a/lib/spack/spack/__init__.py
+++ b/lib/spack/spack/__init__.py
@@ -42,7 +42,8 @@ test_path      = join_path(module_path, "test")
 hooks_path     = join_path(module_path, "hooks")
 var_path       = join_path(prefix, "var", "spack")
 stage_path     = join_path(var_path, "stage")
-install_path   = join_path(prefix, "opt")
+opt_path       = join_path(prefix, "opt")
+install_path   = join_path(opt_path, "spack")
 share_path     = join_path(prefix, "share", "spack")
 
 #
@@ -65,8 +66,8 @@ mock_user_config = join_path(mock_config_path, "user_spackconfig")
 # This controls how spack lays out install prefixes and
 # stage directories.
 #
-from spack.directory_layout import SpecHashDirectoryLayout
-install_layout = SpecHashDirectoryLayout(install_path)
+from spack.directory_layout import YamlDirectoryLayout
+install_layout = YamlDirectoryLayout(install_path)
 
 #
 # This controls how things are concretized in spack.
diff --git a/lib/spack/spack/directory_layout.py b/lib/spack/spack/directory_layout.py
index b2cf5dc801..67708c47b5 100644
--- a/lib/spack/spack/directory_layout.py
+++ b/lib/spack/spack/directory_layout.py
@@ -27,8 +27,9 @@ import os
 import exceptions
 import hashlib
 import shutil
+import glob
 import tempfile
-from contextlib import closing
+from external import yaml
 
 import llnl.util.tty as tty
 from llnl.util.lang import memoized
@@ -81,7 +82,7 @@ class DirectoryLayout(object):
         raise NotImplementedError()
 
 
-    def make_path_for_spec(self, spec):
+    def create_install_directory(self, spec):
         """Creates the installation directory for a spec."""
         raise NotImplementedError()
 
@@ -131,7 +132,7 @@ class DirectoryLayout(object):
         return os.path.join(self.root, path)
 
 
-    def remove_path_for_spec(self, spec):
+    def remove_install_directory(self, spec):
         """Removes a prefix and any empty parent directories from the root.
            Raised RemoveFailedError if something goes wrong.
         """
@@ -153,94 +154,70 @@ class DirectoryLayout(object):
             path = os.path.dirname(path)
 
 
-def traverse_dirs_at_depth(root, depth, path_tuple=(), curdepth=0):
-    """For each directory at <depth> within <root>, return a tuple representing
-       the ancestors of that directory.
-    """
-    if curdepth == depth and curdepth != 0:
-        yield path_tuple
-    elif depth > curdepth:
-        for filename in os.listdir(root):
-            child = os.path.join(root, filename)
-            if os.path.isdir(child):
-                child_tuple = path_tuple + (filename,)
-                for tup in traverse_dirs_at_depth(
-                        child, depth, child_tuple, curdepth+1):
-                    yield tup
-
-
-class SpecHashDirectoryLayout(DirectoryLayout):
+class YamlDirectoryLayout(DirectoryLayout):
     """Lays out installation directories like this::
-           <install_root>/
+           <install root>/
                <architecture>/
-                   <compiler>/
-                       name@version+variant-<dependency_hash>
-
-       Where dependency_hash is a SHA-1 hash prefix for the full package spec.
-       This accounts for dependencies.
+                   <compiler>-<compiler version>/
+                       <name>-<version>-<variants>-<hash>
 
-       If there is ever a hash collision, you won't be able to install a new
-       package unless you use a larger prefix.  However, the full spec is stored
-       in a file called .spec in each directory, so you can migrate an entire
-       install directory to a new hash size pretty easily.
+       The hash here is a SHA-1 hash for the full DAG plus the build
+       spec.  TODO: implement the build spec.
 
-       TODO: make a tool to migrate install directories to different hash sizes.
+       To avoid special characters (like ~) in the directory name,
+       only enabled variants are included in the install path.
+       Disabled variants are omitted.
     """
     def __init__(self, root, **kwargs):
-        """Prefix size is number of characters in the SHA-1 prefix to use
-           to make each hash unique.
-        """
-        spec_file_name = kwargs.get('spec_file_name', '.spec')
-        extension_file_name = kwargs.get('extension_file_name', '.extensions')
-        super(SpecHashDirectoryLayout, self).__init__(root)
-        self.spec_file_name = spec_file_name
-        self.extension_file_name = extension_file_name
+        super(YamlDirectoryLayout, self).__init__(root)
+        self.metadata_dir   = kwargs.get('metadata_dir', '.spack')
+        self.hash_len       = kwargs.get('hash_len', None)
+
+        self.spec_file_name = 'spec'
+        self.extension_file_name = 'extensions'
 
         # Cache of already written/read extension maps.
         self._extension_maps = {}
 
     @property
     def hidden_file_paths(self):
-        return ('.spec', '.extensions')
+        return (self.metadata_dir)
 
 
     def relative_path_for_spec(self, spec):
         _check_concrete(spec)
-        dir_name = spec.format('$_$@$+$#')
-        return join_path(spec.architecture, spec.compiler, dir_name)
+        enabled_variants = (
+            '-' + v.name for v in spec.variants.values()
+            if v.enabled)
+
+        dir_name = "%s-%s%s-%s" % (
+            spec.name,
+            spec.version,
+            ''.join(enabled_variants),
+            spec.dag_hash(self.hash_len))
+
+        path = join_path(
+            spec.architecture,
+            "%s-%s" % (spec.compiler.name, spec.compiler.version),
+            dir_name)
+
+        return path
 
 
     def write_spec(self, spec, path):
         """Write a spec out to a file."""
-        with closing(open(path, 'w')) as spec_file:
-            spec_file.write(spec.tree(ids=False, cover='nodes'))
+        _check_concrete(spec)
+        with open(path, 'w') as f:
+            f.write(spec.to_yaml())
 
 
     def read_spec(self, path):
         """Read the contents of a file and parse them as a spec"""
-        with closing(open(path)) as spec_file:
-            # Specs from files are assumed normal and concrete
-            spec = Spec(spec_file.read().replace('\n', ''))
-
-        if all(spack.db.exists(s.name) for s in spec.traverse()):
-            copy = spec.copy()
-
-            # TODO: It takes a lot of time to normalize every spec on read.
-            # TODO: Storing graph info with spec files would fix this.
-            copy.normalize()
-            if copy.concrete:
-                return copy   # These are specs spack still understands.
-
-        # If we get here, either the spec is no longer in spack, or
-        # something about its dependencies has changed. So we need to
-        # just assume the read spec is correct.  We'll lose graph
-        # information if we do this, but this is just for best effort
-        # for commands like uninstall and find.  Currently Spack
-        # doesn't do anything that needs the graph info after install.
-
-        # TODO: store specs with full connectivity information, so
-        # that we don't have to normalize or reconstruct based on
-        # changing dependencies in the Spack tree.
+        with open(path) as f:
+            yaml_text = f.read()
+        spec = Spec.from_yaml(yaml_text)
+
+        # Specs read from actual installations are always concrete
         spec._normal = True
         spec._concrete = True
         return spec
@@ -249,10 +226,14 @@ class SpecHashDirectoryLayout(DirectoryLayout):
     def spec_file_path(self, spec):
         """Gets full path to spec file"""
         _check_concrete(spec)
-        return join_path(self.path_for_spec(spec), self.spec_file_name)
+        return join_path(self.metadata_path(spec), self.spec_file_name)
+
 
+    def metadata_path(self, spec):
+        return join_path(self.path_for_spec(spec), self.metadata_dir)
 
-    def make_path_for_spec(self, spec):
+
+    def create_install_directory(self, spec):
         _check_concrete(spec)
 
         path = self.path_for_spec(spec)
@@ -267,16 +248,13 @@ class SpecHashDirectoryLayout(DirectoryLayout):
             if installed_spec == self.spec:
                 raise InstallDirectoryAlreadyExistsError(path)
 
-            spec_hash = self.hash_spec(spec)
-            installed_hash = self.hash_spec(installed_spec)
-            if installed_spec == spec_hash:
+            if spec.dag_hash() == installed_spec.dag_hash():
                 raise SpecHashCollisionError(installed_hash, spec_hash)
             else:
                 raise InconsistentInstallDirectoryError(
-                    'Spec file in %s does not match SHA-1 hash!'
-                    % spec_file_path)
+                    'Spec file in %s does not match hash!' % spec_file_path)
 
-        mkdirp(path)
+        mkdirp(self.metadata_path(spec))
         self.write_spec(spec, spec_file_path)
 
 
@@ -284,22 +262,14 @@ class SpecHashDirectoryLayout(DirectoryLayout):
     def all_specs(self):
         if not os.path.isdir(self.root):
             return []
-
-        specs = []
-        for path in traverse_dirs_at_depth(self.root, 3):
-            arch, compiler, last_dir = path
-            spec_file_path = join_path(
-                self.root, arch, compiler, last_dir, self.spec_file_name)
-            if os.path.exists(spec_file_path):
-                spec = self.read_spec(spec_file_path)
-                specs.append(spec)
-        return specs
+        spec_files = glob.glob("%s/*/*/*/.spack/spec" % self.root)
+        return [self.read_spec(s) for s in spec_files]
 
 
     def extension_file_path(self, spec):
         """Gets full path to an installed package's extension file"""
         _check_concrete(spec)
-        return join_path(self.path_for_spec(spec), self.extension_file_name)
+        return join_path(self.metadata_path(spec), self.extension_file_name)
 
 
     def _extension_map(self, spec):
@@ -314,7 +284,7 @@ class SpecHashDirectoryLayout(DirectoryLayout):
 
             else:
                 exts = {}
-                with closing(open(path)) as ext_file:
+                with open(path) as ext_file:
                     for line in ext_file:
                         try:
                             spec = Spec(line.strip())
@@ -358,7 +328,7 @@ class SpecHashDirectoryLayout(DirectoryLayout):
             prefix=basename, dir=dirname, delete=False)
 
         # Write temp file.
-        with closing(tmp):
+        with tmp:
             for extension in sorted(extensions.values()):
                 tmp.write("%s\n" % extension)
 
@@ -392,6 +362,7 @@ class SpecHashDirectoryLayout(DirectoryLayout):
         self._write_extensions(spec, exts)
 
 
+
 class DirectoryLayoutError(SpackError):
     """Superclass for directory layout errors."""
     def __init__(self, message):
@@ -399,9 +370,9 @@ class DirectoryLayoutError(SpackError):
 
 
 class SpecHashCollisionError(DirectoryLayoutError):
-    """Raised when there is a hash collision in an SpecHashDirectoryLayout."""
+    """Raised when there is a hash collision in an install layout."""
     def __init__(self, installed_spec, new_spec):
-        super(SpecHashDirectoryLayout, self).__init__(
+        super(SpecHashCollisionError, self).__init__(
             'Specs %s and %s have the same SHA-1 prefix!'
             % installed_spec, new_spec)
 
@@ -422,7 +393,7 @@ class InconsistentInstallDirectoryError(DirectoryLayoutError):
 
 
 class InstallDirectoryAlreadyExistsError(DirectoryLayoutError):
-    """Raised when make_path_for_sec is called unnecessarily."""
+    """Raised when create_install_directory is called unnecessarily."""
     def __init__(self, path):
         super(InstallDirectoryAlreadyExistsError, self).__init__(
             "Install path %s already exists!")
@@ -455,5 +426,3 @@ class NoSuchExtensionError(DirectoryLayoutError):
         super(NoSuchExtensionError, self).__init__(
             "%s cannot be removed from %s because it's not activated."% (
                 ext_spec.short_spec, spec.short_spec))
-
-
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 908fd86a87..bbb64cd05b 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -658,7 +658,7 @@ class Package(object):
 
     def remove_prefix(self):
         """Removes the prefix for a package along with any empty parent directories."""
-        spack.install_layout.remove_path_for_spec(self.spec)
+        spack.install_layout.remove_install_directory(self.spec)
 
 
     def do_fetch(self):
@@ -810,7 +810,7 @@ class Package(object):
         # create the install directory.  The install layout
         # handles this in case so that it can use whatever
         # package naming scheme it likes.
-        spack.install_layout.make_path_for_spec(self.spec)
+        spack.install_layout.create_install_directory(self.spec)
 
         def cleanup():
             if not keep_prefix:
@@ -831,11 +831,11 @@ class Package(object):
                 spack.hooks.pre_install(self)
 
                 # Set up process's build environment before running install.
-                self.stage.chdir_to_source()
                 if fake_install:
                     self.do_fake_install()
                 else:
                     # Subclasses implement install() to do the real work.
+                    self.stage.chdir_to_source()
                     self.install(self.spec, self.prefix)
 
                 # Ensure that something was actually installed.
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index dd9ec5dbe3..e36a1d6f0b 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -93,6 +93,7 @@ expansion when it is the first character in an id typed on the command line.
 import sys
 import itertools
 import hashlib
+import base64
 from StringIO import StringIO
 from operator import attrgetter
 from external import yaml
@@ -578,27 +579,12 @@ class Spec(object):
         return Prefix(spack.install_layout.path_for_spec(self))
 
 
-    def dep_hash(self, length=None):
-        """Return a hash representing all dependencies of this spec
-           (direct and indirect).
-
-           If you want this hash to be consistent, you should
-           concretize the spec first so that it is not ambiguous.
-        """
-        sha = hashlib.sha1()
-        sha.update(self.dep_string())
-        full_hash = sha.hexdigest()
-
-        return full_hash[:length]
-
-
     def dag_hash(self, length=None):
         """Return a hash of the entire spec DAG, including connectivity."""
-        sha = hashlib.sha1()
-        hash_text = yaml.dump(
+        yaml_text = yaml.dump(
             self.to_node_dict(), default_flow_style=True, width=sys.maxint)
-        sha.update(hash_text)
-        return sha.hexdigest()[:length]
+        sha = hashlib.sha1(yaml_text)
+        return base64.b32encode(sha.digest()).lower()[:length]
 
 
     def to_node_dict(self):
@@ -1363,7 +1349,7 @@ class Spec(object):
                         write(fmt % (c + str(self.architecture)), c)
                 elif c == '#':
                     if self.dependencies:
-                        out.write(fmt % ('-' + self.dep_hash(8)))
+                        out.write(fmt % ('-' + self.dag_hash(8)))
                 elif c == '$':
                     if fmt != '':
                         raise ValueError("Can't use format width with $$.")
diff --git a/lib/spack/spack/test/directory_layout.py b/lib/spack/spack/test/directory_layout.py
index 3e52954cfe..34374628be 100644
--- a/lib/spack/spack/test/directory_layout.py
+++ b/lib/spack/spack/test/directory_layout.py
@@ -36,7 +36,11 @@ from llnl.util.filesystem import *
 import spack
 from spack.spec import Spec
 from spack.packages import PackageDB
-from spack.directory_layout import SpecHashDirectoryLayout
+from spack.directory_layout import YamlDirectoryLayout
+
+# number of packages to test (to reduce test time)
+max_packages = 10
+
 
 class DirectoryLayoutTest(unittest.TestCase):
     """Tests that a directory layout works correctly and produces a
@@ -44,11 +48,11 @@ class DirectoryLayoutTest(unittest.TestCase):
 
     def setUp(self):
         self.tmpdir = tempfile.mkdtemp()
-        self.layout = SpecHashDirectoryLayout(self.tmpdir)
+        self.layout = YamlDirectoryLayout(self.tmpdir)
 
 
     def tearDown(self):
-        shutil.rmtree(self.tmpdir, ignore_errors=True)
+        #shutil.rmtree(self.tmpdir, ignore_errors=True)
         self.layout = None
 
 
@@ -59,7 +63,9 @@ class DirectoryLayoutTest(unittest.TestCase):
            finally that the directory can be removed by the directory
            layout.
         """
-        for pkg in spack.db.all_packages():
+        packages = list(spack.db.all_packages())[:max_packages]
+
+        for pkg in packages:
             spec = pkg.spec
 
             # If a spec fails to concretize, just skip it.  If it is a
@@ -69,7 +75,7 @@ class DirectoryLayoutTest(unittest.TestCase):
             except:
                 continue
 
-            self.layout.make_path_for_spec(spec)
+            self.layout.create_install_directory(spec)
 
             install_dir = self.layout.path_for_spec(spec)
             spec_path = self.layout.spec_file_path(spec)
@@ -90,7 +96,7 @@ class DirectoryLayoutTest(unittest.TestCase):
 
             # Ensure that specs that come out "normal" are really normal.
             with closing(open(spec_path)) as spec_file:
-                read_separately = Spec(spec_file.read())
+                read_separately = Spec.from_yaml(spec_file.read())
 
                 read_separately.normalize()
                 self.assertEqual(read_separately, spec_from_file)
@@ -98,11 +104,11 @@ class DirectoryLayoutTest(unittest.TestCase):
                 read_separately.concretize()
                 self.assertEqual(read_separately, spec_from_file)
 
-            # Make sure the dep hash of the read-in spec is the same
-            self.assertEqual(spec.dep_hash(), spec_from_file.dep_hash())
+            # Make sure the hash of the read-in spec is the same
+            self.assertEqual(spec.dag_hash(), spec_from_file.dag_hash())
 
             # Ensure directories are properly removed
-            self.layout.remove_path_for_spec(spec)
+            self.layout.remove_install_directory(spec)
             self.assertFalse(os.path.isdir(install_dir))
             self.assertFalse(os.path.exists(install_dir))
 
@@ -120,12 +126,14 @@ class DirectoryLayoutTest(unittest.TestCase):
         """
         mock_db = PackageDB(spack.mock_packages_path)
 
-        not_in_mock = set(spack.db.all_package_names()).difference(
+        not_in_mock = set.difference(
+            set(spack.db.all_package_names()),
             set(mock_db.all_package_names()))
+        packages = list(not_in_mock)[:max_packages]
 
         # Create all the packages that are not in mock.
         installed_specs = {}
-        for pkg_name in not_in_mock:
+        for pkg_name in packages:
             spec = spack.db.get(pkg_name).spec
 
             # If a spec fails to concretize, just skip it.  If it is a
@@ -135,7 +143,7 @@ class DirectoryLayoutTest(unittest.TestCase):
             except:
                 continue
 
-            self.layout.make_path_for_spec(spec)
+            self.layout.create_install_directory(spec)
             installed_specs[spec] = self.layout.path_for_spec(spec)
 
         tmp = spack.db
@@ -144,12 +152,28 @@ class DirectoryLayoutTest(unittest.TestCase):
         # Now check that even without the package files, we know
         # enough to read a spec from the spec file.
         for spec, path in installed_specs.items():
-            spec_from_file = self.layout.read_spec(join_path(path, '.spec'))
+            spec_from_file = self.layout.read_spec(join_path(path, '.spack', 'spec'))
 
             # To satisfy these conditions, directory layouts need to
             # read in concrete specs from their install dirs somehow.
             self.assertEqual(path, self.layout.path_for_spec(spec_from_file))
             self.assertEqual(spec, spec_from_file)
-            self.assertEqual(spec.dep_hash(), spec_from_file.dep_hash())
+            self.assertTrue(spec.eq_dag(spec_from_file))
+            self.assertEqual(spec.dag_hash(), spec_from_file.dag_hash())
 
         spack.db = tmp
+
+
+    def test_find(self):
+        """Test that finding specs within an install layout works."""
+        packages = list(spack.db.all_packages())[:max_packages]
+        installed_specs = {}
+        for pkg in packages:
+            spec = pkg.spec.concretized()
+            installed_specs[spec.name] = spec
+            self.layout.create_install_directory(spec)
+
+        found_specs = dict((s.name, s) for s in self.layout.all_specs())
+        for name, spec in found_specs.items():
+            self.assertTrue(name in found_specs)
+            self.assertTrue(found_specs[name].eq_dag(spec))
diff --git a/lib/spack/spack/test/install.py b/lib/spack/spack/test/install.py
index e052f53e77..d240a393a6 100644
--- a/lib/spack/spack/test/install.py
+++ b/lib/spack/spack/test/install.py
@@ -33,7 +33,7 @@ from llnl.util.filesystem import *
 import spack
 from spack.stage import Stage
 from spack.fetch_strategy import URLFetchStrategy
-from spack.directory_layout import SpecHashDirectoryLayout
+from spack.directory_layout import YamlDirectoryLayout
 from spack.util.executable import which
 from spack.test.mock_packages_test import *
 from spack.test.mock_repo import MockArchive
@@ -55,7 +55,7 @@ class InstallTest(MockPackagesTest):
         # installed pkgs and mock packages.
         self.tmpdir = tempfile.mkdtemp()
         self.orig_layout = spack.install_layout
-        spack.install_layout = SpecHashDirectoryLayout(self.tmpdir)
+        spack.install_layout = YamlDirectoryLayout(self.tmpdir)
 
 
     def tearDown(self):