summaryrefslogtreecommitdiff
path: root/buildstream
diff options
context:
space:
mode:
authorTristan Van Berkom <tristan.vanberkom@codethink.co.uk>2018-07-29 17:39:16 +0900
committerTristan Van Berkom <tristan.vanberkom@codethink.co.uk>2018-07-29 19:26:08 +0900
commitdf18d38c917e3e067c2b19a36447b23c932baad9 (patch)
tree4e3d92057cbba699e160ed72e7e3516756324e95 /buildstream
parente84f8b24170f718dcdbe775910ab1dd13cb8be32 (diff)
downloadbuildstream-df18d38c917e3e067c2b19a36447b23c932baad9.tar.gz
git.py: Handle concurrent download completions properly
Use os.rename() to rename the cloned temporary repository into place in the source cache, and issue a STATUS message when discarding a duplicate clone, in the case where the same repository is cloned twice in parallel. The problem with using shutil.move() is that it will create the source directory in a subdirectory of the destination when the destination exists, so it's behavior depends on whether the destination exists. This shutil.move() behavior has so far hidden the race condition where a duplicate repo is created in a subdirectory, as you need to have three concurrent downloads of the same repo in order to trigger the error. This fixes issue #503
Diffstat (limited to 'buildstream')
-rw-r--r--buildstream/plugins/sources/git.py19
1 files changed, 15 insertions, 4 deletions
diff --git a/buildstream/plugins/sources/git.py b/buildstream/plugins/sources/git.py
index bccf5d6b7..32cdcbb97 100644
--- a/buildstream/plugins/sources/git.py
+++ b/buildstream/plugins/sources/git.py
@@ -71,6 +71,7 @@ git - stage files from a git repository
"""
import os
+import errno
import re
import shutil
from collections import Mapping
@@ -119,11 +120,21 @@ 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:
- shutil.move(tmpdir, self.mirror)
- except (shutil.Error, OSError) as e:
- raise SourceError("{}: Failed to move cloned git repository {} from '{}' to '{}'"
- .format(self.source, url, tmpdir, self.mirror)) from e
+ 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
def _fetch(self, alias_override=None):
url = self.source.translate_url(self.url, alias_override=alias_override)