[Notes] [Git][BuildStream/buildstream][bst-1.2] 7 commits: source.py: Document Source.get_source_fetchers() to return an iterable

[Tristan Van Berkom <gitlab mg gitlab com>]

Title: GitLab

Tristan Van Berkom pushed to branch bst-1.2 at BuildStream / buildstream

Commits:

• 2a27b9c4
  by Tristan Van Berkom at 2018-09-03T06:18:17Z

  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
  

• a6dbf5fc
  by Tristan Van Berkom at 2018-09-03T06:18:23Z

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

• 62b7ec45
  by Tristan Van Berkom at 2018-09-03T06:18:31Z

  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
  

• 4df625c8
  by Tristan Van Berkom at 2018-09-03T06:19:48Z

  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
  

• 435dc56c
  by Tristan Van Berkom at 2018-09-03T06:20:13Z

  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
  

• 51eaa2c3
  by Tristan Van Berkom at 2018-09-03T06:20:27Z

  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
  

• b422959f
  by Tristan Van Berkom at 2018-09-03T06:50:11Z

  Merge branch 'tristan/source-fetcher-changes-1.2' into 'bst-1.2'
  
  Source fetcher changes 1.2
  
  See merge request BuildStream/buildstream!773

5 changed files:

• buildstream/element.py
• buildstream/plugin.py
• buildstream/plugins/sources/git.py
• buildstream/source.py
• tests/frontend/project/sources/fetch_source.py

Changes:

buildstream/element.py

---

...	...	@@ -262,7 +262,7 @@ class Element(Plugin):
262	262	# Collect the composited element configuration and
263	263	# ask the element to configure itself.
264	264	self.__config = self.__extract_config(meta)
265		- self.configure(self.__config)
	265	+ self._configure(self.__config)
266	266
267	267	# Extract Sandbox config
268	268	self.__sandbox_config = self.__extract_sandbox_config(meta)

buildstream/plugin.py

---

...	...	@@ -162,6 +162,7 @@ class Plugin():
162	162	self.__provenance = provenance # The Provenance information
163	163	self.__type_tag = type_tag # The type of plugin (element or source)
164	164	self.__unique_id = _plugin_register(self) # Unique ID
	165	+ self.__configuring = False # Whether we are currently configuring
165	166
166	167	# Infer the kind identifier
167	168	modulename = type(self).__module__
...	...	@@ -651,7 +652,32 @@ class Plugin():
651	652	else:
652	653	yield log
653	654
	655	+ # _configure():
	656	+ #
	657	+ # Calls configure() for the plugin, this must be called by
	658	+ # the core instead of configure() directly, so that the
	659	+ # _get_configuring() state is up to date.
	660	+ #
	661	+ # Args:
	662	+ # node (dict): The loaded configuration dictionary
	663	+ #
	664	+ def _configure(self, node):
	665	+ self.__configuring = True
	666	+ self.configure(node)
	667	+ self.__configuring = False
	668	+
	669	+ # _get_configuring():
	670	+ #
	671	+ # Checks whether the plugin is in the middle of having
	672	+ # its Plugin.configure() method called
	673	+ #
	674	+ # Returns:
	675	+ # (bool): Whether we are currently configuring
	676	+ def _get_configuring(self):
	677	+ return self.__configuring
	678	+
654	679	# _preflight():
	680	+ #
655	681	# Calls preflight() for the plugin, and allows generic preflight
656	682	# checks to be added
657	683	#
...	...	@@ -659,6 +685,7 @@ class Plugin():
659	685	# SourceError: If it's a Source implementation
660	686	# ElementError: If it's an Element implementation
661	687	# ProgramNotFoundError: If a required host tool is not found
	688	+ #
662	689	def _preflight(self):
663	690	self.preflight()
664	691

buildstream/plugins/sources/git.py

---

