From 6f42dd556d1ead8e7cad9788004dc33c11240564 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Wed, 24 Feb 2016 17:37:58 +0100
Subject: [PATCH 01/13] stage : on-going refactoring

---
 lib/spack/spack/package.py    | 149 +++++++++++++++++-----------------
 lib/spack/spack/stage.py      |  68 ++++++++++------
 lib/spack/spack/test/stage.py | 122 +++++++++++-----------------
 3 files changed, 167 insertions(+), 172 deletions(-)

diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 8019b29cba..4fdc582479 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -847,93 +847,94 @@ def do_install(self,
                 make_jobs=make_jobs)
 
         start_time = time.time()
-        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)
-
-        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)
-
-                # 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()
+        with self.stage:
+            if not fake:
+                if not skip_patch:
+                    self.do_patch()
                 else:
-                    # Do the real install in the source directory.
-                    self.stage.chdir_to_source()
+                    self.do_stage()
 
-                    # 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):
-                        self.install(self.spec, self.prefix)
+            # 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)
 
-                # Ensure that something was actually installed.
-                self._sanity_check_install()
+            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)
 
-                # Move build log into install directory on success
-                if not fake:
-                    log_install_path = spack.install_layout.build_log_path(self.spec)
-                    install(log_path, log_install_path)
 
-                # On successful install, remove the stage.
-                if not keep_stage:
-                    self.stage.destroy()
+            def real_work():
+                try:
+                    tty.msg("Building %s." % self.name)
 
-                # Stop timer.
-                self._total_time = time.time() - start_time
-                build_time = self._total_time - self._fetch_time
+                    # Run the pre-install hook in the child process after
+                    # the directory is created.
+                    spack.hooks.pre_install(self)
 
-                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)
+                    # 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 ProcessError, e:
-                # Annotate with location of build log.
-                e.build_log = log_path
-                cleanup()
-                raise e
+                        # 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):
+                            self.install(self.spec, self.prefix)
 
-            except:
-                # other exceptions just clean up and raise.
-                cleanup()
-                raise
+                    # Ensure that something was actually installed.
+                    self._sanity_check_install()
 
-        # Set parallelism before starting build.
-        self.make_jobs = make_jobs
+                    # Move build log into install directory on success
+                    if not fake:
+                        log_install_path = spack.install_layout.build_log_path(self.spec)
+                        install(log_path, log_install_path)
 
-        # Do the build.
-        spack.build_environment.fork(self, real_work)
+                    # On successful install, remove the stage.
+                    if not keep_stage:
+                        self.stage.destroy()
 
-        # 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)
+                    # Stop timer.
+                    self._total_time = time.time() - start_time
+                    build_time = self._total_time - self._fetch_time
 
-        # Once everything else is done, run post install hooks
-        spack.hooks.post_install(self)
+                    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
+
+                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):
diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py
index f217450d42..96b1eaf3f2 100644
--- a/lib/spack/spack/stage.py
+++ b/lib/spack/spack/stage.py
@@ -96,12 +96,44 @@ def __init__(self, url_or_fetch_strategy, **kwargs):
         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
+        # 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)
+
+        self.delete_on_exit = True
+
+    def __enter__(self):
+        """
+        Entering a stage context will create the stage directory
+        """
+        # FIXME : if _setup is used only here, then it makes no sense to retain the function
         self._setup()
+        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 _cleanup_dead_links(self):
         """Remove any dead links in the stage directory."""
@@ -163,35 +195,17 @@ def _setup(self):
         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
+            if self._need_to_create_path():
                 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)
+                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)
+            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)
@@ -389,6 +403,12 @@ def source_path(self):
     def path(self):
         return self[0].path
 
+    def __enter__(self):
+        return self[0].__enter__()
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        return self[0].__exit__(exc_type, exc_val, exc_tb)
+
     def chdir_to_source(self):
         return self[0].chdir_to_source()
 
