From f51b541ef8ca5378020d6caf19a8e59c6c0e8aa6 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Tue, 22 Aug 2017 14:35:17 -0700 Subject: Make install command reusable within single Spack run - install and probably other commands were designed to run once, but now we can to test them from within Spack with SpackCommand - cmd/install.py assumed that it could modify do_install in PackageBase and leave it that way; this makes the decorator temporary - package.py didn't properly initialize its stage if the same package had been built successfully before (and the stage removed). - manage stage lifecycle better and remember when Package needs to re-create the stage --- lib/spack/spack/cmd/install.py | 40 +++++++++++++++++++++++++--------------- lib/spack/spack/package.py | 15 +++++++++++---- lib/spack/spack/stage.py | 14 ++++++++++++-- 3 files changed, 48 insertions(+), 21 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/cmd/install.py b/lib/spack/spack/cmd/install.py index 9f35837220..ec1efdc9cf 100644 --- a/lib/spack/spack/cmd/install.py +++ b/lib/spack/spack/cmd/install.py @@ -334,31 +334,41 @@ def install(parser, args, **kwargs): tty.error('The `spack install` command requires a spec to install.') for spec in specs: + saved_do_install = PackageBase.do_install + decorator = lambda fn: fn + # Check if we were asked to produce some log for dashboards if args.log_format is not None: # Compute the filename for logging log_filename = args.log_file if not log_filename: log_filename = default_log_file(spec) + # Create the test suite in which to log results test_suite = TestSuite(spec) - # Decorate PackageBase.do_install to get installation status - PackageBase.do_install = junit_output( - spec, test_suite - )(PackageBase.do_install) + + # Temporarily decorate PackageBase.do_install to monitor + # recursive calls. + decorator = junit_output(spec, test_suite) # Do the actual installation - if args.things_to_install == 'dependencies': - # Install dependencies as-if they were installed - # for root (explicit=False in the DB) - kwargs['explicit'] = False - for s in spec.dependencies(): - p = spack.repo.get(s) - p.do_install(**kwargs) - else: - package = spack.repo.get(spec) - kwargs['explicit'] = True - package.do_install(**kwargs) + try: + # decorate the install if necessary + PackageBase.do_install = decorator(PackageBase.do_install) + + if args.things_to_install == 'dependencies': + # Install dependencies as-if they were installed + # for root (explicit=False in the DB) + kwargs['explicit'] = False + for s in spec.dependencies(): + p = spack.repo.get(s) + p.do_install(**kwargs) + else: + package = spack.repo.get(spec) + kwargs['explicit'] = True + package.do_install(**kwargs) + finally: + PackageBase.do_install = saved_do_install # Dump log file if asked to if args.log_format is not None: diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index ef1c411f66..64db8013d0 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -760,10 +760,6 @@ class PackageBase(with_metaclass(PackageMeta, object)): # Append the item to the composite composite_stage.append(stage) - # Create stage on first access. Needed because fetch, stage, - # patch, and install can be called independently of each - # other, so `with self.stage:` in do_install isn't sufficient. - composite_stage.create() return composite_stage @property @@ -772,6 +768,12 @@ class PackageBase(with_metaclass(PackageMeta, object)): raise ValueError("Can only get a stage for a concrete package.") if self._stage is None: self._stage = self._make_stage() + + # Create stage on first access. Needed because fetch, stage, + # patch, and install can be called independently of each + # other, so `with self.stage:` in do_install isn't sufficient. + self._stage.create() + return self._stage @stage.setter @@ -1390,6 +1392,11 @@ class PackageBase(with_metaclass(PackageMeta, object)): if not keep_prefix: self.remove_prefix() + # The subprocess *may* have removed the build stage. Mark it + # not created so that the next time self.stage is invoked, we + # check the filesystem for it. + self.stage.created = False + def check_for_unfinished_installation( self, keep_prefix=False, restage=False): """Check for leftover files from partially-completed prior install to diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py index 09f18f7d67..f9995bce6c 100644 --- a/lib/spack/spack/stage.py +++ b/lib/spack/spack/stage.py @@ -236,6 +236,10 @@ class Stage(object): self._lock = Stage.stage_locks[self.name] + # When stages are reused, we need to know whether to re-create + # it. This marks whether it has been created/destroyed. + self.created = False + def __enter__(self): """ Entering a stage context will create the stage directory @@ -521,6 +525,7 @@ class Stage(object): mkdirp(self.path) # Make sure we can actually do something with the stage we made. ensure_access(self.path) + self.created = True def destroy(self): """Removes this stage directory.""" @@ -532,6 +537,9 @@ class Stage(object): except OSError: os.chdir(os.path.dirname(self.path)) + # mark as destroyed + self.created = False + class ResourceStage(Stage): @@ -573,8 +581,9 @@ class ResourceStage(Stage): shutil.move(source_path, destination_path) -@pattern.composite(method_list=['fetch', 'create', 'check', 'expand_archive', - 'restage', 'destroy', 'cache_local']) +@pattern.composite(method_list=[ + 'fetch', 'create', 'created', 'check', 'expand_archive', 'restage', + 'destroy', 'cache_local']) class StageComposite: """Composite for Stage type objects. The first item in this composite is considered to be the root package, and operations that return a value are @@ -623,6 +632,7 @@ class DIYStage(object): self.archive_file = None self.path = path self.source_path = path + self.created = True def chdir(self): if os.path.isdir(self.path): -- cgit v1.2.3-70-g09d2