Merge pull request #477 from alalazo/refactoring/stage

refactoring proposal : turn Stage into a context manager

36 files changed, 434 insertions, 515 deletions

diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py
index da3cf96050..366237ef8f 100644
--- a/lib/spack/llnl/util/filesystem.py
+++ b/lib/spack/llnl/util/filesystem.py
@@ -25,7 +25,7 @@
 __all__ = ['set_install_permissions', 'install', 'install_tree', 'traverse_tree',
            'expand_user', 'working_dir', 'touch', 'touchp', 'mkdirp',
            'force_remove', 'join_path', 'ancestor', 'can_access', 'filter_file',
-           'FileFilter', 'change_sed_delimiter', 'is_exe', 'force_symlink']
+           'FileFilter', 'change_sed_delimiter', 'is_exe', 'force_symlink', 'remove_dead_links', 'remove_linked_tree']
 
 import os
 import sys
@@ -240,7 +240,7 @@ def touchp(path):
 def force_symlink(src, dest):
     try:
         os.symlink(src, dest)
-    except OSError, e:
+    except OSError as e:
         os.remove(dest)
         os.symlink(src, dest)
 
@@ -344,3 +344,34 @@ def traverse_tree(source_root, dest_root, rel_path='', **kwargs):
 
     if order == 'post':
         yield (source_path, dest_path)
+
+def remove_dead_links(root):
+    """
+    Removes any dead link that is present in root
+
+    Args:
+        root: path where to search for dead links
+
+    """
+    for file in os.listdir(root):
+        path = join_path(root, file)
+        if os.path.islink(path):
+            real_path = os.path.realpath(path)
+            if not os.path.exists(real_path):
+                os.unlink(path)
+
+def remove_linked_tree(path):
+    """
+    Removes a directory and its contents.  If the directory is a symlink, follows the link and removes the real
+    directory before removing the link.
+
+    Args:
+        path: directory to be removed
+
+    """
+    if os.path.exists(path):
+        if os.path.islink(path):
+            shutil.rmtree(os.path.realpath(path), True)
+            os.unlink(path)
+        else:
+            shutil.rmtree(path, True)
diff --git a/lib/spack/spack/cmd/clean.py b/lib/spack/spack/cmd/clean.py
index 6e7179122c..0c8bd1d528 100644
--- a/lib/spack/spack/cmd/clean.py
+++ b/lib/spack/spack/cmd/clean.py
@@ -43,4 +43,4 @@ def clean(parser, args):
     specs = spack.cmd.parse_specs(args.packages, concretize=True)
     for spec in specs:
         package = spack.repo.get(spec)
-        package.do_clean()
+        package.stage.destroy()
diff --git a/lib/spack/spack/mirror.py b/lib/spack/spack/mirror.py
index fa29e20803..5ed7aff176 100644
--- a/lib/spack/spack/mirror.py
+++ b/lib/spack/spack/mirror.py
@@ -110,7 +110,6 @@ def suggest_archive_basename(resource):
     return basename
 
 