diff --git a/lib/spack/spack/test/stage.py b/lib/spack/spack/test/stage.py
index c1b2a2a573..a7314eba4c 100644
--- a/lib/spack/spack/test/stage.py
+++ b/lib/spack/spack/test/stage.py
@@ -192,116 +192,90 @@ def check_destroy(self, stage, stage_name):
 
     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)
+        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)
 
-        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.")
 
-        # 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))
 
-        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()
+            # 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)

From 726b350689bf6da7b82eec170001adc8100beb71 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Wed, 2 Mar 2016 12:52:38 +0100
Subject: [PATCH 02/13] test : fixed failing unit tests

---
 lib/spack/spack/mirror.py               | 49 +++++++++----------
 lib/spack/spack/stage.py                |  7 ++-
 lib/spack/spack/test/configure_guess.py | 12 ++---
 lib/spack/spack/test/git_fetch.py       | 31 +++++++------
 lib/spack/spack/test/hg_fetch.py        | 31 +++++++------
 lib/spack/spack/test/link_tree.py       |  6 +--
 lib/spack/spack/test/mirror.py          | 62 ++++++++++++-------------
 lib/spack/spack/test/svn_fetch.py       | 31 +++++++------
 8 files changed, 116 insertions(+), 113 deletions(-)

diff --git a/lib/spack/spack/mirror.py b/lib/spack/spack/mirror.py
index fa29e20803..bc8870926f 100644
--- a/lib/spack/spack/mirror.py
+++ b/lib/spack/spack/mirror.py
@@ -168,32 +168,33 @@ def create(path, specs, **kwargs):
         pkg = spec.package
         tty.msg("Adding package {pkg} to mirror".format(pkg=spec.format("$_$@")))
         try:
-            for ii, stage in enumerate(pkg.stage):
-                fetcher = stage.fetcher
-                if ii == 0:
-                    # create a subdirectory for the current package@version
-                    archive_path = os.path.abspath(join_path(mirror_root, mirror_archive_path(spec, fetcher)))
-                    name = spec.format("$_$@")
-                else:
-                    resource = stage.resource
-                    archive_path = join_path(subdir, suggest_archive_basename(resource))
-                    name = "{resource} ({pkg}).".format(resource=resource.name, pkg=spec.format("$_$@"))
-                subdir = os.path.dirname(archive_path)
-                mkdirp(subdir)
+            with pkg.stage:
+                for ii, stage in enumerate(pkg.stage):
+                    fetcher = stage.fetcher
+                    if ii == 0:
+                        # create a subdirectory for the current package@version
+                        archive_path = os.path.abspath(join_path(mirror_root, mirror_archive_path(spec, fetcher)))
+                        name = spec.format("$_$@")
+                    else:
+                        resource = stage.resource
+                        archive_path = join_path(subdir, suggest_archive_basename(resource))
+                        name = "{resource} ({pkg}).".format(resource=resource.name, pkg=spec.format("$_$@"))
+                    subdir = os.path.dirname(archive_path)
+                    mkdirp(subdir)
 
-                if os.path.exists(archive_path):
-                    tty.msg("{name} : already added".format(name=name))
-                else:
-                    everything_already_exists = False
-                    fetcher.fetch()
-                    if not kwargs.get('no_checksum', False):
-                        fetcher.check()
-                        tty.msg("{name} : checksum passed".format(name=name))
+                    if os.path.exists(archive_path):
+                        tty.msg("{name} : already added".format(name=name))
+                    else:
+                        everything_already_exists = False
+                        fetcher.fetch()
+                        if not kwargs.get('no_checksum', False):
+                            fetcher.check()
+                            tty.msg("{name} : checksum passed".format(name=name))
 
-                    # Fetchers have to know how to archive their files.  Use
-                    # that to move/copy/create an archive in the mirror.
-                    fetcher.archive(archive_path)
-                    tty.msg("{name} : added".format(name=name))
+                        # Fetchers have to know how to archive their files.  Use
+                        # that to move/copy/create an archive in the mirror.
+                        fetcher.archive(archive_path)
+                        tty.msg("{name} : added".format(name=name))
 
             if everything_already_exists:
                 present.append(spec)
diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py
index 96b1eaf3f2..956d1c8706 100644
--- a/lib/spack/spack/stage.py
+++ b/lib/spack/spack/stage.py
@@ -404,10 +404,13 @@ def path(self):
         return self[0].path
 
     def __enter__(self):
-        return self[0].__enter__()
+        for item in self:
+            item.__enter__()
+        return self
 
     def __exit__(self, exc_type, exc_val, exc_tb):
-        return self[0].__exit__(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()
diff --git a/lib/spack/spack/test/configure_guess.py b/lib/spack/spack/test/configure_guess.py
index a4e8565b62..bc2332acc2 100644
--- a/lib/spack/spack/test/configure_guess.py
+++ b/lib/spack/spack/test/configure_guess.py
@@ -52,8 +52,6 @@ def setUp(self):
 
     def tearDown(self):
         shutil.rmtree(self.tmpdir, ignore_errors=True)
-        if self.stage:
-            self.stage.destroy()
         os.chdir(self.orig_dir)
 
 
@@ -64,12 +62,12 @@ def check_archive(self, filename, system):
 
         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/git_fetch.py b/lib/spack/spack/test/git_fetch.py
index d84433176a..caa076823e 100644
--- a/lib/spack/spack/test/git_fetch.py
+++ b/lib/spack/spack/test/git_fetch.py
@@ -76,26 +76,27 @@ def try_fetch(self, rev, test_file, args):
         """
         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..75aa7ab17e 100644
--- a/lib/spack/spack/test/hg_fetch.py
+++ b/lib/spack/spack/test/hg_fetch.py
@@ -68,26 +68,27 @@ def try_fetch(self, rev, test_file, args):
         """
         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/link_tree.py b/lib/spack/spack/test/link_tree.py
index 886b7ef4c5..7b67e873dd 100644
--- a/lib/spack/spack/test/link_tree.py
+++ b/lib/spack/spack/test/link_tree.py
@@ -38,6 +38,8 @@ class LinkTreeTest(unittest.TestCase):
 
     def setUp(self):
         self.stage = Stage('link-tree-test')
+        # FIXME : possibly this test needs to be refactored to avoid the explicit call to __enter__ and __exit__
+        self.stage.__enter__()
 
         with working_dir(self.stage.path):
             touchp('source/1')
@@ -51,10 +53,8 @@ def setUp(self):
         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.__exit__(None, None, None)
 
 
     def check_file_link(self, filename):
diff --git a/lib/spack/spack/test/mirror.py b/lib/spack/spack/test/mirror.py
index f83cc8090c..9e2c631978 100644
--- a/lib/spack/spack/test/mirror.py
+++ b/lib/spack/spack/test/mirror.py
@@ -74,14 +74,14 @@ def set_up_package(self, name, MockRepoClass, url_attr):
 
 
     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 +97,36 @@ def check_mirror(self):
                 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
+                # 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)
+                    saved_checksum_setting = spack.do_checksum
+                    with pkg.stage:
+                        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)
+                            # 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)
+                            dcmp = dircmp(original_path, pkg.stage.source_path)
 
-                    # make sure there are no new files in the expanded tarball
-                    self.assertFalse(dcmp.right_only)
+                            # 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))
+                            # 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()
+                        finally:
+                            spack.do_checksum = saved_checksum_setting
+                            pkg.do_clean()
 
 
     def test_git_mirror(self):
