[Notes] [Git][BuildStream/buildstream][master] 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 master at BuildStream / buildstream

Commits:

• 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

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

---

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

---

...	...	@@ -100,13 +100,14 @@ INCONSISTENT_SUBMODULE = "inconsistent-submodules"
100	100	#
101	101	class GitMirror(SourceFetcher):
102	102
103		- def __init__(self, source, path, url, ref):
	103	+ def __init__(self, source, path, url, ref, *, primary=False):
104	104
105	105	super().__init__()
106	106	self.source = source
107	107	self.path = path
108	108	self.url = url
109	109	self.ref = ref
	110	+ self.primary = primary
110	111	self.mirror = os.path.join(source.get_mirror_directory(), utils.url_directory_name(url))
111	112	self.mark_download_url(url)
112	113
...	...	@@ -124,7 +125,8 @@ class GitMirror(SourceFetcher):
124	125	# system configured tmpdir is not on the same partition.
125	126	#
126	127	with self.source.tempdir() as tmpdir:
127		- url = self.source.translate_url(self.url, alias_override=alias_override)
	128	+ url = self.source.translate_url(self.url, alias_override=alias_override,
	129	+ primary=self.primary)
128	130	self.source.call([self.source.host_git, 'clone', '--mirror', '-n', url, tmpdir],
129	131	fail="Failed to clone git repository {}".format(url),
130	132	fail_temporarily=True)
...	...	@@ -146,7 +148,9 @@ class GitMirror(SourceFetcher):
146	148	.format(self.source, url, tmpdir, self.mirror, e)) from e
147	149
148	150	def _fetch(self, alias_override=None):
149		- url = self.source.translate_url(self.url, alias_override=alias_override)
	151	+ url = self.source.translate_url(self.url,
	152	+ alias_override=alias_override,
	153	+ primary=self.primary)
150	154
151	155	if alias_override:
152	156	remote_name = utils.url_directory_name(alias_override)
...	...	@@ -307,7 +311,7 @@ class GitSource(Source):
307	311	self.node_validate(node, config_keys + Source.COMMON_CONFIG_KEYS)
308	312
309	313	self.original_url = self.node_get_member(node, str, 'url')
310		- self.mirror = GitMirror(self, '', self.original_url, ref)
	314	+ self.mirror = GitMirror(self, '', self.original_url, ref, primary=True)
311	315	self.tracking = self.node_get_member(node, str, 'track', None)
312	316
313	317	# At this point we now know if the source has a ref and/or a track.
...	...	@@ -327,6 +331,11 @@ class GitSource(Source):
327	331	for path, _ in self.node_items(modules):
328	332	submodule = self.node_get_member(modules, Mapping, path)
329	333	url = self.node_get_member(submodule, str, 'url', None)
	334	+
	335	+ # Make sure to mark all URLs that are specified in the configuration
	336	+ if url:
	337	+ self.mark_download_url(url, primary=False)
	338	+
330	339	self.submodule_overrides[path] = url
331	340	if 'checkout' in submodule:
332	341	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
...	...	@@ -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

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.