-
 def create(path, specs, **kwargs):
     """Create a directory to be used as a spack mirror, and fill it with
        package archives.
@@ -158,17 +157,29 @@ def create(path, specs, **kwargs):
                 "Cannot create directory '%s':" % mirror_root, str(e))
 
     # Things to keep track of while parsing specs.
-    present  = []
-    mirrored = []
-    error    = []
+    categories = {
+        'present': [],
+        'mirrored': [],
+        'error': []
+    }
 
     # Iterate through packages and download all the safe tarballs for each of them
-    everything_already_exists = True
     for spec in version_specs:
-        pkg = spec.package
-        tty.msg("Adding package {pkg} to mirror".format(pkg=spec.format("$_$@")))
-        try:
-            for ii, stage in enumerate(pkg.stage):
+        add_single_spec(spec, mirror_root, categories, **kwargs)
+
+    return categories['present'], categories['mirrored'], categories['error']
+
+
+def add_single_spec(spec, mirror_root, categories, **kwargs):
+    tty.msg("Adding package {pkg} to mirror".format(pkg=spec.format("$_$@")))
+    spec_exists_in_mirror = True
+    try:
+        with spec.package.stage:
+            # fetcher = stage.fetcher
+            # fetcher.fetch()
+            # ...
+            # fetcher.archive(archive_path)
+            for ii, stage in enumerate(spec.package.stage):
                 fetcher = stage.fetcher
                 if ii == 0:
                     # create a subdirectory for the current package@version
@@ -184,7 +195,7 @@ def create(path, specs, **kwargs):
                 if os.path.exists(archive_path):
                     tty.msg("{name} : already added".format(name=name))
                 else:
-                    everything_already_exists = False
+                    spec_exists_in_mirror = False
                     fetcher.fetch()
                     if not kwargs.get('no_checksum', False):
                         fetcher.check()
@@ -195,20 +206,16 @@ def create(path, specs, **kwargs):
                     fetcher.archive(archive_path)
                     tty.msg("{name} : added".format(name=name))
 
-            if everything_already_exists:
-                present.append(spec)
-            else:
-                mirrored.append(spec)
-        except Exception, e:
-            if spack.debug:
-                sys.excepthook(*sys.exc_info())
-            else:
-                tty.warn("Error while fetching %s." % spec.format('$_$@'), e.message)
-            error.append(spec)
-        finally:
-            pkg.stage.destroy()
-
-    return (present, mirrored, error)
+        if spec_exists_in_mirror:
+            categories['present'].append(spec)
+        else:
+            categories['mirrored'].append(spec)
+    except Exception as e:
+        if spack.debug:
+            sys.excepthook(*sys.exc_info())
+        else:
+            tty.warn("Error while fetching %s." % spec.format('$_$@'), e.message)
+        categories['error'].append(spec)
 
 
 class MirrorError(spack.error.SpackError):
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 9f1825ca21..106c546d5c 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -293,7 +293,6 @@ class Package(object):
 
     .. code-block:: python
 
-       p.do_clean()              # removes the stage directory entirely
        p.do_restage()            # removes the build directory and
                                  # re-expands the archive.
 
@@ -503,7 +502,6 @@ class Package(object):
             self._fetcher = self._make_fetcher()
         return self._fetcher
 
-
     @fetcher.setter
     def fetcher(self, f):
         self._fetcher = f
@@ -735,7 +733,7 @@ class Package(object):
         # If we encounter an archive that failed to patch, restage it
         # so that we can apply all the patches again.
         if os.path.isfile(bad_file):
-            tty.msg("Patching failed last time.  Restaging.")
+            tty.msg("Patching failed last time. Restaging.")
             self.stage.restage()
 
         self.stage.chdir_to_source()
@@ -850,102 +848,103 @@ class Package(object):
                 make_jobs=make_jobs)
 
         start_time = time.time()
-        if not fake:
-            if not skip_patch:
-                self.do_patch()
-            else:
-                self.do_stage()
+        with self.stage:
+            if not fake:
+                if not skip_patch:
+                    self.do_patch()
+                else:
+                    self.do_stage()
 
-        # 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.create_install_directory(self.spec)
+            # 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.create_install_directory(self.spec)
 
-        def cleanup():
-            if not keep_prefix:
-                # If anything goes wrong, remove the install prefix
-                self.remove_prefix()
-            else:
-                tty.warn("Keeping install prefix in place despite error.",
-                         "Spack will think this package is installed." +
-                         "Manually remove this directory to fix:",
-                         self.prefix, wrap=True)
+            def cleanup():
+                if not keep_prefix:
+                    # If anything goes wrong, remove the install prefix
+                    self.remove_prefix()
+                else:
+                    tty.warn("Keeping install prefix in place despite error.",
+                             "Spack will think this package is installed." +
+                             "Manually remove this directory to fix:",
+                             self.prefix, wrap=True)
 
 
-        def real_work():
-            try:
-                tty.msg("Building %s." % self.name)
+            def real_work():
+                try:
+                    tty.msg("Building %s." % self.name)
 
-                # Run the pre-install hook in the child process after
-                # the directory is created.
-                spack.hooks.pre_install(self)
+                    # Run the pre-install hook in the child process after
+                    # the directory is created.
+                    spack.hooks.pre_install(self)
 
-                # Set up process's build environment before running install.
-                if fake:
-                    self.do_fake_install()
-                else:
-                    # Do the real install in the source directory.
-                    self.stage.chdir_to_source()
-
-                    # Save the build environment in a file before building.
-                    env_path = join_path(os.getcwd(), 'spack-build.env')
-
-                    # This redirects I/O to a build log (and optionally to the terminal)
-                    log_path = join_path(os.getcwd(), 'spack-build.out')
-                    log_file = open(log_path, 'w')
-                    with log_output(log_file, verbose, sys.stdout.isatty(), True):
-                        dump_environment(env_path)
-                        self.install(self.spec, self.prefix)
-
-                # Ensure that something was actually installed.
-                self._sanity_check_install()
-
-                # Move build log into install directory on success
-                if not fake:
-                    log_install_path = spack.install_layout.build_log_path(self.spec)
-                    env_install_path = spack.install_layout.build_env_path(self.spec)
-                    install(log_path, log_install_path)
-                    install(env_path, env_install_path)
-
-                packages_dir = spack.install_layout.build_packages_path(self.spec)
-                dump_packages(self.spec, packages_dir)
-
-                # On successful install, remove the stage.
-                if not keep_stage:
-                    self.stage.destroy()
-
-                # Stop timer.
-                self._total_time = time.time() - start_time
-                build_time = self._total_time - self._fetch_time
-
-                tty.msg("Successfully installed %s." % self.name,
-                        "Fetch: %s.  Build: %s.  Total: %s."
-                        % (_hms(self._fetch_time), _hms(build_time), _hms(self._total_time)))
-                print_pkg(self.prefix)
-
-            except ProcessError, e:
-                # Annotate with location of build log.
-                e.build_log = log_path
-                cleanup()
-                raise e
+                    # Set up process's build environment before running install.
+                    if fake:
+                        self.do_fake_install()
+                    else:
+                        # Do the real install in the source directory.
+                        self.stage.chdir_to_source()
 
-            except:
-                # other exceptions just clean up and raise.
-                cleanup()
-                raise
+                        # Save the build environment in a file before building.
+                        env_path = join_path(os.getcwd(), 'spack-build.env')
 
-        # Set parallelism before starting build.
-        self.make_jobs = make_jobs
+                        # This redirects I/O to a build log (and optionally to the terminal)
+                        log_path = join_path(os.getcwd(), 'spack-build.out')
+                        log_file = open(log_path, 'w')
+                        with log_output(log_file, verbose, sys.stdout.isatty(), True):
+                            dump_environment(env_path)
+                            self.install(self.spec, self.prefix)
 
-        # Do the build.
-        spack.build_environment.fork(self, real_work)
+                    # Ensure that something was actually installed.
+                    self._sanity_check_install()
 
-        # note: PARENT of the build process adds the new package to
-        # the database, so that we don't need to re-read from file.
-        spack.installed_db.add(self.spec, self.prefix)
+                    # Move build log into install directory on success
+                    if not fake:
+                        log_install_path = spack.install_layout.build_log_path(self.spec)
+                        env_install_path = spack.install_layout.build_env_path(self.spec)
+                        install(log_path, log_install_path)
+                        install(env_path, env_install_path)
 
-        # Once everything else is done, run post install hooks
-        spack.hooks.post_install(self)
+                    packages_dir = spack.install_layout.build_packages_path(self.spec)
+                    dump_packages(self.spec, packages_dir)
+
+                    # On successful install, remove the stage.
+                    if not keep_stage:
+                        self.stage.destroy()
+
+                    # Stop timer.
+                    self._total_time = time.time() - start_time
+                    build_time = self._total_time - self._fetch_time
+
+                    tty.msg("Successfully installed %s." % self.name,
+                            "Fetch: %s.  Build: %s.  Total: %s."
+                            % (_hms(self._fetch_time), _hms(build_time), _hms(self._total_time)))
+                    print_pkg(self.prefix)
+
+                except ProcessError as e:
+                    # Annotate with location of build log.
+                    e.build_log = log_path
+                    cleanup()
+                    raise e
+
+                except:
+                    # other exceptions just clean up and raise.
+                    cleanup()
+                    raise
+
+            # Set parallelism before starting build.
+            self.make_jobs = make_jobs
+
+            # Do the build.
+            spack.build_environment.fork(self, real_work)
+
+            # note: PARENT of the build process adds the new package to
+            # the database, so that we don't need to re-read from file.
+            spack.installed_db.add(self.spec, self.prefix)
+
+            # Once everything else is done, run post install hooks
+            spack.hooks.post_install(self)
 
 
     def _sanity_check_install(self):
@@ -1149,13 +1148,6 @@ class Package(object):
         """Reverts expanded/checked out source to a pristine state."""
         self.stage.restage()
 
-
-    def do_clean(self):
-        """Removes the package's build stage and source tarball."""
-        if os.path.exists(self.stage.path):
-            self.stage.destroy()
-
-
     def format_doc(self, **kwargs):
         """Wrap doc string at 72 characters and format nicely"""
         indent = kwargs.get('indent', 0)
@@ -1192,7 +1184,7 @@ class Package(object):
         try:
             return spack.util.web.find_versions_of_archive(
                 *self.all_urls, list_url=self.list_url, list_depth=self.list_depth)
-        except spack.error.NoNetworkConnectionError, e:
+        except spack.error.NoNetworkConnectionError as e:
             tty.die("Package.fetch_versions couldn't connect to:",
                     e.url, e.message)
 
diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py
index f217450d42..985043c103 100644
--- a/lib/spack/spack/stage.py
+++ b/lib/spack/spack/stage.py
@@ -42,33 +42,26 @@ STAGE_PREFIX = 'spack-stage-'
 
 
 class Stage(object):
