[Notes] [Git][BuildStream/buildstream][mac_fixes] 23 commits: buildstream/_project.py: Report if project.conf is missing name

[Phillip Smyth <gitlab mg gitlab com>]

Title: GitLab

Phillip Smyth pushed to branch mac_fixes at BuildStream / buildstream

Commits:

• 18d0bfb4
  by Tom Pollard at 2018-08-30T19:14:33Z

  buildstream/_project.py: Report if project.conf is missing name
  
  Explicitly check that project.conf contains a name. This resolves
  the issue of the provenance check from _yaml.py incorrectly reporting
  the offending file as the default_config_node projectconfig.yaml
  when no name key exists in the pre_config_node dict.
  

• 95121148
  by Tiago Gomes at 2018-08-30T20:52:29Z

  Merge branch 'tpollard/591' into 'master'
  
  buildstream/_project.py: Report if project.conf is missing name
  
  See merge request BuildStream/buildstream!680

• 775d3fca
  by Tristan Van Berkom at 2018-08-31T06:05:32Z

  _project.py: Adding FIXME comment to address #591 properly
  

• 0682aa1c
  by Josh Smith at 2018-08-31T10:58:12Z

  job.py: Modify retry messages to be FAIL
  
  This adjusts the message handler for the child processes to no longer
  override the message type.
  
  This also removes the ability for unhandled non BstError exceptions to
  retry.
  

• 68eff9ba
  by Josh Smith at 2018-08-31T10:58:20Z

  tests: Modify tests to ensure retry FAILURE
  

• 9b18e1bf
  by Tristan Van Berkom at 2018-08-31T11:22:51Z

  Merge branch 'Qinusty/retries-should-fail' into 'master'
  
  Retries log as failures
  
  See merge request BuildStream/buildstream!766

• e2d8fedf
  by Josh Smith at 2018-08-31T15:53:43Z

  scheduler.py: Move cache_size logs into folder
  
  This prevents the cache_size.pid.log files from cluttering the root
  log directory.
  

• 6805a2ab
  by Tristan Van Berkom at 2018-09-01T08:24:05Z

  Merge branch 'Qinusty/cache-size-directory' into 'master'
  
  Move cache_size.pid.log files into a subdirectory of logs
  
  See merge request BuildStream/buildstream!769

• f8b06acc
  by Tristan Van Berkom at 2018-09-02T09:33:39Z

  source.py: Document Source.get_source_fetchers() to return an iterable
  
  Also highlight the fact that the plugin can rely on the fetcher's
  fetch() method getting called before consuming the next item in the
  list, which is the magick behavior that the git plugin relies on.
  
  This is a part of #620
  

• 559b6fbc
  by Tristan Van Berkom at 2018-09-02T09:34:10Z

  source.py: Fixing docs link formatting to be consistent.
  

• 43ad22d7
  by Tristan Van Berkom at 2018-09-02T09:34:10Z

  source.py: Documenting that marking download URLs is mandatory
  
  A download URL must be interpreted by the core at `Plugin.configure()`
  time, even if only employed later on.
  
  This is a part of #620
  

• 16462e9c
  by Tristan Van Berkom at 2018-09-02T09:36:49Z

  plugin.py: Added _configure() and _get_configuring() private APIs
  
  Keeps track of whether the plugin is currently being configured.
  
  Adjusted Element and Source classes to call _configure() in place
  of calling configure() directly.
  
  This is a part of #620
  

• 1f9c4147
  by Tristan Van Berkom at 2018-09-02T09:37:12Z

  source.py: Added `primary` argument to URL marking APIs
  
  The Source must now mention whether the marked or translated
  URL is "primary" or not. Even when a Source may have multiple
  URLs, the auxilliary URLs are derived from the primary one, not
  only is this true for git, but it is mandated by our tracking
  API which assumes there is a primary URL.
  
  This adjusts the `git` source and the test `fetch_source.py` source
  to behave properly and advertize it's primary URL properly.
  
  This is a part of #620
  

• f9b2f1a9
  by Tristan Van Berkom at 2018-09-02T09:37:21Z

  source.py: Track marked URLs and assert they are marked during Plugin.configure()
  
  This cannot test for unaliased URLs, as those can be discovered later
  on outside of user provided element configuration; at least we
  assert that if an alias was used, we have seen it at load time.
  
  This will cause a BUG to occur for a plugin which marks an aliased
  URL (or attempts to translate one) outside of `Plugin.configure()`,
  if that URL was not previously seen.
  
  This is a part of #620
  

• 88460cd2
  by Tristan Van Berkom at 2018-09-03T06:23:38Z

  Merge branch 'tristan/source-fetcher-changes' into 'master'
  
  Source fetcher changes
  
  See merge request BuildStream/buildstream!772

• 2339f0c4
  by Tom Pollard at 2018-09-03T09:44:36Z

  plugins/git.py: Warn if ref is not in given track
  
  Add a helper function assert_ref_in_track to git.py GitMirror()
  which is used when staging & initing the source workspace. It
  checks the element's ref exists in the track (branch/tag) if it
  has been specified, raising a warning if necessary. The warning makes
  use of the warning token 'REF_NOT_IN_TRACK' from the configurable
  CoreWarnings. If the element has been tracked with bst, it is assumed
  that the value of ref exists in the track as it was generated from it
  & as such is not asserted.
  

• c96fec5d
  by Tom Pollard at 2018-09-03T09:44:36Z

  tests/sources/git.py: Add tests for REF_NOT_IN_TRACK
  
  Add tests that cover assert_ref_in_track & the configurable
  CoreWarnings REF_NOT_IN_TRACK warnings token.
  

• 6a0cdedf
  by Tom Pollard at 2018-09-03T10:16:39Z

  Merge branch 'tpollard/483' into 'master'
  
  plugins/git.py: Warn if ref is not in given track
  
  See merge request BuildStream/buildstream!564

• bba998f5
  by knownexus at 2018-09-03T15:23:26Z

  Moved `resource.getrlimit()`
  
  Removed `resource.getrlimit()` functionality from app.py
  Added `resource.getrlimit()` functionality to linux.py
  Added `resource.getrlimit()` functionality to unix.py
  

