zhangyiss/spack
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Small refactor: add keep parameter to stage, get rid of stage.destroy call.

Browse Source 
- package.py uses context manager more effectively.

- Stage.__init__ has easier to understand method signature now.
  - keep can be used to override the default behavior either to keep
    the stage ALL the time or to delete the stage ALL the time.
This commit is contained in:
Todd Gamblin
2016-03-05 19:55:07 -08:00
parent 14d48eba46
commit ad103dcafa
4 changed files with 141 additions and 90 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
lib/spack/llnl/util/filesystem.py
View File

@@ -362,8 +362,9 @@ def remove_dead_links(root):
def remove_linked_tree(path): def remove_linked_tree(path):
""" """
Removes a directory and its contents. If the directory is a symlink, follows the link and removes the real Removes a directory and its contents. If the directory is a
directory before removing the link. symlink, follows the link and removes the real directory before
removing the link.
Args: Args:
path: directory to be removed path: directory to be removed

2
lib/spack/spack/cmd/clean.py
View File

@@ -43,4 +43,4 @@ def clean(parser, args):
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.repo.get(spec) package = spack.repo.get(spec)
package.stage.destroy() package.do_clean()

108
lib/spack/spack/package.py
View File

@@ -293,6 +293,7 @@ class SomePackage(Package):
.. code-block:: python .. code-block:: python
p.do_clean() # removes the stage directory entirely
p.do_restage() # removes the build directory and p.do_restage() # removes the build directory and
# re-expands the archive. # re-expands the archive.
@@ -455,7 +456,7 @@ def _make_stage(self):
# Construct a composite stage on top of the composite FetchStrategy # Construct a composite stage on top of the composite FetchStrategy
composite_fetcher = self.fetcher composite_fetcher = self.fetcher
composite_stage = StageComposite() composite_stage = StageComposite()
resources = self._get_resources() resources = self._get_needed_resources()
for ii, fetcher in enumerate(composite_fetcher): for ii, fetcher in enumerate(composite_fetcher):
if ii == 0: if ii == 0:
# Construct root stage first # Construct root stage first
@@ -484,12 +485,14 @@ def stage(self, stage):
def _make_fetcher(self): def _make_fetcher(self):
# Construct a composite fetcher that always contains at least one element (the root package). In case there # Construct a composite fetcher that always contains at least
# are resources associated with the package, append their fetcher to the composite. # one element (the root package). In case there are resources
# associated with the package, append their fetcher to the
# composite.
root_fetcher = fs.for_package_version(self, self.version) root_fetcher = fs.for_package_version(self, self.version)
fetcher = fs.FetchStrategyComposite() # Composite fetcher fetcher = fs.FetchStrategyComposite() # Composite fetcher
fetcher.append(root_fetcher) # Root fetcher is always present fetcher.append(root_fetcher) # Root fetcher is always present
resources = self._get_resources() resources = self._get_needed_resources()
for resource in resources: for resource in resources:
fetcher.append(resource.fetcher) fetcher.append(resource.fetcher)
return fetcher return fetcher
@@ -706,6 +709,7 @@ def do_stage(self, mirror_only=False):
self.stage.expand_archive() self.stage.expand_archive()
self.stage.chdir_to_source() self.stage.chdir_to_source()
def do_patch(self): def do_patch(self):
"""Calls do_stage(), then applied patches to the expanded tarball if they """Calls do_stage(), then applied patches to the expanded tarball if they
haven't been applied already.""" haven't been applied already."""
@@ -798,7 +802,7 @@ def do_fake_install(self):
mkdirp(self.prefix.man1) mkdirp(self.prefix.man1)
def _get_resources(self): def _get_needed_resources(self):
resources = [] resources = []
# Select the resources that are needed for this build # Select the resources that are needed for this build
for when_spec, resource_list in self.resources.items(): for when_spec, resource_list in self.resources.items():
@@ -816,7 +820,7 @@ def _resource_stage(self, resource):
def do_install(self, def do_install(self,
keep_prefix=False, keep_stage=False, ignore_deps=False, keep_prefix=False, keep_stage=None, ignore_deps=False,
skip_patch=False, verbose=False, make_jobs=None, fake=False): skip_patch=False, verbose=False, make_jobs=None, fake=False):
"""Called by commands to install a package and its dependencies. """Called by commands to install a package and its dependencies.
@@ -825,7 +829,8 @@ def do_install(self,
Args: Args:
keep_prefix -- Keep install prefix on failure. By default, destroys it. keep_prefix -- Keep install prefix on failure. By default, destroys it.
keep_stage -- Keep stage on successful build. By default, destroys it. keep_stage -- Set to True or false to always keep or always delete stage.
By default, stage is destroyed only if there are no exceptions.
ignore_deps -- Do not install dependencies before installing this package. ignore_deps -- Do not install dependencies before installing this package.
fake -- Don't really build -- install fake stub files instead. fake -- Don't really build -- install fake stub files instead.
skip_patch -- Skip patch stage of build if True. skip_patch -- Skip patch stage of build if True.
@@ -848,32 +853,33 @@ def do_install(self,
make_jobs=make_jobs) make_jobs=make_jobs)
start_time = time.time() start_time = time.time()
with self.stage: if not fake:
if not fake: if not skip_patch:
if not skip_patch: self.do_patch()
self.do_patch() else:
else: self.do_stage()
self.do_stage()
# create the install directory. The install layout # create the install directory. The install layout
# handles this in case so that it can use whatever # handles this in case so that it can use whatever
# package naming scheme it likes. # package naming scheme it likes.
spack.install_layout.create_install_directory(self.spec) spack.install_layout.create_install_directory(self.spec)
def cleanup(): def cleanup():
if not keep_prefix: if not keep_prefix:
# If anything goes wrong, remove the install prefix # If anything goes wrong, remove the install prefix
self.remove_prefix() self.remove_prefix()
else: else:
tty.warn("Keeping install prefix in place despite error.", tty.warn("Keeping install prefix in place despite error.",
"Spack will think this package is installed." + "Spack will think this package is installed." +
"Manually remove this directory to fix:", "Manually remove this directory to fix:",
self.prefix, wrap=True) self.prefix, wrap=True)
def real_work(): def real_work():
try: try:
tty.msg("Building %s" % self.name) tty.msg("Building %s" % self.name)
self.stage.keep = keep_stage
with self.stage:
# Run the pre-install hook in the child process after # Run the pre-install hook in the child process after
# the directory is created. # the directory is created.
spack.hooks.pre_install(self) spack.hooks.pre_install(self)
@@ -888,7 +894,7 @@ def real_work():
# Save the build environment in a file before building. # Save the build environment in a file before building.
env_path = join_path(os.getcwd(), 'spack-build.env') env_path = join_path(os.getcwd(), 'spack-build.env')
# This redirects 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')
with log_output(log_file, verbose, sys.stdout.isatty(), True): with log_output(log_file, verbose, sys.stdout.isatty(), True):
@@ -908,29 +914,25 @@ def real_work():
packages_dir = spack.install_layout.build_packages_path(self.spec) packages_dir = spack.install_layout.build_packages_path(self.spec)
dump_packages(self.spec, packages_dir) dump_packages(self.spec, packages_dir)
# On successful install, remove the stage. # Stop timer.
if not keep_stage: self._total_time = time.time() - start_time
self.stage.destroy() build_time = self._total_time - self._fetch_time
# Stop timer. tty.msg("Successfully installed %s" % self.name,
self._total_time = time.time() - start_time "Fetch: %s. Build: %s. Total: %s."
build_time = self._total_time - self._fetch_time % (_hms(self._fetch_time), _hms(build_time), _hms(self._total_time)))
print_pkg(self.prefix)
tty.msg("Successfully installed %s" % self.name, except ProcessError as e:
"Fetch: %s. Build: %s. Total: %s." # Annotate with location of build log.
% (_hms(self._fetch_time), _hms(build_time), _hms(self._total_time))) e.build_log = log_path
print_pkg(self.prefix) cleanup()
raise e
except ProcessError as e: except:
# Annotate with location of build log. # other exceptions just clean up and raise.
e.build_log = log_path cleanup()
cleanup() raise
raise e
except:
# other exceptions just clean up and raise.
cleanup()
raise
# Set parallelism before starting build. # Set parallelism before starting build.
self.make_jobs = make_jobs self.make_jobs = make_jobs
@@ -1147,6 +1149,12 @@ def do_restage(self):
"""Reverts expanded/checked out source to a pristine state.""" """Reverts expanded/checked out source to a pristine state."""
self.stage.restage() self.stage.restage()
def do_clean(self):
"""Removes the package's build stage and source tarball."""
self.stage.destroy()
def format_doc(self, **kwargs): def format_doc(self, **kwargs):
"""Wrap doc string at 72 characters and format nicely""" """Wrap doc string at 72 characters and format nicely"""
indent = kwargs.get('indent', 0) indent = kwargs.get('indent', 0)

