[Notes] [Git][BuildStream/buildstream][valentindavid/pull-chmod-bug] 5 commits: loader.py: Stage junctions into .bst instead of a tmpdir

[Valentin David <gitlab mg gitlab com>]

Title: GitLab

Valentin David pushed to branch valentindavid/pull-chmod-bug at BuildStream / buildstream

Commits:

• 352f4ad9
  by Jonathan Maw at 2019-02-12T16:44:35Z

  loader.py: Stage junctions into .bst instead of a tmpdir
  

• 62396af9
  by Jonathan Maw at 2019-02-12T16:44:35Z

  loader.py: Load junction from workspace if one's open
  

• f1e9cb66
  by Jonathan Maw at 2019-02-12T16:44:35Z

  Remove cleanup functionality from loader
  
  Since junction check-outs are now persistent, there is no need to clean
  up temporary directories once finished.
  Now, junction checkouts are stored within the top-level project dir and
  will be cleaned-up when that directory is removed.
  
  This involves changes in:
  * loader.py: Remove cleanup logic and passing-around of tmpdir.
  * _project.py: Remove passing around tmpdir and calling loader's
                 cleanup.
  

• e51116d5
  by Jonathan Maw at 2019-02-12T17:59:25Z

  Merge branch 'jonathan/junction-no-tmpdir' into 'master'
  
  Stage junctions into .bst instead of a tmpdir
  
  Closes #895
  
  See merge request BuildStream/buildstream!1134

• 478e5c47
  by Valentin David at 2019-02-12T18:06:50Z

  buildstream/_cas/cascache.py: Set 0644 rights to pulled files
  
  This was broken by 5ef19a0b31df84caed1e41719ef7ea5c6bd8a8bc.
  

4 changed files:

• buildstream/_cas/cascache.py
• buildstream/_loader/loader.py
• buildstream/_project.py
• tests/frontend/pull.py

Changes:

buildstream/_cas/cascache.py

---

...	...	@@ -376,9 +376,7 @@ class CASCache():
376	376	for chunk in iter(lambda: tmp.read(_BUFFER_SIZE), b""):
377	377	h.update(chunk)
378	378	else:
379		- tmp = stack.enter_context(utils._tempnamedfile(dir=self.tmpdir))
380		- # Set mode bits to 0644
381		- os.chmod(tmp.name, stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH)
	379	+ tmp = stack.enter_context(self._temporary_object())
382	380
383	381	if path:
384	382	with open(path, 'rb') as f:
...	...	@@ -827,6 +825,19 @@ class CASCache():
827	825	for dirnode in directory.directories:
828	826	yield from self._required_blobs(dirnode.digest)
829	827
	828	+ # _temporary_object():
	829	+ #
	830	+ # Returns:
	831	+ # (file): A file object to a named temporary file.
	832	+ #
	833	+ # Create a named temporary file with 0o0644 access rights.
	834	+ @contextlib.contextmanager
	835	+ def _temporary_object(self):
	836	+ with utils._tempnamedfile(dir=self.tmpdir) as f:
	837	+ os.chmod(f.name,
	838	+ stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH)
	839	+ yield f
	840	+
830	841	# _ensure_blob():
831	842	#
832	843	# Fetch and add blob if it's not already local.
...	...	@@ -844,7 +855,7 @@ class CASCache():
844	855	# already in local repository
845	856	return objpath
846	857
847		- with utils._tempnamedfile(dir=self.tmpdir) as f:
	858	+ with self._temporary_object() as f:
848	859	remote._fetch_blob(digest, f)
849	860
850	861	added_digest = self.add_object(path=f.name, link_directly=True)
...	...	@@ -854,7 +865,7 @@ class CASCache():
854	865
855	866	def _batch_download_complete(self, batch):
856	867	for digest, data in batch.send():
857		- with utils._tempnamedfile(dir=self.tmpdir) as f:
	868	+ with self._temporary_object() as f:
858	869	f.write(data)
859	870	f.flush()
860	871

buildstream/_loader/loader.py

---

...	...	@@ -20,8 +20,6 @@
20	20	import os
21	21	from functools import cmp_to_key
22	22	from collections.abc import Mapping
23		-import tempfile
24		-import shutil
25	23
26	24	from .._exceptions import LoadError, LoadErrorReason
27	25	from .. import Consistency
...	...	@@ -49,12 +47,10 @@ from .._message import Message, MessageType
49	47	# context (Context): The Context object
50	48	# project (Project): The toplevel Project object
51	49	# parent (Loader): A parent Loader object, in the case this is a junctioned Loader
52		-# tempdir (str): A directory to cleanup with the Loader, given to the loader by a parent
53		-# loader in the case that this loader is a subproject loader.
54	50	#
55	51	class Loader():
56	52
57		- def __init__(self, context, project, *, parent=None, tempdir=None):
	53	+ def __init__(self, context, project, *, parent=None):