-    """A Stage object manages a directory where some source code is
-       downloaded and built before being installed.  It handles
-       fetching the source code, either as an archive to be expanded
-       or by checking it out of a repository.  A stage's lifecycle
-       looks like this:
-
-       Stage()
-         Constructor creates the stage directory.
-       fetch()
-         Fetch a source archive into the stage.
-       expand_archive()
-         Expand the source archive.
-       <install>
-         Build and install the archive.  This is handled by the Package class.
-       destroy()
-         Remove the stage once the package has been installed.
-
-       If spack.use_tmp_stage is True, spack will attempt to create stages
-       in a tmp directory.  Otherwise, stages are created directly in
-       spack.stage_path.
-
-       There are two kinds of stages: named and unnamed.  Named stages can
-       persist between runs of spack, e.g. if you fetched a tarball but
-       didn't finish building it, you won't have to fetch it again.
-
-       Unnamed stages are created using standard mkdtemp mechanisms or
-       similar, and are intended to persist for only one run of spack.
+    """
+    A Stage object is a context manager that handles a directory where some source code is downloaded and built
+    before being installed.  It handles fetching the source code, either as an archive to be expanded or by checking
+    it out of a repository.  A stage's lifecycle looks like this:
+
+    ```
+    with Stage() as stage:  # Context manager creates and destroys the stage directory
+        fetch()  # Fetch a source archive into the stage.
+        expand_archive()  # Expand the source archive.
+        <install>  # Build and install the archive.  This is handled by the Package class.
+    ```
+
+    If spack.use_tmp_stage is True, spack will attempt to create stages in a tmp directory.
+    Otherwise, stages are created directly in spack.stage_path.
+
+    There are two kinds of stages: named and unnamed.  Named stages can persist between runs of spack, e.g. if you
+    fetched a tarball but didn't finish building it, you won't have to fetch it again.
+
+    Unnamed stages are created using standard mkdtemp mechanisms or similar, and are intended to persist for
+    only one run of spack.
     """
 
     def __init__(self, url_or_fetch_strategy, **kwargs):
@@ -96,21 +89,46 @@ class Stage(object):
         self.default_fetcher = self.fetcher  # self.fetcher can change with mirrors.
         self.skip_checksum_for_mirror = True  # used for mirrored archives of repositories.
 
-        self.name = kwargs.get('name')
+        # TODO : this uses a protected member of tempfile, but seemed the only way to get a temporary name
+        # TODO : besides, the temporary link name won't be the same as the temporary stage area in tmp_root
+        self.name = kwargs.get('name') if 'name' in kwargs else STAGE_PREFIX + next(tempfile._get_candidate_names())
         self.mirror_path = kwargs.get('mirror_path')
         self.tmp_root = find_tmp_root()
 
-        self.path = None
-        self._setup()
+        # Try to construct here a temporary name for the stage directory
+        # If this is a named stage, then construct a named path.
+        self.path = join_path(spack.stage_path, self.name)
+        # Flag to decide whether to delete the stage folder on exit or not
+        self.delete_on_exit = True
 
-    def _cleanup_dead_links(self):
-        """Remove any dead links in the stage directory."""
-        for file in os.listdir(spack.stage_path):
-            path = join_path(spack.stage_path, file)
-            if os.path.islink(path):
-                real_path = os.path.realpath(path)
-                if not os.path.exists(path):
-                    os.unlink(path)
+    def __enter__(self):
+        """
+        Entering a stage context will create the stage directory
+
+        Returns:
+            self
+        """
+        self.create()
+        return self
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        """
+        Exiting from a stage context will delete the stage directory unless:
+        - it was explicitly requested not to do so
+        - an exception has been raised
+
+        Args:
+            exc_type: exception type
+            exc_val: exception value
+            exc_tb: exception traceback
+
+        Returns:
+            Boolean
+        """
+        self.delete_on_exit = False if exc_type is not None else self.delete_on_exit
+
+        if self.delete_on_exit:
+            self.destroy()
 
     def _need_to_create_path(self):
         """Makes sure nothing weird has happened since the last time we
@@ -148,54 +166,6 @@ class Stage(object):
 
         return False
 
-    def _setup(self):
-        """Creates the stage directory.
-           If spack.use_tmp_stage is False, the stage directory is created
-           directly under spack.stage_path.
-
-           If spack.use_tmp_stage is True, this will attempt to create a
-           stage in a temporary directory and link it into spack.stage_path.
-           Spack will use the first writable location in spack.tmp_dirs to
-           create a stage.  If there is no valid location in tmp_dirs, fall
-           back to making the stage inside spack.stage_path.
-        """
-        # Create the top-level stage directory
-        mkdirp(spack.stage_path)
-        self._cleanup_dead_links()
-
-        # If this is a named stage, then construct a named path.
-        if self.name is not None:
-            self.path = join_path(spack.stage_path, self.name)
-
-        # If this is a temporary stage, them make the temp directory
-        tmp_dir = None
-        if self.tmp_root:
-            if self.name is None:
-                # Unnamed tmp root.  Link the path in
-                tmp_dir = tempfile.mkdtemp('', STAGE_PREFIX, self.tmp_root)
-                self.name = os.path.basename(tmp_dir)
-                self.path = join_path(spack.stage_path, self.name)
-                if self._need_to_create_path():
-                    os.symlink(tmp_dir, self.path)
-
-            else:
-                if self._need_to_create_path():
-                    tmp_dir = tempfile.mkdtemp('', STAGE_PREFIX, self.tmp_root)
-                    os.symlink(tmp_dir, self.path)
-
-        # if we're not using a tmp dir, create the stage directly in the
-        # stage dir, rather than linking to it.
-        else:
-            if self.name is None:
-                self.path = tempfile.mkdtemp('', STAGE_PREFIX, spack.stage_path)
-                self.name = os.path.basename(self.path)
-            else:
-                if self._need_to_create_path():
-                    mkdirp(self.path)
-
-        # Make sure we can actually do something with the stage we made.
-        ensure_access(self.path)
-
     @property
     def archive_file(self):
         """Path to the source archive within this stage directory."""
@@ -276,7 +246,7 @@ class Stage(object):
                 self.fetcher = fetcher
                 self.fetcher.fetch()
                 break
-            except spack.error.SpackError, e:
+            except spack.error.SpackError as e:
                 tty.msg("Fetching from %s failed." % fetcher)
                 tty.debug(e)
                 continue
