diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py index 34aa83730bf..efe5fe5e187 100644 --- a/lib/spack/spack/build_environment.py +++ b/lib/spack/spack/build_environment.py @@ -898,7 +898,6 @@ def effective_deptypes( root=True, all_edges=True, ) - traverse.traverse_depth_first_with_visitor(traverse.with_artificial_edges(specs), visitor) # Dictionary with "no mode" as default value, so it's easy to write modes[x] |= flag. use_modes = defaultdict(lambda: UseMode(0)) @@ -1461,7 +1460,7 @@ def exitcode_msg(process): return child_result -CONTEXT_BASES = (spack.package_base.PackageBase, spack.build_systems._checks.BaseBuilder) +CONTEXT_BASES = (spack.package_base.PackageBase, spack.builder.BaseBuilder) def get_package_context(traceback, context=3): diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py index ba77afdb27a..a866023bd92 100644 --- a/lib/spack/spack/installer.py +++ b/lib/spack/spack/installer.py @@ -874,6 +874,7 @@ class Task: success_result: Optional[ExecuteResult] = None error_result: Optional[BaseException] = None + no_op: bool = False def __init__( self, @@ -1183,6 +1184,12 @@ def priority(self): """The priority is based on the remaining uninstalled dependencies.""" return len(self.uninstalled_deps) + def terminate(self) -> None: + """End any processes and clean up any resources allocated by this Task. + + By default this is a no-op. + """ + def check_db(spec: "spack.spec.Spec") -> Tuple[Optional[spack.database.InstallRecord], bool]: """Determine if the spec is flagged as installed in the database @@ -1208,7 +1215,7 @@ def check_db(spec: "spack.spec.Spec") -> Tuple[Optional[spack.database.InstallRe class BuildTask(Task): """Class for representing a build task for a package.""" - process_handle: "spack.build_environment.ProcessHandle" = None + process_handle: Optional["spack.build_environment.ProcessHandle"] = None started: bool = False no_op: bool = False @@ -1319,6 +1326,11 @@ def complete(self): return ExecuteResult.SUCCESS + def terminate(self) -> None: + """Terminate any processes this task still has running.""" + if self.process_handle: + self.process_handle.terminate_processes() + class RewireTask(Task): """Class for representing a rewire task for a package.""" @@ -2262,7 +2274,7 @@ def complete_task(self, task: Task, install_status: InstallStatus) -> Optional[T # Overwrite process exception handling except fs.CouldNotRestoreDirectoryBackup as e: - self.database.remove(task.pkg.spec) + spack.store.STORE.db.remove(task.pkg.spec) tty.error( f"Recovery of install dir of {task.pkg.name} failed due to " f"{e.outer_exception.__class__.__name__}: {str(e.outer_exception)}. " @@ -2306,6 +2318,8 @@ def complete_task(self, task: Task, install_status: InstallStatus) -> Optional[T if pkg.spec.installed: self._cleanup_task(pkg) + return None + def install(self) -> None: """Install the requested package(s) and or associated dependencies.""" @@ -2347,11 +2361,10 @@ def install(self) -> None: failure = self.complete_task(task, install_status) if failure: failed_build_requests.append(failure) - except Exception as e: + except Exception: # Terminate any active child processes if there's an installation error for task in active_tasks: - if task.process_handle is not None: - task.process_handle.terminate_processes() + task.terminate() raise finally: active_tasks.remove(task) diff --git a/lib/spack/spack/test/installer.py b/lib/spack/spack/test/installer.py index d960319b3a6..33682b0b76e 100644 --- a/lib/spack/spack/test/installer.py +++ b/lib/spack/spack/test/installer.py @@ -1198,7 +1198,7 @@ def test_install_implicit(install_mockery, mock_fetch): assert not create_build_task(pkg).explicit -#### WIP ##### +# WIP def test_overwrite_install_backup_success(temporary_store, config, mock_packages, tmpdir): """ When doing an overwrite install that fails, Spack should restore the backup @@ -1221,6 +1221,13 @@ def test_overwrite_install_backup_success(temporary_store, config, mock_packages installed_file = os.path.join(task.pkg.prefix, "some_file") fs.touchp(installed_file) + # TODO: remove this, as it uses the old install_task method + class InstallerThatWipesThePrefixDir: + def _install_task(self, task, install_status): + shutil.rmtree(task.pkg.prefix, ignore_errors=True) + fs.mkdirp(task.pkg.prefix) + raise Exception("Some fatal install error") + # Install that wipes the prefix directory def wiped_installer(): shutil.rmtree(task.pkg.prefix)