58	54
59	55	# Ensure we have an absolute path for the base directory
60	56	basedir = project.element_path
...	...	@@ -73,7 +69,6 @@ class Loader():
73	69	self._options = project.options # Project options (OptionPool)
74	70	self._basedir = basedir # Base project directory
75	71	self._first_pass_options = project.first_pass_config.options # Project options (OptionPool)
76		- self._tempdir = tempdir # A directory to cleanup
77	72	self._parent = parent # The parent loader
78	73
79	74	self._meta_elements = {} # Dict of resolved meta elements by name
...	...	@@ -159,30 +154,6 @@ class Loader():
159	154
160	155	return ret
161	156
162		- # cleanup():
163		- #
164		- # Remove temporary checkout directories of subprojects
165		- #
166		- def cleanup(self):
167		- if self._parent and not self._tempdir:
168		- # already done
169		- return
170		-
171		- # recurse
172		- for loader in self._loaders.values():
173		- # value may be None with nested junctions without overrides
174		- if loader is not None:
175		- loader.cleanup()
176		-
177		- if not self._parent:
178		- # basedir of top-level loader is never a temporary directory
179		- return
180		-
181		- # safe guard to not accidentally delete directories outside builddir
182		- if self._tempdir.startswith(self._context.builddir + os.sep):
183		- if os.path.exists(self._tempdir):
184		- shutil.rmtree(self._tempdir)
185		-
186	157	###########################################
187	158	# Private Methods #
188	159	###########################################
...	...	@@ -540,23 +511,28 @@ class Loader():
540	511	"Subproject has no ref for junction: {}".format(filename),
541	512	detail=detail)
542	513
543		- if len(sources) == 1 and sources[0]._get_local_path():
	514	+ workspace = element._get_workspace()
	515	+ if workspace:
	516	+ # If a workspace is open, load it from there instead
	517	+ basedir = workspace.get_absolute_path()
	518	+ elif len(sources) == 1 and sources[0]._get_local_path():
544	519	# Optimization for junctions with a single local source
545	520	basedir = sources[0]._get_local_path()
546		- tempdir = None
547	521	else:
548	522	# Stage sources
549		- os.makedirs(self._context.builddir, exist_ok=True)
550		- basedir = tempfile.mkdtemp(prefix="{}-".format(element.normal_name), dir=self._context.builddir)
551		- element._stage_sources_at(basedir, mount_workspaces=False)
552		- tempdir = basedir
	523	+ element._update_state()
	524	+ basedir = os.path.join(self.project.directory, ".bst", "staged-junctions",
	525	+ filename, element._get_cache_key())
	526	+ if not os.path.exists(basedir):
	527	+ os.makedirs(basedir, exist_ok=True)
	528	+ element._stage_sources_at(basedir, mount_workspaces=False)
553	529
554	530	# Load the project
555	531	project_dir = os.path.join(basedir, element.path)
556	532	try:
557	533	from .._project import Project
558	534	project = Project(project_dir, self._context, junction=element,
559		- parent_loader=self, tempdir=tempdir)
	535	+ parent_loader=self)
560	536	except LoadError as e:
561	537	if e.reason == LoadErrorReason.MISSING_PROJECT_CONF:
562	538	raise LoadError(reason=LoadErrorReason.INVALID_JUNCTION,

buildstream/_project.py

---

...	...	@@ -91,7 +91,7 @@ class ProjectConfig:
91	91	class Project():
92	92
93	93	def __init__(self, directory, context, *, junction=None, cli_options=None,
94		- default_mirror=None, parent_loader=None, tempdir=None):
	94	+ default_mirror=None, parent_loader=None):
95	95
96	96	# The project name
97	97	self.name = None
...	...	@@ -147,7 +147,7 @@ class Project():
147	147	self._project_includes = None
148	148
149	149	profile_start(Topics.LOAD_PROJECT, self.directory.replace(os.sep, '-'))
150		- self._load(parent_loader=parent_loader, tempdir=tempdir)
	150	+ self._load(parent_loader=parent_loader)
151	151	profile_end(Topics.LOAD_PROJECT, self.directory.replace(os.sep, '-'))
152	152
153	153	self._partially_loaded = True
...	...	@@ -389,8 +389,6 @@ class Project():
389	389	# Cleans up resources used loading elements
390	390	#
391	391	def cleanup(self):
392		- self.loader.cleanup()
393		-
394	392	# Reset the element loader state
395	393	Element._reset_load_state()
396	394
...	...	@@ -439,7 +437,7 @@ class Project():
439	437	#
440	438	# Raises: LoadError if there was a problem with the project.conf
441	439	#
442		- def _load(self, parent_loader=None, tempdir=None):
	440	+ def _load(self, parent_loader=None):