diff --git a/lib/spack/spack/test/svn_fetch.py b/lib/spack/spack/test/svn_fetch.py
index 454a7f1d1f..6ac9e2f343 100644
--- a/lib/spack/spack/test/svn_fetch.py
+++ b/lib/spack/spack/test/svn_fetch.py
@@ -82,26 +82,27 @@ def try_fetch(self, rev, test_file, args):
         """
         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):

From 65b2a24f7c12380d0815e5705d1c1e66e5fd22a9 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Wed, 2 Mar 2016 13:03:40 +0100
Subject: [PATCH 03/13] stage : removed _setup method

---
 lib/spack/spack/stage.py | 57 ++++++++++++++++++----------------------
 1 file changed, 25 insertions(+), 32 deletions(-)

diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py
index 956d1c8706..48770fb407 100644
--- a/lib/spack/spack/stage.py
+++ b/lib/spack/spack/stage.py
@@ -111,9 +111,32 @@ def __init__(self, url_or_fetch_strategy, **kwargs):
     def __enter__(self):
         """
         Entering a stage context will create 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.
         """
-        # FIXME : if _setup is used only here, then it makes no sense to retain the function
-        self._setup()
+        # Create the top-level stage directory
+        mkdirp(spack.stage_path)
+        self._cleanup_dead_links()
+
+        # If this is a temporary stage, them make the temp directory
+        if self.tmp_root:
+            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._need_to_create_path():
+                mkdirp(self.path)
+
+        # Make sure we can actually do something with the stage we made.
+        ensure_access(self.path)
+
         return self
 
     def __exit__(self, exc_type, exc_val, exc_tb):
@@ -180,36 +203,6 @@ def _need_to_create_path(self):
 
         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 temporary stage, them make the temp directory
-        if self.tmp_root:
-            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._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."""

From d649b715edae0871254695e2deacf078554a1475 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Wed, 2 Mar 2016 13:16:04 +0100
Subject: [PATCH 04/13] stage : updated functions doc _cleanup_dead_links :
 fixed minor bug

---
 lib/spack/spack/stage.py | 41 +++++++++++++++++-----------------------
 1 file changed, 17 insertions(+), 24 deletions(-)

diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py
index 48770fb407..900acd664d 100644
--- a/lib/spack/spack/stage.py
+++ b/lib/spack/spack/stage.py
@@ -42,33 +42,26 @@
 
 
 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:
