Fix os.rename in git source element to correctly handle error codes

According to the documentation
(https://www.unix.com/man-page/POSIX/3posix/rename/), when the directory
already is there, either EEXIST or ENOTEMPTY could be thrown.

Previously only ENOTEMPTY was checked.

Done:
  - Separated the move into its own function
  - Check for both errors
  - Create unit tests for it, covering most test cases

1 files changed, 18 insertions, 15 deletions

diff --git a/buildstream/plugins/sources/git.py b/buildstream/plugins/sources/git.py
index 23e3369b1..b51ed79d9 100644
--- a/buildstream/plugins/sources/git.py
+++ b/buildstream/plugins/sources/git.py
@@ -141,21 +141,24 @@ class GitMirror(SourceFetcher):
                                  fail="Failed to clone git repository {}".format(url),
                                  fail_temporarily=True)
 
-                # Attempt atomic rename into destination, this will fail if
-                # another process beat us to the punch
-                try:
-                    os.rename(tmpdir, self.mirror)
-                except OSError as e:
-
-                    # When renaming and the destination repo already exists, os.rename()
-                    # will fail with ENOTEMPTY, since an empty directory will be silently
-                    # replaced
-                    if e.errno == errno.ENOTEMPTY:
-                        self.source.status("{}: Discarding duplicate clone of {}"
-                                           .format(self.source, url))
-                    else:
-                        raise SourceError("{}: Failed to move cloned git repository {} from '{}' to '{}': {}"
-                                          .format(self.source, url, tmpdir, self.mirror, e)) from e
+                self._atomic_move_mirror(tmpdir, url)
+
+    def _atomic_move_mirror(self, tmpdir, url):
+        # Attempt atomic rename into destination, this will fail if
+        # another process beat us to the punch
+        try:
+            os.rename(tmpdir, self.mirror)
+        except OSError as e:
+            # When renaming and the destination repo already exists, os.rename()
+            # will fail with either ENOTEMPTY or EEXIST, depending on the underlying
+            # implementation.
+            # An empty directory would always be replaced.
+            if e.errno in (errno.EEXIST, errno.ENOTEMPTY):
+                self.source.status("{}: Discarding duplicate clone of {}"
+                                   .format(self.source, url))
+            else:
+                raise SourceError("{}: Failed to move cloned git repository {} from '{}' to '{}': {}"
+                                  .format(self.source, url, tmpdir, self.mirror, e)) from e
 
     def _fetch(self, alias_override=None):
         url = self.source.translate_url(self.url,