fix style issues and refactor a bit

Signed-off-by: Todd Gamblin <tgamblin@llnl.gov>

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):

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)

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)