Merge branch 'bschubert/mr938-comments' into 'master'

Followup on MR 938, addressing additional comments

See merge request BuildStream/buildstream!958

1 files changed, 9 insertions, 3 deletions

diff --git a/buildstream/utils.py b/buildstream/utils.py
index 968d87f7b..94c990357 100644
--- a/buildstream/utils.py
+++ b/buildstream/utils.py
@@ -505,17 +505,19 @@ def get_bst_version():
                         .format(__version__))
 
 
-def move_atomic(source, destination, ensure_parents=True):
+def move_atomic(source, destination, *, ensure_parents=True):
     """Move the source to the destination using atomic primitives.
 
     This uses `os.rename` to move a file or directory to a new destination.
     It wraps some `OSError` thrown errors to ensure their handling is correct.
 
     The main reason for this to exist is that rename can throw different errors
-    for the same symptom (https://www.unix.com/man-page/POSIX/3posix/rename/).
+    for the same symptom (https://www.unix.com/man-page/POSIX/3posix/rename/)
+    when we are moving a directory.
 
     We are especially interested here in the case when the destination already
-    exists. In this case, either EEXIST or ENOTEMPTY are thrown.
+    exists, is a directory and is not empty. In this case, either EEXIST or
+    ENOTEMPTY can be thrown.
 
     In order to ensure consistent handling of these exceptions, this function
     should be used instead of `os.rename`
@@ -525,6 +527,10 @@ def move_atomic(source, destination, ensure_parents=True):
       destination (str or Path): destination to which to move the source
       ensure_parents (bool): Whether or not to create the parent's directories
                              of the destination (default: True)
+    Raises:
+      DirectoryExistsError: if the destination directory already exists and is
+                            not empty
+      OSError: if another filesystem level error occured
     """
     if ensure_parents:
         os.makedirs(os.path.dirname(str(destination)), exist_ok=True)