From b813b40c4fdccd593118024083f1e42b472f4e32 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Fri, 25 Apr 2014 15:29:39 -0700 Subject: 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) --- lib/spack/spack/cmd/checksum.py | 8 +++++--- lib/spack/spack/cmd/create.py | 4 ++-- lib/spack/spack/cmd/install.py | 10 ++++------ lib/spack/spack/cmd/uninstall.py | 2 +- lib/spack/spack/package.py | 32 ++++++++++++++++++-------------- 5 files changed, 30 insertions(+), 26 deletions(-) (limited to 'lib') 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" -- cgit v1.2.3-60-g2f50