[Notes] [Git][BuildStream/buildstream][jmac/vdir_import_test] 9 commits: source.py: Fix re-instantiation

[Jim MacArthur <gitlab mg gitlab com>]

Title: GitLab

Jim MacArthur pushed to branch jmac/vdir_import_test at BuildStream / buildstream

Commits:

• 68ef69e4
  by Tristan Van Berkom at 2018-09-21T05:20:46Z

  source.py: Fix re-instantiation
  
  This fixes an issue where the re-instantiated Source used with
  Source mirroring enabled is not completely initialized.
  
  Failing to load the ref from the project.refs file for instance,
  will result in a crash at `fetch` time.
  
  This fixes issue #666
  

• 662c729f
  by Tristan Van Berkom at 2018-09-21T05:59:30Z

  Merge branch 'tristan/fix-source-reinstantiation' into 'master'
  
  source.py: Fix re-instantiation
  
  Closes #666
  
  See merge request BuildStream/buildstream!817

• 461a0588
  by Jim MacArthur at 2018-09-21T10:53:11Z

  element.py: Remove early warning and check directory is not None
  
  The check for directory should have always been here. The early warning
  test doesn't work for `bst checkout` because caches haven't been configured
  at the point of the test, so I've had to remove it.
  

• aa9caaac
  by Jim MacArthur at 2018-09-21T10:53:11Z

  _sandboxremote.py: Ignore return value of push_directory
  
  The return value is always upload_vdir.ref.
  

• 2aae68c7
  by Jim MacArthur at 2018-09-21T10:53:11Z

  cascache.py: make push_remote raise exception if no push remotes exist
  
  Also add docstring to method and remove return value since it was useless
  

• ca1bb72c
  by Jim MacArthur at 2018-09-21T10:53:11Z

  tests/artifactcache/push.py: Ignore return value of push_directory
  

• 55c93a82
  by Jim MacArthur at 2018-09-21T11:26:55Z

  Merge branch 'jmac/remote_exec_checkout_fix' into 'master'
  
  Remote exec: Remove early warning and check directory is not None
  
  See merge request BuildStream/buildstream!800

• 42168657
  by Jim MacArthur at 2018-09-21T13:27:04Z

  buildstream/storage/__init__.py: import CasBasedDirectory
  

• d673cdf8
  by Jim MacArthur at 2018-09-21T13:27:04Z

  tests/storage/virtual_directory_import.py: New file
  

7 changed files:

• buildstream/_artifactcache/cascache.py
• buildstream/element.py
• buildstream/sandbox/_sandboxremote.py
• buildstream/source.py
• buildstream/storage/__init__.py
• tests/artifactcache/push.py
• + tests/storage/virtual_directory_import.py

Changes:

buildstream/_artifactcache/cascache.py

---

...	...	@@ -348,19 +348,29 @@ class CASCache(ArtifactCache):
348	348	return pushed
349	349
350	350	def push_directory(self, project, directory):
	351	+ """ Push the given virtual directory to all remotes.
	352	+
	353	+ Args:
	354	+ project (Project): The current project
	355	+ directory (Directory): A virtual directory object to push.
	356	+
	357	+ Raises: ArtifactError if no push remotes are configured.
	358	+ """
351	359
352	360	push_remotes = [r for r in self._remotes[project] if r.spec.push]
353	361
	362	+ if not push_remotes:
	363	+ raise ArtifactError("CASCache: push_directory was called, but no remote artifact " +
	364	+ "servers are configured as push remotes.")
	365	+
354	366	if directory.ref is None:
355		- return None
	367	+ return
356	368
357	369	for remote in push_remotes:
358	370	remote.init()
359	371
360	372	self._send_directory(remote, directory.ref)
361	373
362		- return directory.ref
363		-
364	374	def push_message(self, project, message):
365	375
366	376	push_remotes = [r for r in self._remotes[project] if r.spec.push]

buildstream/element.py

---

