[Notes] [Git][BuildStream/buildstream][tristan/fix-spurious-errors] 5 commits: buildelement.py: Only display one activity message (#507)

[Tristan Van Berkom <gitlab mg gitlab com>]

Title: GitLab

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

Commits:

• cb8c99e5
  by Tristan Maat at 2018-07-29T07:52:06Z

  buildelement.py: Only display one activity message (#507)
  

• 08b7562b
  by Tristan Van Berkom at 2018-07-29T10:23:43Z

  Merge branch '507-some-log-lines-appear-to-be-duplicates' into 'master'
  
  Resolve "Some log lines appear to be duplicates"
  
  Closes #507
  
  See merge request BuildStream/buildstream!573

• 3b7158b3
  by Tristan Van Berkom at 2018-07-29T10:25:02Z

  source.py: Bring down since version for source mirroring APIs
  
  Since we have now backported this to `bst-1.2`, the APIs have
  been introduced in 1.2 and not 1.4
  

• e84f8b24
  by Tristan Van Berkom at 2018-07-29T10:25:02Z

  NEWS: Bring down since version of backported features to 1.1.5
  
  These features will be released in 1.1.5 since they have been
  backported, lets reflect this in the NEWS file in master.
  
  Also this adds the backported `remote` plugin to the NEWS
  

• df18d38c
  by Tristan Van Berkom at 2018-07-29T10:26:08Z

  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
  

4 changed files:

• NEWS
• buildstream/buildelement.py
• buildstream/plugins/sources/git.py
• buildstream/source.py

Changes:

NEWS

---

1	1	=================
2		-buildstream 1.3.1
	2	+buildstream 1.1.5
3	3	=================
4	4
5	5	o Add a `--tar` option to `bst checkout` which allows a tarball to be
...	...	@@ -9,6 +9,8 @@ buildstream 1.3.1
9	9	and the preferred mirror to fetch from can be defined in the command
10	10	line or user config.
11	11
	12	+ o Added new `remote` source plugin for downloading file blobs
	13	+
12	14	=================
13	15	buildstream 1.1.4
14	16	=================

buildstream/buildelement.py

---

1	1	#
2	2	# Copyright (C) 2016 Codethink Limited
	3	+# Copyright (C) 2018 Bloomberg Finance LP
3	4	#
4	5	# This program is free software; you can redistribute it and/or
5	6	# modify it under the terms of the GNU Lesser General Public
...	...	@@ -204,8 +205,9 @@ class BuildElement(Element):
204	205	def prepare(self, sandbox):
205	206	commands = self.__commands['configure-commands']
206	207	if commands:
207		- for cmd in commands:
208		- self.__run_command(sandbox, cmd, 'configure-commands')
	208	+ with self.timed_activity("Running configure-commands"):
	209	+ for cmd in commands:
	210	+ self.__run_command(sandbox, cmd, 'configure-commands')
209	211
210	212	def generate_script(self):
211	213	script = ""
...	...	@@ -231,13 +233,12 @@ class BuildElement(Element):
231	233	return commands
232	234
233	235	def __run_command(self, sandbox, cmd, cmd_name):
234		- with self.timed_activity("Running {}".format(cmd_name)):
235		- self.status("Running {}".format(cmd_name), detail=cmd)
236		-
237		- # Note the -e switch to 'sh' means to exit with an error
238		- # if any untested command fails.
239		- #
240		- exitcode = sandbox.run(['sh', '-c', '-e', cmd + '\n'],
241		- SandboxFlags.ROOT_READ_ONLY)
242		- if exitcode != 0:
243		- raise ElementError("Command '{}' failed with exitcode {}".format(cmd, exitcode))
	236	+ self.status("Running {}".format(cmd_name), detail=cmd)
	237	+
	238	+ # Note the -e switch to 'sh' means to exit with an error
	239	+ # if any untested command fails.
	240	+ #
	241	+ exitcode = sandbox.run(['sh', '-c', '-e', cmd + '\n'],
	242	+ SandboxFlags.ROOT_READ_ONLY)
	243	+ if exitcode != 0:
	244	+ raise ElementError("Command '{}' failed with exitcode {}".format(cmd, exitcode))

buildstream/plugins/sources/git.py

---

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

buildstream/source.py

---

...	...	@@ -148,7 +148,7 @@ class SourceFetcher():
148	148	places (e.g. a git source with submodules) has a consistent interface for
149	149	fetching and substituting aliases.
150	150
151		- *Since: 1.4*
	151	+ *Since: 1.2*
152	152	"""
153	153	def __init__(self):
154	154	self.__alias = None
...	...	@@ -382,7 +382,7 @@ class Source(Plugin):
382	382	Args:
383	383	url (str): The url used to download
384	384
385		- *Since: 1.4*
	385	+ *Since: 1.2*
386	386	"""
387	387	alias, _ = url.split(utils._ALIAS_SEPARATOR, 1)
388	388	self.__expected_alias = alias
...	...	@@ -398,7 +398,7 @@ class Source(Plugin):
398	398	list: A list of SourceFetchers. If SourceFetchers are not supported,
399	399	this will be an empty list.
400	400
401		- *Since: 1.4*
	401	+ *Since: 1.2*
402	402	"""
403	403
404	404	return []
...	...	@@ -425,7 +425,7 @@ class Source(Plugin):
425	425
426	426	Args:
427	427	url (str): A url, which may be using an alias
428		- alias_override (str): Optionally, an URI to override the alias with. (*Since: 1.4*)
	428	+ alias_override (str): Optionally, an URI to override the alias with. (*Since: 1.2*)
429	429
430	430	Returns:
431	431	str: The fully qualified url, with aliases resolved

—
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.