@@ -328,8 +298,34 @@ class Stage(object):
         """
         self.fetcher.reset()
 
+    def create(self):
+        """
+        Creates the stage directory
+
+        If self.tmp_root evaluates to False, the stage directory is created directly under spack.stage_path, otherwise
+        this will attempt to create a stage in a temporary directory and link it into spack.stage_path.
+
+        Spack will use the first writable location in spack.tmp_dirs to create a stage. If there is no valid location
+        in tmp_dirs, fall back to making the stage inside spack.stage_path.
+        """
+        # Create the top-level stage directory
+        mkdirp(spack.stage_path)
+        remove_dead_links(spack.stage_path)
+        # If a tmp_root exists then create a directory there and then link it in the stage area,
+        # otherwise create the stage directory in self.path
+        if self._need_to_create_path():
+            if self.tmp_root:
+                tmp_dir = tempfile.mkdtemp('', STAGE_PREFIX, self.tmp_root)
+                os.symlink(tmp_dir, self.path)
+            else:
+                mkdirp(self.path)
+        # Make sure we can actually do something with the stage we made.
+        ensure_access(self.path)
+
     def destroy(self):
-        """Remove this stage directory."""
+        """
+        Removes this stage directory
+        """
         remove_linked_tree(self.path)
 
         # Make sure we don't end up in a removed directory
@@ -389,6 +385,15 @@ class StageComposite:
     def path(self):
         return self[0].path
 
+    def __enter__(self):
+        for item in self:
+            item.__enter__()
+        return self
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        for item in reversed(self):
+            item.__exit__(exc_type, exc_val, exc_tb)
+
     def chdir_to_source(self):
         return self[0].chdir_to_source()
 
@@ -439,19 +444,6 @@ def ensure_access(file=spack.stage_path):
         tty.die("Insufficient permissions for %s" % file)
 
 
-def remove_linked_tree(path):
-    """Removes a directory and its contents.  If the directory is a symlink,
-       follows the link and reamoves the real directory before removing the
-       link.
-    """
-    if os.path.exists(path):
-        if os.path.islink(path):
-            shutil.rmtree(os.path.realpath(path), True)
-            os.unlink(path)
-        else:
-            shutil.rmtree(path, True)
-
-
 def purge():
     """Remove all build directories in the top-level stage path."""
     if os.path.isdir(spack.stage_path):
diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py
index 7f2938aec5..794344fb6a 100644
--- a/lib/spack/spack/test/concretize.py
+++ b/lib/spack/spack/test/concretize.py
@@ -22,8 +22,6 @@
 # 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
-
 import spack
 from spack.spec import Spec, CompilerSpec
 from spack.test.mock_packages_test import *
diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py
index d8be5a855b..0562d2d620 100644
--- a/lib/spack/spack/test/config.py
+++ b/lib/spack/spack/test/config.py
@@ -22,13 +22,13 @@
 # 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
-import shutil
 import os
+import shutil
 from tempfile import mkdtemp
-from ordereddict_backport import OrderedDict
+
 import spack
 import spack.config
+from ordereddict_backport import OrderedDict
 from spack.test.mock_packages_test import *
 
 # Some sample compiler config data
diff --git a/lib/spack/spack/test/configure_guess.py b/lib/spack/spack/test/configure_guess.py
index a4e8565b62..2440d120e5 100644
--- a/lib/spack/spack/test/configure_guess.py
+++ b/lib/spack/spack/test/configure_guess.py
@@ -23,20 +23,15 @@
 # Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##############################################################################
 import os
-import unittest
 import shutil
 import tempfile
+import unittest
 
 from llnl.util.filesystem import *
-
 from spack.cmd.create import ConfigureGuesser
 from spack.stage import Stage
-
-from spack.fetch_strategy import URLFetchStrategy
-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
+from spack.util.executable import which
 
 
 class InstallTest(unittest.TestCase):
@@ -52,8 +47,6 @@ class InstallTest(unittest.TestCase):
 
     def tearDown(self):
         shutil.rmtree(self.tmpdir, ignore_errors=True)
-        if self.stage:
-            self.stage.destroy()
         os.chdir(self.orig_dir)
 
 
@@ -64,12 +57,12 @@ class InstallTest(unittest.TestCase):
 
         url = 'file://' + join_path(os.getcwd(), 'archive.tar.gz')
         print url
-        self.stage = Stage(url)
-        self.stage.fetch()
+        with Stage(url) as stage:
+            stage.fetch()
 
-        guesser = ConfigureGuesser()
-        guesser(self.stage)
-        self.assertEqual(system, guesser.build_system)
+            guesser = ConfigureGuesser()
+            guesser(stage)
+            self.assertEqual(system, guesser.build_system)
 
 
     def test_python(self):
diff --git a/lib/spack/spack/test/database.py b/lib/spack/spack/test/database.py
index 0205f4b8ce..9a57e1f03e 100644
--- a/lib/spack/spack/test/database.py
+++ b/lib/spack/spack/test/database.py
@@ -26,19 +26,18 @@
 These tests check the database is functioning properly,
 both in memory and in its file
 """
-import tempfile
-import shutil
 import multiprocessing
-
-from llnl.util.lock import *
-from llnl.util.filesystem import join_path
+import shutil
+import tempfile
 
 import spack
+from llnl.util.filesystem import join_path
+from llnl.util.lock import *
+from llnl.util.tty.colify import colify
 from spack.database import Database
 from spack.directory_layout import YamlDirectoryLayout
 from spack.test.mock_packages_test import *
 
-from llnl.util.tty.colify import colify
 
 def _print_ref_counts():
     """Print out all ref counts for the graph used here, for debugging"""
diff --git a/lib/spack/spack/test/directory_layout.py b/lib/spack/spack/test/directory_layout.py
index 925cb648ed..d814572d4a 100644
--- a/lib/spack/spack/test/directory_layout.py
+++ b/lib/spack/spack/test/directory_layout.py
@@ -25,20 +25,17 @@
 """\
 This test verifies that the Spack directory layout works properly.
 """
-import unittest
-import tempfile
-import shutil
 import os
-
-from llnl.util.filesystem import *
+import shutil
+import tempfile
 
 import spack
-from spack.spec import Spec
-from spack.repository import RepoPath
+from llnl.util.filesystem import *
 from spack.directory_layout import YamlDirectoryLayout
+from spack.repository import RepoPath
+from spack.spec import Spec
 from spack.test.mock_packages_test import *
 
-
 # number of packages to test (to reduce test time)
 max_packages = 10
 
diff --git a/lib/spack/spack/test/git_fetch.py b/lib/spack/spack/test/git_fetch.py
index d84433176a..3578044116 100644
--- a/lib/spack/spack/test/git_fetch.py
+++ b/lib/spack/spack/test/git_fetch.py
@@ -23,19 +23,12 @@
 # Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##############################################################################
 import os
-import unittest
-import shutil
-import tempfile
-
-from llnl.util.filesystem import *
 
 import spack
-from spack.version import ver
-from spack.stage import Stage
-from spack.util.executable import which
-
+from llnl.util.filesystem import *
 from spack.test.mock_packages_test import *
 from spack.test.mock_repo import MockGitRepo
+from spack.version import ver
 
 
 class GitFetchTest(MockPackagesTest):
@@ -52,19 +45,15 @@ class GitFetchTest(MockPackagesTest):
         spec.concretize()
         self.pkg = spack.repo.get(spec, new=True)
 
-
     def tearDown(self):
         """Destroy the stage space used by this test."""
         super(GitFetchTest, self).tearDown()
         self.repo.destroy()
-        self.pkg.do_clean()
-
 
     def assert_rev(self, rev):
         """Check that the current git revision is equal to the supplied rev."""
         self.assertEqual(self.repo.rev_hash('HEAD'), self.repo.rev_hash(rev))
 
-
     def try_fetch(self, rev, test_file, args):
         """Tries to:
            1. Fetch the repo using a fetch strategy constructed with
@@ -76,26 +65,27 @@ class GitFetchTest(MockPackagesTest):
         """
         self.pkg.versions[ver('git')] = args
 
-        self.pkg.do_stage()
-        self.assert_rev(rev)
+        with self.pkg.stage:
+            self.pkg.do_stage()
+            self.assert_rev(rev)
 
-        file_path = join_path(self.pkg.stage.source_path, test_file)
-        self.assertTrue(os.path.isdir(self.pkg.stage.source_path))
-        self.assertTrue(os.path.isfile(file_path))
+            file_path = join_path(self.pkg.stage.source_path, test_file)
+            self.assertTrue(os.path.isdir(self.pkg.stage.source_path))
+            self.assertTrue(os.path.isfile(file_path))
 
-        os.unlink(file_path)
-        self.assertFalse(os.path.isfile(file_path))
+            os.unlink(file_path)
+            self.assertFalse(os.path.isfile(file_path))
 
