Clean up commands and get rid of inconsistent --dirty options

- checksum  --dirty and create --dirty now changed to --keep-stage
- install   --dirty is now --keep-prefix
- uninstall --force now works properly

- commands use keyword args instead of package instance vars
  where possible (less weird package state)

5 files changed, 30 insertions, 26 deletions

diff --git a/lib/spack/spack/cmd/checksum.py b/lib/spack/spack/cmd/checksum.py
index 97ce91386b..f5cf0d0143 100644
--- a/lib/spack/spack/cmd/checksum.py
+++ b/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)
diff --git a/lib/spack/spack/cmd/create.py b/lib/spack/spack/cmd/create.py
index bc47b77258..8653fafa5f 100644
--- a/lib/spack/spack/cmd/create.py
+++ b/lib/spack/spack/cmd/create.py
@@ -85,7 +85,7 @@ class ${class_name}(Package):
 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)
diff --git a/lib/spack/spack/cmd/install.py b/lib/spack/spack/cmd/install.py
index 02194c1b3f..ea11cb89a9 100644
--- a/lib/spack/spack/cmd/install.py
+++ b/lib/spack/spack/cmd/install.py
@@ -32,10 +32,10 @@ description = "Build and install packages"
 
 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)
diff --git a/lib/spack/spack/cmd/uninstall.py b/lib/spack/spack/cmd/uninstall.py
index 7982892d81..df208b3a6a 100644
--- a/lib/spack/spack/cmd/uninstall.py
+++ b/lib/spack/spack/cmd/uninstall.py
@@ -68,4 +68,4 @@ def uninstall(parser, args):
 
     # Uninstall packages in order now.
     for pkg in pkgs:
-        pkg.do_uninstall()
+        pkg.do_uninstall(force=args.force)
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index d0c5b22211..48fa0e4955 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -317,12 +317,6 @@ class Package(object):
     """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 @@ class Package(object):
 
     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 @@ class Package(object):
         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 @@ class Package(object):
             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()
@@ -685,8 +681,14 @@ class Package(object):
                 os._exit(0)
 
             except:
-                # If anything else goes wrong, get rid of the install dir.
-                self.remove_prefix()
+                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
@@ -717,11 +719,13 @@ class Package(object):
         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"