Cleanup and comments.

8 files changed, 60 insertions, 31 deletions

diff --git a/lib/spack/spack/attr.py b/lib/spack/spack/attr.py
deleted file mode 100644
index 2da77aae54..0000000000
--- a/lib/spack/spack/attr.py
+++ /dev/null
@@ -1,14 +0,0 @@
-import spack.tty as tty
-
-def required(obj, attr_name):
-    """Ensure that a class has a required attribute."""
-    if not hasattr(obj, attr_name):
-        tty.die("No required attribute '%s' in class '%s'"
-                % (attr_name, obj.__class__.__name__))
-
-
-def setdefault(obj, name, value):
-    """Like dict.setdefault, but for objects."""
-    if not hasattr(obj, name):
-        setattr(obj, name, value)
-    return getattr(obj, name)
diff --git a/lib/spack/spack/cmd/__init__.py b/lib/spack/spack/cmd/__init__.py
index 96d02f7855..d1d5529cf0 100644
--- a/lib/spack/spack/cmd/__init__.py
+++ b/lib/spack/spack/cmd/__init__.py
@@ -5,7 +5,7 @@ import sys
 import spack
 import spack.spec
 import spack.tty as tty
-import spack.attr as attr
+from spack.util.lang import attr_setdefault
 
 # Patterns to ignore in the commands directory when looking for commands.
 ignore_files = r'^\.|^__init__.py$|^#'
@@ -34,8 +34,8 @@ def get_module(name):
         module_name, fromlist=[name, SETUP_PARSER, DESCRIPTION],
         level=0)
 
-    attr.setdefault(module, SETUP_PARSER, lambda *args: None) # null-op
-    attr.setdefault(module, DESCRIPTION, "")
+    attr_setdefault(module, SETUP_PARSER, lambda *args: None) # null-op
+    attr_setdefault(module, DESCRIPTION, "")
 
     fn_name = get_cmd_function_name(name)
     if not hasattr(module, fn_name):
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index d4e31d7326..e15a1f97eb 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -21,14 +21,13 @@ import spack.spec
 import spack.error
 import packages
 import tty
-import attr
 import validate
 import url
 
-from version import *
-from multi_function import platform
-from stage import Stage
-from spack.util.lang import memoized, list_modules
+from spack.multi_function import platform
+from spack.version import *
+from spack.stage import Stage
+from spack.util.lang import *
 from spack.util.crypto import md5
 from spack.util.web import get_pages
 
@@ -254,7 +253,7 @@ class Package(object):
     dependencies = {}
 
     """List of specs of virtual packages provided by this package."""
-    provided_virtual_packages = {}
+    provided = {}
 
     #
     # These are default values for instance variables.
@@ -270,9 +269,9 @@ class Package(object):
 
     def __init__(self, spec):
         # These attributes are required for all packages.
-        attr.required(self, 'homepage')
-        attr.required(self, 'url')
-        attr.required(self, 'md5')
+        attr_required(self, 'homepage')
+        attr_required(self, 'url')
+        attr_required(self, 'md5')
 
         # this determines how the package should be built.
         self.spec = spec
@@ -304,7 +303,7 @@ class Package(object):
         self._available_versions = None
 
         # This list overrides available_versions if set by the user.
-        attr.setdefault(self, 'versions', None)
+        attr_setdefault(self, 'versions', None)
         if self.versions and type(self.versions) != VersionList:
             self.versions = VersionList(self.versions)
 
diff --git a/lib/spack/spack/packages/__init__.py b/lib/spack/spack/packages/__init__.py
index 40270afc99..ecdd380e0c 100644
--- a/lib/spack/spack/packages/__init__.py
+++ b/lib/spack/spack/packages/__init__.py
@@ -42,7 +42,7 @@ def get_providers(vpkg_name):
 
 def compute_providers():
     for pkg in all_packages():
-        for vpkg in pkg.provided_virtual_packages:
+        for vpkg in pkg.provided:
             if vpkg not in providers:
                 providers[vpkg] = []
             providers[vpkg].append(pkg)
diff --git a/lib/spack/spack/relations.py b/lib/spack/spack/relations.py
index 101cdd8c83..f936f6e364 100644
--- a/lib/spack/spack/relations.py
+++ b/lib/spack/spack/relations.py
@@ -52,7 +52,8 @@ import spack.spec
 def _caller_locals():
     """This will return the locals of the *parent* of the caller.
        This allows a fucntion to insert variables into its caller's
-       scope.
+       scope.  Yes, this is some black magic, and yes it's useful
+       for implementing things like depends_on and provides.
     """
     stack = inspect.stack()
     try:
