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)
This commit is contained in:
Todd Gamblin 2014-04-25 15:29:39 -07:00
parent 0c99d9ddc3
commit b813b40c4f
5 changed files with 30 additions and 26 deletions

View file

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

View file

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

View file

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

View file

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

View file

@ -317,12 +317,6 @@ class SomePackage(Package):
"""By default we build in parallel. Subclasses can override this.""" """By default we build in parallel. Subclasses can override this."""
parallel = True 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 """Dirty hack for forcing packages with uninterpretable URLs
TODO: get rid of this. TODO: get rid of this.
""" """
@ -532,8 +526,6 @@ def url_for_version(self, version):
def remove_prefix(self): def remove_prefix(self):
"""Removes the prefix for a package along with any empty parent directories.""" """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) spack.install_layout.remove_path_for_spec(self.spec)
@ -627,10 +619,14 @@ def do_patch(self):
touch(good_file) touch(good_file)
def do_install(self): def do_install(self, **kwargs):
"""This class should call this version of the install method. """This class should call this version of the install method.
Package implementations should override install(). 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: if not self.spec.concrete:
raise ValueError("Can only install concrete packages.") raise ValueError("Can only install concrete packages.")
@ -638,7 +634,7 @@ def do_install(self):
tty.msg("%s is already installed." % self.name) tty.msg("%s is already installed." % self.name)
return return
if not self.ignore_dependencies: if not ignore_deps:
self.do_install_dependencies() self.do_install_dependencies()
self.do_patch() self.do_patch()
@ -685,8 +681,14 @@ def do_install(self):
os._exit(0) os._exit(0)
except: except:
# If anything else goes wrong, get rid of the install dir. if not keep_prefix:
# If anything goes wrong, remove the install prefix
self.remove_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 raise
# Parent process just waits for the child to complete. If the # Parent process just waits for the child to complete. If the
@ -717,11 +719,13 @@ def install(self, spec, prefix):
raise InstallError("Package %s provides no install method!" % self.name) 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: if not self.installed:
raise InstallError(self.name + " is not installed.") raise InstallError(self.name + " is not installed.")
if not self.ignore_dependencies: if not force:
deps = self.installed_dependents deps = self.installed_dependents
if deps: raise InstallError( if deps: raise InstallError(
"Cannot uninstall %s. The following installed packages depend on it: %s" "Cannot uninstall %s. The following installed packages depend on it: %s"