Merge pull request #9 in SCALE/spack from bugfix/SPACK-17-delete-install-dir-on-error to develop

# By Todd Gamblin
# Via Todd Gamblin
* commit 'b813b40c4fdccd593118024083f1e42b472f4e32':
  Clean up commands and get rid of inconsistent --dirty options
  Fix SPACK-17: Remove install prefix if install() fails

lib/spack/spack/cmd/checksum.py

@@ -44,7 +44,7 @@ def setup_parser(subparser):
     subparser.add_argument(
         'package', metavar='PACKAGE', help='Package to list versions for')
     subparser.add_argument(
-        '-d', '--dirty', action='store_true', dest='dirty',
+        '--keep-stage', action='store_true', dest='keep_stage',
         help="Don't clean up staging area when command completes.")
     subparser.add_argument(
         'versions', nargs=argparse.REMAINDER, help='Versions to generate checksums for')
@@ -54,6 +54,8 @@ def get_checksums(versions, urls, **kwargs):
     # Allow commands like create() to do some analysis on the first
     # archive after it is downloaded.
     first_stage_function = kwargs.get('first_stage_function', None)
+    keep_stage = kwargs.get('keep_stage', False)
+
 
     tty.msg("Downloading...")
     hashes = []
@@ -71,7 +73,7 @@ def get_checksums(versions, urls, **kwargs):
             continue
 
         finally:
-            if not kwargs.get('dirty', False):
+            if not keep_stage:
                 stage.destroy()
 
     return zip(versions, hashes)
@@ -110,7 +112,7 @@ def checksum(parser, args):
         return
 
     version_hashes = get_checksums(
-        versions[:archives_to_fetch], urls[:archives_to_fetch], dirty=args.dirty)
+        versions[:archives_to_fetch], urls[:archives_to_fetch], keep_stage=args.keep_stage)
 
     if not version_hashes:
         tty.die("Could not fetch any available versions for %s." % pkg.name)

lib/spack/spack/cmd/create.py

@@ -85,7 +85,7 @@ def install(self, spec, prefix):
 def setup_parser(subparser):
     subparser.add_argument('url', nargs='?', help="url of package archive")
     subparser.add_argument(
-        '-d', '--dirty', action='store_true', dest='dirty',
+        '--keep-stage', action='store_true', dest='keep_stage',
         help="Don't clean up staging area when command completes.")
     subparser.add_argument(
         '-f', '--force', action='store_true', dest='force',
@@ -174,7 +174,7 @@ def create(parser, args):
     guesser = ConfigureGuesser()
     ver_hash_tuples = spack.cmd.checksum.get_checksums(
         versions[:archives_to_fetch], urls[:archives_to_fetch],
-        first_stage_function=guesser, dirty=args.dirty)
+        first_stage_function=guesser, keep_stage=args.keep_stage)
 
     if not ver_hash_tuples:
         tty.die("Could not fetch any tarballs for %s." % name)

lib/spack/spack/cmd/install.py

@@ -32,10 +32,10 @@
 
 def setup_parser(subparser):
     subparser.add_argument(
-        '-i', '--ignore-dependencies', action='store_true', dest='ignore_dependencies',
+        '-i', '--ignore-dependencies', action='store_true', dest='ignore_deps',
         help="Do not try to install dependencies of requested packages.")
     subparser.add_argument(
-        '-d', '--dirty', action='store_true', dest='dirty',
+        '--keep-prefix', action='store_true', dest='keep_prefix',
         help="Don't clean up staging area when install completes.")
     subparser.add_argument(
         '-n', '--no-checksum', action='store_true', dest='no_checksum',
@@ -51,10 +51,8 @@ def install(parser, args):
     if args.no_checksum:
         spack.do_checksum = False
 
-    spack.ignore_dependencies = args.ignore_dependencies
     specs = spack.cmd.parse_specs(args.packages, concretize=True)
-
     for spec in specs:
         package = spack.db.get(spec)
-        package.dirty = args.dirty
-        package.do_install()
+        package.do_install(keep_prefix=args.keep_prefix,
+                           ignore_deps=args.ignore_deps)

lib/spack/spack/cmd/uninstall.py

@@ -68,4 +68,4 @@ def num_installed_deps(pkg):
 
     # Uninstall packages in order now.
     for pkg in pkgs:
-        pkg.do_uninstall()
+        pkg.do_uninstall(force=args.force)

lib/spack/spack/package.py

@@ -317,12 +317,6 @@ class SomePackage(Package):
     """By default we build in parallel.  Subclasses can override this."""
     parallel = True
 
-    """Remove tarball and build by default.  If this is true, leave them."""
-    dirty = False
-
-    """Controls whether install and uninstall check deps before running."""
-    ignore_dependencies = False
-
     """Dirty hack for forcing packages with uninterpretable URLs
        TODO: get rid of this.
     """
@@ -532,8 +526,6 @@ def url_for_version(self, version):
 
     def remove_prefix(self):
         """Removes the prefix for a package along with any empty parent directories."""
-        if self.dirty:
-            return
         spack.install_layout.remove_path_for_spec(self.spec)
 
 
@@ -627,10 +619,14 @@ def do_patch(self):
         touch(good_file)
 
 
-    def do_install(self):
+    def do_install(self, **kwargs):
         """This class should call this version of the install method.
            Package implementations should override install().
         """
+        # whether to keep the prefix on failure.  Default is to destroy it.
+        keep_prefix = kwargs.get('keep_prefix', False)
+        ignore_deps = kwargs.get('ignore_deps', False)
+
         if not self.spec.concrete:
             raise ValueError("Can only install concrete packages.")
 
@@ -638,7 +634,7 @@ def do_install(self):
             tty.msg("%s is already installed." % self.name)
             return
 
-        if not self.ignore_dependencies:
+        if not ignore_deps:
             self.do_install_dependencies()
 
         self.do_patch()
@@ -674,16 +670,25 @@ def do_install(self):
                         % self.name)
 
                 # On successful install, remove the stage.
-                # Leave if if there is an error
+                # Leave if there is an error
                 self.stage.destroy()
 
                 tty.msg("Successfully installed %s" % self.name)
                 print_pkg(self.prefix)
 
-                sys.exit(0)
+                # Use os._exit here to avoid raising a SystemExit exception,
+                # which interferes with unit tests.
+                os._exit(0)
 
-            except Exception, e:
+            except:
+                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)
                 raise
 
         # Parent process just waits for the child to complete.  If the
@@ -714,11 +719,13 @@ def install(self, spec, prefix):
         raise InstallError("Package %s provides no install method!" % self.name)
 
 
-    def do_uninstall(self):
+    def do_uninstall(self, **kwargs):
+        force = kwargs.get('force', False)
+
         if not self.installed:
             raise InstallError(self.name + " is not installed.")
 
-        if not self.ignore_dependencies:
+        if not force:
             deps = self.installed_dependents
             if deps: raise InstallError(
                 "Cannot uninstall %s. The following installed packages depend on it: %s"