...	...	@@ -90,13 +90,14 @@ GIT_MODULES = '.gitmodules'
90	90	#
91	91	class GitMirror(SourceFetcher):
92	92
93		- def __init__(self, source, path, url, ref):
	93	+ def __init__(self, source, path, url, ref, *, primary=False):
94	94
95	95	super().__init__()
96	96	self.source = source
97	97	self.path = path
98	98	self.url = url
99	99	self.ref = ref
	100	+ self.primary = primary
100	101	self.mirror = os.path.join(source.get_mirror_directory(), utils.url_directory_name(url))
101	102	self.mark_download_url(url)
102	103
...	...	@@ -114,7 +115,8 @@ class GitMirror(SourceFetcher):
114	115	# system configured tmpdir is not on the same partition.
115	116	#
116	117	with self.source.tempdir() as tmpdir:
117		- url = self.source.translate_url(self.url, alias_override=alias_override)
	118	+ url = self.source.translate_url(self.url, alias_override=alias_override,
	119	+ primary=self.primary)
118	120	self.source.call([self.source.host_git, 'clone', '--mirror', '-n', url, tmpdir],
119	121	fail="Failed to clone git repository {}".format(url),
120	122	fail_temporarily=True)
...	...	@@ -136,7 +138,9 @@ class GitMirror(SourceFetcher):
136	138	.format(self.source, url, tmpdir, self.mirror, e)) from e
137	139
138	140	def _fetch(self, alias_override=None):
139		- url = self.source.translate_url(self.url, alias_override=alias_override)
	141	+ url = self.source.translate_url(self.url,
	142	+ alias_override=alias_override,
	143	+ primary=self.primary)
140	144
141	145	if alias_override:
142	146	remote_name = utils.url_directory_name(alias_override)
...	...	@@ -294,7 +298,7 @@ class GitSource(Source):
294	298	self.node_validate(node, config_keys + Source.COMMON_CONFIG_KEYS)
295	299
296	300	self.original_url = self.node_get_member(node, str, 'url')
297		- self.mirror = GitMirror(self, '', self.original_url, ref)
	301	+ self.mirror = GitMirror(self, '', self.original_url, ref, primary=True)
298	302	self.tracking = self.node_get_member(node, str, 'track', None)
299	303
300	304	# At this point we now know if the source has a ref and/or a track.
...	...	@@ -314,6 +318,11 @@ class GitSource(Source):
314	318	for path, _ in self.node_items(modules):
315	319	submodule = self.node_get_member(modules, Mapping, path)
316	320	url = self.node_get_member(submodule, str, 'url', None)
	321	+
	322	+ # Make sure to mark all URLs that are specified in the configuration
	323	+ if url:
	324	+ self.mark_download_url(url, primary=False)
	325	+
317	326	self.submodule_overrides[path] = url
318	327	if 'checkout' in submodule:
319	328	checkout = self.node_get_member(submodule, bool, 'checkout')

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
...	...	@@ -149,6 +161,13 @@ class SourceFetcher():
149	161	fetching and substituting aliases.
150	162
151	163	*Since: 1.2*
	164	+
	165	+ .. attention::
	166	+
	167	+ When implementing a SourceFetcher, remember to call
	168	+ :func:`Source.mark_download_url() <buildstream.source.Source.mark_download_url>`
	169	+ for every URL found in the configuration data at
	170	+ :func:`Plugin.configure() <buildstream.plugin.Plugin.configure>` time.
152	171	"""
153	172	def __init__(self):
154	173	self.__alias = None
...	...	@@ -171,7 +190,7 @@ class SourceFetcher():
171	190	Implementors should raise :class:`.SourceError` if the there is some
172	191	network error or if the source reference could not be matched.
173	192	"""
174		- raise ImplError("Source fetcher '{}' does not implement fetch()".format(type(self)))
	193	+ raise ImplError("SourceFetcher '{}' does not implement fetch()".format(type(self)))