• 97a7b15d
  by knownexus at 2018-09-03T15:23:26Z

  Add FUSE check to linux.py
  
  This needed to be implemented as Windows WSL
  Does not currently support FUSE
  

• e866ad63
  by knownexus at 2018-09-03T15:23:26Z

  Adding darwin.py platform
  
  Adding functionality to recognise Darwin as a platform in plaform.py
  

• ab122039
  by knownexus at 2018-09-03T16:12:16Z

  Moving `os.sched_getaffinity(0)` logic
  
  Removed `os.sched_getaffinity(0)` from _project.py
  Added `os.sched_getaffinity(0)` to platform.py
  Added `os.cpu_count()` to darwin.py
  

• 6bb43613
  by knownexus at 2018-09-03T16:12:22Z

  Adding EPERM check to utils.safe_remove
  
  Mac seems to allow the unlinking of directories
  

18 changed files:

• buildstream/_frontend/app.py
• + buildstream/_platform/darwin.py
• buildstream/_platform/linux.py
• buildstream/_platform/platform.py
• buildstream/_platform/unix.py
• buildstream/_project.py
• buildstream/_scheduler/jobs/job.py
• buildstream/_scheduler/scheduler.py
• buildstream/element.py
• buildstream/plugin.py
• buildstream/plugins/sources/git.py
• buildstream/source.py
• buildstream/utils.py
• tests/frontend/project/sources/fetch_source.py
• tests/sources/deb.py
• tests/sources/git.py
• tests/sources/tar.py
• tests/sources/zip.py

Changes:

buildstream/_frontend/app.py

---

...	...	@@ -116,14 +116,6 @@ class App():
116	116	else:
117	117	self.colors = False
118	118
119		- # Increase the soft limit for open file descriptors to the maximum.
120		- # SafeHardlinks FUSE needs to hold file descriptors for all processes in the sandbox.
121		- # Avoid hitting the limit too quickly.
122		- limits = resource.getrlimit(resource.RLIMIT_NOFILE)
123		- if limits[0] != limits[1]:
124		- # Set soft limit to hard limit
125		- resource.setrlimit(resource.RLIMIT_NOFILE, (limits[1], limits[1]))
126		-
127	119	# create()
128	120	#
129	121	# Should be used instead of the regular constructor.

buildstream/_platform/darwin.py

---

	1	+#
	2	+# Copyright (C) 2017 Codethink Limited
	3	+# Copyright (C) 2018 Bloomberg Finance LP
	4	+#
	5	+# This program is free software; you can redistribute it and/or
	6	+# modify it under the terms of the GNU Lesser General Public
	7	+# License as published by the Free Software Foundation; either
	8	+# version 2 of the License, or (at your option) any later version.
	9	+#
	10	+# This library is distributed in the hope that it will be useful,
	11	+# but WITHOUT ANY WARRANTY; without even the implied warranty of
	12	+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
	13	+# Lesser General Public License for more details.
	14	+#
	15	+# You should have received a copy of the GNU Lesser General Public
	16	+# License along with this library. If not, see <http://www.gnu.org/licenses/>.
	17	+
	18	+import os
	19	+import resource
	20	+
	21	+from .._exceptions import PlatformError
	22	+from ..sandbox import SandboxChroot
	23	+
	24	+from . import Platform
	25	+
	26	+
	27	+class Darwin(Platform):
	28	+
	29	+ def __init__(self, context):
	30	+
	31	+ super().__init__(context)
	32	+
	33	+ # Need to set resources for _frontend/app.py as this is dependent on the platform
	34	+ # SafeHardlinks FUSE needs to hold file descriptors for all processes in the sandbox.
	35	+ # Avoid hitting the limit too quickly.
	36	+ limits = resource.getrlimit(resource.RLIMIT_NOFILE)
	37	+ if limits[0] != limits[1]:
	38	+ resource.setrlimit(resource.RLIMIT_NOFILE, (49152, limits[1]))
	39	+
	40	+ @property
	41	+ def artifactcache(self):
	42	+ return self._artifact_cache
	43	+
	44	+ def create_sandbox(self, *args, **kwargs):
	45	+ return SandboxChroot(*args, **kwargs)
	46	+
	47	+ def get_cpu_count(self, cap=None):
	48	+ if cap < os.cpu_count():
	49	+ return cap
	50	+ else:
	51	+ return os.cpu_count()

buildstream/_platform/linux.py

---

...	...	@@ -17,10 +17,12 @@
17	17	# Authors:
18	18	# Tristan Maat <tristan maat codethink co uk>
19	19
	20	+import os
20	21	import subprocess
21	22
22	23	from .. import _site
23	24	from .. import utils
	25	+import resource
24	26	from .._artifactcache.cascache import CASCache
25	27	from .._message import Message, MessageType
26	28	from ..sandbox import SandboxBwrap
...	...	@@ -35,9 +37,18 @@ class Linux(Platform):
35	37	super().__init__(context)
36	38
37	39	self._die_with_parent_available = _site.check_bwrap_version(0, 1, 8)
38		- self._user_ns_available = self._check_user_ns_available(context)
	40	+
	41	+ self._user_ns_available = self._check_fuse_available() and self._check_user_ns_available(context)
	42	+
39	43	self._artifact_cache = CASCache(context, enable_push=self._user_ns_available)
40	44
	45	+ # Need to set resources for _frontend/app.py as this is dependent on the platform
	46	+ # SafeHardlinks FUSE needs to hold file descriptors for all processes in the sandbox.
	47	+ # Avoid hitting the limit too quickly.
	48	+ limits = resource.getrlimit(resource.RLIMIT_NOFILE)
	49	+ if limits[0] != limits[1]:
	50	+ resource.setrlimit(resource.RLIMIT_NOFILE, (limits[1], limits[1]))
	51	+