-        untracked_file = 'foobarbaz'
-        touch(untracked_file)
-        self.assertTrue(os.path.isfile(untracked_file))
-        self.pkg.do_restage()
-        self.assertFalse(os.path.isfile(untracked_file))
+            untracked_file = 'foobarbaz'
+            touch(untracked_file)
+            self.assertTrue(os.path.isfile(untracked_file))
+            self.pkg.do_restage()
+            self.assertFalse(os.path.isfile(untracked_file))
 
-        self.assertTrue(os.path.isdir(self.pkg.stage.source_path))
-        self.assertTrue(os.path.isfile(file_path))
+            self.assertTrue(os.path.isdir(self.pkg.stage.source_path))
+            self.assertTrue(os.path.isfile(file_path))
 
-        self.assert_rev(rev)
+            self.assert_rev(rev)
 
 
     def test_fetch_master(self):
diff --git a/lib/spack/spack/test/hg_fetch.py b/lib/spack/spack/test/hg_fetch.py
index bbcb64e4c1..b8a0c1ec46 100644
--- a/lib/spack/spack/test/hg_fetch.py
+++ b/lib/spack/spack/test/hg_fetch.py
@@ -23,16 +23,12 @@
 # Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##############################################################################
 import os
-import unittest
-
-from llnl.util.filesystem import *
-
 import spack
+
 from spack.version import ver
-from spack.stage import Stage
-from spack.util.executable import which
-from spack.test.mock_packages_test import *
 from spack.test.mock_repo import MockHgRepo
+from llnl.util.filesystem import *
+from spack.test.mock_packages_test import *
 
 
 class HgFetchTest(MockPackagesTest):
@@ -49,13 +45,10 @@ class HgFetchTest(MockPackagesTest):
         spec.concretize()
         self.pkg = spack.repo.get(spec, new=True)
 
-
     def tearDown(self):
         """Destroy the stage space used by this test."""
         super(HgFetchTest, self).tearDown()
         self.repo.destroy()
-        self.pkg.do_clean()
-
 
     def try_fetch(self, rev, test_file, args):
         """Tries to:
@@ -68,26 +61,27 @@ class HgFetchTest(MockPackagesTest):
         """
         self.pkg.versions[ver('hg')] = args
 
-        self.pkg.do_stage()
-        self.assertEqual(self.repo.get_rev(), rev)
+        with self.pkg.stage:
+            self.pkg.do_stage()
+            self.assertEqual(self.repo.get_rev(), rev)
 
-        file_path = join_path(self.pkg.stage.source_path, test_file)
-        self.assertTrue(os.path.isdir(self.pkg.stage.source_path))
-        self.assertTrue(os.path.isfile(file_path))
+            file_path = join_path(self.pkg.stage.source_path, test_file)
+            self.assertTrue(os.path.isdir(self.pkg.stage.source_path))
+            self.assertTrue(os.path.isfile(file_path))
 
-        os.unlink(file_path)
-        self.assertFalse(os.path.isfile(file_path))
+            os.unlink(file_path)
+            self.assertFalse(os.path.isfile(file_path))
 
-        untracked = 'foobarbaz'
-        touch(untracked)
-        self.assertTrue(os.path.isfile(untracked))
-        self.pkg.do_restage()
-        self.assertFalse(os.path.isfile(untracked))
+            untracked = 'foobarbaz'
+            touch(untracked)
+            self.assertTrue(os.path.isfile(untracked))
+            self.pkg.do_restage()
+            self.assertFalse(os.path.isfile(untracked))
 
-        self.assertTrue(os.path.isdir(self.pkg.stage.source_path))
-        self.assertTrue(os.path.isfile(file_path))
+            self.assertTrue(os.path.isdir(self.pkg.stage.source_path))
+            self.assertTrue(os.path.isfile(file_path))
 
-        self.assertEqual(self.repo.get_rev(), rev)
+            self.assertEqual(self.repo.get_rev(), rev)
 
 
     def test_fetch_default(self):
diff --git a/lib/spack/spack/test/install.py b/lib/spack/spack/test/install.py
index 8863d13c42..8297893f01 100644
--- a/lib/spack/spack/test/install.py
+++ b/lib/spack/spack/test/install.py
@@ -22,18 +22,13 @@
 # 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 unittest
 import shutil
 import tempfile
 
-from llnl.util.filesystem import *
-
 import spack
-from spack.stage import Stage
-from spack.fetch_strategy import URLFetchStrategy, FetchStrategyComposite
+from llnl.util.filesystem import *
 from spack.directory_layout import YamlDirectoryLayout
-from spack.util.executable import which
+from spack.fetch_strategy import URLFetchStrategy, FetchStrategyComposite
 from spack.test.mock_packages_test import *
 from spack.test.mock_repo import MockArchive
 
diff --git a/lib/spack/spack/test/link_tree.py b/lib/spack/spack/test/link_tree.py
index 886b7ef4c5..ee37e765c7 100644
--- a/lib/spack/spack/test/link_tree.py
+++ b/lib/spack/spack/test/link_tree.py
@@ -24,8 +24,6 @@
 ##############################################################################
 import os
 import unittest
-import shutil
-import tempfile
 
 from llnl.util.filesystem import *
 from llnl.util.link_tree import LinkTree
@@ -38,6 +36,7 @@ class LinkTreeTest(unittest.TestCase):
 
     def setUp(self):
         self.stage = Stage('link-tree-test')
+        self.stage.create()
 
         with working_dir(self.stage.path):
             touchp('source/1')
@@ -51,10 +50,8 @@ class LinkTreeTest(unittest.TestCase):
         source_path = os.path.join(self.stage.path, 'source')
         self.link_tree = LinkTree(source_path)
 
-
     def tearDown(self):
-        if self.stage:
-            self.stage.destroy()
+        self.stage.destroy()
 
 
     def check_file_link(self, filename):
diff --git a/lib/spack/spack/test/lock.py b/lib/spack/spack/test/lock.py
index bc68df01db..3b11d18da4 100644
--- a/lib/spack/spack/test/lock.py
+++ b/lib/spack/spack/test/lock.py
@@ -25,15 +25,13 @@
 """
 These tests ensure that our lock works correctly.
 """
-import unittest
-import os
-import tempfile
 import shutil
+import tempfile
+import unittest
 from multiprocessing import Process
 
-from llnl.util.lock import *
 from llnl.util.filesystem import join_path, touch
-
+from llnl.util.lock import *
 from spack.util.multiproc import Barrier
 
 # This is the longest a failed test will take, as the barriers will
