hlrs-software-stack/spack
7
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Fixes #502. Create install prefix before build, clean up do_install.

Browse source 
- Fix bug introduced during merge of stage refactor.
  - install prefix was not created before build_environment.fork()
    - build_environment.fork() calls setup_dependent_environment
    - python's setup_dependent_environment can inadvertently create
      the install prefix before directory_layout expects it.

- Clean up Package.do_install:
  - simplify control flow: parent process now entirely responsible for
    creating/destroying the install prefix. cleanup is now in one place.
  - Hoisting cleanup out of the child improves nesting of try/catch in
    `real_work`.
  - `real_work` renamed to `build_process`
This commit is contained in:
Todd Gamblin 2016-03-08 02:50:26 -08:00
parent 18ce5ccf8f
commit 5aadb6df19
2 changed files with 91 additions and 80 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

56
lib/spack/spack/directory_layout.py
View file

@ -85,6 +85,16 @@ def create_install_directory(self, spec):
raise NotImplementedError() raise NotImplementedError()
def check_installed(self, spec):
"""Checks whether a spec is installed.
Return the spec's prefix, if it is installed, None otherwise.
Raise an exception if the install is inconsistent or corrupt.
"""
raise NotImplementedError()
def extension_map(self, spec): def extension_map(self, spec):
"""Get a dict of currently installed extension packages for a spec. """Get a dict of currently installed extension packages for a spec.
@ -246,26 +256,36 @@ def build_packages_path(self, spec):
def create_install_directory(self, spec): def create_install_directory(self, spec):
_check_concrete(spec) _check_concrete(spec)
prefix = self.check_installed(spec)
if prefix:
raise InstallDirectoryAlreadyExistsError(prefix)
mkdirp(self.metadata_path(spec))
self.write_spec(spec, self.spec_file_path(spec))
def check_installed(self, spec):
_check_concrete(spec)
path = self.path_for_spec(spec) path = self.path_for_spec(spec)
spec_file_path = self.spec_file_path(spec) spec_file_path = self.spec_file_path(spec)
if os.path.isdir(path): if not os.path.isdir(path):
if not os.path.isfile(spec_file_path): return None
raise InconsistentInstallDirectoryError(
'No spec file found at path %s' % spec_file_path)
installed_spec = self.read_spec(spec_file_path) if not os.path.isfile(spec_file_path):
if installed_spec == self.spec: raise InconsistentInstallDirectoryError(
raise InstallDirectoryAlreadyExistsError(path) 'Inconsistent state: install prefix exists but contains no spec.yaml:',
" " + path)
if spec.dag_hash() == installed_spec.dag_hash(): installed_spec = self.read_spec(spec_file_path)
raise SpecHashCollisionError(installed_hash, spec_hash) if installed_spec == spec:
else: return path
raise InconsistentInstallDirectoryError(
'Spec file in %s does not match hash!' % spec_file_path)
mkdirp(self.metadata_path(spec)) if spec.dag_hash() == installed_spec.dag_hash():
self.write_spec(spec, spec_file_path) raise SpecHashCollisionError(installed_hash, spec_hash)
else:
raise InconsistentInstallDirectoryError(
'Spec file in %s does not match hash!' % spec_file_path)
def all_specs(self): def all_specs(self):
@ -399,8 +419,8 @@ def remove_extension(self, spec, ext_spec):
class DirectoryLayoutError(SpackError): class DirectoryLayoutError(SpackError):
"""Superclass for directory layout errors.""" """Superclass for directory layout errors."""
def __init__(self, message): def __init__(self, message, long_msg=None):
super(DirectoryLayoutError, self).__init__(message) super(DirectoryLayoutError, self).__init__(message, long_msg)
class SpecHashCollisionError(DirectoryLayoutError): class SpecHashCollisionError(DirectoryLayoutError):
@ -422,8 +442,8 @@ def __init__(self, installed_spec, prefix, error):
class InconsistentInstallDirectoryError(DirectoryLayoutError): class InconsistentInstallDirectoryError(DirectoryLayoutError):
"""Raised when a package seems to be installed to the wrong place.""" """Raised when a package seems to be installed to the wrong place."""
def __init__(self, message): def __init__(self, message, long_msg=None):
super(InconsistentInstallDirectoryError, self).__init__(message) super(InconsistentInstallDirectoryError, self).__init__(message, long_msg)
class InstallDirectoryAlreadyExistsError(DirectoryLayoutError): class InstallDirectoryAlreadyExistsError(DirectoryLayoutError):

115
lib/spack/spack/package.py
View file

