spack uninstall: use topo order (#34053)

2 files changed, 17 insertions, 50 deletions

diff --git a/lib/spack/spack/cmd/gc.py b/lib/spack/spack/cmd/gc.py
index dd7ede2411..c50646c150 100644
--- a/lib/spack/spack/cmd/gc.py
+++ b/lib/spack/spack/cmd/gc.py
@@ -43,4 +43,4 @@ def gc(parser, args):
     if not args.yes_to_all:
         spack.cmd.uninstall.confirm_removal(specs)
 
-    spack.cmd.uninstall.do_uninstall(None, specs, force=False)
+    spack.cmd.uninstall.do_uninstall(specs, force=False)
diff --git a/lib/spack/spack/cmd/uninstall.py b/lib/spack/spack/cmd/uninstall.py
index 7134ca7256..8ef361506f 100644
--- a/lib/spack/spack/cmd/uninstall.py
+++ b/lib/spack/spack/cmd/uninstall.py
@@ -230,54 +230,21 @@ def _remove_from_env(spec, env):
         pass  # ignore non-root specs
 
 
-def do_uninstall(env, specs, force):
-    """Uninstalls all the specs in a list.
-
-    Args:
-        env (spack.environment.Environment or None): active environment, or ``None``
-            if there is not one
-        specs (list): list of specs to be uninstalled
-        force (bool): force uninstallation (boolean)
-    """
-    packages = []
-    for item in specs:
-        try:
-            # should work if package is known to spack
-            packages.append(item.package)
-        except spack.repo.UnknownEntityError:
-            # The package.py file has gone away -- but still
-            # want to uninstall.
-            spack.package_base.PackageBase.uninstall_by_spec(item, force=True)
-
-    # A package is ready to be uninstalled when nothing else references it,
-    # unless we are requested to force uninstall it.
-    def is_ready(dag_hash):
-        if force:
-            return True
-
-        record = spack.store.db.query_local_by_spec_hash(dag_hash)
-        if not record.ref_count:
-            return True
-
-        # If this spec is only used as a build dependency, we can uninstall
-        return all(
-            dspec.deptypes == ("build",) or not dspec.parent.installed
-            for dspec in record.spec.edges_from_dependents()
-        )
-
-    while packages:
-        ready = [x for x in packages if is_ready(x.spec.dag_hash())]
-        if not ready:
-            msg = (
-                "unexpected error [cannot proceed uninstalling specs with"
-                " remaining link or run dependents {0}]"
-            )
-            msg = msg.format(", ".join(x.name for x in packages))
-            raise spack.error.SpackError(msg)
-
-        packages = [x for x in packages if x not in ready]
-        for item in ready:
-            item.do_uninstall(force=force)
+def do_uninstall(specs, force=False):
+    # TODO: get rid of the call-sites that use this function,
+    # so that we don't have to do a dance of list -> set -> list -> set
+    hashes_to_remove = set(s.dag_hash() for s in specs)
+
+    for s in traverse.traverse_nodes(
+        specs,
+        order="topo",
+        direction="children",
+        root=True,
+        cover="nodes",
+        deptype="all",
+    ):
+        if s.dag_hash() in hashes_to_remove:
+            spack.package_base.PackageBase.uninstall_by_spec(s, force=force)
 
 
 def get_uninstall_list(args, specs, env):
@@ -419,7 +386,7 @@ def uninstall_specs(args, specs):
         confirm_removal(uninstall_list)
 
     # Uninstall everything on the list
-    do_uninstall(env, uninstall_list, args.force)
+    do_uninstall(uninstall_list, args.force)
 
     if env:
         with env.write_transaction():