Merge pull request #232 from LLNL/bugfix/package-cache-217

Bugfix/package cache 217

3 files changed, 26 insertions, 16 deletions

diff --git a/lib/spack/spack/packages.py b/lib/spack/spack/packages.py
index f6f4cbf025..080644fb90 100644
--- a/lib/spack/spack/packages.py
+++ b/lib/spack/spack/packages.py
@@ -67,27 +67,28 @@ class PackageDB(object):
         if spec.virtual:
             raise UnknownPackageError(spec.name)
 
+        key = hash(spec)
         if kwargs.get('new', False):
-            if spec in self.instances:
-                del self.instances[spec]
+            if key in self.instances:
+                del self.instances[key]
 
-        if not spec in self.instances:
+        if not key in self.instances:
             package_class = self.get_class_for_package_name(spec.name)
             try:
-                copy = spec.copy()
-                self.instances[copy] = package_class(copy)
+                copy = spec.copy() # defensive copy.  Package owns its spec.
+                self.instances[key] = package_class(copy)
             except Exception, e:
                 if spack.debug:
                     sys.excepthook(*sys.exc_info())
                 raise FailedConstructorError(spec.name, e)
 
-        return self.instances[spec]
+        return self.instances[key]
 
 
     @_autospec
     def delete(self, spec):
         """Force a package to be recreated."""
-        del self.instances[spec]
+        del self.instances[spec.dag_hash()]
 
 
     def purge(self):
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index f62182ce76..fb5ea5d02b 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -1481,8 +1481,11 @@ class Spec(object):
 
     def _cmp_node(self):
         """Comparison key for just *this node* and not its deps."""
-        return (self.name, self.versions, self.variants,
-                self.architecture, self.compiler)
+        return (self.name,
+                self.versions,
+                self.variants,
+                self.architecture,
+                self.compiler)
 
 
     def eq_node(self, other):
@@ -1496,11 +1499,15 @@ class Spec(object):
 
 
     def _cmp_key(self):
-        """Comparison key for this node and all dependencies *without*
-           considering structure.  This is the default, as
-           normalization will restore structure.
+        """This returns a key for the spec *including* DAG structure.
+
+        The key is the concatenation of:
+          1. A tuple describing this node in the DAG.
+          2. The hash of each of this node's dependencies' cmp_keys.
         """
-        return self._cmp_node() + (self.sorted_deps(),)
+        return self._cmp_node() + (
+            tuple(hash(self.dependencies[name])
+                  for name in sorted(self.dependencies)),)
 
 
     def colorized(self):
diff --git a/lib/spack/spack/test/spec_dag.py b/lib/spack/spack/test/spec_dag.py
index 94438b6a0c..d3a4d77b32 100644
--- a/lib/spack/spack/test/spec_dag.py
+++ b/lib/spack/spack/test/spec_dag.py
@@ -340,16 +340,18 @@ class SpecDagTest(MockPackagesTest):
         self.assertEqual(spec, expected_flat)
         self.assertTrue(spec.eq_dag(expected_flat))
 
-        self.assertEqual(spec, expected_normalized)
+        # Normalized has different DAG structure, so NOT equal.
+        self.assertNotEqual(spec, expected_normalized)
         self.assertFalse(spec.eq_dag(expected_normalized))
 
-        self.assertEqual(spec, non_unique_nodes)
+        # Again, different DAG structure so not equal.
+        self.assertNotEqual(spec, non_unique_nodes)
         self.assertFalse(spec.eq_dag(non_unique_nodes))
 
         spec.normalize()
 
         # After normalizing, spec_dag_equal should match the normalized spec.
-        self.assertEqual(spec, expected_flat)
+        self.assertNotEqual(spec, expected_flat)
         self.assertFalse(spec.eq_dag(expected_flat))
 
         self.assertEqual(spec, expected_normalized)