[Notes] [Git][BuildStream/buildstream][valentindavid/deterministic-source] 5 commits: buildstream/plugins/sources/local.py: Make staging deterministic.

[Valentin David <gitlab mg gitlab com>]

Title: GitLab

Valentin David pushed to branch valentindavid/deterministic-source at BuildStream / buildstream

Commits:

• 32d571e6
  by Valentin David at 2018-08-10T08:53:20Z

  buildstream/plugins/sources/local.py: Make staging deterministic.
  
  Instead of copying metadata on files staged by local, we manually set
  mode to 0755 or 0644 depending on whether user execution was enabled
  on source file.
  
  This makes file modes deterministic independently on the way source
  was distributed.
  
  Non-deterministic mode copying all metadata can still be enabled by
  disable 'deterministic' Boolean configuration on the plugin.
  
  Fixes #527.
  

• c5362746
  by Valentin David at 2018-08-10T08:53:20Z

  Use deterministic umask when staging sources.
  
  This fix is applied to plugins bzr, git, patch.
  
  Fixes #543 #544 #555.
  

• f52110f7
  by Valentin David at 2018-08-10T08:53:20Z

  buildstream/plugins/sources/zip.py: Fix non-determism in staging.
  
  Staging could end-up with file with different rights depending on the
  umask. The extracted files need to get their access rights fixed.
  

• 118f2798
  by Valentin David at 2018-08-10T08:53:20Z

  buildstream/plugins/sources/remote.py: Make staging deterministic.
  

• 11843cc7
  by Valentin David at 2018-08-10T08:53:20Z

  Add some integration tests for source plugin determinism.
  

8 changed files:

• buildstream/plugins/sources/bzr.py
• buildstream/plugins/sources/git.py
• buildstream/plugins/sources/local.py
• buildstream/plugins/sources/patch.py
• buildstream/plugins/sources/remote.py
• buildstream/plugins/sources/zip.py
• buildstream/utils.py
• + tests/integration/source-determinism.py

Changes:

buildstream/plugins/sources/bzr.py

---

...	...	@@ -123,7 +123,8 @@ class BzrSource(Source):
123	123	"--revision=revno:{}".format(self.ref),
124	124	self._get_branch_dir(), directory],
125	125	fail="Failed to checkout revision {} from branch {} to {}"
126		- .format(self.ref, self._get_branch_dir(), directory))
	126	+ .format(self.ref, self._get_branch_dir(), directory),
	127	+ deterministic_umask=True)
127	128	# Remove .bzr dir
128	129	shutil.rmtree(os.path.join(directory, ".bzr"))
129	130

buildstream/plugins/sources/git.py

---

...	...	@@ -198,11 +198,13 @@ class GitMirror(SourceFetcher):
198	198	# directory.
199	199	self.source.call([self.source.host_git, 'clone', '--no-checkout', '--shared', self.mirror, fullpath],
200	200	fail="Failed to create git mirror {} in directory: {}".format(self.mirror, fullpath),
201		- fail_temporarily=True)
	201	+ fail_temporarily=True,
	202	+ deterministic_umask=True)
202	203
203	204	self.source.call([self.source.host_git, 'checkout', '--force', self.ref],
204	205	fail="Failed to checkout git ref {}".format(self.ref),
205		- cwd=fullpath)
	206	+ cwd=fullpath,
	207	+ deterministic_umask=True)
206	208
207	209	# Remove .git dir
208	210	shutil.rmtree(os.path.join(fullpath, ".git"))

buildstream/plugins/sources/local.py

---