443	441
444	442	# Load builtin default
445	443	projectfile = os.path.join(self.directory, _PROJECT_CONF_FILE)
...	...	@@ -505,8 +503,7 @@ class Project():
505	503	self._fatal_warnings = _yaml.node_get(pre_config_node, list, 'fatal-warnings', default_value=[])
506	504
507	505	self.loader = Loader(self._context, self,
508		- parent=parent_loader,
509		- tempdir=tempdir)
	506	+ parent=parent_loader)
510	507
511	508	self._project_includes = Includes(self.loader, copy_tree=False)
512	509

tests/frontend/pull.py

---

1	1	import os
2	2	import shutil
	3	+import stat
3	4	import pytest
4	5	from buildstream.plugintestutils import cli
5	6	from tests.testutils import create_artifact_share, generate_junction
...	...	@@ -462,3 +463,74 @@ def test_build_remote_option(caplog, cli, tmpdir, datafiles):
462	463	assert shareproject.repo not in result.stderr
463	464	assert shareuser.repo not in result.stderr
464	465	assert sharecli.repo in result.stderr
	466	+
	467	+
	468	+@pytest.mark.datafiles(DATA_DIR)
	469	+def test_pull_access_rights(caplog, cli, tmpdir, datafiles):
	470	+ project = str(datafiles)
	471	+ checkout = os.path.join(str(tmpdir), 'checkout')
	472	+
	473	+ # Work-around datafiles not preserving mode
	474	+ os.chmod(os.path.join(project, 'files/bin-files/usr/bin/hello'), 0o0755)
	475	+
	476	+ # We need a big file that does not go into a batch to test a different
	477	+ # code path
	478	+ os.makedirs(os.path.join(project, 'files/dev-files/usr/share'), exist_ok=True)
	479	+ with open(os.path.join(project, 'files/dev-files/usr/share/big-file'), 'w') as f:
	480	+ buf = ' ' * 4096
	481	+ for _ in range(1024):
	482	+ f.write(buf)
	483	+
	484	+ with create_artifact_share(os.path.join(str(tmpdir), 'artifactshare')) as share:
	485	+
	486	+ cli.configure({
	487	+ 'artifacts': {'url': share.repo, 'push': True}
	488	+ })
	489	+ result = cli.run(project=project, args=['build', 'compose-all.bst'])
	490	+ result.assert_success()
	491	+
	492	+ result = cli.run(project=project,
	493	+ args=['artifact', 'checkout',
	494	+ '--hardlinks', '--no-integrate',
	495	+ 'compose-all.bst',
	496	+ '--directory', checkout])
	497	+ result.assert_success()
	498	+
	499	+ st = os.lstat(os.path.join(checkout, 'usr/include/pony.h'))
	500	+ assert stat.S_ISREG(st.st_mode)
	501	+ assert stat.S_IMODE(st.st_mode) == 0o0644
	502	+
	503	+ st = os.lstat(os.path.join(checkout, 'usr/bin/hello'))
	504	+ assert stat.S_ISREG(st.st_mode)
	505	+ assert stat.S_IMODE(st.st_mode) == 0o0755
	506	+
	507	+ st = os.lstat(os.path.join(checkout, 'usr/share/big-file'))
	508	+ assert stat.S_ISREG(st.st_mode)
	509	+ assert stat.S_IMODE(st.st_mode) == 0o0644
	510	+
	511	+ shutil.rmtree(checkout)
	512	+
	513	+ artifacts = os.path.join(cli.directory, 'artifacts')
	514	+ shutil.rmtree(artifacts)
	515	+
	516	+ result = cli.run(project=project, args=['artifact', 'pull', 'compose-all.bst'])
	517	+ result.assert_success()
	518	+
	519	+ result = cli.run(project=project,
	520	+ args=['artifact', 'checkout',
	521	+ '--hardlinks', '--no-integrate',
	522	+ 'compose-all.bst',
	523	+ '--directory', checkout])
	524	+ result.assert_success()
	525	+
	526	+ st = os.lstat(os.path.join(checkout, 'usr/include/pony.h'))
	527	+ assert stat.S_ISREG(st.st_mode)
	528	+ assert stat.S_IMODE(st.st_mode) == 0o0644
	529	+
	530	+ st = os.lstat(os.path.join(checkout, 'usr/bin/hello'))
	531	+ assert stat.S_ISREG(st.st_mode)
	532	+ assert stat.S_IMODE(st.st_mode) == 0o0755
	533	+
	534	+ st = os.lstat(os.path.join(checkout, 'usr/share/big-file'))
	535	+ assert stat.S_ISREG(st.st_mode)
	536	+ assert stat.S_IMODE(st.st_mode) == 0o0644

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