[Notes] [Git][BuildStream/buildstream][tristan/fix-spurious-errors-1.2] git.py: Handle concurrent download completions properly

[Tristan Van Berkom <gitlab mg gitlab com>]

Title: GitLab

Tristan Van Berkom pushed to branch tristan/fix-spurious-errors-1.2 at BuildStream / buildstream

Commits:

• 95cc9793
  by Tristan Van Berkom at 2018-07-29T17:21:24Z

  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
  

1 changed file:

• buildstream/plugins/sources/git.py

Changes:

buildstream/plugins/sources/git.py

---

...	...	@@ -71,8 +71,8 @@ git - stage files from a git repository
71	71	"""
72	72
73	73	import os
	74	+import errno
74	75	import re
75		-import shutil
76	76	from collections import Mapping
77	77	from io import StringIO
78	78
...	...	@@ -119,11 +119,21 @@ class GitMirror(SourceFetcher):
119	119	fail="Failed to clone git repository {}".format(url),
120	120	fail_temporarily=True)
121	121
	122	+ # Attempt atomic rename into destination, this will fail if
	123	+ # another process beat us to the punch
122	124	try:
123		- shutil.move(tmpdir, self.mirror)
124		- except (shutil.Error, OSError) as e:
125		- raise SourceError("{}: Failed to move cloned git repository {} from '{}' to '{}'"
126		- .format(self.source, url, tmpdir, self.mirror)) from e
	125	+ os.rename(tmpdir, self.mirror)
	126	+ except OSError as e:
	127	+
	128	+ # When renaming and the destination repo already exists, os.rename()
	129	+ # will fail with ENOTEMPTY, since an empty directory will be silently
	130	+ # replaced
	131	+ if e.errno == errno.ENOTEMPTY:
	132	+ self.source.status("{}: Discarding duplicate clone of {}"
	133	+ .format(self.source, url))
	134	+ else:
	135	+ raise SourceError("{}: Failed to move cloned git repository {} from '{}' to '{}': {}"
	136	+ .format(self.source, url, tmpdir, self.mirror, e)) from e
127	137
128	138	def _fetch(self, alias_override=None):
129	139	url = self.source.translate_url(self.url, alias_override=alias_override)

—
View it on GitLab.
You're receiving this email because of your account on gitlab.com. If you'd like to receive fewer emails, you can adjust your notification settings.