...	...	@@ -37,6 +37,7 @@ local - stage local files and directories
37	37	"""
38	38
39	39	import os
	40	+import stat
40	41	from buildstream import Source, Consistency
41	42	from buildstream import utils
42	43
...	...	@@ -94,12 +95,35 @@ class LocalSource(Source):
94	95	# Dont use hardlinks to stage sources, they are not write protected
95	96	# in the sandbox.
96	97	with self.timed_activity("Staging local files at {}".format(self.path)):
	98	+
97	99	if os.path.isdir(self.fullpath):
98		- utils.copy_files(self.fullpath, directory)
	100	+ files = list(utils.list_relative_paths(self.fullpath, list_dirs=True))
	101	+ utils.copy_files(self.fullpath, directory, files=files)
99	102	else:
100	103	destfile = os.path.join(directory, os.path.basename(self.path))
	104	+ files = [os.path.basename(self.path)]
101	105	utils.safe_copy(self.fullpath, destfile)
102	106
	107	+ for f in files:
	108	+ # Non empty directories are not listed by list_relative_paths
	109	+ dirs = f.split(os.sep)
	110	+ for i in range(1, len(dirs)):
	111	+ d = os.path.join(directory, *(dirs[:i]))
	112	+ assert os.path.isdir(d) and not os.path.islink(d)
	113	+ os.chmod(d, stat.S_IRWXU | stat.S_IRGRP | stat.S_IXGRP | stat.S_IROTH | stat.S_IXOTH)
	114	+
	115	+ path = os.path.join(directory, f)
	116	+ if os.path.islink(path):
	117	+ pass
	118	+ elif os.path.isdir(path):
	119	+ os.chmod(path, stat.S_IRWXU | stat.S_IRGRP | stat.S_IXGRP | stat.S_IROTH | stat.S_IXOTH)
	120	+ else:
	121	+ st = os.stat(path)
	122	+ if st.st_mode & stat.S_IXUSR:
	123	+ os.chmod(path, stat.S_IRWXU | stat.S_IRGRP | stat.S_IXGRP | stat.S_IROTH | stat.S_IXOTH)
	124	+ else:
	125	+ os.chmod(path, stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH)
	126	+
103	127
104	128	# Create a unique key for a file
105	129	def unique_key(filename):

buildstream/plugins/sources/patch.py

---

...	...	@@ -90,9 +90,10 @@ class PatchSource(Source):
90	90	raise SourceError("Nothing to patch in directory '{}'".format(directory),
91	91	reason="patch-no-files")
92	92
93		- strip_level_option = "-p{}".format(self.strip_level)
94		- self.call([self.host_patch, strip_level_option, "-i", self.fullpath, "-d", directory],
95		- fail="Failed to apply patch {}".format(self.path))
	93	+ strip_level_option = "-p{}".format(self.strip_level)
	94	+ self.call([self.host_patch, strip_level_option, "-i", self.fullpath, "-d", directory],
	95	+ fail="Failed to apply patch {}".format(self.path),
	96	+ deterministic_umask=True)
96	97
97	98
98	99	# Plugin entry point

buildstream/plugins/sources/remote.py

---

...	...	@@ -49,6 +49,7 @@ remote - stage files from remote urls
49	49
50	50	"""
51	51	import os
	52	+import stat
52	53	from buildstream import SourceError, utils
53	54	from ._downloadablefilesource import DownloadableFileSource
54	55
...	...	@@ -75,6 +76,7 @@ class RemoteSource(DownloadableFileSource):
75	76	dest = os.path.join(directory, self.filename)
76	77	with self.timed_activity("Staging remote file to {}".format(dest)):
77	78	utils.safe_copy(self._get_mirror_file(), dest)
	79	+ os.chmod(dest, stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH)
78	80
79	81
80	82	def setup():

buildstream/plugins/sources/zip.py

---

...	...	@@ -53,6 +53,7 @@ zip - stage files from zip archives
53	53
54	54	import os
55	55	import zipfile
	56	+import stat
56	57
57	58	from buildstream import SourceError
58	59	from buildstream import utils
...	...	@@ -74,6 +75,9 @@ class ZipSource(DownloadableFileSource):
74	75	return super().get_unique_key() + [self.base_dir]
75	76
76	77	def stage(self, directory):
	78	+ exec_rights = (stat.S_IRWXU | stat.S_IRWXG | stat.S_IRWXO) & ~(stat.S_IWGRP | stat.S_IWOTH)
	79	+ noexec_rights = exec_rights & ~(stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH)
	80	+
77	81	try:
78	82	with zipfile.ZipFile(self._get_mirror_file()) as archive:
79	83	base_dir = None
...	...	@@ -81,9 +85,27 @@ class ZipSource(DownloadableFileSource):
81	85	base_dir = self._find_base_dir(archive, self.base_dir)
82	86
83	87	if base_dir:
84		- archive.extractall(path=directory, members=self._extract_members(archive, base_dir))
	88	+ members = self._extract_members(archive, base_dir)
85	89	else:
86		- archive.extractall(path=directory)
	90	+ members = archive.namelist()
	91	+
	92	+ for member in members:
	93	+ written = archive.extract(member, path=directory)
	94	+
	95	+ # zipfile.extract might create missing directories
	96	+ rel = os.path.relpath(written, start=directory)
	97	+ assert not os.path.isabs(rel)
	98	+ rel = os.path.dirname(rel)
	99	+ while rel:
	100	+ os.chmod(os.path.join(directory, rel), exec_rights)
	101	+ rel = os.path.dirname(rel)
	102	+
	103	+ if os.path.islink(written):
	104	+ pass
	105	+ elif os.path.isdir(written):
	106	+ os.chmod(written, exec_rights)
	107	+ else:
	108	+ os.chmod(written, noexec_rights)