116
lib/spack/spack/stage.py
View File

@@ -42,29 +42,53 @@
class Stage(object): class Stage(object):
""" """Manages a temporary stage directory for building.
A Stage object is a context manager that handles a directory where some source code is downloaded and built
before being installed. It handles fetching the source code, either as an archive to be expanded or by checking A Stage object is a context manager that handles a directory where
it out of a repository. A stage's lifecycle looks like this: some source code is downloaded and built before being installed.
It handles fetching the source code, either as an archive to be
expanded or by checking it out of a repository. A stage's
lifecycle looks like this:
``` ```
with Stage() as stage: # Context manager creates and destroys the stage directory with Stage() as stage: # Context manager creates and destroys the stage directory
fetch() # Fetch a source archive into the stage. stage.fetch() # Fetch a source archive into the stage.
expand_archive() # Expand the source archive. stage.expand_archive() # Expand the source archive.
<install> # Build and install the archive. This is handled by the Package class. <install> # Build and install the archive. (handled by user of Stage)
``` ```
If spack.use_tmp_stage is True, spack will attempt to create stages in a tmp directory. When used as a context manager, the stage is automatically
Otherwise, stages are created directly in spack.stage_path. destroyed if no exception is raised by the context. If an
excpetion is raised, the stage is left in the filesystem and NOT
destroyed, for potential reuse later.
There are two kinds of stages: named and unnamed. Named stages can persist between runs of spack, e.g. if you You can also use the stage's create/destroy functions manually,
fetched a tarball but didn't finish building it, you won't have to fetch it again. like this:
Unnamed stages are created using standard mkdtemp mechanisms or similar, and are intended to persist for ```
only one run of spack. stage = Stage()
try:
stage.create() # Explicitly create the stage directory.
stage.fetch() # Fetch a source archive into the stage.
stage.expand_archive() # Expand the source archive.
<install> # Build and install the archive. (handled by user of Stage)
finally:
stage.destroy() # Explicitly destroy the stage directory.
```
If spack.use_tmp_stage is True, spack will attempt to create
stages in a tmp directory. Otherwise, stages are created directly
in spack.stage_path.
There are two kinds of stages: named and unnamed. Named stages
can persist between runs of spack, e.g. if you fetched a tarball
but didn't finish building it, you won't have to fetch it again.
Unnamed stages are created using standard mkdtemp mechanisms or
similar, and are intended to persist for only one run of spack.
""" """
def __init__(self, url_or_fetch_strategy, **kwargs): def __init__(self, url_or_fetch_strategy, name=None, mirror_path=None, keep=None):
"""Create a stage object. """Create a stage object.
Parameters: Parameters:
url_or_fetch_strategy url_or_fetch_strategy
@@ -76,6 +100,18 @@ def __init__(self, url_or_fetch_strategy, **kwargs):
and will persist between runs (or if you construct another and will persist between runs (or if you construct another
stage object later). If name is not provided, then this stage object later). If name is not provided, then this
stage will be given a unique name automatically. stage will be given a unique name automatically.
mirror_path
If provided, Stage will search Spack's mirrors for
this archive at the mirror_path, before using the
default fetch strategy.
keep
By default, when used as a context manager, the Stage
is cleaned up when everything goes well, and it is
kept intact when an exception is raised. You can
override this behavior by setting keep to True
(always keep) or False (always delete).
""" """
# TODO: fetch/stage coupling needs to be reworked -- the logic # TODO: fetch/stage coupling needs to be reworked -- the logic
# TODO: here is convoluted and not modular enough. # TODO: here is convoluted and not modular enough.
@@ -91,15 +127,19 @@ def __init__(self, url_or_fetch_strategy, **kwargs):
# TODO : this uses a protected member of tempfile, but seemed the only way to get a temporary name # TODO : this uses a protected member of tempfile, but seemed the only way to get a temporary name
# TODO : besides, the temporary link name won't be the same as the temporary stage area in tmp_root # TODO : besides, the temporary link name won't be the same as the temporary stage area in tmp_root
self.name = kwargs.get('name') if 'name' in kwargs else STAGE_PREFIX + next(tempfile._get_candidate_names()) self.name = name
self.mirror_path = kwargs.get('mirror_path') if name is None:
self.name = STAGE_PREFIX + next(tempfile._get_candidate_names())
self.mirror_path = mirror_path
self.tmp_root = find_tmp_root() self.tmp_root = find_tmp_root()
# Try to construct here a temporary name for the stage directory # Try to construct here a temporary name for the stage directory
# If this is a named stage, then construct a named path. # If this is a named stage, then construct a named path.
self.path = join_path(spack.stage_path, self.name) self.path = join_path(spack.stage_path, self.name)
# Flag to decide whether to delete the stage folder on exit or not # Flag to decide whether to delete the stage folder on exit or not
self.delete_on_exit = True self.keep = keep
def __enter__(self): def __enter__(self):
""" """
@@ -111,6 +151,7 @@ def __enter__(self):
self.create() self.create()
return self return self
def __exit__(self, exc_type, exc_val, exc_tb): def __exit__(self, exc_type, exc_val, exc_tb):
""" """
Exiting from a stage context will delete the stage directory unless: Exiting from a stage context will delete the stage directory unless:
@@ -125,11 +166,15 @@ def __exit__(self, exc_type, exc_val, exc_tb):
Returns: Returns:
Boolean Boolean
""" """
self.delete_on_exit = False if exc_type is not None else self.delete_on_exit if self.keep is None:
# Default: delete when there are no exceptions.
if exc_type is None: self.destroy()
if self.delete_on_exit: elif not self.keep:
# Overridden. Either always keep or always delete.
self.destroy() self.destroy()
def _need_to_create_path(self): def _need_to_create_path(self):
"""Makes sure nothing weird has happened since the last time we """Makes sure nothing weird has happened since the last time we
looked at path. Returns True if path already exists and is ok. looked at path. Returns True if path already exists and is ok.
@@ -201,7 +246,7 @@ def chdir(self):
if os.path.isdir(self.path): if os.path.isdir(self.path):
os.chdir(self.path) os.chdir(self.path)
else: else:
tty.die("Setup failed: no such directory: " + self.path) raise ChdirError("Setup failed: no such directory: " + self.path)
def fetch(self, mirror_only=False): def fetch(self, mirror_only=False):
"""Downloads an archive or checks out code from a repository.""" """Downloads an archive or checks out code from a repository."""
@@ -302,11 +347,14 @@ def create(self):
""" """
Creates the stage directory Creates the stage directory
If self.tmp_root evaluates to False, the stage directory is created directly under spack.stage_path, otherwise If self.tmp_root evaluates to False, the stage directory is
this will attempt to create a stage in a temporary directory and link it into spack.stage_path. created directly under spack.stage_path, otherwise this will
attempt to create a stage in a temporary directory and link it
into spack.stage_path.
Spack will use the first writable location in spack.tmp_dirs to create a stage. If there is no valid location Spack will use the first writable location in spack.tmp_dirs
in tmp_dirs, fall back to making the stage inside spack.stage_path. to create a stage. If there is no valid location in tmp_dirs,
fall back to making the stage inside spack.stage_path.
""" """
# Create the top-level stage directory # Create the top-level stage directory
mkdirp(spack.stage_path) mkdirp(spack.stage_path)
@@ -323,9 +371,7 @@ def create(self):
ensure_access(self.path) ensure_access(self.path)
def destroy(self): def destroy(self):
""" """Removes this stage directory."""
Removes this stage directory
"""
remove_linked_tree(self.path) remove_linked_tree(self.path)
# Make sure we don't end up in a removed directory # Make sure we don't end up in a removed directory
@@ -370,7 +416,7 @@ def expand_archive(self):
shutil.move(source_path, destination_path) shutil.move(source_path, destination_path)
@pattern.composite(method_list=['fetch', 'check', 'expand_archive', 'restage', 'destroy']) @pattern.composite(method_list=['fetch', 'create', 'check', 'expand_archive', 'restage', 'destroy'])
class StageComposite: class StageComposite:
""" """
Composite for Stage type objects. The first item in this composite is considered to be the root package, and Composite for Stage type objects. The first item in this composite is considered to be the root package, and
@@ -410,7 +456,7 @@ def chdir(self):
if os.path.isdir(self.path): if os.path.isdir(self.path):
os.chdir(self.path) os.chdir(self.path)
else: else:
tty.die("Setup failed: no such directory: " + self.path) raise ChdirError("Setup failed: no such directory: " + self.path)
def chdir_to_source(self): def chdir_to_source(self):
self.chdir() self.chdir()
@@ -472,19 +518,15 @@ def find_tmp_root():
class StageError(spack.error.SpackError): class StageError(spack.error.SpackError):
def __init__(self, message, long_message=None): """"Superclass for all errors encountered during staging."""
super(self, StageError).__init__(message, long_message)
class RestageError(StageError): class RestageError(StageError):
def __init__(self, message, long_msg=None): """"Error encountered during restaging."""
super(RestageError, self).__init__(message, long_msg)
class ChdirError(StageError): class ChdirError(StageError):
def __init__(self, message, long_msg=None): """Raised when Spack can't change directories."""
super(ChdirError, self).__init__(message, long_msg)
# Keep this in namespace for convenience # Keep this in namespace for convenience
FailedDownloadError = fs.FailedDownloadError FailedDownloadError = fs.FailedDownloadError