[Notes] [Git][BuildStream/buildstream][bschubert/dont-cache-errors-from-host-tools] 4 commits: plugins/elements/cmake.yaml: always specify variable types

[Benjamin Schubert <gitlab mg gitlab com>]

Title: GitLab

Benjamin Schubert pushed to branch bschubert/dont-cache-errors-from-host-tools at BuildStream / buildstream

Commits:

• bc827cc8
  by Abderrahim Kitouni at 2018-11-19T14:55:15Z

  plugins/elements/cmake.yaml: always specify variable types
  
  cmake sometimes misinterprets relative paths as relative to the current directory
  if this is not specified. See freedesktop-sdk/freedesktop-sdk#431
  
  adjust tests/format/variables.py accordingly.
  

• 5fbc5f41
  by Valentin David at 2018-11-19T15:31:05Z

  Merge branch 'abderrahim/cmake-variable-types' into 'master'
  
  plugins/elements/cmake.yaml: always specify variable types
  
  Closes #761
  
  See merge request BuildStream/buildstream!947

• 06e28860
  by Benjamin Schubert at 2018-11-19T15:52:24Z

  Don't cache sandbox errors
  
  Sandbox errors (like missing host tools) are dependent on the host
  system and rarely on what is actually done.
  
  It is therefore better to not cache them as they are subject to
  change between two runs.
  
  Also add test to ensure sandbox failure are not cached
  

• dd36cfbc
  by Benjamin Schubert at 2018-11-19T15:52:24Z

  element.py: Only cache on element error or success
  
  Reverse the way we were handling caching by only caching
  when we know it's a problem with the build or the build is a
  success in order not to cache transient errors.
  

5 changed files:

• buildstream/element.py
• buildstream/plugins/elements/cmake.yaml
• conftest.py
• tests/format/variables.py
• tests/integration/cachedfail.py

Changes:

buildstream/element.py

---

...	...	@@ -85,7 +85,8 @@ import shutil
85	85	from . import _yaml
86	86	from ._variables import Variables
87	87	from ._versions import BST_CORE_ARTIFACT_VERSION
88		-from ._exceptions import BstError, LoadError, LoadErrorReason, ImplError, ErrorDomain
	88	+from ._exceptions import BstError, LoadError, LoadErrorReason, ImplError, \
	89	+ ErrorDomain
89	90	from .utils import UtilError
90	91	from . import Plugin, Consistency, Scope
91	92	from . import SandboxFlags
...	...	@@ -1553,7 +1554,6 @@ class Element(Plugin):
1553	1554	self.__dynamic_public = _yaml.node_copy(self.__public)
1554	1555
1555	1556	# Call the abstract plugin methods
1556		- collect = None
1557	1557	try:
1558	1558	# Step 1 - Configure
1559	1559	self.configure_sandbox(sandbox)
...	...	@@ -1564,7 +1564,7 @@ class Element(Plugin):
1564	1564	# Step 4 - Assemble
1565	1565	collect = self.assemble(sandbox) # pylint: disable=assignment-from-no-return
1566	1566	self.__set_build_result(success=True, description="succeeded")
1567		- except BstError as e:
	1567	+ except ElementError as e:
1568	1568	# Shelling into a sandbox is useful to debug this error
1569	1569	e.sandbox = True
1570	1570
...	...	@@ -1586,104 +1586,105 @@ class Element(Plugin):
1586	1586	self.warn("Failed to preserve workspace state for failed build sysroot: {}"
1587	1587	.format(e))
1588	1588
1589		- if isinstance(e, ElementError):
1590		- collect = e.collect # pylint: disable=no-member
1591		-
1592	1589	self.__set_build_result(success=False, description=str(e), detail=e.detail)
	1590	+ self._cache_artifact(rootdir, sandbox, e.collect)
	1591	+
1593	1592	raise
	1593	+ else:
	1594	+ return self._cache_artifact(rootdir, sandbox, collect)
1594	1595	finally:
1595		- if collect is not None:
1596		- try:
1597		- sandbox_vroot = sandbox.get_virtual_directory()
1598		- collectvdir = sandbox_vroot.descend(collect.lstrip(os.sep).split(os.sep))
1599		- except VirtualDirectoryError:
1600		- # No collect directory existed
1601		- collectvdir = None
1602		-
1603		- # Create artifact directory structure
1604		- assembledir = os.path.join(rootdir, 'artifact')
1605		- filesdir = os.path.join(assembledir, 'files')
1606		- logsdir = os.path.join(assembledir, 'logs')
1607		- metadir = os.path.join(assembledir, 'meta')
1608		- buildtreedir = os.path.join(assembledir, 'buildtree')
1609		- os.mkdir(assembledir)
1610		- if collect is not None and collectvdir is not None:
1611		- os.mkdir(filesdir)
1612		- os.mkdir(logsdir)
1613		- os.mkdir(metadir)
1614		- os.mkdir(buildtreedir)
1615		-
1616		- # Hard link files from collect dir to files directory
1617		- if collect is not None and collectvdir is not None:
1618		- collectvdir.export_files(filesdir, can_link=True)
1619		-
1620		- try:
1621		- sandbox_vroot = sandbox.get_virtual_directory()
1622		- sandbox_build_dir = sandbox_vroot.descend(
1623		- self.get_variable('build-root').lstrip(os.sep).split(os.sep))
1624		- # Hard link files from build-root dir to buildtreedir directory
1625		- sandbox_build_dir.export_files(buildtreedir)
1626		- except VirtualDirectoryError:
1627		- # Directory could not be found. Pre-virtual
1628		- # directory behaviour was to continue silently
1629		- # if the directory could not be found.
1630		- pass
1631		-
1632		- # Copy build log
1633		- log_filename = context.get_log_filename()
1634		- self._build_log_path = os.path.join(logsdir, 'build.log')
1635		- if log_filename:
1636		- shutil.copyfile(log_filename, self._build_log_path)
1637		-
1638		- # Store public data
1639		- _yaml.dump(_yaml.node_sanitize(self.__dynamic_public), os.path.join(metadir, 'public.yaml'))
1640		-
1641		- # Store result
1642		- build_result_dict = {"success": self.__build_result[0], "description": self.__build_result[1]}
1643		- if self.__build_result[2] is not None:
1644		- build_result_dict["detail"] = self.__build_result[2]
1645		- _yaml.dump(build_result_dict, os.path.join(metadir, 'build-result.yaml'))
1646		-
1647		- # ensure we have cache keys
1648		- self._assemble_done()
1649		-
1650		- # Store keys.yaml
1651		- _yaml.dump(_yaml.node_sanitize({
1652		- 'strong': self._get_cache_key(),
1653		- 'weak': self._get_cache_key(_KeyStrength.WEAK),
1654		- }), os.path.join(metadir, 'keys.yaml'))
1655		-
1656		- # Store dependencies.yaml
1657		- _yaml.dump(_yaml.node_sanitize({
1658		- e.name: e._get_cache_key() for e in self.dependencies(Scope.BUILD)
1659		- }), os.path.join(metadir, 'dependencies.yaml'))
1660		-
1661		- # Store workspaced.yaml
1662		- _yaml.dump(_yaml.node_sanitize({
1663		- 'workspaced': True if self._get_workspace() else False
1664		- }), os.path.join(metadir, 'workspaced.yaml'))
1665		-
1666		- # Store workspaced-dependencies.yaml
1667		- _yaml.dump(_yaml.node_sanitize({
1668		- 'workspaced-dependencies': [
1669		- e.name for e in self.dependencies(Scope.BUILD)
1670		- if e._get_workspace()
1671		- ]
1672		- }), os.path.join(metadir, 'workspaced-dependencies.yaml'))
1673		-
1674		- with self.timed_activity("Caching artifact"):
1675		- artifact_size = utils._get_dir_size(assembledir)
1676		- self.__artifacts.commit(self, assembledir, self.__get_cache_keys_for_commit())
1677		-
1678		- if collect is not None and collectvdir is None:
1679		- raise ElementError(
1680		- "Directory '{}' was not found inside the sandbox, "
1681		- "unable to collect artifact contents"
1682		- .format(collect))
1683		-
1684		- # Finally cleanup the build dir
1685	1596	cleanup_rootdir()
1686	1597
	1598	+ def _cache_artifact(self, rootdir, sandbox, collect):
	1599	+ if collect is not None:
	1600	+ try:
	1601	+ sandbox_vroot = sandbox.get_virtual_directory()
	1602	+ collectvdir = sandbox_vroot.descend(collect.lstrip(os.sep).split(os.sep))
	1603	+ except VirtualDirectoryError:
	1604	+ # No collect directory existed
	1605	+ collectvdir = None
	1606	+
	1607	+ # Create artifact directory structure
	1608	+ assembledir = os.path.join(rootdir, 'artifact')
	1609	+ filesdir = os.path.join(assembledir, 'files')
	1610	+ logsdir = os.path.join(assembledir, 'logs')
	1611	+ metadir = os.path.join(assembledir, 'meta')
	1612	+ buildtreedir = os.path.join(assembledir, 'buildtree')
	1613	+ os.mkdir(assembledir)
	1614	+ if collect is not None and collectvdir is not None:
	1615	+ os.mkdir(filesdir)
	1616	+ os.mkdir(logsdir)
	1617	+ os.mkdir(metadir)
	1618	+ os.mkdir(buildtreedir)
	1619	+
	1620	+ # Hard link files from collect dir to files directory
	1621	+ if collect is not None and collectvdir is not None:
	1622	+ collectvdir.export_files(filesdir, can_link=True)
	1623	+
	1624	+ try:
	1625	+ sandbox_vroot = sandbox.get_virtual_directory()
	1626	+ sandbox_build_dir = sandbox_vroot.descend(
	1627	+ self.get_variable('build-root').lstrip(os.sep).split(os.sep))
	1628	+ # Hard link files from build-root dir to buildtreedir directory
	1629	+ sandbox_build_dir.export_files(buildtreedir)
	1630	+ except VirtualDirectoryError:
	1631	+ # Directory could not be found. Pre-virtual
	1632	+ # directory behaviour was to continue silently
	1633	+ # if the directory could not be found.
	1634	+ pass
	1635	+
	1636	+ # Copy build log
	1637	+ log_filename = self._get_context().get_log_filename()
	1638	+ self._build_log_path = os.path.join(logsdir, 'build.log')
	1639	+ if log_filename:
	1640	+ shutil.copyfile(log_filename, self._build_log_path)
	1641	+
	1642	+ # Store public data
	1643	+ _yaml.dump(_yaml.node_sanitize(self.__dynamic_public), os.path.join(metadir, 'public.yaml'))
	1644	+
	1645	+ # Store result
	1646	+ build_result_dict = {"success": self.__build_result[0], "description": self.__build_result[1]}
	1647	+ if self.__build_result[2] is not None:
	1648	+ build_result_dict["detail"] = self.__build_result[2]
	1649	+ _yaml.dump(build_result_dict, os.path.join(metadir, 'build-result.yaml'))
	1650	+
	1651	+ # ensure we have cache keys
	1652	+ self._assemble_done()
	1653	+
	1654	+ # Store keys.yaml
	1655	+ _yaml.dump(_yaml.node_sanitize({
	1656	+ 'strong': self._get_cache_key(),
	1657	+ 'weak': self._get_cache_key(_KeyStrength.WEAK),
	1658	+ }), os.path.join(metadir, 'keys.yaml'))
	1659	+
	1660	+ # Store dependencies.yaml
	1661	+ _yaml.dump(_yaml.node_sanitize({
	1662	+ e.name: e._get_cache_key() for e in self.dependencies(Scope.BUILD)
	1663	+ }), os.path.join(metadir, 'dependencies.yaml'))
	1664	+
	1665	+ # Store workspaced.yaml
	1666	+ _yaml.dump(_yaml.node_sanitize({
	1667	+ 'workspaced': True if self._get_workspace() else False
	1668	+ }), os.path.join(metadir, 'workspaced.yaml'))
	1669	+
	1670	+ # Store workspaced-dependencies.yaml
	1671	+ _yaml.dump(_yaml.node_sanitize({
	1672	+ 'workspaced-dependencies': [
	1673	+ e.name for e in self.dependencies(Scope.BUILD)
	1674	+ if e._get_workspace()
	1675	+ ]
	1676	+ }), os.path.join(metadir, 'workspaced-dependencies.yaml'))
	1677	+
	1678	+ with self.timed_activity("Caching artifact"):
	1679	+ artifact_size = utils._get_dir_size(assembledir)
	1680	+ self.__artifacts.commit(self, assembledir, self.__get_cache_keys_for_commit())
	1681	+
	1682	+ if collect is not None and collectvdir is None:
	1683	+ raise ElementError(
	1684	+ "Directory '{}' was not found inside the sandbox, "
	1685	+ "unable to collect artifact contents"
	1686	+ .format(collect))
	1687	+
1687	1688	return artifact_size
1688	1689
1689	1690	def _get_build_log(self):

buildstream/plugins/elements/cmake.yaml

---

...	...	@@ -19,7 +19,7 @@ variables:
19	19	cmake-args: |
20	20
21	21	-DCMAKE_INSTALL_PREFIX:PATH="%{prefix}" \
22		- -DCMAKE_INSTALL_LIBDIR=%{lib} %{cmake-extra} %{cmake-global} %{cmake-local}
	22	+ -DCMAKE_INSTALL_LIBDIR:PATH="%{lib}" %{cmake-extra} %{cmake-global} %{cmake-local}
23	23
24	24	cmake: |
25	25

conftest.py

---

...	...	@@ -56,6 +56,10 @@ def integration_cache(request):
56	56	pass
57	57
58	58
59		-@pytest.fixture(autouse=True)
60	59	def clean_platform_cache():
61	60	Platform._instance = None
	61	+
	62	+
	63	+@pytest.fixture(autouse=True)
	64	+def ensure_platform_cache_is_clean():
	65	+ clean_platform_cache()

tests/format/variables.py

---

...	...	@@ -20,7 +20,7 @@ DATA_DIR = os.path.join(
20	20	('autotools.bst', 'make-install', "make -j1 DESTDIR=\"/buildstream-install\" install"),
21	21	('cmake.bst', 'cmake',
22	22	"cmake -B_builddir -H\".\" -G\"Unix Makefiles\" " + "-DCMAKE_INSTALL_PREFIX:PATH=\"/usr\" \\\n" +
23		- "-DCMAKE_INSTALL_LIBDIR=lib "),
	23	+ "-DCMAKE_INSTALL_LIBDIR:PATH=\"lib\" "),
24	24	('distutils.bst', 'python-install',
25	25	"python3 ./setup.py install --prefix \"/usr\" \\\n" +
26	26	"--root \"/buildstream-install\""),
...	...	@@ -46,7 +46,7 @@ def test_defaults(cli, datafiles, tmpdir, target, varname, expected):
46	46	('autotools.bst', 'make-install', "make -j1 DESTDIR=\"/custom/install/root\" install"),
47	47	('cmake.bst', 'cmake',
48	48	"cmake -B_builddir -H\".\" -G\"Ninja\" " + "-DCMAKE_INSTALL_PREFIX:PATH=\"/opt\" \\\n" +
49		- "-DCMAKE_INSTALL_LIBDIR=lib "),
	49	+ "-DCMAKE_INSTALL_LIBDIR:PATH=\"lib\" "),
50	50	('distutils.bst', 'python-install',
51	51	"python3 ./setup.py install --prefix \"/opt\" \\\n" +
52	52	"--root \"/custom/install/root\""),

tests/integration/cachedfail.py

---

...	...	@@ -4,6 +4,8 @@ import pytest
4	4	from buildstream import _yaml
5	5	from buildstream._exceptions import ErrorDomain
6	6
	7	+from conftest import clean_platform_cache
	8	+
7	9	from tests.testutils import cli_integration as cli, create_artifact_share
8	10	from tests.testutils.site import IS_LINUX
9	11
...	...	@@ -158,3 +160,40 @@ def test_push_cached_fail(cli, tmpdir, datafiles, on_error):
158	160	assert cli.get_element_state(project, 'element.bst') == 'failed'
159	161	# This element should have been pushed to the remote
160	162	assert share.has_artifact('test', 'element.bst', cli.get_element_key(project, 'element.bst'))
	163	+
	164	+
	165	+@pytest.mark.skipif(not IS_LINUX, reason='Only available on linux')
	166	+@pytest.mark.datafiles(DATA_DIR)
	167	+def test_host_tools_errors_are_not_cached(cli, tmpdir, datafiles):
	168	+ project = os.path.join(datafiles.dirname, datafiles.basename)
	169	+ element_path = os.path.join(project, 'elements', 'element.bst')
	170	+
	171	+ # Write out our test target
	172	+ element = {
	173	+ 'kind': 'script',
	174	+ 'depends': [
	175	+ {
	176	+ 'filename': 'base.bst',
	177	+ 'type': 'build',
	178	+ },
	179	+ ],
	180	+ 'config': {
	181	+ 'commands': [
	182	+ 'true',
	183	+ ],
	184	+ },
	185	+ }
	186	+ _yaml.dump(element, element_path)
	187	+
	188	+ # Build without access to host tools, this will fail
	189	+ result1 = cli.run(project=project, args=['build', 'element.bst'], env={'PATH': ''})
	190	+ result1.assert_task_error(ErrorDomain.SANDBOX, 'unavailable-local-sandbox')
	191	+ assert cli.get_element_state(project, 'element.bst') == 'buildable'
	192	+
	193	+ # clean the cache before running again
	194	+ clean_platform_cache()
	195	+
	196	+ # When rebuilding, this should work
	197	+ result2 = cli.run(project=project, args=['build', 'element.bst'])
	198	+ result2.assert_success()
	199	+ assert cli.get_element_state(project, 'element.bst') == 'cached'

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