diff --git a/lib/spack/spack/test/make_executable.py b/lib/spack/spack/test/make_executable.py
index d568a28d44..a2606acf19 100644
--- a/lib/spack/spack/test/make_executable.py
+++ b/lib/spack/spack/test/make_executable.py
@@ -28,13 +28,13 @@ Tests for Spack's built-in parallel make support.
 This just tests whether the right args are getting passed to make.
 """
 import os
-import unittest
-import tempfile
 import shutil
+import tempfile
+import unittest
 
 from llnl.util.filesystem import *
-from spack.util.environment import path_put_first
 from spack.build_environment import MakeExecutable
+from spack.util.environment import path_put_first
 
 
 class MakeExecutableTest(unittest.TestCase):
diff --git a/lib/spack/spack/test/mirror.py b/lib/spack/spack/test/mirror.py
index f83cc8090c..e707adfe9d 100644
--- a/lib/spack/spack/test/mirror.py
+++ b/lib/spack/spack/test/mirror.py
@@ -23,11 +23,10 @@
 # Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##############################################################################
 import os
-from filecmp import dircmp
-
 import spack
 import spack.mirror
-from spack.util.compression import decompressor_for
+
+from filecmp import dircmp
 from spack.test.mock_packages_test import *
 from spack.test.mock_repo import *
 
@@ -74,14 +73,14 @@ class MirrorTest(MockPackagesTest):
 
 
     def check_mirror(self):
-        stage = Stage('spack-mirror-test')
-        mirror_root = join_path(stage.path, 'test-mirror')
+        with Stage('spack-mirror-test') as stage:
+            mirror_root = join_path(stage.path, 'test-mirror')
+
+            # register mirror with spack config
+            mirrors = { 'spack-mirror-test' : 'file://' + mirror_root }
+            spack.config.update_config('mirrors', mirrors)
 
-        # register mirror with spack config
-        mirrors = { 'spack-mirror-test' : 'file://' + mirror_root }
-        spack.config.update_config('mirrors', mirrors)
 
-        try:
             os.chdir(stage.path)
             spack.mirror.create(
                 mirror_root, self.repos, no_checksum=True)
@@ -97,38 +96,28 @@ class MirrorTest(MockPackagesTest):
                 files = os.listdir(subdir)
                 self.assertEqual(len(files), 1)
 
-            # Now try to fetch each package.
-            for name, mock_repo in self.repos.items():
-                spec = Spec(name).concretized()
-                pkg = spec.package
-
-                pkg._stage = None
-                saved_checksum_setting = spack.do_checksum
-                try:
-                    # Stage the archive from the mirror and cd to it.
-                    spack.do_checksum = False
-                    pkg.do_stage(mirror_only=True)
-
-                    # Compare the original repo with the expanded archive
-                    original_path = mock_repo.path
-                    if 'svn' in name:
-                        # have to check out the svn repo to compare.
-                        original_path = join_path(mock_repo.path, 'checked_out')
-                        svn('checkout', mock_repo.url, original_path)
-
-                    dcmp = dircmp(original_path, pkg.stage.source_path)
-
-                    # make sure there are no new files in the expanded tarball
-                    self.assertFalse(dcmp.right_only)
-
-                    # and that all original files are present.
-                    self.assertTrue(all(l in exclude for l in dcmp.left_only))
-
-                finally:
-                    spack.do_checksum = saved_checksum_setting
-                    pkg.do_clean()
-        finally:
-            stage.destroy()
+                # Now try to fetch each package.
+                for name, mock_repo in self.repos.items():
+                    spec = Spec(name).concretized()
+                    pkg = spec.package
+
+                    saved_checksum_setting = spack.do_checksum
+                    with pkg.stage:
+                        # Stage the archive from the mirror and cd to it.
+                        spack.do_checksum = False
+                        pkg.do_stage(mirror_only=True)
+                        # Compare the original repo with the expanded archive
+                        original_path = mock_repo.path
+                        if 'svn' in name:
+                            # have to check out the svn repo to compare.
+                            original_path = join_path(mock_repo.path, 'checked_out')
+                            svn('checkout', mock_repo.url, original_path)
+                        dcmp = dircmp(original_path, pkg.stage.source_path)
+                        # make sure there are no new files in the expanded tarball
+                        self.assertFalse(dcmp.right_only)
+                        # and that all original files are present.
+                        self.assertTrue(all(l in exclude for l in dcmp.left_only))
+                        spack.do_checksum = saved_checksum_setting
 
 
     def test_git_mirror(self):
diff --git a/lib/spack/spack/test/mock_packages_test.py b/lib/spack/spack/test/mock_packages_test.py
index e9f1f95df5..0b8867b61e 100644
--- a/lib/spack/spack/test/mock_packages_test.py
+++ b/lib/spack/spack/test/mock_packages_test.py
@@ -22,17 +22,15 @@
 # along with this program; if not, write to the Free Software Foundation,
 # Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##############################################################################
-import sys
 import os
 import shutil
-import unittest
 import tempfile
-from ordereddict_backport import OrderedDict
-
-from llnl.util.filesystem import mkdirp
+import unittest
 
 import spack
 import spack.config
+from llnl.util.filesystem import mkdirp
+from ordereddict_backport import OrderedDict
 from spack.repository import RepoPath
 from spack.spec import Spec
 
diff --git a/lib/spack/spack/test/mock_repo.py b/lib/spack/spack/test/mock_repo.py
index ed94023b0e..a8bdfb5571 100644
--- a/lib/spack/spack/test/mock_repo.py
+++ b/lib/spack/spack/test/mock_repo.py
@@ -26,13 +26,9 @@ import os
 import shutil
 
 from llnl.util.filesystem import *
-
-import spack
-from spack.version import ver
 from spack.stage import Stage
 from spack.util.executable import which
 
-
 #
 # VCS Systems used by mock repo code.
 #
diff --git a/lib/spack/spack/test/multimethod.py b/lib/spack/spack/test/multimethod.py
index 7bf4ff0a0a..2d4b8cd584 100644
--- a/lib/spack/spack/test/multimethod.py
+++ b/lib/spack/spack/test/multimethod.py
@@ -25,14 +25,11 @@
 """
 Test for multi_method dispatch.
 """
-import unittest
 
 import spack
 from spack.multimethod import *
-from spack.version import *
-from spack.spec import Spec
-from spack.multimethod import when
 from spack.test.mock_packages_test import *
+from spack.version import *
 
 
 class MultiMethodTest(MockPackagesTest):
diff --git a/lib/spack/spack/test/namespace_trie.py b/lib/spack/spack/test/namespace_trie.py
index 647976df21..2023ba6d96 100644
--- a/lib/spack/spack/test/namespace_trie.py
+++ b/lib/spack/spack/test/namespace_trie.py
@@ -23,6 +23,7 @@
 # Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##############################################################################
 import unittest
+
 from spack.util.naming import NamespaceTrie
 
 
diff --git a/lib/spack/spack/test/optional_deps.py b/lib/spack/spack/test/optional_deps.py
index ebd7281999..55f35ea4c9 100644
--- a/lib/spack/spack/test/optional_deps.py
+++ b/lib/spack/spack/test/optional_deps.py
@@ -22,10 +22,8 @@
 # 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
 
-import spack
-from spack.spec import Spec, CompilerSpec
+from spack.spec import Spec
 from spack.test.mock_packages_test import *
 
 class ConcretizeTest(MockPackagesTest):
diff --git a/lib/spack/spack/test/packages.py b/lib/spack/spack/test/packages.py
index 83984dc5f6..f0b5e05f3b 100644
--- a/lib/spack/spack/test/packages.py
+++ b/lib/spack/spack/test/packages.py
@@ -22,14 +22,12 @@
 # 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 join_path
 
 import spack
+from llnl.util.filesystem import join_path
 from spack.repository import Repo
-from spack.util.naming import mod_to_class
 from spack.test.mock_packages_test import *
+from spack.util.naming import mod_to_class
 
 
 class PackagesTest(MockPackagesTest):
diff --git a/lib/spack/spack/test/python_version.py b/lib/spack/spack/test/python_version.py
index d74d3b9b7d..4294975304 100644
--- a/lib/spack/spack/test/python_version.py
+++ b/lib/spack/spack/test/python_version.py
@@ -28,12 +28,11 @@ This test ensures that all Spack files are Python version 2.6 or less.
 Spack was originally 2.7, but enough systems in 2014 are still using
 2.6 on their frontend nodes that we need 2.6 to get adopted.
 """