41	52	@property
42	53	def artifactcache(self):
43	54	return self._artifact_cache
...	...	@@ -51,6 +62,9 @@ class Linux(Platform):
51	62	################################################
52	63	# Private Methods #
53	64	################################################
	65	+ def _check_fuse_available(self):
	66	+ return os.path.exists('/dev/fuse')
	67	+
54	68	def _check_user_ns_available(self, context):
55	69
56	70	# Here, lets check if bwrap is able to create user namespaces,

buildstream/_platform/platform.py

---

...	...	@@ -21,6 +21,7 @@ import os
21	21	import sys
22	22
23	23	from .._exceptions import PlatformError, ImplError
	24	+from .._artifactcache.cascache import CASCache
24	25
25	26
26	27	class Platform():
...	...	@@ -37,22 +38,27 @@ class Platform():
37	38	#
38	39	def __init__(self, context):
39	40	self.context = context
	41	+ self._artifact_cache = CASCache(context)
40	42
41	43	@classmethod
42	44	def create_instance(cls, *args, **kwargs):
43		- if sys.platform.startswith('linux'):
44		- backend = 'linux'
45		- else:
46		- backend = 'unix'
47	45
48	46	# Meant for testing purposes and therefore hidden in the
49	47	# deepest corners of the source code. Try not to abuse this,
50	48	# please?
51	49	if os.getenv('BST_FORCE_BACKEND'):
52	50	backend = os.getenv('BST_FORCE_BACKEND')
	51	+ elif sys.platform.startswith('linux'):
	52	+ backend = 'linux'
	53	+ elif sys.platform.startswith('darwin'):
	54	+ backend = 'darwin'
	55	+ else:
	56	+ backend = 'unix'
53	57
54	58	if backend == 'linux':
55	59	from .linux import Linux as PlatformImpl
	60	+ elif backend == 'darwin':
	61	+ from .darwin import Darwin as PlatformImpl
56	62	elif backend == 'unix':
57	63	from .unix import Unix as PlatformImpl
58	64	else:
...	...	@@ -92,3 +98,9 @@ class Platform():
92	98	def create_sandbox(self, *args, **kwargs):
93	99	raise ImplError("Platform {platform} does not implement create_sandbox()"
94	100	.format(platform=type(self).__name__))
	101	+
	102	+ def get_cpu_count(self, cap=None):
	103	+ if cap is None:
	104	+ return len(os.sched_getaffinity(0))
	105	+ else:
	106	+ return min(len(os.sched_getaffinity(0)), cap)

buildstream/_platform/unix.py

---

...	...	@@ -18,8 +18,8 @@
18	18	# Tristan Maat <tristan maat codethink co uk>
19	19
20	20	import os
	21	+import resource
21	22
22		-from .._artifactcache.cascache import CASCache
23	23	from .._exceptions import PlatformError
24	24	from ..sandbox import SandboxChroot
25	25
...	...	@@ -31,7 +31,13 @@ class Unix(Platform):
31	31	def __init__(self, context):
32	32
33	33	super().__init__(context)
34		- self._artifact_cache = CASCache(context)
	34	+
	35	+ # Need to set resources for _frontend/app.py as this is dependent on the platform
	36	+ # SafeHardlinks FUSE needs to hold file descriptors for all processes in the sandbox.
	37	+ # Avoid hitting the limit too quickly.
	38	+ limits = resource.getrlimit(resource.RLIMIT_NOFILE)
	39	+ if limits[0] != limits[1]:
	40	+ resource.setrlimit(resource.RLIMIT_NOFILE, (limits[1], limits[1]))
35	41
36	42	# Not necessarily 100% reliable, but we want to fail early.
37	43	if os.geteuid() != 0:

buildstream/_project.py

---

...	...	@@ -19,6 +19,7 @@
19	19	# Tiago Gomes <tiago gomes codethink co uk>
20	20
21	21	import os
	22	+import sys
22	23	from collections import Mapping, OrderedDict
23	24	from pluginbase import PluginBase
24	25	from . import utils
...	...	@@ -38,6 +39,7 @@ from ._loader import Loader
38	39	from .element import Element
39	40	from ._message import Message, MessageType
40	41	from ._includes import Includes
	42	+from ._platform import Platform
41	43
42	44
43	45	# Project Configuration file
...	...	@@ -398,6 +400,17 @@ class Project():
398	400	"Project requested format version {}, but BuildStream {}.{} only supports up until format version {}"
399	401	.format(format_version, major, minor, BST_FORMAT_VERSION))
400	402
	403	+ # FIXME:
	404	+ #
	405	+ # Performing this check manually in the absense
	406	+ # of proper support from _yaml.node_get(), this should
	407	+ # be removed in favor of a proper accessor function
	408	+ # from the _yaml module when #591 is fixed.
	409	+ #
	410	+ if self._project_conf.get('name') is None:
	411	+ raise LoadError(LoadErrorReason.INVALID_DATA,
	412	+ "{}: project.conf does not contain expected key '{}'".format(projectfile, 'name'))
	413	+
401	414	# The project name, element path and option declarations
402	415	# are constant and cannot be overridden by option conditional statements
403	416	self.name = _yaml.node_get(pre_config_node, str, 'name')
...	...	@@ -594,7 +607,8 @@ class Project():
594	607	# Based on some testing (mainly on AWS), maximum effective
595	608	# max-jobs value seems to be around 8-10 if we have enough cores
596	609	# users should set values based on workload and build infrastructure
597		- output.base_variables['max-jobs'] = str(min(len(os.sched_getaffinity(0)), 8))
	610	+ platform = Platform.get_platform()
	611	+ output.base_variables['max-jobs'] = str(platform.get_cpu_count(8))
598	612
599	613	# Export options into variables, if that was requested
600	614	output.options.export_variables(output.base_variables)

buildstream/_scheduler/jobs/job.py

---

...	...	@@ -403,7 +403,7 @@ class Job():
403	403	if self._retry_flag and (self._tries <= self._max_retries):
404	404	self.message(MessageType.FAIL,
405	405	"Try #{} failed, retrying".format(self._tries),
406		- elapsed=elapsed)
	406	+ elapsed=elapsed, logfile=filename)