175	194
176	195	#############################################################
177	196	# Public Methods #
...	...	@@ -216,8 +235,11 @@ class Source(Plugin):
216	235	self.__element_kind = meta.element_kind # The kind of the element owning this source
217	236	self.__directory = meta.directory # Staging relative directory
218	237	self.__consistency = Consistency.INCONSISTENT # Cached consistency state
	238	+
	239	+ # The alias_override is only set on a re-instantiated Source
219	240	self.__alias_override = alias_override # Tuple of alias and its override to use instead
220		- self.__expected_alias = None # A hacky way to store the first alias used
	241	+ self.__expected_alias = None # The primary alias
	242	+ self.__marked_urls = set() # Set of marked download URLs
221	243
222	244	# FIXME: Reconstruct a MetaSource from a Source instead of storing it.
223	245	self.__meta = meta # MetaSource stored so we can copy this source later.
...	...	@@ -228,7 +250,7 @@ class Source(Plugin):
228	250	self.__config = self.__extract_config(meta)
229	251	self.__first_pass = meta.first_pass
230	252
231		- self.configure(self.__config)
	253	+ self._configure(self.__config)
232	254
233	255	COMMON_CONFIG_KEYS = ['kind', 'directory']
234	256	"""Common source config keys
...	...	@@ -290,10 +312,10 @@ class Source(Plugin):
290	312	Args:
291	313	ref (simple object): The internal source reference to set, or ``None``
292	314	node (dict): The same dictionary which was previously passed
293		- to :func:`~buildstream.source.Source.configure`
	315	+ to :func:`Plugin.configure() <buildstream.plugin.Plugin.configure>`
294	316
295		- See :func:`~buildstream.source.Source.get_ref` for a discussion on
296		- the *ref* parameter.
	317	+ See :func:`Source.get_ref() <buildstream.source.Source.get_ref>`
	318	+ for a discussion on the *ref* parameter.
297	319
298	320	.. note::
299	321
...	...	@@ -317,8 +339,8 @@ class Source(Plugin):
317	339	backend store allows one to query for a new ref from a symbolic
318	340	tracking data without downloading then that is desirable.
319	341
320		- See :func:`~buildstream.source.Source.get_ref` for a discussion on
321		- the *ref* parameter.
	342	+ See :func:`Source.get_ref() <buildstream.source.Source.get_ref>`
	343	+ for a discussion on the *ref* parameter.
322	344	"""
323	345	# Allow a non implementation
324	346	return None
...	...	@@ -362,7 +384,7 @@ class Source(Plugin):
362	384	:class:`.SourceError`
363	385
364	386	Default implementation is to call
365		- :func:`~buildstream.source.Source.stage`.
	387	+ :func:`Source.stage() <buildstream.source.Source.stage>`.
366	388
367	389	Implementors overriding this method should assume that *directory*
368	390	already exists.
...	...	@@ -380,8 +402,15 @@ class Source(Plugin):
380	402	is recommended.
381	403
382	404	Returns:
383		- list: A list of SourceFetchers. If SourceFetchers are not supported,
384		- this will be an empty list.
	405	+ iterable: The Source's SourceFetchers, if any.
	406	+
	407	+ .. note::
	408	+
	409	+ Implementors can implement this as a generator.
	410	+
	411	+ The :func:`SourceFetcher.fetch() <buildstream.source.SourceFetcher.fetch>`
	412	+ method will be called on the returned fetchers one by one,
	413	+ before consuming the next fetcher in the list.
385	414
386	415	*Since: 1.2*
387	416	"""
...	...	@@ -404,17 +433,27 @@ class Source(Plugin):
404	433	os.makedirs(directory, exist_ok=True)
405	434	return directory
406	435
407		- def translate_url(self, url, *, alias_override=None):
	436	+ def translate_url(self, url, *, alias_override=None, primary=True):
408	437	"""Translates the given url which may be specified with an alias
409	438	into a fully qualified url.
410	439
411	440	Args:
412		- url (str): A url, which may be using an alias
	441	+ url (str): A URL, which may be using an alias