-import unittest
 import os
 import re
+import unittest
 
 import llnl.util.tty as tty
-
 import pyqver2
 import spack
 
diff --git a/lib/spack/spack/test/spec_dag.py b/lib/spack/spack/test/spec_dag.py
index 632f777cde..5e6162b6e6 100644
--- a/lib/spack/spack/test/spec_dag.py
+++ b/lib/spack/spack/test/spec_dag.py
@@ -31,8 +31,6 @@ You can find the dummy packages here::
 import spack
 import spack.package
 
-from llnl.util.lang import list_modules
-
 from spack.spec import Spec
 from spack.test.mock_packages_test import *
 
diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py
index 44a09cbd7f..8c33d1ff6e 100644
--- a/lib/spack/spack/test/spec_semantics.py
+++ b/lib/spack/spack/test/spec_semantics.py
@@ -22,7 +22,6 @@
 # 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 spack.spec import *
 from spack.test.mock_packages_test import *
 
diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py
index 1daaa4be8f..6e08e30e13 100644
--- a/lib/spack/spack/test/spec_syntax.py
+++ b/lib/spack/spack/test/spec_syntax.py
@@ -23,9 +23,10 @@
 # Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##############################################################################
 import unittest
+
 import spack.spec
-from spack.spec import *
 from spack.parse import Token
+from spack.spec import *
 
 # Sample output for a complex lexing.
 complex_lex = [Token(ID, 'mvapich_foo'),
diff --git a/lib/spack/spack/test/stage.py b/lib/spack/spack/test/stage.py
index c1b2a2a573..dbcf89d864 100644
--- a/lib/spack/spack/test/stage.py
+++ b/lib/spack/spack/test/stage.py
@@ -25,15 +25,13 @@
 """\
 Test that the Stage class works correctly.
 """
-import unittest
-import shutil
 import os
-import getpass
+import shutil
+import unittest
 from contextlib import *
 
-from llnl.util.filesystem import *
-
 import spack
+from llnl.util.filesystem import *
 from spack.stage import Stage
 from spack.util.executable import which
 
@@ -192,116 +190,90 @@ class StageTest(unittest.TestCase):
 
     def test_setup_and_destroy_name_with_tmp(self):
         with use_tmp(True):
-            stage = Stage(archive_url, name=stage_name)
-            self.check_setup(stage, stage_name)
-
-            stage.destroy()
+            with Stage(archive_url, name=stage_name) as stage:
+                self.check_setup(stage, stage_name)
             self.check_destroy(stage, stage_name)
 
 
     def test_setup_and_destroy_name_without_tmp(self):
         with use_tmp(False):
-            stage = Stage(archive_url, name=stage_name)
-            self.check_setup(stage, stage_name)
-
-            stage.destroy()
+            with Stage(archive_url, name=stage_name) as stage:
+                self.check_setup(stage, stage_name)
             self.check_destroy(stage, stage_name)
 
 
     def test_setup_and_destroy_no_name_with_tmp(self):
         with use_tmp(True):
-            stage = Stage(archive_url)
-            self.check_setup(stage, None)
-
-            stage.destroy()
+            with Stage(archive_url) as stage:
+                self.check_setup(stage, None)
             self.check_destroy(stage, None)
 
 
     def test_setup_and_destroy_no_name_without_tmp(self):
         with use_tmp(False):
-            stage = Stage(archive_url)
-            self.check_setup(stage, None)
-
-            stage.destroy()
+            with Stage(archive_url) as stage:
+                self.check_setup(stage, None)
             self.check_destroy(stage, None)
 
 
     def test_chdir(self):
-        stage = Stage(archive_url, name=stage_name)
-
-        stage.chdir()
-        self.check_setup(stage, stage_name)
-        self.check_chdir(stage, stage_name)
-
-        stage.destroy()
+        with Stage(archive_url, name=stage_name) as stage:
+            stage.chdir()
+            self.check_setup(stage, stage_name)
+            self.check_chdir(stage, stage_name)
         self.check_destroy(stage, stage_name)
 
 
     def test_fetch(self):
-        stage = Stage(archive_url, name=stage_name)
-
-        stage.fetch()
-        self.check_setup(stage, stage_name)
-        self.check_chdir(stage, stage_name)
-        self.check_fetch(stage, stage_name)
-
-        stage.destroy()
+        with Stage(archive_url, name=stage_name) as stage:
+            stage.fetch()
+            self.check_setup(stage, stage_name)
+            self.check_chdir(stage, stage_name)
+            self.check_fetch(stage, stage_name)
         self.check_destroy(stage, stage_name)
 
 
     def test_expand_archive(self):
-        stage = Stage(archive_url, name=stage_name)
-
-        stage.fetch()
-        self.check_setup(stage, stage_name)
-        self.check_fetch(stage, stage_name)
-
-        stage.expand_archive()
-        self.check_expand_archive(stage, stage_name)
-
-        stage.destroy()
+        with Stage(archive_url, name=stage_name) as stage:
+            stage.fetch()
+            self.check_setup(stage, stage_name)
+            self.check_fetch(stage, stage_name)
+            stage.expand_archive()
+            self.check_expand_archive(stage, stage_name)
         self.check_destroy(stage, stage_name)
 
 
     def test_expand_archive(self):
-        stage = Stage(archive_url, name=stage_name)
-
-        stage.fetch()
-        self.check_setup(stage, stage_name)
-        self.check_fetch(stage, stage_name)
-
-        stage.expand_archive()
-        stage.chdir_to_source()
-        self.check_expand_archive(stage, stage_name)
-        self.check_chdir_to_source(stage, stage_name)
-
-        stage.destroy()
+        with Stage(archive_url, name=stage_name) as stage:
+            stage.fetch()
+            self.check_setup(stage, stage_name)
+            self.check_fetch(stage, stage_name)
+            stage.expand_archive()
+            stage.chdir_to_source()
+            self.check_expand_archive(stage, stage_name)
+            self.check_chdir_to_source(stage, stage_name)
         self.check_destroy(stage, stage_name)
 
 
     def test_restage(self):
-        stage = Stage(archive_url, name=stage_name)
-
-        stage.fetch()
-        stage.expand_archive()
-        stage.chdir_to_source()
-        self.check_expand_archive(stage, stage_name)
-        self.check_chdir_to_source(stage, stage_name)
-
-        # Try to make a file in the old archive dir
-        with open('foobar', 'w') as file:
-            file.write("this file is to be destroyed.")
-
-        self.assertTrue('foobar' in os.listdir(stage.source_path))
-
-        # Make sure the file is not there after restage.
-        stage.restage()
-        self.check_chdir(stage, stage_name)
-        self.check_fetch(stage, stage_name)
-
-        stage.chdir_to_source()
-        self.check_chdir_to_source(stage, stage_name)
-        self.assertFalse('foobar' in os.listdir(stage.source_path))
-
-        stage.destroy()
+        with Stage(archive_url, name=stage_name) as stage:
+            stage.fetch()
+            stage.expand_archive()
+            stage.chdir_to_source()
+            self.check_expand_archive(stage, stage_name)
+            self.check_chdir_to_source(stage, stage_name)
+
+            # Try to make a file in the old archive dir
+            with open('foobar', 'w') as file:
+                file.write("this file is to be destroyed.")
+
+            self.assertTrue('foobar' in os.listdir(stage.source_path))
+
+            # Make sure the file is not there after restage.
+            stage.restage()
+            self.check_chdir(stage, stage_name)
+            self.check_fetch(stage, stage_name)
+            stage.chdir_to_source()
+            self.check_chdir_to_source(stage, stage_name)
+            self.assertFalse('foobar' in os.listdir(stage.source_path))
         self.check_destroy(stage, stage_name)
diff --git a/lib/spack/spack/test/svn_fetch.py b/lib/spack/spack/test/svn_fetch.py
index 454a7f1d1f..1ee4ee700e 100644
--- a/lib/spack/spack/test/svn_fetch.py
+++ b/lib/spack/spack/test/svn_fetch.py
@@ -24,18 +24,12 @@
 ##############################################################################
 import os
 import re
-import unittest
-import shutil
-import tempfile
-
-from llnl.util.filesystem import *
-
 import spack
+
+from spack.test.mock_repo import svn, MockSvnRepo
 from spack.version import ver
-from spack.stage import Stage
-from spack.util.executable import which
 from spack.test.mock_packages_test import *
-from spack.test.mock_repo import svn, MockSvnRepo
+from llnl.util.filesystem import *
 
 
 class SvnFetchTest(MockPackagesTest):
@@ -51,13 +45,10 @@ class SvnFetchTest(MockPackagesTest):
         spec.concretize()
         self.pkg = spack.repo.get(spec, new=True)
 
-
     def tearDown(self):
         """Destroy the stage space used by this test."""
         super(SvnFetchTest, self).tearDown()
         self.repo.destroy()
-        self.pkg.do_clean()
-
 
     def assert_rev(self, rev):
         """Check that the current revision is equal to the supplied rev."""
@@ -70,7 +61,6 @@ class SvnFetchTest(MockPackagesTest):
                     return match.group(1)
         self.assertEqual(get_rev(), rev)
 
-
     def try_fetch(self, rev, test_file, args):
         """Tries to:
            1. Fetch the repo using a fetch strategy constructed with
@@ -82,26 +72,27 @@ class SvnFetchTest(MockPackagesTest):
         """
         self.pkg.versions[ver('svn')] = args
 
-        self.pkg.do_stage()
-        self.assert_rev(rev)
+        with self.pkg.stage:
+            self.pkg.do_stage()
+            self.assert_rev(rev)
 
-        file_path = join_path(self.pkg.stage.source_path, test_file)
-        self.assertTrue(os.path.isdir(self.pkg.stage.source_path))
-        self.assertTrue(os.path.isfile(file_path))
+            file_path = join_path(self.pkg.stage.source_path, test_file)
+            self.assertTrue(os.path.isdir(self.pkg.stage.source_path))
+            self.assertTrue(os.path.isfile(file_path))
 
-        os.unlink(file_path)
-        self.assertFalse(os.path.isfile(file_path))
+            os.unlink(file_path)
+            self.assertFalse(os.path.isfile(file_path))
 
-        untracked = 'foobarbaz'
-        touch(untracked)
-        self.assertTrue(os.path.isfile(untracked))
-        self.pkg.do_restage()
-        self.assertFalse(os.path.isfile(untracked))
+            untracked = 'foobarbaz'
+            touch(untracked)
+            self.assertTrue(os.path.isfile(untracked))
+            self.pkg.do_restage()
+            self.assertFalse(os.path.isfile(untracked))
 
-        self.assertTrue(os.path.isdir(self.pkg.stage.source_path))
-        self.assertTrue(os.path.isfile(file_path))
+            self.assertTrue(os.path.isdir(self.pkg.stage.source_path))
+            self.assertTrue(os.path.isfile(file_path))
 
-        self.assert_rev(rev)
+            self.assert_rev(rev)
 
 
     def test_fetch_default(self):
diff --git a/lib/spack/spack/test/tally_plugin.py b/lib/spack/spack/test/tally_plugin.py
index e0b9618e0c..4163ab95dd 100644
--- a/lib/spack/spack/test/tally_plugin.py
+++ b/lib/spack/spack/test/tally_plugin.py
@@ -22,10 +22,10 @@
 # along with this program; if not, write to the Free Software Foundation,
 # Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##############################################################################
-from nose.plugins import Plugin
-
 import os
 
+from nose.plugins import Plugin
+
 class Tally(Plugin):
     name = 'tally'
 
diff --git a/lib/spack/spack/test/unit_install.py b/lib/spack/spack/test/unit_install.py
index ccc409dd60..18615b7efe 100644
--- a/lib/spack/spack/test/unit_install.py
+++ b/lib/spack/spack/test/unit_install.py
@@ -22,10 +22,11 @@
 # 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
 import itertools
+import unittest
 
 import spack
+
 test_install = __import__("spack.cmd.test-install",
     fromlist=["BuildId", "create_test_output", "TestResult"])
 
diff --git a/lib/spack/spack/test/url_extrapolate.py b/lib/spack/spack/test/url_extrapolate.py
index 87adf89401..068a335b49 100644
--- a/lib/spack/spack/test/url_extrapolate.py
+++ b/lib/spack/spack/test/url_extrapolate.py
@@ -25,10 +25,7 @@
 """\
 Tests ability of spack to extrapolate URL versions from existing versions.
 """
-import spack
 import spack.url as url
-from spack.spec import Spec
-from spack.version import ver
 from spack.test.mock_packages_test import *
 
 
diff --git a/lib/spack/spack/test/url_parse.py b/lib/spack/spack/test/url_parse.py
index efde7c0c73..561d4658a1 100644
--- a/lib/spack/spack/test/url_parse.py
+++ b/lib/spack/spack/test/url_parse.py
@@ -27,8 +27,8 @@ This file has a bunch of versions tests taken from the excellent version
 detection in Homebrew.
 """
 import unittest
+
 import spack.url as url
-from pprint import pprint
 
 
 class UrlParseTest(unittest.TestCase):
diff --git a/lib/spack/spack/test/url_substitution.py b/lib/spack/spack/test/url_substitution.py
index aec8baf4ea..2be38af0d3 100644
--- a/lib/spack/spack/test/url_substitution.py
+++ b/lib/spack/spack/test/url_substitution.py
@@ -27,7 +27,6 @@ This test does sanity checks on substituting new versions into URLs
 """
 import unittest
 
-import spack
 import spack.url as url
 
 
diff --git a/lib/spack/spack/test/versions.py b/lib/spack/spack/test/versions.py
index 108450e098..2732006eb3 100644
--- a/lib/spack/spack/test/versions.py
+++ b/lib/spack/spack/test/versions.py
@@ -28,6 +28,7 @@ We try to maintain compatibility with RPM's version semantics
 where it makes sense.
 """
 import unittest
+
 from spack.version import *
 
 
diff --git a/lib/spack/spack/test/yaml.py b/lib/spack/spack/test/yaml.py
index 5a357b8e69..b930c022f2 100644
--- a/lib/spack/spack/test/yaml.py
+++ b/lib/spack/spack/test/yaml.py
@@ -26,6 +26,7 @@
 Test Spack's custom YAML format.
 """
 import unittest
+
 import spack.util.spack_yaml as syaml
 
 test_file = """\