[Notes] [Git][BuildStream/buildstream][tpollard/494] 4 commits: contributing: add guidance on unit tests

[Jürg Billeter <gitlab mg gitlab com>]

Title: GitLab

Jürg Billeter pushed to branch tpollard/494 at BuildStream / buildstream

Commits:

• 082b3811
  by Angelos Evripiotis at 2018-11-15T19:48:12Z

  contributing: add guidance on unit tests
  
  Decrease uncertainty around whether unit tests are welcome in the
  project or not.
  

• b5b79056
  by Jürg Billeter at 2018-11-15T20:24:58Z

  Merge branch 'aevri/unit_tests' into 'master'
  
  contributing: add guidance on unit tests
  
  See merge request BuildStream/buildstream!943

• 199bfff1
  by Tom Pollard at 2018-11-17T13:07:59Z

  Add cli main and user config option for 'pull-buildtrees' context.
  
  _context.py: Add pull_buildtrees global user context, the default
  of which is set to False via the addition of pull-buildtrees to
  userconfig.yaml cache group.
  
  _frontend/app.py & cli.py: Add --pull-buildtrees as a bst main
  option, which when passed will override the default or user defined
  context for pull_buildtrees.
  
  tests/completions/completions.py: Update for the added flag.
  

• 454dcaf1
  by Tom Pollard at 2018-11-17T13:07:59Z

  Don't pull artifact build trees by default.
  
  The addition of cached build trees being included in element
  artifacts has led to mostly redundant download overheads when
  pulling from a remote artifact server. As such the default behaviour
  of pull shouldn't fetch the build tree object if available.
  
  element.py: extend relevant pull logic for specified subdir
  consideration and ensure push logic does not lead to partial
  artifact pushes. Change assumption that the buildtree can be
  extracted if artifact is cached. __cached_buildtree() and
  __pull_directories() helpers added.
  
  _artifactcache/: artifactcache.py & cascache.py inclusion of
  helper functions for subdir artifact checking & fetching, fetch
  logic extended to only pull required artifact directories.
  extract & checkout updated to handle full/partial operation.
  
  tests/: addition of integration test pullbuildtrees.py,
  buildtrees.py adapted cli options, testutils/artifactshare.py
  has_artifact changed to return artifact digest on true condition.
  

13 changed files:

• CONTRIBUTING.rst
• NEWS
• buildstream/_artifactcache/artifactcache.py
• buildstream/_artifactcache/cascache.py
• buildstream/_context.py
• buildstream/_frontend/app.py
• buildstream/_frontend/cli.py
• buildstream/data/userconfig.yaml
• buildstream/element.py
• tests/completions/completions.py
• tests/integration/build-tree.py
• + tests/integration/pullbuildtrees.py
• tests/testutils/artifactshare.py

Changes:

CONTRIBUTING.rst

---

...	...	@@ -1547,6 +1547,24 @@ Tests that run a sandbox should be decorated with::
1547	1547
1548	1548	and use the integration cli helper.
1549	1549
	1550	+You should first aim to write tests that exercise your changes from the cli.
	1551	+This is so that the testing is end-to-end, and the changes are guaranteed to
	1552	+work for the end-user. The cli is considered stable, and so tests written in
	1553	+terms of it are unlikely to require updating as the internals of the software
	1554	+change over time.
	1555	+
	1556	+It may be impractical to sufficiently examine some changes this way. For
	1557	+example, the number of cases to test and the running time of each test may be
	1558	+too high. It may also be difficult to contrive circumstances to cover every
	1559	+line of the change. If this is the case, next you can consider also writing
	1560	+unit tests that work more directly on the changes.
	1561	+
	1562	+It is important to write unit tests in such a way that they do not break due to
	1563	+changes unrelated to what they are meant to test. For example, if the test
	1564	+relies on a lot of BuildStream internals, a large refactoring will likely
	1565	+require the test to be rewritten. Pure functions that only rely on the Python
	1566	+Standard Library are excellent candidates for unit testing.
	1567	+
1550	1568
1551	1569	Measuring performance
1552	1570	---------------------

NEWS

---

...	...	@@ -38,13 +38,23 @@ buildstream 1.3.1
38	38	a bug fix to workspaces so they can be build in workspaces too.
39	39
40	40	o Creating a build shell through the interactive mode or `bst shell --build`
41		- will now use the cached build tree. It is now easier to debug local build
42		- failures.
	41	+ will now use the cached build tree if available locally. It is now easier to
	42	+ debug local build failures.
43	43
44	44	o `bst shell --sysroot` now takes any directory that contains a sysroot,
45	45	instead of just a specially-formatted build-root with a `root` and `scratch`
46	46	subdirectory.
47	47
	48	+ o Due to the element `build tree` being cached in the respective artifact their
	49	+ size in some cases has significantly increased. In *most* cases the build trees
	50	+ are not utilised when building targets, as such by default bst 'pull' & 'build'
	51	+ will not fetch build trees from remotes. This behaviour can be overridden with
	52	+ the cli main option '--pull-buildtrees', or the user configuration cache group
	53	+ option 'pull-buildtrees = True'. The override will also add the build tree to
	54	+ already cached artifacts. When attempting to populate an artifactcache server
	55	+ with cached artifacts, only 'complete' elements can be pushed. If the element
	56	+ is expected to have a populated build tree then it must be cached before pushing.
	57	+
48	58
49	59	=================
50	60	buildstream 1.1.5

buildstream/_artifactcache/artifactcache.py

---

...	...	@@ -476,6 +476,22 @@ class ArtifactCache():
476	476
477	477	return self.cas.contains(ref)
478	478
	479	+ # contains_subdir_artifact():
	480	+ #
	481	+ # Check whether an artifact element contains a digest for a subdir
	482	+ # which is populated in the cache, i.e non dangling.
	483	+ #
	484	+ # Args:
	485	+ # element (Element): The Element to check
	486	+ # key (str): The cache key to use
	487	+ # subdir (str): The subdir to check
	488	+ #
	489	+ # Returns: True if the subdir exists & is populated in the cache, False otherwise
	490	+ #
	491	+ def contains_subdir_artifact(self, element, key, subdir):
	492	+ ref = self.get_artifact_fullname(element, key)
	493	+ return self.cas.contains_subdir_artifact(ref, subdir)
	494	+
479	495	# list_artifacts():
480	496	#
481	497	# List artifacts in this cache in LRU order.
...	...	@@ -533,6 +549,7 @@ class ArtifactCache():
533	549	# Args:
534	550	# element (Element): The Element to extract
535	551	# key (str): The cache key to use
	552	+ # subdir (str): Optional specific subdir to extract
536	553	#
537	554	# Raises:
538	555	# ArtifactError: In cases there was an OSError, or if the artifact
...	...	@@ -540,12 +557,12 @@ class ArtifactCache():
540	557	#
541	558	# Returns: path to extracted artifact
542	559	#
543		- def extract(self, element, key):
	560	+ def extract(self, element, key, subdir=None):
544	561	ref = self.get_artifact_fullname(element, key)
545	562
546	563	path = os.path.join(self.extractdir, element._get_project().name, element.normal_name)
547	564
548		- return self.cas.extract(ref, path)
	565	+ return self.cas.extract(ref, path, subdir=subdir)
549	566
550	567	# commit():
551	568	#
...	...	@@ -666,11 +683,13 @@ class ArtifactCache():
666	683	# element (Element): The Element whose artifact is to be fetched
667	684	# key (str): The cache key to use
668	685	# progress (callable): The progress callback, if any
	686	+ # subdir (str): The optional specific subdir to pull
	687	+ # excluded_subdirs (list): The optional list of subdirs to not pull
669	688	#
670	689	# Returns:
671	690	# (bool): True if pull was successful, False if artifact was not available
672	691	#
673		- def pull(self, element, key, *, progress=None):
	692	+ def pull(self, element, key, *, progress=None, subdir=None, excluded_subdirs=None):
674	693	ref = self.get_artifact_fullname(element, key)
675	694
676	695	project = element._get_project()
...	...	@@ -680,8 +699,13 @@ class ArtifactCache():
680	699	display_key = element._get_brief_display_key()
681	700	element.status("Pulling artifact {} <- {}".format(display_key, remote.spec.url))
682	701
683		- if self.cas.pull(ref, remote, progress=progress):
	702	+ if self.cas.pull(ref, remote, progress=progress, subdir=subdir, excluded_subdirs=excluded_subdirs):
684	703	element.info("Pulled artifact {} <- {}".format(display_key, remote.spec.url))
	704	+ if subdir:
	705	+ # Attempt to extract subdir into artifact extract dir if it already exists
	706	+ # without containing the subdir. If the respective artifact extract dir does not
	707	+ # exist a complete extraction will complete.
	708	+ self.extract(element, key, subdir)
685	709	# no need to pull from additional remotes
686	710	return True
687	711	else:

buildstream/_artifactcache/cascache.py

---

...	...	@@ -82,6 +82,27 @@ class CASCache():
82	82	# This assumes that the repository doesn't have any dangling pointers
83	83	return os.path.exists(refpath)
84	84
	85	+ # contains_subdir_artifact():
	86	+ #
	87	+ # Check whether the specified artifact element tree has a digest for a subdir
	88	+ # which is populated in the cache, i.e non dangling.
	89	+ #
	90	+ # Args:
	91	+ # ref (str): The ref to check
	92	+ # subdir (str): The subdir to check
	93	+ #
	94	+ # Returns: True if the subdir exists & is populated in the cache, False otherwise
	95	+ #
	96	+ def contains_subdir_artifact(self, ref, subdir):
	97	+ tree = self.resolve_ref(ref)
	98	+
	99	+ # This assumes that the subdir digest is present in the element tree
	100	+ subdirdigest = self._get_subdir(tree, subdir)
	101	+ objpath = self.objpath(subdirdigest)
	102	+
	103	+ # True if subdir content is cached or if empty as expected
	104	+ return os.path.exists(objpath)
	105	+
85	106	# extract():
86	107	#
87	108	# Extract cached directory for the specified ref if it hasn't
...	...	@@ -90,19 +111,30 @@ class CASCache():
90	111	# Args:
91	112	# ref (str): The ref whose directory to extract
92	113	# path (str): The destination path
	114	+ # subdir (str): Optional specific dir to extract
93	115	#
94	116	# Raises:
95	117	# CASError: In cases there was an OSError, or if the ref did not exist.
96	118	#
97	119	# Returns: path to extracted directory
98	120	#
99		- def extract(self, ref, path):
	121	+ def extract(self, ref, path, subdir=None):
100	122	tree = self.resolve_ref(ref, update_mtime=True)
101	123
102		- dest = os.path.join(path, tree.hash)
	124	+ originaldest = dest = os.path.join(path, tree.hash)
	125	+
	126	+ # If artifact is already extracted, check if the optional subdir
	127	+ # has also been extracted. If the artifact has not been extracted
	128	+ # a full extraction would include the optional subdir
103	129	if os.path.isdir(dest):
104		- # directory has already been extracted
105		- return dest
	130	+ if subdir:
	131	+ if not os.path.isdir(os.path.join(dest, subdir)):
	132	+ dest = os.path.join(dest, subdir)
	133	+ tree = self._get_subdir(tree, subdir)
	134	+ else:
	135	+ return dest
	136	+ else:
	137	+ return dest
106	138
107	139	with tempfile.TemporaryDirectory(prefix='tmp', dir=self.tmpdir) as tmpdir:
108	140	checkoutdir = os.path.join(tmpdir, ref)
...	...	@@ -120,7 +152,7 @@ class CASCache():
120	152	if e.errno not in [errno.ENOTEMPTY, errno.EEXIST]:
121	153	raise CASError("Failed to extract directory for ref '{}': {}".format(ref, e)) from e
122	154
123		- return dest
	155	+ return originaldest
124	156
125	157	# commit():
126	158	#
...	...	@@ -193,11 +225,13 @@ class CASCache():
193	225	# ref (str): The ref to pull
194	226	# remote (CASRemote): The remote repository to pull from
195	227	# progress (callable): The progress callback, if any
	228	+ # subdir (str): The optional specific subdir to pull
	229	+ # excluded_subdirs (list): The optional list of subdirs to not pull
196	230	#
197	231	# Returns:
198	232	# (bool): True if pull was successful, False if ref was not available
199	233	#
200		- def pull(self, ref, remote, *, progress=None):
	234	+ def pull(self, ref, remote, *, progress=None, subdir=None, excluded_subdirs=None):
201	235	try:
202	236	remote.init()
203	237
...	...	@@ -209,7 +243,12 @@ class CASCache():
209	243	tree.hash = response.digest.hash
210	244	tree.size_bytes = response.digest.size_bytes
211	245
212		- self._fetch_directory(remote, tree)
	246	+ # Check if the element artifact is present, if so just fetch the subdir.
	247	+ if subdir and os.path.exists(self.objpath(tree)):
	248	+ self._fetch_subdir(remote, tree, subdir)
	249	+ else:
	250	+ # Fetch artifact, excluded_subdirs determined in pullqueue
	251	+ self._fetch_directory(remote, tree, excluded_subdirs=excluded_subdirs)
213	252
214	253	self.set_ref(ref, tree)
215	254
...	...	@@ -607,8 +646,10 @@ class CASCache():
607	646	stat.S_IRGRP | stat.S_IXGRP | stat.S_IROTH | stat.S_IXOTH)
608	647
609	648	for dirnode in directory.directories:
610		- fullpath = os.path.join(dest, dirnode.name)
611		- self._checkout(fullpath, dirnode.digest)
	649	+ # Don't try to checkout a dangling ref
	650	+ if os.path.exists(self.objpath(dirnode.digest)):
	651	+ fullpath = os.path.join(dest, dirnode.name)
	652	+ self._checkout(fullpath, dirnode.digest)
612	653
613	654	for symlinknode in directory.symlinks:
614	655	# symlink
...	...	@@ -863,11 +904,14 @@ class CASCache():
863	904	# Args:
864	905	# remote (Remote): The remote to use.
865	906	# dir_digest (Digest): Digest object for the directory to fetch.
	907	+ # excluded_subdirs (list): The optional list of subdirs to not fetch
866	908	#
867		- def _fetch_directory(self, remote, dir_digest):
	909	+ def _fetch_directory(self, remote, dir_digest, *, excluded_subdirs=None):
868	910	fetch_queue = [dir_digest]
869	911	fetch_next_queue = []
870	912	batch = _CASBatchRead(remote)
	913	+ if not excluded_subdirs:
	914	+ excluded_subdirs = []
871	915
872	916	while len(fetch_queue) + len(fetch_next_queue) > 0:
873	917	if not fetch_queue:
...	...	@@ -882,8 +926,9 @@ class CASCache():
882	926	directory.ParseFromString(f.read())
883	927
884	928	for dirnode in directory.directories:
885		- batch = self._fetch_directory_node(remote, dirnode.digest, batch,
886		- fetch_queue, fetch_next_queue, recursive=True)
	929	+ if dirnode.name not in excluded_subdirs:
	930	+ batch = self._fetch_directory_node(remote, dirnode.digest, batch,
	931	+ fetch_queue, fetch_next_queue, recursive=True)
887	932
888	933	for filenode in directory.files:
889	934	batch = self._fetch_directory_node(remote, filenode.digest, batch,
...	...	@@ -892,6 +937,10 @@ class CASCache():
892	937	# Fetch final batch
893	938	self._fetch_directory_batch(remote, batch, fetch_queue, fetch_next_queue)
894	939
	940	+ def _fetch_subdir(self, remote, tree, subdir):
	941	+ subdirdigest = self._get_subdir(tree, subdir)
	942	+ self._fetch_directory(remote, subdirdigest)
	943	+
895	944	def _fetch_tree(self, remote, digest):
896	945	# download but do not store the Tree object
897	946	with tempfile.NamedTemporaryFile(dir=self.tmpdir) as out:

buildstream/_context.py

---

...	...	@@ -104,6 +104,9 @@ class Context():
104	104	# What to do when a build fails in non interactive mode
105	105	self.sched_error_action = 'continue'
106	106
	107	+ # Whether or not to attempt to pull build trees globally
	108	+ self.pull_buildtrees = None
	109	+
107	110	# Whether elements must be rebuilt when their dependencies have changed
108	111	self._strict_build_plan = None
109	112
...	...	@@ -178,13 +181,16 @@ class Context():
178	181	# our artifactdir - the artifactdir may not have been created
179	182	# yet.
180	183	cache = _yaml.node_get(defaults, Mapping, 'cache')
181		- _yaml.node_validate(cache, ['quota'])
	184	+ _yaml.node_validate(cache, ['quota', 'pull-buildtrees'])
182	185
183	186	self.config_cache_quota = _yaml.node_get(cache, str, 'quota', default_value='infinity')
184	187
185	188	# Load artifact share configuration
186	189	self.artifact_cache_specs = ArtifactCache.specs_from_config_node(defaults)
187	190
	191	+ # Load pull build trees configuration
	192	+ self.pull_buildtrees = _yaml.node_get(cache, bool, 'pull-buildtrees')
	193	+
188	194	# Load logging config
189	195	logging = _yaml.node_get(defaults, Mapping, 'logging')
190	196	_yaml.node_validate(logging, [

buildstream/_frontend/app.py

---

...	...	@@ -182,7 +182,8 @@ class App():
182	182	'fetchers': 'sched_fetchers',
183	183	'builders': 'sched_builders',
184	184	'pushers': 'sched_pushers',
185		- 'network_retries': 'sched_network_retries'
	185	+ 'network_retries': 'sched_network_retries',
	186	+ 'pull_buildtrees': 'pull_buildtrees'
186	187	}
187	188	for cli_option, context_attr in override_map.items():
188	189	option_value = self._main_options.get(cli_option)

buildstream/_frontend/cli.py

---

...	...	@@ -219,6 +219,8 @@ def print_version(ctx, param, value):
219	219	help="Specify a project option")
220	220	@click.option('--default-mirror', default=None,
221	221	help="The mirror to fetch from first, before attempting other mirrors")
	222	+@click.option('--pull-buildtrees', is_flag=True, default=None,
	223	+ help="Include an element's build tree when pulling remote element artifacts")
222	224	@click.pass_context
223	225	def cli(context, **kwargs):
224	226	"""Build and manipulate BuildStream projects

buildstream/data/userconfig.yaml

---

...	...	@@ -35,6 +35,9 @@ cache:
35	35	# to the isize of the file system containing the cache.
36	36	quota: infinity
37	37
	38	+ # Whether to pull build trees when downloading element artifacts
	39	+ pull-buildtrees: False
	40	+
38	41	#
39	42	# Scheduler
40	43	#

buildstream/element.py

---

...	...	@@ -1397,12 +1397,12 @@ class Element(Plugin):
1397	1397	with self.timed_activity("Staging local files at {}"
1398	1398	.format(workspace.get_absolute_path())):
1399	1399	workspace.stage(temp_staging_directory)
1400		- elif self._cached():
1401		- # We have a cached buildtree to use, instead
	1400	+ # Check if we have a cached buildtree to use
	1401	+ elif self.__cached_buildtree():
1402	1402	artifact_base, _ = self.__extract()
1403	1403	import_dir = os.path.join(artifact_base, 'buildtree')
1404	1404	else:
1405		- # No workspace, stage directly
	1405	+ # No workspace or cached buildtree, stage source directly
1406	1406	for source in self.sources():
1407	1407	source._stage(temp_staging_directory)
1408	1408
...	...	@@ -1691,7 +1691,9 @@ class Element(Plugin):
1691	1691
1692	1692	# _pull_pending()
1693	1693	#
1694		- # Check whether the artifact will be pulled.
	1694	+ # Check whether the artifact will be pulled. If the pull operation is to
	1695	+ # include a specific subdir of the element artifact (from cli or user conf)
	1696	+ # then the local cache is queried for the subdirs existence.
1695	1697	#
1696	1698	# Returns:
1697	1699	# (bool): Whether a pull operation is pending
...	...	@@ -1701,8 +1703,15 @@ class Element(Plugin):
1701	1703	# Workspace builds are never pushed to artifact servers
1702	1704	return False
1703	1705
1704		- if self.__strong_cached:
1705		- # Artifact already in local cache
	1706	+ # Check whether the pull has been invoked with a specific subdir requested
	1707	+ # in user context, as to complete a partial artifact
	1708	+ subdir, _ = self.__pull_directories()
	1709	+
	1710	+ if self.__strong_cached and subdir:
	1711	+ # If we've specified a subdir, check if the subdir is cached locally
	1712	+ if self.__artifacts.contains_subdir_artifact(self, self.__strict_cache_key, subdir):
	1713	+ return False
	1714	+ elif self.__strong_cached:
1706	1715	return False
1707	1716
1708	1717	# Pull is pending if artifact remote server available
...	...	@@ -1724,33 +1733,6 @@ class Element(Plugin):
1724	1733
1725	1734	self._update_state()
1726	1735
1727		- def _pull_strong(self, *, progress=None):
1728		- weak_key = self._get_cache_key(strength=_KeyStrength.WEAK)
1729		-
1730		- key = self.__strict_cache_key
1731		- if not self.__artifacts.pull(self, key, progress=progress):
1732		- return False
1733		-
1734		- # update weak ref by pointing it to this newly fetched artifact
1735		- self.__artifacts.link_key(self, key, weak_key)
1736		-
1737		- return True
1738		-
1739		- def _pull_weak(self, *, progress=None):
1740		- weak_key = self._get_cache_key(strength=_KeyStrength.WEAK)
1741		-
1742		- if not self.__artifacts.pull(self, weak_key, progress=progress):
1743		- return False
1744		-
1745		- # extract strong cache key from this newly fetched artifact
1746		- self._pull_done()
1747		-
1748		- # create tag for strong cache key
1749		- key = self._get_cache_key(strength=_KeyStrength.STRONG)
1750		- self.__artifacts.link_key(self, weak_key, key)
1751		-
1752		- return True
1753		-
1754	1736	# _pull():
1755	1737	#
1756	1738	# Pull artifact from remote artifact repository into local artifact cache.
...	...	@@ -1763,11 +1745,15 @@ class Element(Plugin):
1763	1745	def progress(percent, message):
1764	1746	self.status(message)
1765	1747
	1748	+ # Get optional specific subdir to pull and optional list to not pull
	1749	+ # based off of user context
	1750	+ subdir, excluded_subdirs = self.__pull_directories()
	1751	+
1766	1752	# Attempt to pull artifact without knowing whether it's available
1767		- pulled = self._pull_strong(progress=progress)
	1753	+ pulled = self.__pull_strong(progress=progress, subdir=subdir, excluded_subdirs=excluded_subdirs)
1768	1754
1769	1755	if not pulled and not self._cached() and not context.get_strict():
1770		- pulled = self._pull_weak(progress=progress)
	1756	+ pulled = self.__pull_weak(progress=progress, subdir=subdir, excluded_subdirs=excluded_subdirs)
1771	1757
1772	1758	if not pulled:
1773	1759	return False
...	...	@@ -1787,10 +1773,12 @@ class Element(Plugin):
1787	1773	# No push remotes for this element's project
1788	1774	return True
1789	1775
1790		- if not self._cached():
	1776	+ # Do not push elements that aren't cached, or that are cached with a dangling buildtree
	1777	+ # artifact unless element type is expected to have an an empty buildtree directory
	1778	+ if not self.__cached_buildtree():
1791	1779	return True
1792	1780
1793		- # Do not push tained artifact
	1781	+ # Do not push tainted artifact
1794	1782	if self.__get_tainted():
1795	1783	return True
1796	1784
...	...	@@ -2674,6 +2662,106 @@ class Element(Plugin):
2674	2662
2675	2663	return utils._deduplicate(keys)
2676	2664
	2665	+ # __pull_strong():
	2666	+ #
	2667	+ # Attempt pulling given element from configured artifact caches with
	2668	+ # the strict cache key
	2669	+ #
	2670	+ # Args:
	2671	+ # progress (callable): The progress callback, if any
	2672	+ # subdir (str): The optional specific subdir to pull
	2673	+ # excluded_subdirs (list): The optional list of subdirs to not pull
	2674	+ #
	2675	+ # Returns:
	2676	+ # (bool): Whether or not the pull was successful
	2677	+ #
	2678	+ def __pull_strong(self, *, progress=None, subdir=None, excluded_subdirs=None):
	2679	+ weak_key = self._get_cache_key(strength=_KeyStrength.WEAK)
	2680	+ key = self.__strict_cache_key
	2681	+ if not self.__artifacts.pull(self, key, progress=progress, subdir=subdir,
	2682	+ excluded_subdirs=excluded_subdirs):
	2683	+ return False
	2684	+
	2685	+ # update weak ref by pointing it to this newly fetched artifact
	2686	+ self.__artifacts.link_key(self, key, weak_key)
	2687	+
	2688	+ return True
	2689	+
	2690	+ # __pull_weak():
	2691	+ #
	2692	+ # Attempt pulling given element from configured artifact caches with
	2693	+ # the weak cache key
	2694	+ #
	2695	+ # Args:
	2696	+ # progress (callable): The progress callback, if any
	2697	+ # subdir (str): The optional specific subdir to pull
	2698	+ # excluded_subdirs (list): The optional list of subdirs to not pull
	2699	+ #
	2700	+ # Returns:
	2701	+ # (bool): Whether or not the pull was successful
	2702	+ #
	2703	+ def __pull_weak(self, *, progress=None, subdir=None, excluded_subdirs=None):
	2704	+ weak_key = self._get_cache_key(strength=_KeyStrength.WEAK)
	2705	+ if not self.__artifacts.pull(self, weak_key, progress=progress, subdir=subdir,
	2706	+ excluded_subdirs=excluded_subdirs):
	2707	+ return False
	2708	+
	2709	+ # extract strong cache key from this newly fetched artifact
	2710	+ self._pull_done()
	2711	+
	2712	+ # create tag for strong cache key
	2713	+ key = self._get_cache_key(strength=_KeyStrength.STRONG)
	2714	+ self.__artifacts.link_key(self, weak_key, key)
	2715	+
	2716	+ return True
	2717	+
	2718	+ # __cached_buildtree():
	2719	+ #
	2720	+ # Check if cached element artifact contains expected buildtree
	2721	+ #
	2722	+ # Returns:
	2723	+ # (bool): True if artifact cached with buildtree, False if
	2724	+ # element not cached or missing expected buildtree
	2725	+ #
	2726	+ def __cached_buildtree(self):
	2727	+ context = self._get_context()
	2728	+
	2729	+ if not self._cached():
	2730	+ return False
	2731	+ elif context.get_strict():
	2732	+ if not self.__artifacts.contains_subdir_artifact(self, self.__strict_cache_key, 'buildtree'):
	2733	+ return False
	2734	+ elif not self.__artifacts.contains_subdir_artifact(self, self.__weak_cache_key, 'buildtree'):
	2735	+ return False
	2736	+
	2737	+ return True
	2738	+
	2739	+ # __pull_directories():
	2740	+ #
	2741	+ # Which directories to include or exclude given the current
	2742	+ # context
	2743	+ #
	2744	+ # Returns:
	2745	+ # subdir (str): The optional specific subdir to include, based
	2746	+ # on user context
	2747	+ # excluded_subdirs (list): The optional list of subdirs to not
	2748	+ # pull, referenced against subdir value
	2749	+ #
	2750	+ def __pull_directories(self):
	2751	+ context = self._get_context()
	2752	+
	2753	+ # Current default exclusions on pull
	2754	+ excluded_subdirs = ["buildtree"]
	2755	+ subdir = ''
	2756	+
	2757	+ # If buildtrees are to be pulled, remove the value from exclusion list
	2758	+ # and set specific subdir
	2759	+ if context.pull_buildtrees:
	2760	+ subdir = "buildtree"
	2761	+ excluded_subdirs.remove(subdir)
	2762	+
	2763	+ return (subdir, excluded_subdirs)
	2764	+
2677	2765
2678	2766	def _overlap_error_detail(f, forbidden_overlap_elements, elements):
2679	2767	if forbidden_overlap_elements:

tests/completions/completions.py

---

...	...	@@ -42,6 +42,7 @@ MAIN_OPTIONS = [
42	42	"-o ",
43	43	"--option ",
44	44	"--on-error ",
	45	+ "--pull-buildtrees ",
45	46	"--pushers ",
46	47	"--strict ",
47	48	"--verbose ",

tests/integration/build-tree.py

---

...	...	@@ -70,8 +70,8 @@ def test_buildtree_pulled(cli, tmpdir, datafiles):
70	70	})
71	71	assert cli.get_element_state(project, element_name) != 'cached'
72	72
73		- # Pull from cache
74		- result = cli.run(project=project, args=['pull', '--deps', 'all', element_name])
	73	+ # Pull from cache, ensuring cli options is set to pull the buildtree
	74	+ result = cli.run(project=project, args=['--pull-buildtrees', 'pull', '--deps', 'all', element_name])
75	75	result.assert_success()
76	76
77	77	# Check it's using the cached build tree

tests/integration/pullbuildtrees.py

---

	1	+import os
	2	+import shutil
	3	+import pytest
	4	+
	5	+from tests.testutils import cli_integration as cli, create_artifact_share
	6	+from tests.testutils.integration import assert_contains
	7	+from buildstream._exceptions import ErrorDomain, LoadErrorReason
	8	+
	9	+
	10	+DATA_DIR = os.path.join(
	11	+ os.path.dirname(os.path.realpath(__file__)),
	12	+ "project"
	13	+)
	14	+
	15	+
	16	+# Remove artifact cache & set cli.config value of pull-buildtrees
	17	+# to false, which is the default user context. The cache has to be
	18	+# cleared as just forcefully removing the refpath leaves dangling objects.
	19	+def default_state(cli, tmpdir, share):
	20	+ shutil.rmtree(os.path.join(str(tmpdir), 'artifacts'))
	21	+ cli.configure({
	22	+ 'artifacts': {'url': share.repo, 'push': False},
	23	+ 'artifactdir': os.path.join(str(tmpdir), 'artifacts'),
	24	+ 'cache': {'pull-buildtrees': False},
	25	+ })
	26	+
	27	+
	28	+# A test to capture the integration of the pullbuildtrees
	29	+# behaviour, which by default is to not include the buildtree
	30	+# directory of an element.
	31	+@pytest.mark.integration
	32	+@pytest.mark.datafiles(DATA_DIR)
	33	+def test_pullbuildtrees(cli, tmpdir, datafiles, integration_cache):
	34	+ project = os.path.join(datafiles.dirname, datafiles.basename)
	35	+ element_name = 'autotools/amhello.bst'
	36	+
	37	+ # Create artifact shares for pull & push testing
	38	+ with create_artifact_share(os.path.join(str(tmpdir), 'share1')) as share1,\
	39	+ create_artifact_share(os.path.join(str(tmpdir), 'share2')) as share2:
	40	+ cli.configure({
	41	+ 'artifacts': {'url': share1.repo, 'push': True},
	42	+ 'artifactdir': os.path.join(str(tmpdir), 'artifacts')
	43	+ })
	44	+
	45	+ # Build autotools element, checked pushed, delete local
	46	+ result = cli.run(project=project, args=['build', element_name])
	47	+ assert result.exit_code == 0
	48	+ assert cli.get_element_state(project, element_name) == 'cached'
	49	+ assert share1.has_artifact('test', element_name, cli.get_element_key(project, element_name))
	50	+ default_state(cli, tmpdir, share1)
	51	+
	52	+ # Pull artifact with default config, assert that pulling again
	53	+ # doesn't create a pull job, then assert with buildtrees user
	54	+ # config set creates a pull job.
	55	+ result = cli.run(project=project, args=['pull', element_name])
	56	+ assert element_name in result.get_pulled_elements()
	57	+ result = cli.run(project=project, args=['pull', element_name])
	58	+ assert element_name not in result.get_pulled_elements()
	59	+ cli.configure({'cache': {'pull-buildtrees': True}})
	60	+ result = cli.run(project=project, args=['pull', element_name])
	61	+ assert element_name in result.get_pulled_elements()
	62	+ default_state(cli, tmpdir, share1)
	63	+
	64	+ # Pull artifact with default config, then assert that pulling
	65	+ # with buildtrees cli flag set creates a pull job.
	66	+ # Also assert that the buildtree is added to the artifact's
	67	+ # extract dir
	68	+ result = cli.run(project=project, args=['pull', element_name])
	69	+ assert element_name in result.get_pulled_elements()
	70	+ elementdigest = share1.has_artifact('test', element_name, cli.get_element_key(project, element_name))
	71	+ buildtreedir = os.path.join(str(tmpdir), 'artifacts', 'extract', 'test', 'autotools-amhello',
	72	+ elementdigest.hash, 'buildtree')
	73	+ assert not os.path.isdir(buildtreedir)
	74	+ result = cli.run(project=project, args=['--pull-buildtrees', 'pull', element_name])
	75	+ assert element_name in result.get_pulled_elements()
	76	+ assert os.path.isdir(buildtreedir)
	77	+ default_state(cli, tmpdir, share1)
	78	+
	79	+ # Pull artifact with pullbuildtrees set in user config, then assert
	80	+ # that pulling with the same user config doesn't creates a pull job,
	81	+ # or when buildtrees cli flag is set.
	82	+ cli.configure({'cache': {'pull-buildtrees': True}})
	83	+ result = cli.run(project=project, args=['pull', element_name])
	84	+ assert element_name in result.get_pulled_elements()
	85	+ result = cli.run(project=project, args=['pull', element_name])
	86	+ assert element_name not in result.get_pulled_elements()
	87	+ result = cli.run(project=project, args=['--pull-buildtrees', 'pull', element_name])
	88	+ assert element_name not in result.get_pulled_elements()
	89	+ default_state(cli, tmpdir, share1)
	90	+
	91	+ # Pull artifact with default config and buildtrees cli flag set, then assert
	92	+ # that pulling with pullbuildtrees set in user config doesn't create a pull
	93	+ # job.
	94	+ result = cli.run(project=project, args=['--pull-buildtrees', 'pull', element_name])
	95	+ assert element_name in result.get_pulled_elements()
	96	+ cli.configure({'cache': {'pull-buildtrees': True}})
	97	+ result = cli.run(project=project, args=['pull', element_name])
	98	+ assert element_name not in result.get_pulled_elements()
	99	+ default_state(cli, tmpdir, share1)
	100	+
	101	+ # Assert that a partial build element (not containing a populated buildtree dir)
	102	+ # can't be pushed to an artifact share, then assert that a complete build element
	103	+ # can be. This will attempt a partial pull from share1 and then a partial push
	104	+ # to share2
	105	+ result = cli.run(project=project, args=['pull', element_name])
	106	+ assert element_name in result.get_pulled_elements()
	107	+ cli.configure({'artifacts': {'url': share2.repo, 'push': True}})
	108	+ result = cli.run(project=project, args=['push', element_name])
	109	+ assert element_name not in result.get_pushed_elements()
	110	+ assert not share2.has_artifact('test', element_name, cli.get_element_key(project, element_name))
	111	+
	112	+ # Assert that after pulling the missing buildtree the element artifact can be
	113	+ # successfully pushed to the remote. This will attempt to pull the buildtree
	114	+ # from share1 and then a 'complete' push to share2
	115	+ cli.configure({'artifacts': {'url': share1.repo, 'push': False}})
	116	+ result = cli.run(project=project, args=['--pull-buildtrees', 'pull', element_name])
	117	+ assert element_name in result.get_pulled_elements()
	118	+ cli.configure({'artifacts': {'url': share2.repo, 'push': True}})
	119	+ result = cli.run(project=project, args=['push', element_name])
	120	+ assert element_name in result.get_pushed_elements()
	121	+ assert share2.has_artifact('test', element_name, cli.get_element_key(project, element_name))
	122	+ default_state(cli, tmpdir, share1)
	123	+
	124	+
	125	+# Ensure that only valid pull-buildtrees boolean options make it through the loading
	126	+# process.
	127	+@pytest.mark.parametrize("value,success", [
	128	+ (True, True),
	129	+ (False, True),
	130	+ ("pony", False),
	131	+ ("1", False)
	132	+])
	133	+@pytest.mark.datafiles(DATA_DIR)
	134	+def test_invalid_cache_pullbuildtrees(cli, datafiles, tmpdir, value, success):
	135	+ project = os.path.join(datafiles.dirname, datafiles.basename)
	136	+
	137	+ cli.configure({
	138	+ 'cache': {
	139	+ 'pull-buildtrees': value,
	140	+ }
	141	+ })
	142	+
	143	+ res = cli.run(project=project, args=['workspace', 'list'])
	144	+ if success:
	145	+ res.assert_success()
	146	+ else:
	147	+ res.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.ILLEGAL_COMPOSITE)

tests/testutils/artifactshare.py

---

...	...	@@ -114,7 +114,7 @@ class ArtifactShare():
114	114	# cache_key (str): The cache key
115	115	#
116	116	# Returns:
117		- # (bool): True if the artifact exists in the share, otherwise false.
	117	+ # (str): artifact digest if the artifact exists in the share, otherwise None.
118	118	def has_artifact(self, project_name, element_name, cache_key):
119	119
120	120	# NOTE: This should be kept in line with our
...	...	@@ -134,9 +134,9 @@ class ArtifactShare():
134	134
135	135	try:
136	136	tree = self.cas.resolve_ref(artifact_key)
137		- return True
	137	+ return tree
138	138	except CASError:
139		- return False
	139	+ return None
140	140
141	141	# close():
142	142	#

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