+    """
+    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:
 
-       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.
+    ```
+    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.
+    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.
+    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.
+    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):
@@ -164,7 +157,7 @@ def _cleanup_dead_links(self):
             path = join_path(spack.stage_path, file)
             if os.path.islink(path):
                 real_path = os.path.realpath(path)
-                if not os.path.exists(path):
+                if not os.path.exists(real_path):
                     os.unlink(path)
 
     def _need_to_create_path(self):

From 9001b9ed3c01f8ccaceaca60d6a34c3551f77240 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Wed, 2 Mar 2016 15:56:09 +0100
Subject: [PATCH 05/13] package : minor syntax fixes mirror : extracted
 add_single_spec from create

---
 lib/spack/spack/mirror.py  | 98 ++++++++++++++++++++------------------
 lib/spack/spack/package.py |  4 +-
 2 files changed, 54 insertions(+), 48 deletions(-)

diff --git a/lib/spack/spack/mirror.py b/lib/spack/spack/mirror.py
index bc8870926f..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,58 +157,65 @@ 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:
-            with pkg.stage:
-                for ii, stage in enumerate(pkg.stage):
-                    fetcher = stage.fetcher
-                    if ii == 0:
-                        # create a subdirectory for the current package@version
-                        archive_path = os.path.abspath(join_path(mirror_root, mirror_archive_path(spec, fetcher)))
-                        name = spec.format("$_$@")
-                    else:
-                        resource = stage.resource
-                        archive_path = join_path(subdir, suggest_archive_basename(resource))
-                        name = "{resource} ({pkg}).".format(resource=resource.name, pkg=spec.format("$_$@"))
-                    subdir = os.path.dirname(archive_path)
-                    mkdirp(subdir)
+        add_single_spec(spec, mirror_root, categories, **kwargs)
 
-                    if os.path.exists(archive_path):
-                        tty.msg("{name} : already added".format(name=name))
-                    else:
-                        everything_already_exists = False
-                        fetcher.fetch()
-                        if not kwargs.get('no_checksum', False):
-                            fetcher.check()
-                            tty.msg("{name} : checksum passed".format(name=name))
+    return categories['present'], categories['mirrored'], categories['error']
 
-                        # Fetchers have to know how to archive their files.  Use
-                        # that to move/copy/create an archive in the mirror.
-                        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()
+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
+                    archive_path = os.path.abspath(join_path(mirror_root, mirror_archive_path(spec, fetcher)))
+                    name = spec.format("$_$@")
+                else:
+                    resource = stage.resource
+                    archive_path = join_path(subdir, suggest_archive_basename(resource))
+                    name = "{resource} ({pkg}).".format(resource=resource.name, pkg=spec.format("$_$@"))
+                subdir = os.path.dirname(archive_path)
+                mkdirp(subdir)
 
-    return (present, mirrored, error)
+                if os.path.exists(archive_path):
+                    tty.msg("{name} : already added".format(name=name))
+                else:
+                    spec_exists_in_mirror = False
+                    fetcher.fetch()
+                    if not kwargs.get('no_checksum', False):
+                        fetcher.check()
+                        tty.msg("{name} : checksum passed".format(name=name))
+
+                    # Fetchers have to know how to archive their files.  Use
+                    # that to move/copy/create an archive in the mirror.
+                    fetcher.archive(archive_path)
+                    tty.msg("{name} : added".format(name=name))
+
+        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 4fdc582479..29b781a749 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -732,7 +732,7 @@ def do_patch(self):
         # 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()
@@ -912,7 +912,7 @@ def real_work():
                             % (_hms(self._fetch_time), _hms(build_time), _hms(self._total_time)))
                     print_pkg(self.prefix)
 
-                except ProcessError, e:
+                except ProcessError as e:
                     # Annotate with location of build log.
                     e.build_log = log_path
                     cleanup()

From 901e4851b993f73c4bdd92b85b2469d6673b7617 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Wed, 2 Mar 2016 16:19:32 +0100
Subject: [PATCH 06/13] _cleanup_dead_links : factored method into a function
 and put it in llnl.filesystem

---
 lib/spack/llnl/util/filesystem.py | 19 +++++++++++++++++--
 lib/spack/spack/stage.py          | 20 ++++----------------
 2 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py
index 24cfbfde71..4a708b639a 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']
 
 import os
 import sys
@@ -235,7 +235,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)
 
@@ -339,3 +339,18 @@ 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)
\ No newline at end of file
diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py
index 900acd664d..e910643192 100644
--- a/lib/spack/spack/stage.py
+++ b/lib/spack/spack/stage.py
@@ -98,7 +98,7 @@ def __init__(self, url_or_fetch_strategy, **kwargs):
         # 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 __enter__(self):
@@ -113,20 +113,17 @@ def __enter__(self):
         """
         # Create the top-level stage directory
         mkdirp(spack.stage_path)
-        self._cleanup_dead_links()
+        remove_dead_links(spack.stage_path)
 
-        # If this is a temporary stage, them make the temp directory
+        # 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.tmp_root:
             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._need_to_create_path():
                 mkdirp(self.path)
-
         # Make sure we can actually do something with the stage we made.
         ensure_access(self.path)
 
@@ -151,15 +148,6 @@ def __exit__(self, exc_type, exc_val, exc_tb):
         if self.delete_on_exit:
             self.destroy()
 