@@ -78,6 +79,6 @@ def provides(*args):
        can use the providing package to satisfy the dependency.
     """
     # Get the enclosing package's scope and add deps to it.
-    provides = _caller_locals().setdefault("provides", [])
+    provided = _caller_locals().setdefault("provided", [])
     for name in args:
         provides.append(name)
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index 6d8742c73a..73542b9179 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -336,6 +336,10 @@ class Spec(object):
 
     @property
     def concrete(self):
+        """A spec is concrete if it can describe only ONE build of a package.
+           If any of the name, version, architecture, compiler, or depdenencies
+           are ambiguous,then it is not concrete.
+        """
         return bool(not self.virtual
                     and self.versions.concrete
                     and self.architecture
@@ -344,6 +348,27 @@ class Spec(object):
 
 
     def preorder_traversal(self, visited=None, d=0, **kwargs):
+        """Generic preorder traversal of the DAG represented by this spec.
+           This will yield each node in the spec.  Options:
+
+           unique   [=True]
+               When True (default) every node in the DAG is yielded only once.
+               When False, the traversal will yield already visited nodes but
+               not their children.  This lets you see that a node ponts to
+               an already-visited subgraph without descending into it.
+
+           depth    [=False]
+               Defaults to False.  When True, yields not just nodes in the
+               spec, but also their depth from the root in a (depth, node)
+               tuple.
+
+           keyfun   [=id]
+               Allow a custom key function to track the identity of nodes
+               in the traversal.
+
+           noroot   [=False]
+               If true, this won't yield the root node, just its descendents.
+        """
         unique = kwargs.setdefault('unique', True)
         depth  = kwargs.setdefault('depth', False)
         keyfun = kwargs.setdefault('key', id)
@@ -368,6 +393,7 @@ class Spec(object):
 
 
     def _concretize_helper(self, presets):
+        """Recursive helper function for concretize()."""
         for name in sorted(self.dependencies.keys()):
             self.dependencies[name]._concretize_helper(presets)
 
@@ -416,6 +442,9 @@ class Spec(object):
 
 
     def concretized(self, *presets):
+        """This is a non-destructive version of concretize().  First clones,
+           then returns a concrete version of this package without modifying
+           this package. """
         clone = self.copy()
         clone.concretize(*presets)
         return clone
diff --git a/lib/spack/spack/test/mock_packages/mpich.py b/lib/spack/spack/test/mock_packages/mpich.py
index 337e7f6629..85c4c2d939 100644
--- a/lib/spack/spack/test/mock_packages/mpich.py
+++ b/lib/spack/spack/test/mock_packages/mpich.py
@@ -3,10 +3,10 @@ from spack import *
 class Mpich(Package):
     homepage = "http://www.mpich.org"
     url      = "http://www.mpich.org/static/downloads/3.0.4/mpich-3.0.4.tar.gz"
+    md5      = "9c5d5d4fe1e17dd12153f40bc5b6dbc0"
 
     list_url   = "http://www.mpich.org/static/downloads/"
     list_depth = 2
-    md5      = "9c5d5d4fe1e17dd12153f40bc5b6dbc0"
 
     versions = '1.0.3, 1.3.2p1, 1.4.1p1, 3.0.4, 3.1b1'
 
diff --git a/lib/spack/spack/util/lang.py b/lib/spack/spack/util/lang.py
index d983141045..a916dfee57 100644
--- a/lib/spack/spack/util/lang.py
+++ b/lib/spack/spack/util/lang.py
@@ -9,6 +9,20 @@ from spack.util.filesystem import new_path
 ignore_modules = [r'^\.#', '~$']
 
 
+def attr_required(obj, attr_name):
+    """Ensure that a class has a required attribute."""
+    if not hasattr(obj, attr_name):
+        tty.die("No required attribute '%s' in class '%s'"
+                % (attr_name, obj.__class__.__name__))
+
+
+def attr_setdefault(obj, name, value):
+    """Like dict.setdefault, but for objects."""
+    if not hasattr(obj, name):
+        setattr(obj, name, value)
+    return getattr(obj, name)
+
+
 def has_method(cls, name):
     for base in inspect.getmro(cls):
         if base is object: