[Notes] [Git][BuildStream/buildstream][jmac/cas_to_cas_oct] 6 commits: CasBasedDirectory: Remove some prints

[Jim MacArthur <gitlab mg gitlab com>]

Title: GitLab

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

Commits:

• d0a11e7f
  by Jim MacArthur at 2018-10-25T16:25:51Z

  CasBasedDirectory: Remove some prints
  

• 4bb8d311
  by Jim MacArthur at 2018-10-25T16:26:21Z

  Make virtual_directory_test do the cas roundtrip test instead of _casbaseddirectory
  

• 5878f0ec
  by Jim MacArthur at 2018-10-25T16:32:05Z

  casbaseddirectory: Remove roundtrip checking code
  

• a52da6e2
  by Jim MacArthur at 2018-10-25T16:49:45Z

  _casbaseddirectory.py: Remove some unnecessary things, label others
  

• a82e6d95
  by Jim MacArthur at 2018-10-25T17:02:12Z

  casbaseddirectory: Combine all the _resolve functions
  

• 711c9847
  by Jim MacArthur at 2018-10-26T13:11:37Z

  Rearrange comment
  

2 changed files:

• buildstream/storage/_casbaseddirectory.py
• tests/storage/virtual_directory_import.py

Changes:

buildstream/storage/_casbaseddirectory.py

---

...	...	@@ -244,10 +244,8 @@ class CasBasedDirectory(Directory):
244	244	for collection in [self.pb2_directory.files, self.pb2_directory.symlinks, self.pb2_directory.directories]:
245	245	for thing in collection:
246	246	if thing.name == name:
247		- print("Removing {} from PB2".format(name))
248	247	collection.remove(thing)
249	248	if name in self.index:
250		- print("Removing {} from index".format(name))
251	249	del self.index[name]
252	250
253	251	def descend(self, subdirectory_spec, create=False):
...	...	@@ -330,26 +328,7 @@ class CasBasedDirectory(Directory):
330	328	as a directory as long as it's within this directory tree.
331	329	"""
332	330
333		- if isinstance(self.index[name].buildstream_object, Directory):
334		- return self.index[name].buildstream_object
335		- # OK then, it's a symlink
336		- symlink = self._find_pb2_entry(name)
337		- assert isinstance(symlink, remote_execution_pb2.SymlinkNode)
338		- absolute = symlink.target.startswith(CasBasedDirectory._pb2_absolute_path_prefix)
339		- if absolute:
340		- root = self.find_root()
341		- else:
342		- root = self
343		- directory = root
344		- components = symlink.target.split(CasBasedDirectory._pb2_path_sep)
345		- for c in components:
346		- if c == ".":
347		- pass
348		- elif c == "..":
349		- directory = directory.parent
350		- else:
351		- directory = directory.descend(c, create=True)
352		- return directory
	331	+ return self._resolve(name, force_create=True)
353	332
354	333	def _is_followable(self, name):
355	334	""" Returns true if this is a directory or symlink to a valid directory. """
...	...	@@ -364,35 +343,16 @@ class CasBasedDirectory(Directory):
364	343	def _resolve_symlink(self, node, force_create=True):
365	344	"""Same as _resolve_symlink_or_directory but takes a SymlinkNode.
366	345	"""
367		-
368		- # OK then, it's a symlink
369		- symlink = node
370		- absolute = symlink.target.startswith(CasBasedDirectory._pb2_absolute_path_prefix)
371		- if absolute:
372		- root = self.find_root()
373		- else:
374		- root = self
375		- directory = root
376		- components = symlink.target.split(CasBasedDirectory._pb2_path_sep)
377		- for c in components:
378		- if c == ".":
379		- pass
380		- elif c == "..":
381		- directory = directory.parent
382		- else:
383		- if c in directory.index or force_create:
384		- directory = directory.descend(c, create=True)
385		- else:
386		- return None
387		- return directory
388		-
	346	+ return self._resolve(node.name, force_create=True)
389	347
390	348	def _resolve(self, name, absolute_symlinks_resolve=True, force_create=False, first_seen_object = None):
391	349	""" Resolves any name to an object. If the name points to a symlink in
392	350	this directory, it returns the thing it points to,
393	351	recursively. Returns a CasBasedDirectory, FileNode or
394		- None. Never creates a directory or otherwise alters the
395		- directory.
	352	+ None.
	353	+
	354	+ If force_create is on, will attempt to create directories to make symlinks and directories resolve.
	355	+ If force_create is off, this will never alter this directory.
396	356
397	357	"""
398	358	# First check if it's a normal object and return that
...	...	@@ -439,27 +399,23 @@ class CasBasedDirectory(Directory):
439	399	if c == ".":
440	400	pass
441	401	elif c == "..":
442		- print(" resolving {}: up-dir".format(c))
	402	+ if directory.parent is not None:
	403	+ directory = directory.parent
443	404	# If directory.parent *is* None, this is an attempt to access
444	405	# '..' from the root, which is valid under POSIX; it just
445	406	# returns the root.
446		- if directory.parent is not None:
447		- directory = directory.parent
448	407	else:
449	408	if c in directory.index:
450	409	f = directory._resolve(c, absolute_symlinks_resolve, first_seen_object=first_seen_object)
451	410	# Ultimately f must now be a file or directory
452	411	if isinstance(f, CasBasedDirectory):
453	412	directory = f
454		- print(" resolving {}: dir".format(c))
455	413
456	414	else:
457	415	# This is a file or None (i.e. broken symlink)
458		- print(" resolving {}: file/broken link".format(c))
459	416	if f is None and force_create:
460		- print("Creating target of broken link {}".format(c))
461	417	directory = directory.descend(c, create=True)
462		- elif components:
	418	+ elif components and force_create:
463	419	# Oh dear. We have components left to resolve, but the one we're trying to resolve points to a file.
464	420	print("Trying to resolve {}, but found {} was a file.".format(symlink.target, c))
465	421	self.delete_entry(c)
...	...	@@ -468,9 +424,7 @@ class CasBasedDirectory(Directory):
468	424	else:
469	425	return f
470	426	else:
471		- print(" resolving {}: Non-existent file; must be from a broken symlink.".format(c))
472	427	if force_create:
473		- print("Creating target of broken link {} (2)".format(c))
474	428	directory = directory.descend(c, create=True)
475	429	else:
476	430	return None
...	...	@@ -534,9 +488,6 @@ class CasBasedDirectory(Directory):
534	488	""" Imports files from a traditional directory """
535	489	result = FileListResult()
536	490	for entry in files:
537		- print("Importing {} from file system".format(entry))
538		- print("...Order of elements was {}".format(", ".join(self.index.keys())))
539		-
540	491	split_path = entry.split(os.path.sep)
541	492	# The actual file on the FS we're importing
542	493	import_file = os.path.join(source_directory, entry)
...	...	@@ -547,7 +498,6 @@ class CasBasedDirectory(Directory):
547	498	# Hand this off to the importer for that subdir. This will only do one file -
548	499	# a better way would be to hand off all the files in this subdir at once.
549	500	# failed here because directory_name didn't point to a directory...
550		- print("Attempting to import into {} from {}".format(directory_name, source_directory))
551	501	subdir_result = self._import_directory_recursively(directory_name, source_directory,
552	502	split_path[1:], path_prefix)
553	503	result.combine(subdir_result)
...	...	@@ -563,8 +513,6 @@ class CasBasedDirectory(Directory):
563	513	if self._check_replacement(entry, path_prefix, result):
564	514	self._add_new_file(source_directory, entry)
565	515	result.files_written.append(relative_pathname)
566		- print("...Order of elements is now {}".format(", ".join(self.index.keys())))
567		-
568	516	return result
569	517
570	518
...	...	@@ -621,17 +569,6 @@ class CasBasedDirectory(Directory):
621	569	x = self._resolve_symlink(symlink_node, force_create=False)
622	570	return isinstance(x, CasBasedDirectory)
623	571
624		- def _verify_unique(self):
625		- # Verifies that there are no duplicate names in this directory or subdirectories.
626		- names = []
627		- for entrylist in [self.pb2_directory.files, self.pb2_directory.directories, self.pb2_directory.symlinks]:
628		- for e in entrylist:
629		- if e.name in names:
630		- raise VirtualDirectoryError("Duplicate entry for name {} found".format(e.name))
631		- names.append(e.name)
632		- for d in self.pb2_directory.directories:
633		- self.index[d.name].buildstream_object._verify_unique()
634		-
635	572	def _partial_import_cas_into_cas(self, source_directory, files, path_prefix="", file_list_required=True):
636	573	""" Import only the files and symlinks listed in 'files' from source_directory to this one.
637	574	Args:
...	...	@@ -640,11 +577,9 @@ class CasBasedDirectory(Directory):
640	577	path_prefix (str): Prefix used to add entries to the file list result.
641	578	file_list_required: Whether to update the file list while processing.
642	579	"""
643		- print("Beginning partial import of {} into {}. Files are: >{}<".format(source_directory, self, ", ".join(files)))
644	580	result = FileListResult()
645	581	processed_directories = set()
646	582	for f in files:
647		- #if f == ".": continue
648	583	fullname = os.path.join(path_prefix, f)
649	584	components = f.split(os.path.sep)
650	585	if len(components)>1:
...	...	@@ -664,12 +599,9 @@ class CasBasedDirectory(Directory):
664	599	else:
665	600	dest_subdir = x
666	601	else:
667		- print("Importing {}: {} does not exist in {}, so it is created as a directory".format(f, dirname, self))
668		-
669	602	self.create_directory(dirname)
670	603	dest_subdir = self._resolve_symlink_or_directory(dirname)
671	604	src_subdir = source_directory.descend(dirname)
672		- print("Now recursing into {} to continue adding {}".format(src_subdir, f))
673	605	import_result = dest_subdir._partial_import_cas_into_cas(src_subdir, subcomponents,
674	606	path_prefix=fullname, file_list_required=file_list_required)
675	607	result.combine(import_result)
...	...	@@ -693,23 +625,17 @@ class CasBasedDirectory(Directory):
693	625	else:
694	626	# We're importing a file or symlink - replace anything with the same name.
695	627	print("Import of file/symlink {} into this directory. Removing anything existing...".format(f))
696		- print(" ... ordering of nodes in this dir was: {}".format(self.index.keys()))
697		- print(" ... symlinks were {}".format([x.name for x in self.pb2_directory.symlinks]))
698	628	importable = self._check_replacement(f, path_prefix, result)
699	629	if importable:
700	630	print(" ... after replacement of '{}', symlinks are now {}".format(f, [x.name for x in self.pb2_directory.symlinks]))
701	631	item = source_directory.index[f].pb_object
702	632	if isinstance(item, remote_execution_pb2.FileNode):
703		- print(" ... importing file")
704	633	filenode = self.pb2_directory.files.add(digest=item.digest, name=f,
705	634	is_executable=item.is_executable)
706	635	self.index[f] = IndexEntry(filenode, modified=(fullname in result.overwritten))
707	636	else:
708		- print(" ... importing symlink")
709	637	assert(isinstance(item, remote_execution_pb2.SymlinkNode))
710	638	self._add_new_link_direct(name=f, target=item.target)
711		- print(" ... symlinks are now {}".format([x.name for x in self.pb2_directory.symlinks]))
712		- print(" ... ordering of nodes in this dir is now: {}".format(self.index.keys()))
713	639	return result
714	640
715	641	def transfer_node_contents(destination, source):
...	...	@@ -754,12 +680,12 @@ class CasBasedDirectory(Directory):
754	680	replace one directory with another's hash, without doing any recursion.
755	681	"""
756	682	if files is None:
757		- #return self._full_import_cas_into_cas(source_directory, can_hardlink=True)
758		- files = list(source_directory.list_relative_paths())
759		- print("Extracted all files from source directory '{}': {}".format(source_directory, files))
	683	+ files = source_directory.list_relative_paths()
	684	+ # You must pass a list into _partial_import (not a generator)
760	685	return self._partial_import_cas_into_cas(source_directory, list(files))
761	686
762	687	def _describe(self, thing):
	688	+ """ Only used by showdiff, and as such, not called """
763	689	# Describes protocol buffer objects
764	690	if isinstance(thing, remote_execution_pb2.DirectoryNode):
765	691	return "directory called {}".format(thing.name)
...	...	@@ -770,10 +696,8 @@ class CasBasedDirectory(Directory):
770	696	else:
771	697	return "strange thing"
772	698
773		-
774	699	def showdiff(self, other):
775		- print("Diffing {} and {}:".format(self, other))
776		-
	700	+ """ An old function used to show differences between two directories. No longer in use. """
777	701	def compare_list(l1, l2, name):
778	702	item2 = None
779	703	index = 0
...	...	@@ -848,39 +772,10 @@ class CasBasedDirectory(Directory):
848	772
849	773	print("Directory before import: {}".format(self.show_files_recursive()))
850	774
851		- # Sync self
852		- self._recalculate_recursing_down()
853		- if self.parent:
854		- self.parent._recalculate_recursing_up(self)
855		-
856		- duplicate_test = False
857		-
858		- print("Original CAS before CAS-based import: {}".format(self.show_files_recursive()))
859		- print("Original CAS hash: {}".format(self.ref.hash))
860		- duplicate_cas = None
861		- self._verify_unique()
862	775	if isinstance(external_pathspec, CasBasedDirectory):
863		- if duplicate_test:
864		- duplicate_cas = CasBasedDirectory(self.context, ref=copy.copy(self.ref))
865		- duplicate_cas._verify_unique()
866		- print("Duplicated CAS before file-based import: {}".format(duplicate_cas.show_files_recursive()))
867		- print("Duplicate CAS hash: {}".format(duplicate_cas.ref.hash))
868	776	print("-"*80 + "Performing direct CAS-to-CAS import")
869	777	result = self._import_cas_into_cas(external_pathspec, files=files)
870		- self._verify_unique()
871	778	print("Result of cas-to-cas import: {}".format(self.show_files_recursive()))
872		- print("-"*80 + "Performing round-trip import via file system")
873		- if duplicate_test:
874		- with tempfile.TemporaryDirectory(prefix="roundtrip") as tmpdir:
875		- external_pathspec.export_files(tmpdir)
876		- if files is None:
877		- files = list(list_relative_paths(tmpdir))
878		- print("Importing from filesystem: filelist is: {}".format(files))
879		- duplicate_cas._import_files_from_directory(tmpdir, files=files)
880		- duplicate_cas._recalculate_recursing_down()
881		- if duplicate_cas.parent:
882		- duplicate_cas.parent._recalculate_recursing_up(duplicate_cas)
883		- print("Result of direct import: {}".format(duplicate_cas.show_files_recursive()))
884	779	else:
885	780	print("-"*80 + "Performing initial import")
886	781	if isinstance(external_pathspec, FileBasedDirectory):
...	...	@@ -904,10 +799,6 @@ class CasBasedDirectory(Directory):
904	799	self._recalculate_recursing_down()
905	800	if self.parent:
906	801	self.parent._recalculate_recursing_up(self)
907		- if duplicate_cas:
908		- if duplicate_cas.ref.hash != self.ref.hash:
909		- self.showdiff(duplicate_cas)
910		- raise VirtualDirectoryError("Mismatch between file-imported result {} and cas-to-cas imported result {}.".format(duplicate_cas.ref.hash,self.ref.hash))
911	802
912	803	return result
913	804

tests/storage/virtual_directory_import.py

---

1	1	import os
2	2	import pytest
3	3	import random
	4	+import copy
	5	+import tempfile
4	6	from tests.testutils import cli
5	7
	8	+
6	9	from buildstream.storage import CasBasedDirectory
7	10	from buildstream.storage import FileBasedDirectory
8		-
	11	+from buildstream import utils
9	12
10	13	class FakeContext():
11	14	def __init__(self):
...	...	@@ -84,7 +87,6 @@ def generate_random_root(rootno, directory):
84	87	os.symlink(symlink_destination, target)
85	88	description = "symlink pointing to {}".format(symlink_destination)
86	89	things.append(os.path.join(location, thingname))
87		- print("Generated {}/{}, a {}".format(rootdir, things[-1], description))
88	90
89	91
90	92	def file_contents(path):
...	...	@@ -160,15 +162,24 @@ def _import_test(tmpdir, original, overlay, generator_function, verify_contents=
160	162	generator_function(overlay, tmpdir)
161	163
162	164	d = create_new_casdir(original, fake_context, tmpdir)
	165	+
	166	+ #duplicate_cas = CasBasedDirectory(fake_context, ref=copy.copy(d.ref))
	167	+ duplicate_cas = create_new_casdir(original, fake_context, tmpdir)
	168	+
	169	+ assert duplicate_cas.ref.hash == d.ref.hash
	170	+
163	171	d2 = create_new_casdir(overlay, fake_context, tmpdir)
164	172	print("Importing dir {} into {}".format(overlay, original))
165	173	d.import_files(d2)
166		- d.export_files(os.path.join(tmpdir, "output"))
	174	+ export_dir = os.path.join(tmpdir, "output")
	175	+ roundtrip_dir = os.path.join(tmpdir, "roundtrip")
	176	+ d2.export_files(roundtrip_dir)
	177	+ d.export_files(export_dir)
167	178
168	179	if verify_contents:
169	180	for item in root_filesets[overlay - 1]:
170	181	(path, typename, content) = item
171		- realpath = resolve_symlinks(path, os.path.join(tmpdir, "output"))
	182	+ realpath = resolve_symlinks(path, export_dir)
172	183	if typename == 'F':
173	184	if os.path.isdir(realpath) and directory_not_empty(realpath):
174	185	# The file should not have overwritten the directory in this case.
...	...	@@ -189,10 +200,21 @@ def _import_test(tmpdir, original, overlay, generator_function, verify_contents=
189	200	assert os.path.lexists(realpath)
190	201
191	202	# Now do the same thing with filebaseddirectories and check the contents match
192		- d3 = create_new_casdir(original, fake_context, tmpdir)
193		- d4 = create_new_filedir(overlay, tmpdir)
194		- d3.import_files(d2)
195		- assert d.ref.hash == d3.ref.hash
	203	+
	204	+ files = list(utils.list_relative_paths(roundtrip_dir))
	205	+ print("Importing from filesystem: filelist is: {}".format(files))
	206	+ duplicate_cas._import_files_from_directory(roundtrip_dir, files=files)
	207	+ duplicate_cas._recalculate_recursing_down()
	208	+ if duplicate_cas.parent:
	209	+ duplicate_cas.parent._recalculate_recursing_up(duplicate_cas)
	210	+ print("Result of direct import: {}".format(duplicate_cas.show_files_recursive()))
	211	+
	212	+ assert duplicate_cas.ref.hash == d.ref.hash
	213	+
	214	+ #d3 = create_new_casdir(original, fake_context, tmpdir)
	215	+ #d4 = create_new_filedir(overlay, tmpdir)
	216	+ #d3.import_files(d2)
	217	+ #assert d.ref.hash == d3.ref.hash
196	218
197	219	@pytest.mark.parametrize("original,overlay", combinations(range(1,len(root_filesets)+1)))
198	220	def test_fixed_cas_import(cli, tmpdir, original, overlay):

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