413	442	alias_override (str): Optionally, an URI to override the alias with. (*Since: 1.2*)
	443	+ primary (bool): Whether this is the primary URL for the source. (*Since: 1.2*)
414	444
415	445	Returns:
416		- str: The fully qualified url, with aliases resolved
	446	+ str: The fully qualified URL, with aliases resolved
	447	+ .. note::
	448	+
	449	+ This must be called for every URL in the configuration during
	450	+ :func:`Plugin.configure() <buildstream.plugin.Plugin.configure>` if
	451	+ :func:`Source.mark_download_url() <buildstream.source.Source.mark_download_url>`
	452	+ is not called.
417	453	"""
	454	+ # Ensure that the download URL is also marked
	455	+ self.mark_download_url(url, primary=primary)
	456	+
418	457	# Alias overriding can happen explicitly (by command-line) or
419	458	# implicitly (the Source being constructed with an __alias_override).
420	459	if alias_override or self.__alias_override:
...	...	@@ -433,25 +472,55 @@ class Source(Plugin):
433	472	url = override_url + url_body
434	473	return url
435	474	else:
436		- # Sneakily store the alias if it hasn't already been stored
437		- if not self.__expected_alias and url and utils._ALIAS_SEPARATOR in url:
438		- self.mark_download_url(url)
439		-
440	475	project = self._get_project()
441	476	return project.translate_url(url, first_pass=self.__first_pass)
442	477
443		- def mark_download_url(self, url):
	478	+ def mark_download_url(self, url, *, primary=True):
444	479	"""Identifies the URL that this Source uses to download
445	480
446		- This must be called during :func:`~buildstream.plugin.Plugin.configure` if
447		- :func:`~buildstream.source.Source.translate_url` is not called.
448		-
449	481	Args:
450		- url (str): The url used to download
	482	+ url (str): The URL used to download
	483	+ primary (bool): Whether this is the primary URL for the source
	484	+
	485	+ .. note::
	486	+
	487	+ This must be called for every URL in the configuration during
	488	+ :func:`Plugin.configure() <buildstream.plugin.Plugin.configure>` if
	489	+ :func:`Source.translate_url() <buildstream.source.Source.translate_url>`
	490	+ is not called.
451	491
452	492	*Since: 1.2*
453	493	"""
454		- self.__expected_alias = _extract_alias(url)
	494	+ # Only mark the Source level aliases on the main instance, not in
	495	+ # a reinstantiated instance in mirroring.
	496	+ if not self.__alias_override:
	497	+ if primary:
	498	+ expected_alias = _extract_alias(url)
	499	+
	500	+ assert (self.__expected_alias is None or
	501	+ self.__expected_alias == expected_alias), \
	502	+ "Primary URL marked twice with different URLs"
	503	+
	504	+ self.__expected_alias = expected_alias
	505	+
	506	+ # Enforce proper behaviour of plugins by ensuring that all
	507	+ # aliased URLs have been marked at Plugin.configure() time.
	508	+ #
	509	+ if self._get_configuring():
	510	+ # Record marked urls while configuring
	511	+ #
	512	+ self.__marked_urls.add(url)
	513	+ else:
	514	+ # If an unknown aliased URL is seen after configuring,
	515	+ # this is an error.
	516	+ #
	517	+ # It is still possible that a URL that was not mentioned
	518	+ # in the element configuration can be marked, this is
	519	+ # the case for git submodules which might be automatically
	520	+ # discovered.
	521	+ #
	522	+ assert (url in self.__marked_urls or not _extract_alias(url)), \
	523	+ "URL was not seen at configure time: {}".format(url)
455	524
456	525	def get_project_directory(self):
457	526	"""Fetch the project base directory

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

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