-    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(real_path):
-                    os.unlink(path)
-
     def _need_to_create_path(self):
         """Makes sure nothing weird has happened since the last time we
            looked at path.  Returns True if path already exists and is ok.

From 4d63544fe918393de9265f8879bb0199a542cc3b Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Wed, 2 Mar 2016 16:55:57 +0100
Subject: [PATCH 07/13] remove_link_tree : moved to llnl.util.filesystem

---
 lib/spack/llnl/util/filesystem.py | 20 ++++++++++++++++++--
 lib/spack/spack/stage.py          | 21 +++------------------
 2 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py
index 4a708b639a..015eeb9aa1 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', 'remove_dead_links']
+           'FileFilter', 'change_sed_delimiter', 'is_exe', 'force_symlink', 'remove_dead_links', 'remove_linked_tree']
 
 import os
 import sys
@@ -353,4 +353,20 @@ def remove_dead_links(root):
         if os.path.islink(path):
             real_path = os.path.realpath(path)
             if not os.path.exists(real_path):
-                os.unlink(path)
\ No newline at end of file
+                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/stage.py b/lib/spack/spack/stage.py
index e910643192..e87b822a8f 100644
--- a/lib/spack/spack/stage.py
+++ b/lib/spack/spack/stage.py
@@ -114,15 +114,13 @@ def __enter__(self):
         # 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.tmp_root:
-            if self._need_to_create_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:
-            if self._need_to_create_path():
+            else:
                 mkdirp(self.path)
         # Make sure we can actually do something with the stage we made.
         ensure_access(self.path)
@@ -436,19 +434,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):

From ca41909ec52ab2b8f82ca91421d3882300531ac2 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Wed, 2 Mar 2016 17:19:27 +0100
Subject: [PATCH 08/13] package : removed do_clean()

---
 lib/spack/spack/cmd/clean.py      |  2 +-
 lib/spack/spack/package.py        |  9 +-------
 lib/spack/spack/test/git_fetch.py |  4 ----
 lib/spack/spack/test/hg_fetch.py  |  3 ---
 lib/spack/spack/test/mirror.py    | 38 ++++++++++++-------------------
 lib/spack/spack/test/svn_fetch.py |  4 ----
 6 files changed, 17 insertions(+), 43 deletions(-)

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/package.py b/lib/spack/spack/package.py
index 29b781a749..fa64b2c047 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -1138,13 +1138,6 @@ def do_restage(self):
         """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)
@@ -1181,7 +1174,7 @@ def fetch_remote_versions(self):
         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/test/git_fetch.py b/lib/spack/spack/test/git_fetch.py
index caa076823e..6d6a67a1d3 100644
--- a/lib/spack/spack/test/git_fetch.py
+++ b/lib/spack/spack/test/git_fetch.py
@@ -52,19 +52,15 @@ def setUp(self):
         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
diff --git a/lib/spack/spack/test/hg_fetch.py b/lib/spack/spack/test/hg_fetch.py
index 75aa7ab17e..d884ed78a0 100644
--- a/lib/spack/spack/test/hg_fetch.py
+++ b/lib/spack/spack/test/hg_fetch.py
@@ -49,13 +49,10 @@ def setUp(self):
         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:
diff --git a/lib/spack/spack/test/mirror.py b/lib/spack/spack/test/mirror.py
index 9e2c631978..f117e04242 100644
--- a/lib/spack/spack/test/mirror.py
+++ b/lib/spack/spack/test/mirror.py
@@ -104,29 +104,21 @@ def check_mirror(self):
 
                     saved_checksum_setting = spack.do_checksum
                     with pkg.stage:
-                        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()
+                        # 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/svn_fetch.py b/lib/spack/spack/test/svn_fetch.py
index 6ac9e2f343..13a00f5df7 100644
--- a/lib/spack/spack/test/svn_fetch.py
+++ b/lib/spack/spack/test/svn_fetch.py
@@ -51,13 +51,10 @@ def setUp(self):
         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 +67,6 @@ def get_rev():
                     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

From 21cd05aad5e8f45a7ebac7b213855a16685d229f Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Wed, 2 Mar 2016 17:28:21 +0100
Subject: [PATCH 09/13] package : updated doc

