Clean up usage of shutil.rmtree()tlater/clean-shutils

shutil.rmtree() can fail if the user changes permissions of files
during BuildStream's run time, or sometimes if drives break or some
other horrors occur.

These situations are rare, but if we use shutil.rmtree() they will
result in stacktraces. Instead, we use utils._force_rmtree() now,
which will convert OSErrors into proper BstErrors, which in turn will
bubble up nicely and be reported as usual.

4 files changed, 26 insertions, 11 deletions

diff --git a/src/buildstream/_cas/casdprocessmanager.py b/src/buildstream/_cas/casdprocessmanager.py
index 4c9d80209..d9dbf1a49 100644
--- a/src/buildstream/_cas/casdprocessmanager.py
+++ b/src/buildstream/_cas/casdprocessmanager.py
@@ -19,7 +19,6 @@
 import contextlib
 import os
 import random
-import shutil
 import signal
 import stat
 import subprocess
@@ -162,7 +161,26 @@ class CASDProcessManager:
     def release_resources(self, messenger=None):
         self._terminate(messenger)
         self.process = None
-        shutil.rmtree(self._socket_tempdir)
+        try:
+            utils._force_rmtree(self._socket_tempdir)
+        except utils.UtilError as e:
+            if messenger:
+                messenger.message(
+                    Message(
+                        MessageType.BUG,
+                        "Could not remove the CASD socket from {}. Error: {}".format(self._socket_tempdir, e),
+                        detail="This should not cause any immediate problems, but should be removed manually",
+                    )
+                )
+            # If no messenger is active, it should be ok to just
+            # ignore the error; the user is likely either already
+            # aware of the problem (since the drive is experiencing
+            # catastrophic failure) or somehow deliberately caused it
+            # (by making our socket tempdir immutable).
+            #
+            # We could consider falling back to the logging module in
+            # the future, but this would need to be done throughout
+            # this file.
 
     # _terminate()
     #
diff --git a/src/buildstream/_context.py b/src/buildstream/_context.py
index 47001440a..d38dcadac 100644
--- a/src/buildstream/_context.py
+++ b/src/buildstream/_context.py
@@ -18,7 +18,6 @@
 #        Tristan Van Berkom <tristan.vanberkom@codethink.co.uk>
 
 import os
-import shutil
 from . import utils
 from . import _site
 from . import _yaml
@@ -286,7 +285,7 @@ class Context:
         old_extractdirs = [os.path.join(self.cachedir, "artifacts", "extract"), os.path.join(self.cachedir, "extract")]
         for old_extractdir in old_extractdirs:
             if os.path.isdir(old_extractdir):
-                shutil.rmtree(old_extractdir, ignore_errors=True)
+                utils._force_rmtree(old_extractdir)
 
         # Load quota configuration
         # We need to find the first existing directory in the path of our
diff --git a/src/buildstream/_stream.py b/src/buildstream/_stream.py
index c2945a2f6..0350fcd6d 100644
--- a/src/buildstream/_stream.py
+++ b/src/buildstream/_stream.py
@@ -23,7 +23,6 @@ import os
 import sys
 import stat
 import shlex
-import shutil
 import tarfile
 import tempfile
 from contextlib import contextmanager, suppress
@@ -889,10 +888,7 @@ class Stream:
             with self._context.messenger.timed_activity(
                 "Removing workspace directory {}".format(workspace.get_absolute_path())
             ):
-                try:
-                    shutil.rmtree(workspace.get_absolute_path())
-                except OSError as e:
-                    raise StreamError("Could not remove  '{}': {}".format(workspace.get_absolute_path(), e)) from e
+                utils._force_rmtree(workspace.get_absolute_path())
 
         # Delete the workspace and save the configuration
         workspaces.delete_workspace(element_name)
diff --git a/src/buildstream/sandbox/_sandboxbwrap.py b/src/buildstream/sandbox/_sandboxbwrap.py
index 433b0f754..de0597eb9 100644
--- a/src/buildstream/sandbox/_sandboxbwrap.py
+++ b/src/buildstream/sandbox/_sandboxbwrap.py
@@ -28,7 +28,6 @@ import time
 import errno
 import signal
 import subprocess
-import shutil
 from contextlib import ExitStack, suppress
 from tempfile import TemporaryFile
 
@@ -309,7 +308,10 @@ class SandboxBwrap(Sandbox):
                     #       cleanup any debris it creates at startup time, and do
                     #       the same ourselves for any directories we explicitly create.
                     #
-                    shutil.rmtree(base_directory, ignore_errors=True)
+                    try:
+                        utils._force_rmtree(base_directory)
+                    except utils.UtilError:
+                        pass
                 else:
                     try:
                         os.rmdir(base_directory)