407	407	else:
408	408	self.message(MessageType.FAIL, str(e),
409	409	elapsed=elapsed, detail=e.detail,
...	...	@@ -430,7 +430,8 @@ class Job():
430	430	self.message(MessageType.BUG, self.action_name,
431	431	elapsed=elapsed, detail=detail,
432	432	logfile=filename)
433		- self._child_shutdown(RC_FAIL)
	433	+ # Unhandled exceptions should permenantly fail
	434	+ self._child_shutdown(RC_PERM_FAIL)
434	435
435	436	else:
436	437	# No exception occurred in the action
...	...	@@ -509,11 +510,6 @@ class Job():
509	510	message.action_name = self.action_name
510	511	message.task_id = self._task_id
511	512
512		- if (message.message_type == MessageType.FAIL and
513		- self._tries <= self._max_retries and self._retry_flag):
514		- # Job will be retried, display failures as warnings in the frontend
515		- message.message_type = MessageType.WARN
516		-
517	513	# Send to frontend if appropriate
518	514	if context.silent_messages() and (message.message_type not in unconditional_messages):
519	515	return

buildstream/_scheduler/scheduler.py

---

...	...	@@ -329,7 +329,7 @@ class Scheduler():
329	329	self.schedule_jobs([job])
330	330
331	331	def _check_cache_size_real(self):
332		- job = CacheSizeJob(self, 'cache_size', 'cache_size',
	332	+ job = CacheSizeJob(self, 'cache_size', 'cache_size/cache_size',
333	333	resources=[ResourceType.CACHE,
334	334	ResourceType.PROCESS],
335	335	exclusive_resources=[ResourceType.CACHE],

buildstream/element.py

---

...	...	@@ -245,7 +245,7 @@ class Element(Plugin):
245	245	# Collect the composited element configuration and
246	246	# ask the element to configure itself.
247	247	self.__config = self.__extract_config(meta)
248		- self.configure(self.__config)
	248	+ self._configure(self.__config)
249	249
250	250	# Extract Sandbox config
251	251	self.__sandbox_config = self.__extract_sandbox_config(meta)

buildstream/plugin.py

---

...	...	@@ -179,6 +179,7 @@ class Plugin():
179	179	self.__provenance = provenance # The Provenance information
180	180	self.__type_tag = type_tag # The type of plugin (element or source)
181	181	self.__unique_id = _plugin_register(self) # Unique ID
	182	+ self.__configuring = False # Whether we are currently configuring
182	183
183	184	# Infer the kind identifier
184	185	modulename = type(self).__module__
...	...	@@ -682,7 +683,32 @@ class Plugin():
682	683	else:
683	684	yield log
684	685
	686	+ # _configure():
	687	+ #
	688	+ # Calls configure() for the plugin, this must be called by
	689	+ # the core instead of configure() directly, so that the
	690	+ # _get_configuring() state is up to date.
	691	+ #
	692	+ # Args:
	693	+ # node (dict): The loaded configuration dictionary
	694	+ #
	695	+ def _configure(self, node):
	696	+ self.__configuring = True
	697	+ self.configure(node)
	698	+ self.__configuring = False
	699	+
	700	+ # _get_configuring():
	701	+ #
	702	+ # Checks whether the plugin is in the middle of having
	703	+ # its Plugin.configure() method called
	704	+ #
	705	+ # Returns:
	706	+ # (bool): Whether we are currently configuring
	707	+ def _get_configuring(self):
	708	+ return self.__configuring
	709	+
685	710	# _preflight():
	711	+ #
686	712	# Calls preflight() for the plugin, and allows generic preflight
687	713	# checks to be added
688	714	#
...	...	@@ -690,6 +716,7 @@ class Plugin():
690	716	# SourceError: If it's a Source implementation
691	717	# ElementError: If it's an Element implementation
692	718	# ProgramNotFoundError: If a required host tool is not found
	719	+ #
693	720	def _preflight(self):
694	721	self.preflight()
695	722

buildstream/plugins/sources/git.py

---

...	...	@@ -74,6 +74,9 @@ This plugin provides the following configurable warnings:
74	74
75	75	- 'git:inconsistent-submodule' - A submodule was found to be missing from the underlying git repository.
76	76
	77	+This plugin also utilises the following configurable core plugin warnings:
	78	+
	79	+- 'ref-not-in-track' - The provided ref was not found in the provided track in the element's git repository.
77	80	"""
78	81
79	82	import os
...	...	@@ -87,6 +90,7 @@ from configparser import RawConfigParser
87	90
88	91	from buildstream import Source, SourceError, Consistency, SourceFetcher
89	92	from buildstream import utils
	93	+from buildstream.plugin import CoreWarnings
90	94
91	95	GIT_MODULES = '.gitmodules'
92	96
...	...	@@ -100,13 +104,14 @@ INCONSISTENT_SUBMODULE = "inconsistent-submodules"
100	104	#
101	105	class GitMirror(SourceFetcher):
102	106
103		- def __init__(self, source, path, url, ref):
	107	+ def __init__(self, source, path, url, ref, *, primary=False):
104	108
105	109	super().__init__()
106	110	self.source = source
107	111	self.path = path
108	112	self.url = url
109	113	self.ref = ref
	114	+ self.primary = primary
110	115	self.mirror = os.path.join(source.get_mirror_directory(), utils.url_directory_name(url))
111	116	self.mark_download_url(url)
112	117
...	...	@@ -124,7 +129,8 @@ class GitMirror(SourceFetcher):
124	129	# system configured tmpdir is not on the same partition.
125	130	#
126	131	with self.source.tempdir() as tmpdir:
127		- url = self.source.translate_url(self.url, alias_override=alias_override)
	132	+ url = self.source.translate_url(self.url, alias_override=alias_override,
	133	+ primary=self.primary)
128	134	self.source.call([self.source.host_git, 'clone', '--mirror', '-n', url, tmpdir],
129	135	fail="Failed to clone git repository {}".format(url),
130	136	fail_temporarily=True)
...	...	@@ -146,7 +152,9 @@ class GitMirror(SourceFetcher):
146	152	.format(self.source, url, tmpdir, self.mirror, e)) from e
147	153
148	154	def _fetch(self, alias_override=None):
149		- url = self.source.translate_url(self.url, alias_override=alias_override)
	155	+ url = self.source.translate_url(self.url,
	156	+ alias_override=alias_override,
	157	+ primary=self.primary)
150	158
151	159	if alias_override:
152	160	remote_name = utils.url_directory_name(alias_override)
...	...	@@ -199,7 +207,7 @@ class GitMirror(SourceFetcher):
199	207	cwd=self.mirror)
200	208	return output.rstrip('\n')
201	209
202		- def stage(self, directory):
	210	+ def stage(self, directory, track=None):
203	211	fullpath = os.path.join(directory, self.path)
204	212
205	213	# Using --shared here avoids copying the objects into the checkout, in any
...	...	@@ -213,10 +221,14 @@ class GitMirror(SourceFetcher):
213	221	fail="Failed to checkout git ref {}".format(self.ref),
214	222	cwd=fullpath)
215	223
	224	+ # Check that the user specified ref exists in the track if provided & not already tracked
	225	+ if track:
	226	+ self.assert_ref_in_track(fullpath, track)
	227	+
216	228	# Remove .git dir
217	229	shutil.rmtree(os.path.join(fullpath, ".git"))
218	230
219		- def init_workspace(self, directory):
	231	+ def init_workspace(self, directory, track=None):
220	232	fullpath = os.path.join(directory, self.path)
221	233	url = self.source.translate_url(self.url)
222	234
...	...	@@ -232,6 +244,10 @@ class GitMirror(SourceFetcher):
232	244	fail="Failed to checkout git ref {}".format(self.ref),
233	245	cwd=fullpath)
234	246
	247	+ # Check that the user specified ref exists in the track if provided & not already tracked
	248	+ if track:
	249	+ self.assert_ref_in_track(fullpath, track)
	250	+
235	251	# List the submodules (path/url tuples) present at the given ref of this repo
236	252	def submodule_list(self):
237	253	modules = "{}:{}".format(self.ref, GIT_MODULES)
...	...	@@ -296,6 +312,28 @@ class GitMirror(SourceFetcher):
296	312
297	313	return None
298	314
	315	+ # Assert that ref exists in track, if track has been specified.
	316	+ def assert_ref_in_track(self, fullpath, track):
	317	+ _, branch = self.source.check_output([self.source.host_git, 'branch', '--list', track,
	318	+ '--contains', self.ref],
	319	+ cwd=fullpath,)
	320	+ if branch:
	321	+ return True
	322	+ else:
	323	+ _, tag = self.source.check_output([self.source.host_git, 'tag', '--list', track,
	324	+ '--contains', self.ref],
	325	+ cwd=fullpath,)
	326	+ if tag:
	327	+ return True
	328	+
	329	+ detail = "The ref provided for the element does not exist locally in the provided track branch / tag " + \
	330	+ "'{}'.\nYou may wish to track the element to update the ref from '{}' ".format(track, track) + \
	331	+ "with `bst track`,\nor examine the upstream at '{}' for the specific ref.".format(self.url)
	332	+
	333	+ self.source.warn("{}: expected ref '{}' was not found in given track '{}' for staged repository: '{}'\n"
	334	+ .format(self.source, self.ref, track, self.url),
	335	+ detail=detail, warning_token=CoreWarnings.REF_NOT_IN_TRACK)
	336	+
299	337
300	338	class GitSource(Source):
301	339	# pylint: disable=attribute-defined-outside-init
...	...	@@ -307,7 +345,7 @@ class GitSource(Source):
307	345	self.node_validate(node, config_keys + Source.COMMON_CONFIG_KEYS)
308	346
309	347	self.original_url = self.node_get_member(node, str, 'url')
310		- self.mirror = GitMirror(self, '', self.original_url, ref)
	348	+ self.mirror = GitMirror(self, '', self.original_url, ref, primary=True)
311	349	self.tracking = self.node_get_member(node, str, 'track', None)
312	350
313	351	# At this point we now know if the source has a ref and/or a track.
...	...	@@ -327,12 +365,18 @@ class GitSource(Source):
327	365	for path, _ in self.node_items(modules):
328	366	submodule = self.node_get_member(modules, Mapping, path)
329	367	url = self.node_get_member(submodule, str, 'url', None)
	368	+
	369	+ # Make sure to mark all URLs that are specified in the configuration
	370	+ if url:
	371	+ self.mark_download_url(url, primary=False)
	372	+
330	373	self.submodule_overrides[path] = url
331	374	if 'checkout' in submodule:
332	375	checkout = self.node_get_member(submodule, bool, 'checkout')
333	376	self.submodule_checkout_overrides[path] = checkout
334	377
335	378	self.mark_download_url(self.original_url)
	379	+ self.tracked = False
336	380
337	381	def preflight(self):
338	382	# Check if git is installed, get the binary at the same time
...	...	@@ -398,6 +442,8 @@ class GitSource(Source):
398	442	# Update self.mirror.ref and node.ref from the self.tracking branch
399	443	ret = self.mirror.latest_commit(self.tracking)
400	444
	445	+ # Set tracked attribute, parameter for if self.mirror.assert_ref_in_track is needed
	446	+ self.tracked = True
401	447	return ret
402	448
403	449	def init_workspace(self, directory):
...	...	@@ -405,7 +451,7 @@ class GitSource(Source):
405	451	self.refresh_submodules()
406	452
407	453	with self.timed_activity('Setting up workspace "{}"'.format(directory), silent_nested=True):
408		- self.mirror.init_workspace(directory)
	454	+ self.mirror.init_workspace(directory, track=(self.tracking if not self.tracked else None))
409	455	for mirror in self.submodules:
410	456	mirror.init_workspace(directory)
411	457
...	...	@@ -421,7 +467,7 @@ class GitSource(Source):
421	467	# Stage the main repo in the specified directory
422	468	#
423	469	with self.timed_activity("Staging {}".format(self.mirror.url), silent_nested=True):
424		- self.mirror.stage(directory)
	470	+ self.mirror.stage(directory, track=(self.tracking if not self.tracked else None))
425	471	for mirror in self.submodules:
426	472	if mirror.path in self.submodule_checkout_overrides:
427	473	checkout = self.submodule_checkout_overrides[mirror.path]

buildstream/source.py

---

...	...	@@ -28,6 +28,18 @@ Abstract Methods
28	28	For loading and configuration purposes, Sources must implement the
29	29	:ref:`Plugin base class abstract methods <core_plugin_abstract_methods>`.
30	30
	31	+.. attention::
	32	+
	33	+ In order to ensure that all configuration data is processed at
	34	+ load time, it is important that all URLs have been processed during
	35	+ :func:`Plugin.configure() <buildstream.plugin.Plugin.configure>`.
	36	+
	37	+ Source implementations *must* either call
	38	+ :func:`Source.translate_url() <buildstream.source.Source.translate_url>` or
	39	+ :func:`Source.mark_download_url() <buildstream.source.Source.mark_download_url>`
	40	+ for every URL that has been specified in the configuration during
	41	+ :func:`Plugin.configure() <buildstream.plugin.Plugin.configure>`
	42	+
31	43	Sources expose the following abstract methods. Unless explicitly mentioned,
32	44	these methods are mandatory to implement.
33	45
...	...	@@ -184,6 +196,13 @@ class SourceFetcher():
184	196	fetching and substituting aliases.
185	197
186	198	*Since: 1.2*
	199	+
	200	+ .. attention::
	201	+
	202	+ When implementing a SourceFetcher, remember to call
	203	+ :func:`Source.mark_download_url() <buildstream.source.Source.mark_download_url>`
	204	+ for every URL found in the configuration data at
	205	+ :func:`Plugin.configure() <buildstream.plugin.Plugin.configure>` time.
187	206	"""
188	207	def __init__(self):
189	208	self.__alias = None
...	...	@@ -206,7 +225,7 @@ class SourceFetcher():
206	225	Implementors should raise :class:`.SourceError` if the there is some
207	226	network error or if the source reference could not be matched.
208	227	"""
209		- raise ImplError("Source fetcher '{}' does not implement fetch()".format(type(self)))
	228	+ raise ImplError("SourceFetcher '{}' does not implement fetch()".format(type(self)))
210	229
211	230	#############################################################
212	231	# Public Methods #
...	...	@@ -277,8 +296,11 @@ class Source(Plugin):
277	296	self.__element_kind = meta.element_kind # The kind of the element owning this source
278	297	self.__directory = meta.directory # Staging relative directory
279	298	self.__consistency = Consistency.INCONSISTENT # Cached consistency state
	299	+
	300	+ # The alias_override is only set on a re-instantiated Source
280	301	self.__alias_override = alias_override # Tuple of alias and its override to use instead
281		- self.__expected_alias = None # A hacky way to store the first alias used
	302	+ self.__expected_alias = None # The primary alias
	303	+ self.__marked_urls = set() # Set of marked download URLs
282	304
283	305	# FIXME: Reconstruct a MetaSource from a Source instead of storing it.
284	306	self.__meta = meta # MetaSource stored so we can copy this source later.
...	...	@@ -289,7 +311,7 @@ class Source(Plugin):
289	311	self.__config = self.__extract_config(meta)
290	312	self.__first_pass = meta.first_pass
291	313
292		- self.configure(self.__config)
	314	+ self._configure(self.__config)
293	315
294	316	COMMON_CONFIG_KEYS = ['kind', 'directory']
295	317	"""Common source config keys
...	...	@@ -351,10 +373,10 @@ class Source(Plugin):
351	373	Args:
352	374	ref (simple object): The internal source reference to set, or ``None``
353	375	node (dict): The same dictionary which was previously passed
354		- to :func:`~buildstream.source.Source.configure`
	376	+ to :func:`Plugin.configure() <buildstream.plugin.Plugin.configure>`
355	377
356		- See :func:`~buildstream.source.Source.get_ref` for a discussion on
357		- the *ref* parameter.
	378	+ See :func:`Source.get_ref() <buildstream.source.Source.get_ref>`
	379	+ for a discussion on the *ref* parameter.
358	380
359	381	.. note::
360	382
...	...	@@ -384,8 +406,8 @@ class Source(Plugin):
384	406	backend store allows one to query for a new ref from a symbolic
385	407	tracking data without downloading then that is desirable.
386	408
387		- See :func:`~buildstream.source.Source.get_ref` for a discussion on
388		- the *ref* parameter.
	409	+ See :func:`Source.get_ref() <buildstream.source.Source.get_ref>`
	410	+ for a discussion on the *ref* parameter.
389	411	"""
390	412	# Allow a non implementation
391	413	return None
...	...	@@ -435,7 +457,7 @@ class Source(Plugin):
435	457	:class:`.SourceError`
436	458
437	459	Default implementation is to call
438		- :func:`~buildstream.source.Source.stage`.
	460	+ :func:`Source.stage() <buildstream.source.Source.stage>`.
439	461
440	462	Implementors overriding this method should assume that *directory*
441	463	already exists.
...	...	@@ -453,8 +475,15 @@ class Source(Plugin):
453	475	is recommended.
454	476
455	477	Returns:
456		- list: A list of SourceFetchers. If SourceFetchers are not supported,
457		- this will be an empty list.
	478	+ iterable: The Source's SourceFetchers, if any.
	479	+
	480	+ .. note::
	481	+
	482	+ Implementors can implement this as a generator.
	483	+
	484	+ The :func:`SourceFetcher.fetch() <buildstream.source.SourceFetcher.fetch>`
	485	+ method will be called on the returned fetchers one by one,
	486	+ before consuming the next fetcher in the list.
458	487
459	488	*Since: 1.2*
460	489	"""
...	...	@@ -477,17 +506,27 @@ class Source(Plugin):
477	506	os.makedirs(directory, exist_ok=True)
478	507	return directory
479	508
480		- def translate_url(self, url, *, alias_override=None):
	509	+ def translate_url(self, url, *, alias_override=None, primary=True):
481	510	"""Translates the given url which may be specified with an alias
482	511	into a fully qualified url.
483	512
484	513	Args:
485		- url (str): A url, which may be using an alias
	514	+ url (str): A URL, which may be using an alias
486	515	alias_override (str): Optionally, an URI to override the alias with. (*Since: 1.2*)
	516	+ primary (bool): Whether this is the primary URL for the source. (*Since: 1.2*)
487	517
488	518	Returns:
489		- str: The fully qualified url, with aliases resolved
	519	+ str: The fully qualified URL, with aliases resolved
	520	+ .. note::
	521	+
	522	+ This must be called for every URL in the configuration during
	523	+ :func:`Plugin.configure() <buildstream.plugin.Plugin.configure>` if
	524	+ :func:`Source.mark_download_url() <buildstream.source.Source.mark_download_url>`
	525	+ is not called.
490	526	"""
	527	+ # Ensure that the download URL is also marked
	528	+ self.mark_download_url(url, primary=primary)
	529	+
491	530	# Alias overriding can happen explicitly (by command-line) or
492	531	# implicitly (the Source being constructed with an __alias_override).
493	532	if alias_override or self.__alias_override:
...	...	@@ -506,25 +545,55 @@ class Source(Plugin):
506	545	url = override_url + url_body
507	546	return url
508	547	else:
509		- # Sneakily store the alias if it hasn't already been stored
510		- if not self.__expected_alias and url and utils._ALIAS_SEPARATOR in url:
511		- self.mark_download_url(url)
512		-
513	548	project = self._get_project()
514	549	return project.translate_url(url, first_pass=self.__first_pass)
515	550
516		- def mark_download_url(self, url):
	551	+ def mark_download_url(self, url, *, primary=True):
517	552	"""Identifies the URL that this Source uses to download
518	553
519		- This must be called during :func:`~buildstream.plugin.Plugin.configure` if
520		- :func:`~buildstream.source.Source.translate_url` is not called.
521		-
522	554	Args:
523		- url (str): The url used to download
	555	+ url (str): The URL used to download
	556	+ primary (bool): Whether this is the primary URL for the source
	557	+
	558	+ .. note::
	559	+
	560	+ This must be called for every URL in the configuration during
	561	+ :func:`Plugin.configure() <buildstream.plugin.Plugin.configure>` if
	562	+ :func:`Source.translate_url() <buildstream.source.Source.translate_url>`
	563	+ is not called.
524	564
525	565	*Since: 1.2*
526	566	"""
527		- self.__expected_alias = _extract_alias(url)
	567	+ # Only mark the Source level aliases on the main instance, not in
	568	+ # a reinstantiated instance in mirroring.
	569	+ if not self.__alias_override:
	570	+ if primary:
	571	+ expected_alias = _extract_alias(url)
	572	+
	573	+ assert (self.__expected_alias is None or
	574	+ self.__expected_alias == expected_alias), \
	575	+ "Primary URL marked twice with different URLs"
	576	+
	577	+ self.__expected_alias = expected_alias
	578	+
	579	+ # Enforce proper behaviour of plugins by ensuring that all
	580	+ # aliased URLs have been marked at Plugin.configure() time.
	581	+ #
	582	+ if self._get_configuring():
	583	+ # Record marked urls while configuring
	584	+ #
	585	+ self.__marked_urls.add(url)
	586	+ else:
	587	+ # If an unknown aliased URL is seen after configuring,
	588	+ # this is an error.
	589	+ #
	590	+ # It is still possible that a URL that was not mentioned
	591	+ # in the element configuration can be marked, this is
	592	+ # the case for git submodules which might be automatically
	593	+ # discovered.
	594	+ #
	595	+ assert (url in self.__marked_urls or not _extract_alias(url)), \
	596	+ "URL was not seen at configure time: {}".format(url)
528	597
529	598	def get_project_directory(self):
530	599	"""Fetch the project base directory

buildstream/utils.py

---

...	...	@@ -335,7 +335,7 @@ def safe_remove(path):
335	335	try:
336	336	os.unlink(path)
337	337	except OSError as e:
338		- if e.errno != errno.EISDIR:
	338	+ if e.errno != errno.EISDIR and (e.errno != errno.EPERM or not os.path.isdir(path)):
339	339	raise UtilError("Failed to remove '{}': {}"
340	340	.format(path, e))
341	341

tests/frontend/project/sources/fetch_source.py

---

...	...	@@ -15,14 +15,17 @@ from buildstream import Source, Consistency, SourceError, SourceFetcher
15	15
16	16
17	17	class FetchFetcher(SourceFetcher):
18		- def __init__(self, source, url):
	18	+ def __init__(self, source, url, primary=False):
19	19	super().__init__()
20	20	self.source = source
21	21	self.original_url = url
	22	+ self.primary = primary
22	23	self.mark_download_url(url)
23	24
24	25	def fetch(self, alias_override=None):
25		- url = self.source.translate_url(self.original_url, alias_override=alias_override)
	26	+ url = self.source.translate_url(self.original_url,
	27	+ alias_override=alias_override,
	28	+ primary=self.primary)
26	29	with open(self.source.output_file, "a") as f:
27	30	success = url in self.source.fetch_succeeds and self.source.fetch_succeeds[url]
28	31	message = "Fetch {} {} from {}\n".format(self.original_url,
...	...	@@ -37,12 +40,21 @@ class FetchSource(Source):
37	40	# Read config to know which URLs to fetch
38	41	def configure(self, node):
39	42	self.original_urls = self.node_get_member(node, list, 'urls')
40		- self.fetchers = [FetchFetcher(self, url) for url in self.original_urls]
41	43	self.output_file = self.node_get_member(node, str, 'output-text')
42	44	self.fetch_succeeds = {}
43	45	if 'fetch-succeeds' in node:
44	46	self.fetch_succeeds = {x[0]: x[1] for x in self.node_items(node['fetch-succeeds'])}
45	47
	48	+ # First URL is the primary one for this test
	49	+ #
	50	+ primary = True
	51	+ self.fetchers = []
	52	+ for url in self.original_urls:
	53	+ self.mark_download_url(url, primary=primary)
	54	+ fetcher = FetchFetcher(self, url, primary=primary)
	55	+ self.fetchers.append(fetcher)
	56	+ primary = False
	57	+
46	58	def get_source_fetchers(self):
47	59	return self.fetchers
48	60

tests/sources/deb.py

---

...	...	@@ -56,7 +56,7 @@ def test_fetch_bad_url(cli, tmpdir, datafiles):
56	56	result = cli.run(project=project, args=[
57	57	'fetch', 'target.bst'
58	58	])
59		- assert "Try #" in result.stderr
	59	+ assert "FAILURE Try #" in result.stderr
60	60	result.assert_main_error(ErrorDomain.STREAM, None)
61	61	result.assert_task_error(ErrorDomain.SOURCE, None)
62	62

tests/sources/git.py

---

...	...	@@ -25,6 +25,7 @@ import pytest
25	25
26	26	from buildstream._exceptions import ErrorDomain
27	27	from buildstream import _yaml
	28	+from buildstream.plugin import CoreWarnings
28	29
29	30	from tests.testutils import cli, create_repo
30	31	from tests.testutils.site import HAVE_GIT
...	...	@@ -408,3 +409,70 @@ def test_submodule_track_no_ref_or_track(cli, tmpdir, datafiles):
408	409	result = cli.run(project=project, args=['show', 'target.bst'])
409	410	result.assert_main_error(ErrorDomain.SOURCE, "missing-track-and-ref")
410	411	result.assert_task_error(None, None)
	412	+
	413	+
	414	+@pytest.mark.skipif(HAVE_GIT is False, reason="git is not available")
	415	+@pytest.mark.datafiles(os.path.join(DATA_DIR, 'template'))
	416	+def test_ref_not_in_track_warn(cli, tmpdir, datafiles):
	417	+ project = os.path.join(datafiles.dirname, datafiles.basename)
	418	+
	419	+ # Create the repo from 'repofiles', create a branch without latest commit
	420	+ repo = create_repo('git', str(tmpdir))
	421	+ ref = repo.create(os.path.join(project, 'repofiles'))
	422	+
	423	+ gitsource = repo.source_config(ref=ref)
	424	+
	425	+ # Overwrite the track value to the added branch
	426	+ gitsource['track'] = 'foo'
	427	+
	428	+ # Write out our test target
	429	+ element = {
	430	+ 'kind': 'import',
	431	+ 'sources': [
	432	+ gitsource
	433	+ ]
	434	+ }
	435	+ _yaml.dump(element, os.path.join(project, 'target.bst'))
	436	+
	437	+ # Assert the warning is raised as ref is not in branch foo.
	438	+ # Assert warning not error to the user, when not set as fatal.
	439	+ result = cli.run(project=project, args=['build', 'target.bst'])
	440	+ assert "The ref provided for the element does not exist locally" in result.stderr
	441	+
	442	+
	443	+@pytest.mark.skipif(HAVE_GIT is False, reason="git is not available")
	444	+@pytest.mark.datafiles(os.path.join(DATA_DIR, 'template'))
	445	+def test_ref_not_in_track_warn_error(cli, tmpdir, datafiles):
	446	+ project = os.path.join(datafiles.dirname, datafiles.basename)
	447	+
	448	+ # Add fatal-warnings ref-not-in-track to project.conf
	449	+ project_template = {
	450	+ "name": "foo",
	451	+ "fatal-warnings": [CoreWarnings.REF_NOT_IN_TRACK]
	452	+ }
	453	+
	454	+ _yaml.dump(project_template, os.path.join(project, 'project.conf'))
	455	+
	456	+ # Create the repo from 'repofiles', create a branch without latest commit
	457	+ repo = create_repo('git', str(tmpdir))
	458	+ ref = repo.create(os.path.join(project, 'repofiles'))
	459	+
	460	+ gitsource = repo.source_config(ref=ref)
	461	+
	462	+ # Overwrite the track value to the added branch
	463	+ gitsource['track'] = 'foo'
	464	+
	465	+ # Write out our test target
	466	+ element = {
	467	+ 'kind': 'import',
	468	+ 'sources': [
	469	+ gitsource
	470	+ ]
	471	+ }
	472	+ _yaml.dump(element, os.path.join(project, 'target.bst'))
	473	+
	474	+ # Assert that build raises a warning here that is captured
	475	+ # as plugin error, due to the fatal warning being set
	476	+ result = cli.run(project=project, args=['build', 'target.bst'])
	477	+ result.assert_main_error(ErrorDomain.STREAM, None)
	478	+ result.assert_task_error(ErrorDomain.PLUGIN, CoreWarnings.REF_NOT_IN_TRACK)

tests/sources/tar.py

---

...	...	@@ -67,7 +67,7 @@ def test_fetch_bad_url(cli, tmpdir, datafiles):
67	67	result = cli.run(project=project, args=[
68	68	'fetch', 'target.bst'
69	69	])
70		- assert "Try #" in result.stderr
	70	+ assert "FAILURE Try #" in result.stderr
71	71	result.assert_main_error(ErrorDomain.STREAM, None)
72	72	result.assert_task_error(ErrorDomain.SOURCE, None)
73	73

tests/sources/zip.py

---

...	...	@@ -53,7 +53,7 @@ def test_fetch_bad_url(cli, tmpdir, datafiles):
53	53	result = cli.run(project=project, args=[
54	54	'fetch', 'target.bst'
55	55	])
56		- assert "Try #" in result.stderr
	56	+ assert "FAILURE Try #" in result.stderr
57	57	result.assert_main_error(ErrorDomain.STREAM, None)
58	58	result.assert_task_error(ErrorDomain.SOURCE, None)
59	59

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