You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@buildstream.apache.org by ro...@apache.org on 2020/12/29 13:44:46 UTC
[buildstream] branch tlater/clean-shutils created (now 0d5aeb3)
This is an automated email from the ASF dual-hosted git repository.
root pushed a change to branch tlater/clean-shutils
in repository https://gitbox.apache.org/repos/asf/buildstream.git.
at 0d5aeb3 Clean up usage of shutil.rmtree()
This branch includes the following new commits:
new 0d5aeb3 Clean up usage of shutil.rmtree()
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[buildstream] 01/01: Clean up usage of shutil.rmtree()
Posted by ro...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
root pushed a commit to branch tlater/clean-shutils
in repository https://gitbox.apache.org/repos/asf/buildstream.git
commit 0d5aeb3f22b28408e7e5e48a21bf944d1764d175
Author: Tristan Maat <tr...@codethink.co.uk>
AuthorDate: Tue Dec 31 12:01:58 2019 +0000
Clean up usage of shutil.rmtree()
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.
---
src/buildstream/_cas/casdprocessmanager.py | 22 ++++++++++++++++++++--
src/buildstream/_context.py | 3 +--
src/buildstream/_stream.py | 6 +-----
src/buildstream/sandbox/_sandboxbwrap.py | 6 ++++--
4 files changed, 26 insertions(+), 11 deletions(-)
diff --git a/src/buildstream/_cas/casdprocessmanager.py b/src/buildstream/_cas/casdprocessmanager.py
index 4c9d802..d9dbf1a 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 4700144..d38dcad 100644
--- a/src/buildstream/_context.py
+++ b/src/buildstream/_context.py
@@ -18,7 +18,6 @@
# Tristan Van Berkom <tr...@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 c2945a2..0350fcd 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 433b0f7..de0597e 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)