Make install command reusable within single Spack run

- install and probably other commands were designed to run once, but now
  we can to test them from within Spack with SpackCommand

- cmd/install.py assumed that it could modify do_install in PackageBase
  and leave it that way; this makes the decorator temporary

- package.py didn't properly initialize its stage if the same package had
  been built successfully before (and the stage removed).
  - manage stage lifecycle better and remember when Package needs to
    re-create the stage

3 files changed, 48 insertions, 21 deletions

diff --git a/lib/spack/spack/cmd/install.py b/lib/spack/spack/cmd/install.py
index 9f35837220..ec1efdc9cf 100644
--- a/lib/spack/spack/cmd/install.py
+++ b/lib/spack/spack/cmd/install.py
@@ -334,31 +334,41 @@ def install(parser, args, **kwargs):
         tty.error('The `spack install` command requires a spec to install.')
 
     for spec in specs:
+        saved_do_install = PackageBase.do_install
+        decorator = lambda fn: fn
+
         # Check if we were asked to produce some log for dashboards
         if args.log_format is not None:
             # Compute the filename for logging
             log_filename = args.log_file
             if not log_filename:
                 log_filename = default_log_file(spec)
+
             # Create the test suite in which to log results
             test_suite = TestSuite(spec)
-            # Decorate PackageBase.do_install to get installation status
-            PackageBase.do_install = junit_output(
-                spec, test_suite
-            )(PackageBase.do_install)
+
+            # Temporarily decorate PackageBase.do_install to monitor
+            # recursive calls.
+            decorator = junit_output(spec, test_suite)
 
         # Do the actual installation
-        if args.things_to_install == 'dependencies':
-            # Install dependencies as-if they were installed
-            # for root (explicit=False in the DB)
-            kwargs['explicit'] = False
-            for s in spec.dependencies():
-                p = spack.repo.get(s)
-                p.do_install(**kwargs)
-        else:
-            package = spack.repo.get(spec)
-            kwargs['explicit'] = True
-            package.do_install(**kwargs)
+        try:
+            # decorate the install if necessary
+            PackageBase.do_install = decorator(PackageBase.do_install)
+
+            if args.things_to_install == 'dependencies':
+                # Install dependencies as-if they were installed
+                # for root (explicit=False in the DB)
+                kwargs['explicit'] = False
+                for s in spec.dependencies():
+                    p = spack.repo.get(s)
+                    p.do_install(**kwargs)
+            else:
+                package = spack.repo.get(spec)
+                kwargs['explicit'] = True
+                package.do_install(**kwargs)
+        finally:
+            PackageBase.do_install = saved_do_install
 
         # Dump log file if asked to
         if args.log_format is not None:
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index ef1c411f66..64db8013d0 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -760,10 +760,6 @@ class PackageBase(with_metaclass(PackageMeta, object)):
             # Append the item to the composite
             composite_stage.append(stage)
 
-        # Create stage on first access.  Needed because fetch, stage,
-        # patch, and install can be called independently of each
-        # other, so `with self.stage:` in do_install isn't sufficient.
-        composite_stage.create()
         return composite_stage
 
     @property
@@ -772,6 +768,12 @@ class PackageBase(with_metaclass(PackageMeta, object)):
             raise ValueError("Can only get a stage for a concrete package.")
         if self._stage is None:
             self._stage = self._make_stage()
+
+        # Create stage on first access.  Needed because fetch, stage,
+        # patch, and install can be called independently of each
+        # other, so `with self.stage:` in do_install isn't sufficient.
+        self._stage.create()
+
         return self._stage
 
     @stage.setter
@@ -1390,6 +1392,11 @@ class PackageBase(with_metaclass(PackageMeta, object)):
             if not keep_prefix:
                 self.remove_prefix()
 
+            # The subprocess *may* have removed the build stage. Mark it
+            # not created so that the next time self.stage is invoked, we
+            # check the filesystem for it.
+            self.stage.created = False
+
     def check_for_unfinished_installation(
             self, keep_prefix=False, restage=False):
         """Check for leftover files from partially-completed prior install to
diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py
index 09f18f7d67..f9995bce6c 100644
--- a/lib/spack/spack/stage.py
+++ b/lib/spack/spack/stage.py
@@ -236,6 +236,10 @@ class Stage(object):
 
             self._lock = Stage.stage_locks[self.name]
 
+        # When stages are reused, we need to know whether to re-create
+        # it.  This marks whether it has been created/destroyed.
+        self.created = False
+
     def __enter__(self):
         """
         Entering a stage context will create the stage directory
@@ -521,6 +525,7 @@ class Stage(object):
                 mkdirp(self.path)
         # Make sure we can actually do something with the stage we made.
         ensure_access(self.path)
+        self.created = True
 
     def destroy(self):
         """Removes this stage directory."""
@@ -532,6 +537,9 @@ class Stage(object):
         except OSError:
             os.chdir(os.path.dirname(self.path))
 
+        # mark as destroyed
+        self.created = False
+
 
 class ResourceStage(Stage):
 
@@ -573,8 +581,9 @@ class ResourceStage(Stage):
                 shutil.move(source_path, destination_path)
 
 
-@pattern.composite(method_list=['fetch', 'create', 'check', 'expand_archive',
-                                'restage', 'destroy', 'cache_local'])
+@pattern.composite(method_list=[
+    'fetch', 'create', 'created', 'check', 'expand_archive', 'restage',
+    'destroy', 'cache_local'])
 class StageComposite:
     """Composite for Stage type objects. The first item in this composite is
     considered to be the root package, and operations that return a value are
@@ -623,6 +632,7 @@ class DIYStage(object):
         self.archive_file = None
         self.path = path
         self.source_path = path
+        self.created = True
 
     def chdir(self):
         if os.path.isdir(self.path):