87	109
88	110	except (zipfile.BadZipFile, zipfile.LargeZipFile, OSError) as e:
89	111	raise SourceError("{}: Error staging source: {}".format(self, e)) from e

buildstream/utils.py

---

...	...	@@ -998,18 +998,29 @@ def _kill_process_tree(pid):
998	998	# Args:
999	999	# popenargs (list): Popen() arguments
1000	1000	# terminate (bool): Whether to attempt graceful termination before killing
	1001	+# deterministic_umask (bool): Whether to set umask to 0o022
1001	1002	# rest_of_args (kwargs): Remaining arguments to subprocess.call()
1002	1003	#
1003	1004	# Returns:
1004	1005	# (int): The process exit code.
1005	1006	# (str): The program output.
1006	1007	#
1007		-def _call(*popenargs, terminate=False, **kwargs):
	1008	+def _call(*popenargs, terminate=False, deterministic_umask=False, **kwargs):
1008	1009
1009	1010	kwargs['start_new_session'] = True
1010	1011
1011	1012	process = None
1012	1013
	1014	+ if deterministic_umask:
	1015	+ old_preexec_fn = kwargs.get('preexec_fn')
	1016	+
	1017	+ def preexec_fn():
	1018	+ os.umask(stat.S_IWGRP | stat.S_IWOTH)
	1019	+ if old_preexec_fn is not None:
	1020	+ old_preexec_fn()
	1021	+
	1022	+ kwargs['preexec_fn'] = preexec_fn
	1023	+
1013	1024	# Handle termination, suspend and resume
1014	1025	def kill_proc():
1015	1026	if process:

tests/integration/source-determinism.py