---
 lib/spack/spack/package.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index fa64b2c047..0214dcd771 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -291,7 +291,6 @@ class SomePackage(Package):
 
     .. code-block:: python
 
-       p.do_clean()              # removes the stage directory entirely
        p.do_restage()            # removes the build directory and
                                  # re-expands the archive.
 

From 717bcb862428a1b1a3b9c70f925c3fe6c325f622 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Thu, 3 Mar 2016 09:17:49 +0100
Subject: [PATCH 10/13] test : optimized import statements

---
 lib/spack/spack/test/concretize.py         |  2 --
 lib/spack/spack/test/config.py             |  6 ++---
 lib/spack/spack/test/configure_guess.py    |  9 ++------
 lib/spack/spack/test/database.py           | 11 +++++----
 lib/spack/spack/test/directory_layout.py   | 13 +++++------
 lib/spack/spack/test/git_fetch.py          | 11 ++-------
 lib/spack/spack/test/hg_fetch.py           | 10 +++------
 lib/spack/spack/test/install.py            |  9 ++------
 lib/spack/spack/test/link_tree.py          |  7 ++----
 lib/spack/spack/test/lock.py               |  8 +++----
 lib/spack/spack/test/make_executable.py    |  6 ++---
 lib/spack/spack/test/mirror.py             |  5 ++---
 lib/spack/spack/test/mock_packages_test.py |  8 +++----
 lib/spack/spack/test/mock_repo.py          |  4 ----
 lib/spack/spack/test/multimethod.py        |  5 +----
 lib/spack/spack/test/namespace_trie.py     |  1 +
 lib/spack/spack/test/optional_deps.py      |  4 +---
 lib/spack/spack/test/packages.py           | 26 ++++++----------------
 lib/spack/spack/test/python_version.py     |  3 +--
 lib/spack/spack/test/spec_dag.py           |  2 --
 lib/spack/spack/test/spec_semantics.py     |  1 -
 lib/spack/spack/test/spec_syntax.py        |  3 ++-
 lib/spack/spack/test/stage.py              |  8 +++----
 lib/spack/spack/test/svn_fetch.py          | 14 ++++--------
 lib/spack/spack/test/tally_plugin.py       |  4 ++--
 lib/spack/spack/test/unit_install.py       |  3 ++-
 lib/spack/spack/test/url_extrapolate.py    |  3 ---
 lib/spack/spack/test/url_parse.py          |  2 +-
 lib/spack/spack/test/url_substitution.py   |  1 -
 lib/spack/spack/test/versions.py           |  1 +
 lib/spack/spack/test/yaml.py               |  1 +
 31 files changed, 62 insertions(+), 129 deletions(-)

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 bc2332acc2..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):
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 6d6a67a1d3..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):
diff --git a/lib/spack/spack/test/hg_fetch.py b/lib/spack/spack/test/hg_fetch.py
index d884ed78a0..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):
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 7b67e873dd..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,8 +36,7 @@ class LinkTreeTest(unittest.TestCase):
 
     def setUp(self):
         self.stage = Stage('link-tree-test')
-        # FIXME : possibly this test needs to be refactored to avoid the explicit call to __enter__ and __exit__
-        self.stage.__enter__()
+        self.stage.create()
 
         with working_dir(self.stage.path):
             touchp('source/1')
@@ -54,7 +51,7 @@ def setUp(self):
         self.link_tree = LinkTree(source_path)
 
     def tearDown(self):
-        self.stage.__exit__(None, None, None)
+        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 @@
 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 f117e04242..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 *
 
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 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 d0d809004d..83fb34bf76 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..07ff0b21af 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):
@@ -77,30 +75,20 @@ def test_package_class_names(self):
     #
 
     def test_import_package(self):
-        import spack.pkg.builtin.mock.mpich
+        pass
 
 
     def test_import_package_as(self):
-        import spack.pkg.builtin.mock.mpich as mp
+        pass
 
 
     def test_import_class_from_package(self):