@ -845,7 +845,8 @@ def do_install(self,
if not self.spec.concrete: if not self.spec.concrete:
raise ValueError("Can only install concrete packages.") raise ValueError("Can only install concrete packages.")
if os.path.exists(self.prefix): # Ensure package is not already installed
if spack.install_layout.check_installed(self.spec):
tty.msg("%s is already installed in %s" % (self.name, self.prefix)) tty.msg("%s is already installed in %s" % (self.name, self.prefix))
return return
@ -857,18 +858,11 @@ def do_install(self,
keep_prefix=keep_prefix, keep_stage=keep_stage, ignore_deps=ignore_deps, keep_prefix=keep_prefix, keep_stage=keep_stage, ignore_deps=ignore_deps,
fake=fake, skip_patch=skip_patch, verbose=verbose, make_jobs=make_jobs) fake=fake, skip_patch=skip_patch, verbose=verbose, make_jobs=make_jobs)
def cleanup(): # Set parallelism before starting build.
"""Handles removing install prefix on error.""" self.make_jobs = make_jobs
if not keep_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, wrap=True)
# Then install the package itself. # Then install the package itself.
def real_work(): def build_process():
"""Forked for each build. Has its own process and python """Forked for each build. Has its own process and python
module space set up by build_environment.fork().""" module space set up by build_environment.fork()."""
start_time = time.time() start_time = time.time()
@ -878,30 +872,24 @@ def real_work():
else: else:
self.do_stage() self.do_stage()
# create the install directory. The install layout tty.msg("Building %s" % self.name)
# handles this in case so that it can use whatever
# package naming scheme it likes.
spack.install_layout.create_install_directory(self.spec)
try: self.stage.keep = keep_stage
tty.msg("Building %s" % self.name) with self.stage:
# Run the pre-install hook in the child process after
# the directory is created.
spack.hooks.pre_install(self)
self.stage.keep = keep_stage if fake:
with self.stage: self.do_fake_install()
# Run the pre-install hook in the child process after else:
# the directory is created. # Do the real install in the source directory.
spack.hooks.pre_install(self) self.stage.chdir_to_source()
if fake: # Save the build environment in a file before building.
self.do_fake_install() env_path = join_path(os.getcwd(), 'spack-build.env')
else:
# Do the real install in the source directory.
self.stage.chdir_to_source()
# Save the build environment in a file before building.
env_path = join_path(os.getcwd(), 'spack-build.env')
try:
# Redirect I/O to a build log (and optionally to the terminal) # Redirect I/O to a build log (and optionally to the terminal)
log_path = join_path(os.getcwd(), 'spack-build.out') log_path = join_path(os.getcwd(), 'spack-build.out')
log_file = open(log_path, 'w') log_file = open(log_path, 'w')
@ -909,43 +897,46 @@ def real_work():
dump_environment(env_path) dump_environment(env_path)
self.install(self.spec, self.prefix) self.install(self.spec, self.prefix)
# Ensure that something was actually installed. except ProcessError as e:
self._sanity_check_install() # Annotate ProcessErrors with the location of the build log.
e.build_log = log_path
raise e
# Copy provenance into the install directory on success # Ensure that something was actually installed.
log_install_path = spack.install_layout.build_log_path(self.spec) self._sanity_check_install()
env_install_path = spack.install_layout.build_env_path(self.spec)
packages_dir = spack.install_layout.build_packages_path(self.spec)
install(log_path, log_install_path) # Copy provenance into the install directory on success
install(env_path, env_install_path) log_install_path = spack.install_layout.build_log_path(self.spec)
dump_packages(self.spec, packages_dir) env_install_path = spack.install_layout.build_env_path(self.spec)
packages_dir = spack.install_layout.build_packages_path(self.spec)
# Stop timer. install(log_path, log_install_path)
self._total_time = time.time() - start_time install(env_path, env_install_path)
build_time = self._total_time - self._fetch_time dump_packages(self.spec, packages_dir)
tty.msg("Successfully installed %s" % self.name, # Stop timer.
"Fetch: %s. Build: %s. Total: %s." self._total_time = time.time() - start_time
% (_hms(self._fetch_time), _hms(build_time), _hms(self._total_time))) build_time = self._total_time - self._fetch_time
print_pkg(self.prefix)
except ProcessError as e: tty.msg("Successfully installed %s" % self.name,
# Annotate with location of build log. "Fetch: %s. Build: %s. Total: %s."
e.build_log = log_path % (_hms(self._fetch_time), _hms(build_time), _hms(self._total_time)))
cleanup() print_pkg(self.prefix)
raise e
except: try:
# other exceptions just clean up and raise. # Create the install prefix and fork the build process.
cleanup() spack.install_layout.create_install_directory(self.spec)
raise spack.build_environment.fork(self, build_process)
except:
# Set parallelism before starting build. # remove the install prefix if anything went wrong during install.
self.make_jobs = make_jobs if not keep_prefix:
self.remove_prefix()
# Do the build. else:
spack.build_environment.fork(self, real_work) tty.warn("Keeping install prefix in place despite error.",
"Spack will think this package is installed. " +
"Manually remove this directory to fix:",
self.prefix, wrap=True)
raise
# note: PARENT of the build process adds the new package to # note: PARENT of the build process adds the new package to
# the database, so that we don't need to re-read from file. # the database, so that we don't need to re-read from file.