---

	1	+import os
	2	+import pytest
	3	+
	4	+from buildstream import _yaml, utils
	5	+from tests.testutils import cli, create_repo, ALL_REPO_KINDS
	6	+
	7	+
	8	+DATA_DIR = os.path.join(
	9	+ os.path.dirname(os.path.realpath(__file__)),
	10	+ "project"
	11	+)
	12	+
	13	+
	14	+def create_test_file(*path, mode=0o644, content='content\n'):
	15	+ path = os.path.join(*path)
	16	+ os.makedirs(os.path.dirname(path), exist_ok=True)
	17	+ with open(path, 'w') as f:
	18	+ f.write(content)
	19	+ os.fchmod(f.fileno(), mode)
	20	+
	21	+
	22	+def create_test_directory(*path, mode=0o644):
	23	+ create_test_file(*path, '.keep', content='')
	24	+ path = os.path.join(*path)
	25	+ os.chmod(path, mode)
	26	+
	27	+
	28	+@pytest.mark.integration
	29	+@pytest.mark.datafiles(DATA_DIR)
	30	+@pytest.mark.parametrize("kind", [(kind) for kind in ALL_REPO_KINDS] + ['local'])
	31	+def test_deterministic_source_umask(cli, tmpdir, datafiles, kind):
	32	+ project = str(datafiles)
	33	+ element_name = 'list'
	34	+ element_path = os.path.join(project, 'elements', element_name)
	35	+ repodir = os.path.join(str(tmpdir), 'repo')
	36	+ sourcedir = os.path.join(project, 'source')
	37	+
	38	+ create_test_file(sourcedir, 'a.txt', mode=0o700)
	39	+ create_test_file(sourcedir, 'b.txt', mode=0o755)
	40	+ create_test_file(sourcedir, 'c.txt', mode=0o600)
	41	+ create_test_file(sourcedir, 'd.txt', mode=0o400)
	42	+ create_test_file(sourcedir, 'e.txt', mode=0o644)
	43	+ create_test_file(sourcedir, 'f.txt', mode=0o4755)
	44	+ create_test_file(sourcedir, 'g.txt', mode=0o2755)
	45	+ create_test_file(sourcedir, 'h.txt', mode=0o1755)
	46	+ create_test_directory(sourcedir, 'dir-a', mode=0o0700)
	47	+ create_test_directory(sourcedir, 'dir-c', mode=0o0755)
	48	+ create_test_directory(sourcedir, 'dir-d', mode=0o4755)
	49	+ create_test_directory(sourcedir, 'dir-e', mode=0o2755)
	50	+ create_test_directory(sourcedir, 'dir-f', mode=0o1755)
	51	+
	52	+ if kind == 'local':
	53	+ source = {'kind': 'local',
	54	+ 'path': 'source'}
	55	+ else:
	56	+ repo = create_repo(kind, repodir)
	57	+ ref = repo.create(sourcedir)
	58	+ source = repo.source_config(ref=ref)
	59	+ element = {
	60	+ 'kind': 'manual',
	61	+ 'depends': [
	62	+ {
	63	+ 'filename': 'base.bst',
	64	+ 'type': 'build'
	65	+ }
	66	+ ],
	67	+ 'sources': [
	68	+ source
	69	+ ],
	70	+ 'config': {
	71	+ 'install-commands': [
	72	+ 'ls -l >"%{install-root}/ls-l"'
	73	+ ]
	74	+ }
	75	+ }
	76	+ _yaml.dump(element, element_path)
	77	+
	78	+ def get_value_for_umask(umask):
	79	+ checkoutdir = os.path.join(str(tmpdir), 'checkout-{}'.format(umask))
	80	+
	81	+ old_umask = os.umask(umask)
	82	+
	83	+ try:
	84	+ result = cli.run(project=project, args=['build', element_name])
	85	+ result.assert_success()
	86	+
	87	+ result = cli.run(project=project, args=['checkout', element_name, checkoutdir])
	88	+ result.assert_success()
	89	+
	90	+ with open(os.path.join(checkoutdir, 'ls-l'), 'r') as f:
	91	+ return f.read()
	92	+ finally:
	93	+ os.umask(old_umask)
	94	+ cli.remove_artifact_from_cache(project, element_name)
	95	+
	96	+ assert get_value_for_umask(0o022) == get_value_for_umask(0o077)
	97	+
	98	+
	99	+@pytest.mark.integration
	100	+@pytest.mark.datafiles(DATA_DIR)
	101	+def test_deterministic_source_local(cli, tmpdir, datafiles):
	102	+ """Only user rights should be considered for local source.
	103	+ """
	104	+ project = str(datafiles)
	105	+ element_name = 'test'
	106	+ element_path = os.path.join(project, 'elements', element_name)
	107	+ sourcedir = os.path.join(project, 'source')
	108	+
	109	+ element = {
	110	+ 'kind': 'manual',
	111	+ 'depends': [
	112	+ {
	113	+ 'filename': 'base.bst',
	114	+ 'type': 'build'
	115	+ }
	116	+ ],
	117	+ 'sources': [
	118	+ {
	119	+ 'kind': 'local',
	120	+ 'path': 'source'
	121	+ }
	122	+ ],
	123	+ 'config': {
	124	+ 'install-commands': [
	125	+ 'ls -l >"%{install-root}/ls-l"'
	126	+ ]
	127	+ }
	128	+ }
	129	+ _yaml.dump(element, element_path)
	130	+
	131	+ def get_value_for_mask(mask):
	132	+ checkoutdir = os.path.join(str(tmpdir), 'checkout-{}'.format(mask))
	133	+
	134	+ create_test_file(sourcedir, 'a.txt', mode=0o644 & mask)
	135	+ create_test_file(sourcedir, 'b.txt', mode=0o755 & mask)
	136	+ create_test_file(sourcedir, 'c.txt', mode=0o4755 & mask)
	137	+ create_test_file(sourcedir, 'd.txt', mode=0o2755 & mask)
	138	+ create_test_file(sourcedir, 'e.txt', mode=0o1755 & mask)
	139	+ create_test_directory(sourcedir, 'dir-a', mode=0o0755 & mask)
	140	+ create_test_directory(sourcedir, 'dir-b', mode=0o4755 & mask)
	141	+ create_test_directory(sourcedir, 'dir-c', mode=0o2755 & mask)
	142	+ create_test_directory(sourcedir, 'dir-d', mode=0o1755 & mask)
	143	+ try:
	144	+ result = cli.run(project=project, args=['build', element_name])
	145	+ result.assert_success()
	146	+
	147	+ result = cli.run(project=project, args=['checkout', element_name, checkoutdir])
	148	+ result.assert_success()
	149	+
	150	+ with open(os.path.join(checkoutdir, 'ls-l'), 'r') as f:
	151	+ return f.read()
	152	+ finally:
	153	+ cli.remove_artifact_from_cache(project, element_name)
	154	+
	155	+ assert get_value_for_mask(0o7777) == get_value_for_mask(0o0700)

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