hooks: remove 7 unused hooks (#42575)

These 7 hooks were not used.

- Six of them related to install phases were unused after `spack`
  `monitor` was removed, and the code seems to have bit rotten as there
  were reports they were not (always?) triggered when they should.
- The post environment one was made redundant after spack install for
  environment started following the common code path for generating
  module files in #42147.

It should not be a breaking change to remove, since users cannot define
hooks in extensions, they would have to fork Spack.

If we ever _were_ to make those hooks extendable outside of core Spack,
it would also be better to start with fewer rather than more, cause
everything you expose gets relied upon...

Removing those also allows us to rethink what hooks we really need, and
in particular it seems like we need a hook that runs post install also when
the spec is inserted into the database.

4 files changed, 12 insertions, 120 deletions

diff --git a/lib/spack/docs/developer_guide.rst b/lib/spack/docs/developer_guide.rst
index 440fe01880..af69b219a7 100644
--- a/lib/spack/docs/developer_guide.rst
+++ b/lib/spack/docs/developer_guide.rst
@@ -357,91 +357,23 @@ If there is a hook that you would like and is missing, you can propose to add a
 ``pre_install(spec)``
 """""""""""""""""""""
 
-A ``pre_install`` hook is run within an install subprocess, directly before
-the install starts. It expects a single argument of a spec, and is run in
-a multiprocessing subprocess. Note that if you see ``pre_install`` functions associated with packages these are not hooks
-as we have defined them here, but rather callback functions associated with
-a package install.
+A ``pre_install`` hook is run within the install subprocess, directly before the install starts.
+It expects a single argument of a spec.
 
 
-""""""""""""""""""""""
-``post_install(spec)``
-""""""""""""""""""""""
+"""""""""""""""""""""""""""""""""""""
+``post_install(spec, explicit=None)``
+"""""""""""""""""""""""""""""""""""""
 
-A ``post_install`` hook is run within an install subprocess, directly after
-the install finishes, but before the build stage is removed. If you
-write one of these hooks, you should expect it to accept a spec as the only
-argument. This is run in a multiprocessing subprocess. This ``post_install`` is
-also seen in packages, but in this context not related to the hooks described
-here.
+A ``post_install`` hook is run within the install subprocess, directly after the install finishes,
+but before the build stage is removed and the spec is registered in the database. It expects two
+arguments: spec and an optional boolean indicating whether this spec is being installed explicitly.
 
+""""""""""""""""""""""""""""""""""""""""""""""""""""
+``pre_uninstall(spec)`` and ``post_uninstall(spec)``
+""""""""""""""""""""""""""""""""""""""""""""""""""""
 
-""""""""""""""""""""""""""
-``on_install_start(spec)``
-""""""""""""""""""""""""""
-
-This hook is run at the beginning of ``lib/spack/spack/installer.py``,
-in the install function of a ``PackageInstaller``,
-and importantly is not part of a build process, but before it. This is when
-we have just newly grabbed the task, and are preparing to install. If you
-write a hook of this type, you should provide the spec to it.
-
-.. code-block:: python
-
-    def on_install_start(spec):
-        """On start of an install, we want to...
-        """
-        print('on_install_start')
-
-
-""""""""""""""""""""""""""""
-``on_install_success(spec)``
-""""""""""""""""""""""""""""
-
-This hook is run on a successful install, and is also run inside the build
-process, akin to ``post_install``. The main difference is that this hook
-is run outside of the context of the stage directory, meaning after the
-build stage has been removed and the user is alerted that the install was
-successful. If you need to write a hook that is run on success of a particular
-phase, you should use ``on_phase_success``.
-
-""""""""""""""""""""""""""""
-``on_install_failure(spec)``
-""""""""""""""""""""""""""""
-
-This hook is run given an install failure that happens outside of the build
-subprocess, but somewhere in ``installer.py`` when something else goes wrong.
-If you need to write a hook that is relevant to a failure within a build
-process, you would want to instead use ``on_phase_failure``.
-
-
-"""""""""""""""""""""""""""
-``on_install_cancel(spec)``
-"""""""""""""""""""""""""""
-
-The same, but triggered if a spec install is cancelled for any reason.
-
-
-"""""""""""""""""""""""""""""""""""""""""""""""
-``on_phase_success(pkg, phase_name, log_file)``
-"""""""""""""""""""""""""""""""""""""""""""""""
-
-This hook is run within the install subprocess, and specifically when a phase
-successfully finishes. Since we are interested in the package, the name of
-the phase, and any output from it, we require:
-
- - **pkg**: the package variable, which also has the attached spec at ``pkg.spec``
- - **phase_name**: the name of the phase that was successful (e.g., configure)
- - **log_file**: the path to the file with output, in case you need to inspect or otherwise interact with it.
-
-"""""""""""""""""""""""""""""""""""""""""""""
-``on_phase_error(pkg, phase_name, log_file)``
-"""""""""""""""""""""""""""""""""""""""""""""
-
-In the case of an error during a phase, we might want to trigger some event
-with a hook, and this is the purpose of this particular hook. Akin to
-``on_phase_success`` we require the same variables - the package that failed,
-the name of the phase, and the log file where we might find errors.
+These hooks are currently used for cleaning up module files after uninstall.
 
 
 ^^^^^^^^^^^^^^^^^^^^^^
diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py
index e54ff5709c..2433e59b21 100644
--- a/lib/spack/spack/environment/environment.py
+++ b/lib/spack/spack/environment/environment.py
@@ -2090,7 +2090,6 @@ class Environment:
 
         if regenerate:
             self.regenerate_views()
-            spack.hooks.post_env_write(self)
 
         self.new_specs.clear()
 
diff --git a/lib/spack/spack/hooks/__init__.py b/lib/spack/spack/hooks/__init__.py
index 5eb2c0bf57..029f9ca7ba 100644
--- a/lib/spack/spack/hooks/__init__.py
+++ b/lib/spack/spack/hooks/__init__.py
@@ -15,13 +15,6 @@ Currently the following hooks are supported:
     * post_install(spec, explicit)
     * pre_uninstall(spec)
     * post_uninstall(spec)
-    * on_install_start(spec)
-    * on_install_success(spec)
-    * on_install_failure(spec)
-    * on_phase_success(pkg, phase_name, log_file)
-    * on_phase_error(pkg, phase_name, log_file)
-    * on_phase_error(pkg, phase_name, log_file)
-    * post_env_write(env)
 
 This can be used to implement support for things like module
 systems (e.g. modules, lmod, etc.) or to add other custom
@@ -78,17 +71,5 @@ class _HookRunner:
 pre_install = _HookRunner("pre_install")
 post_install = _HookRunner("post_install")
 
-# These hooks are run within an install subprocess
 pre_uninstall = _HookRunner("pre_uninstall")
 post_uninstall = _HookRunner("post_uninstall")
-on_phase_success = _HookRunner("on_phase_success")
-on_phase_error = _HookRunner("on_phase_error")
-
-# These are hooks in installer.py, before starting install subprocess
-on_install_start = _HookRunner("on_install_start")
-on_install_success = _HookRunner("on_install_success")
-on_install_failure = _HookRunner("on_install_failure")
-on_install_cancel = _HookRunner("on_install_cancel")
-
-# Environment hooks
-post_env_write = _HookRunner("post_env_write")
diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py
index b2d2da0d8a..915327e3b4 100644
--- a/lib/spack/spack/installer.py
+++ b/lib/spack/spack/installer.py
@@ -1705,7 +1705,6 @@ class PackageInstaller:
         except spack.build_environment.StopPhase as e:
             # A StopPhase exception means that do_install was asked to
             # stop early from clients, and is not an error at this point
-            spack.hooks.on_install_failure(task.request.pkg.spec)
             pid = f"{self.pid}: " if tty.show_pid() else ""
             tty.debug(f"{pid}{str(e)}")
             tty.debug(f"Package stage directory: {pkg.stage.source_path}")
@@ -2011,7 +2010,6 @@ class PackageInstaller:
             if task is None:
                 continue
 
-            spack.hooks.on_install_start(task.request.pkg.spec)
             install_args = task.request.install_args
             keep_prefix = install_args.get("keep_prefix")
 
@@ -2037,9 +2035,6 @@ class PackageInstaller:
                     tty.warn(f"{pkg_id} does NOT actually have any uninstalled deps left")
                 dep_str = "dependencies" if task.priority > 1 else "dependency"
 
-                # Hook to indicate task failure, but without an exception
-                spack.hooks.on_install_failure(task.request.pkg.spec)
-
                 raise InstallError(
                     f"Cannot proceed with {pkg_id}: {task.priority} uninstalled "
                     f"{dep_str}: {','.join(task.uninstalled_deps)}",
@@ -2062,11 +2057,6 @@ class PackageInstaller:
                 tty.warn(f"{pkg_id} failed to install")
                 self._update_failed(task)
 
-                # Mark that the package failed
-                # TODO: this should also be for the task.pkg, but we don't
-                # model transitive yet.
-                spack.hooks.on_install_failure(task.request.pkg.spec)
-
                 if self.fail_fast:
                     raise InstallError(fail_fast_err, pkg=pkg)
 
@@ -2169,7 +2159,6 @@ class PackageInstaller:
                 tty.error(
                     f"Failed to install {pkg.name} due to " f"{exc.__class__.__name__}: {str(exc)}"
                 )
-                spack.hooks.on_install_cancel(task.request.pkg.spec)
                 raise
 
             except binary_distribution.NoChecksumException as exc:
@@ -2188,7 +2177,6 @@ class PackageInstaller:
 
             except (Exception, SystemExit) as exc:
                 self._update_failed(task, True, exc)
-                spack.hooks.on_install_failure(task.request.pkg.spec)
 
                 # Best effort installs suppress the exception and mark the
                 # package as a failure.
@@ -2372,9 +2360,6 @@ class BuildProcessInstaller:
         _print_timer(pre=self.pre, pkg_id=self.pkg_id, timer=self.timer)
         _print_installed_pkg(self.pkg.prefix)
 
-        # Send final status that install is successful
-        spack.hooks.on_install_success(self.pkg.spec)
-
         # preserve verbosity across runs
         return self.echo
 
@@ -2453,15 +2438,10 @@ class BuildProcessInstaller:
                         # Catch any errors to report to logging
                         self.timer.start(phase_fn.name)
                         phase_fn.execute()
-                        spack.hooks.on_phase_success(pkg, phase_fn.name, log_file)
                         self.timer.stop(phase_fn.name)
 
                 except BaseException:
                     combine_phase_logs(pkg.phase_log_files, pkg.log_path)
-                    spack.hooks.on_phase_error(pkg, phase_fn.name, log_file)
-
-                    # phase error indicates install error
-                    spack.hooks.on_install_failure(pkg.spec)
                     raise
 
                 # We assume loggers share echo True/False