...	...	@@ -2137,14 +2137,11 @@ class Element(Plugin):
2137	2137	project = self._get_project()
2138	2138	platform = Platform.get_platform()
2139	2139
2140		- if self.__remote_execution_url and self.BST_VIRTUAL_DIRECTORY:
2141		- if not self.__artifacts.has_push_remotes(element=self):
2142		- # Give an early warning if remote execution will not work
2143		- raise ElementError("Artifact {} is configured to use remote execution but has no push remotes. "
2144		- .format(self.name) +
2145		- "The remote artifact server(s) may not be correctly configured or contactable.")
2146		-
2147		- self.info("Using a remote sandbox for artifact {}".format(self.name))
	2140	+ if (directory is not None and
	2141	+ self.__remote_execution_url and
	2142	+ self.BST_VIRTUAL_DIRECTORY):
	2143	+
	2144	+ self.info("Using a remote sandbox for artifact {} with directory '{}'".format(self.name, directory))
2148	2145
2149	2146	sandbox = SandboxRemote(context, project,
2150	2147	directory,

buildstream/sandbox/_sandboxremote.py

---

...	...	@@ -173,8 +173,8 @@ class SandboxRemote(Sandbox):
173	173	platform = Platform.get_platform()
174	174	cascache = platform.artifactcache
175	175	# Now, push that key (without necessarily needing a ref) to the remote.
176		- vdir_digest = cascache.push_directory(self._get_project(), upload_vdir)
177		- if not vdir_digest or not cascache.verify_digest_pushed(self._get_project(), vdir_digest):
	176	+ cascache.push_directory(self._get_project(), upload_vdir)
	177	+ if not cascache.verify_digest_pushed(self._get_project(), upload_vdir.ref):
178	178	raise SandboxError("Failed to verify that source has been pushed to the remote artifact cache.")
179	179
180	180	# Set up environment and working directory

buildstream/source.py

---

...	...	@@ -930,6 +930,38 @@ class Source(Plugin):
930	930	# Local Private Methods #
931	931	#############################################################
932	932
	933	+ # __clone_for_uri()
	934	+ #
	935	+ # Clone the source with an alternative URI setup for the alias
	936	+ # which this source uses.
	937	+ #
	938	+ # This is used for iteration over source mirrors.
	939	+ #
	940	+ # Args:
	941	+ # uri (str): The alternative URI for this source's alias
	942	+ #
	943	+ # Returns:
	944	+ # (Source): A new clone of this Source, with the specified URI
	945	+ # as the value of the alias this Source has marked as
	946	+ # primary with either mark_download_url() or
	947	+ # translate_url().
	948	+ #
	949	+ def __clone_for_uri(self, uri):
	950	+ project = self._get_project()
	951	+ context = self._get_context()
	952	+ alias = self._get_alias()
	953	+ source_kind = type(self)
	954	+
	955	+ clone = source_kind(context, project, self.__meta, alias_override=(alias, uri))
	956	+
	957	+ # Do the necessary post instantiation routines here
	958	+ #
	959	+ clone._preflight()
	960	+ clone._load_ref()
	961	+ clone._update_state()
	962	+
	963	+ return clone
	964	+
933	965	# Tries to call fetch for every mirror, stopping once it succeeds
934	966	def __do_fetch(self, **kwargs):
935	967	project = self._get_project()
...	...	@@ -968,12 +1000,8 @@ class Source(Plugin):
968	1000	self.fetch(**kwargs)
969	1001	return
970	1002
971		- context = self._get_context()
972		- source_kind = type(self)
973	1003	for uri in project.get_alias_uris(alias, first_pass=self.__first_pass):
974		- new_source = source_kind(context, project, self.__meta,
975		- alias_override=(alias, uri))
976		- new_source._preflight()
	1004	+ new_source = self.__clone_for_uri(uri)
977	1005	try:
978	1006	new_source.fetch(**kwargs)
979	1007	# FIXME: Need to consider temporary vs. permanent failures,
...	...	@@ -1006,9 +1034,7 @@ class Source(Plugin):
1006	1034	# NOTE: We are assuming here that tracking only requires substituting the
1007	1035	# first alias used
1008	1036	for uri in reversed(project.get_alias_uris(alias, first_pass=self.__first_pass)):
1009		- new_source = source_kind(context, project, self.__meta,
1010		- alias_override=(alias, uri))
1011		- new_source._preflight()
	1037	+ new_source = self.__clone_for_uri(uri)
1012	1038	try:
1013	1039	ref = new_source.track(**kwargs)
1014	1040	# FIXME: Need to consider temporary vs. permanent failures,

buildstream/storage/__init__.py

---

...	...	@@ -19,4 +19,5 @@
19	19	# Jim MacArthur <jim macarthur codethink co uk>
20	20
21	21	from ._filebaseddirectory import FileBasedDirectory
	22	+from ._casbaseddirectory import CasBasedDirectory
22	23	from .directory import Directory

tests/artifactcache/push.py

---

...	...	@@ -228,9 +228,9 @@ def _test_push_directory(user_config_file, project_dir, artifact_dir, artifact_d
228	228	directory = CasBasedDirectory(context, ref=artifact_digest)
229	229
230	230	# Push the CasBasedDirectory object
231		- directory_digest = cas.push_directory(project, directory)
	231	+ cas.push_directory(project, directory)
232	232
233		- queue.put(directory_digest.hash)
	233	+ queue.put(directory.ref.hash)
234	234	else:
235	235	queue.put("No remote configured")
236	236

tests/storage/virtual_directory_import.py

---

	1	+import os
	2	+import pytest
	3	+from tests.testutils import cli
	4	+
	5	+from buildstream.storage import CasBasedDirectory
	6	+
	7	+
	8	+class FakeContext():
	9	+ def __init__(self):
	10	+ self.config_cache_quota = "65536"
	11	+
	12	+ def get_projects(self):
	13	+ return []
	14	+
	15	+# This is a set of example file system contents. The test attempts to import
	16	+# each on top of each other to test importing works consistently.
	17	+# Each tuple is defined as (<filename>, <type>, <content>). Type can be
	18	+# 'F' (file), 'S' (symlink) or 'D' (directory) with content being the contents
	19	+# for a file or the destination for a symlink.
	20	+root_filesets = [
	21	+ [('a/b/c/textfile1', 'F', 'This is textfile 1\n')],
	22	+ [('a/b/c/textfile1', 'F', 'This is the replacement textfile 1\n')],
	23	+ [('a/b/d', 'D', '')],
	24	+ [('a/b/c', 'S', '/a/b/d')],
	25	+ [('a/b/d', 'D', ''), ('a/b/c', 'S', '/a/b/d')],
	26	+]
	27	+
	28	+empty_hash_ref = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
	29	+
	30	+
	31	+def generate_import_roots(directory):
	32	+ for fileset in range(1, len(root_filesets) + 1):
	33	+ rootname = "root{}".format(fileset)
	34	+ rootdir = os.path.join(directory, "content", rootname)
	35	+
	36	+ for (path, typesymbol, content) in root_filesets[fileset - 1]:
	37	+ if typesymbol == 'F':
	38	+ (dirnames, filename) = os.path.split(path)
	39	+ os.makedirs(os.path.join(rootdir, dirnames), exist_ok=True)
	40	+ with open(os.path.join(rootdir, dirnames, filename), "wt") as f:
	41	+ f.write(content)
	42	+ elif typesymbol == 'D':
	43	+ os.makedirs(os.path.join(rootdir, path), exist_ok=True)
	44	+ elif typesymbol == 'S':
	45	+ (dirnames, filename) = os.path.split(path)
	46	+ os.makedirs(os.path.join(rootdir, dirnames), exist_ok=True)
	47	+ os.symlink(content, os.path.join(rootdir, path))
	48	+
	49	+
	50	+def file_contents(path):
	51	+ with open(path, "r") as f:
	52	+ result = f.read()
	53	+ return result
	54	+
	55	+
	56	+def file_contents_are(path, contents):
	57	+ return file_contents(path) == contents
	58	+
	59	+
	60	+def create_new_vdir(root_number, fake_context, tmpdir):
	61	+ d = CasBasedDirectory(fake_context)
	62	+ d.import_files(os.path.join(tmpdir, "content", "root{}".format(root_number)))
	63	+ assert d.ref.hash != empty_hash_ref
	64	+ return d
	65	+
	66	+
	67	+def combinations(integer_range):
	68	+ for x in integer_range:
	69	+ for y in integer_range:
	70	+ yield (x, y)
	71	+
	72	+
	73	+def resolve_symlinks(path, root):
	74	+ """ A function to resolve symlinks inside 'path' components apart from the last one.
	75	+ For example, resolve_symlinks('/a/b/c/d', '/a/b')
	76	+ will return '/a/b/f/d' if /a/b/c is a symlink to /a/b/f. The final component of
	77	+ 'path' is not resolved, because we typically want to inspect the symlink found
	78	+ at that path, not its target.
	79	+
	80	+ """
	81	+ components = path.split(os.path.sep)
	82	+ location = root
	83	+ for i in range(0, len(components) - 1):
	84	+ location = os.path.join(location, components[i])
	85	+ if os.path.islink(location):
	86	+ # Resolve the link, add on all the remaining components
	87	+ target = os.path.join(os.readlink(location))
	88	+ tail = os.path.sep.join(components[i + 1:])
	89	+
	90	+ if target.startswith(os.path.sep):
	91	+ # Absolute link - relative to root
	92	+ location = os.path.join(root, target, tail)
	93	+ else:
	94	+ # Relative link - relative to symlink location
	95	+ location = os.path.join(location, target)
	96	+ return resolve_symlinks(location, root)
	97	+ # If we got here, no symlinks were found. Add on the final component and return.
	98	+ location = os.path.join(location, components[-1])
	99	+ return location
	100	+
	101	+
	102	+def directory_not_empty(path):
	103	+ return os.listdir(path)
	104	+
	105	+
	106	+@pytest.mark.parametrize("original,overlay", combinations([1, 2, 3, 4, 5]))
	107	+def test_cas_import(cli, tmpdir, original, overlay):
	108	+ tmpdir = str(tmpdir) # Avoids LocalPath issues on some systems
	109	+ fake_context = FakeContext()
	110	+ fake_context.artifactdir = tmpdir
	111	+ # Create some fake content
	112	+ generate_import_roots(tmpdir)
	113	+
	114	+ d = create_new_vdir(original, fake_context, tmpdir)
	115	+ d2 = create_new_vdir(overlay, fake_context, tmpdir)
	116	+ d.import_files(d2)
	117	+ d.export_files(os.path.join(tmpdir, "output"))
	118	+
	119	+ for item in root_filesets[overlay - 1]:
	120	+ (path, typename, content) = item
	121	+ realpath = resolve_symlinks(path, os.path.join(tmpdir, "output"))
	122	+ if typename == 'F':
	123	+ if os.path.isdir(realpath) and directory_not_empty(realpath):
	124	+ # The file should not have overwritten the directory in this case.
	125	+ pass
	126	+ else:
	127	+ assert os.path.isfile(realpath), "{} did not exist in the combined virtual directory".format(path)
	128	+ assert file_contents_are(realpath, content)
	129	+ elif typename == 'S':
	130	+ if os.path.isdir(realpath) and directory_not_empty(realpath):
	131	+ # The symlink should not have overwritten the directory in this case.
	132	+ pass
	133	+ else:
	134	+ assert os.path.islink(realpath)
	135	+ assert os.readlink(realpath) == content
	136	+ elif typename == 'D':
	137	+ # Note that isdir accepts symlinks to dirs, so a symlink to a dir is acceptable.
	138	+ assert os.path.isdir(realpath)

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