-        from spack.pkg.builtin.mock.mpich import Mpich
+        pass
 
 
     def test_import_module_from_package(self):
-        from spack.pkg.builtin.mock import mpich
+        pass
 
 
     def test_import_namespace_container_modules(self):
-        import spack.pkg
-        import spack.pkg as p
-        from spack import pkg
-
-        import spack.pkg.builtin
-        import spack.pkg.builtin as b
-        from spack.pkg import builtin
-
-        import spack.pkg.builtin.mock
-        import spack.pkg.builtin.mock as m
-        from spack.pkg.builtin import mock
+        pass
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 @@
 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 @@
 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 a7314eba4c..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
 
diff --git a/lib/spack/spack/test/svn_fetch.py b/lib/spack/spack/test/svn_fetch.py
index 13a00f5df7..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.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 spack.version import ver
+from spack.test.mock_packages_test import *
+from llnl.util.filesystem import *
 
 
 class SvnFetchTest(MockPackagesTest):
diff --git a/lib/spack/spack/test/tally_plugin.py b/lib/spack/spack/test/tally_plugin.py
index 9ca898c47c..eb1e4a3240 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 @@
 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 @@
 """
 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 @@
 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 = """\

From 353d12d2e6e24c0f62795dd9e649a801b566c4d8 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Thu, 3 Mar 2016 09:18:47 +0100
Subject: [PATCH 11/13] stage : added create function as decided in #477
 comments

---
 lib/spack/spack/stage.py | 50 ++++++++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 20 deletions(-)

diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py
index e87b822a8f..5d8b09fa9e 100644
--- a/lib/spack/spack/stage.py
+++ b/lib/spack/spack/stage.py
@@ -105,26 +105,10 @@ def __enter__(self):
         """
         Entering a stage context will create 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.
+        Returns:
+            self
         """
-        # 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)
-
+        self.create()
         return self
 
     def __exit__(self, exc_type, exc_val, exc_tb):
@@ -314,8 +298,34 @@ def restage(self):
         """
         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

From 5ef36215569e1151164ecb630011de425387732a Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Thu, 3 Mar 2016 10:10:30 +0100
Subject: [PATCH 12/13] stage : minor syntax fix

---
 lib/spack/spack/stage.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py
index 5d8b09fa9e..985043c103 100644
--- a/lib/spack/spack/stage.py
+++ b/lib/spack/spack/stage.py
@@ -246,7 +246,7 @@ def fetch(self, mirror_only=False):
                 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

From db92699c02f9aaf435d6bc073c89cd2c75d8824c Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Thu, 3 Mar 2016 11:05:36 +0100
Subject: [PATCH 13/13] test : fixed tests that were broken by import
 optimization

---
 lib/spack/spack/test/packages.py | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/lib/spack/spack/test/packages.py b/lib/spack/spack/test/packages.py
index 07ff0b21af..f0b5e05f3b 100644
--- a/lib/spack/spack/test/packages.py
+++ b/lib/spack/spack/test/packages.py
@@ -75,20 +75,30 @@ def test_package_class_names(self):
     #
 
     def test_import_package(self):
-        pass
+        import spack.pkg.builtin.mock.mpich
 
 
     def test_import_package_as(self):
-        pass
+        import spack.pkg.builtin.mock.mpich as mp
 
 
     def test_import_class_from_package(self):
-        pass
+        from spack.pkg.builtin.mock.mpich import Mpich
 
 
     def test_import_module_from_package(self):
-        pass
+        from spack.pkg.builtin.mock import mpich
 
 
     def test_import_namespace_container_modules(self):
-        pass
+        import spack.pkg
+        import spack.pkg as p
+        from spack import pkg
+
+        import spack.pkg.builtin
+        import spack.pkg.builtin as b
+        from spack.pkg import builtin
+
+        import spack.pkg.builtin.mock
+        import spack.pkg.builtin.mock as m
+        from spack.pkg.builtin import mock