From b1552ea657a0d9da7ee8733334a02a2f92dc7f97 Mon Sep 17 00:00:00 2001 From: Tristan Van Berkom Date: Fri, 13 Oct 2017 20:24:46 +0900 Subject: element.py: Fix cleanup after collection of artifact with read-only directories Part of issue #81 This was failing because the tmpdir contextmanager was trying to still use it's own mechanics to cleanup the artifact assembly directory. This is fixed by *not* using a tmpdir, which was just wrong to begin with because we're already working inside a temp directory. --- buildstream/element.py | 87 +++++++++++++++++++++++--------------------------- 1 file changed, 40 insertions(+), 47 deletions(-) diff --git a/buildstream/element.py b/buildstream/element.py index 05c92aa19..aa7c7e5ff 100644 --- a/buildstream/element.py +++ b/buildstream/element.py @@ -1075,53 +1075,46 @@ class Element(Plugin): # At this point, we expect an exception was raised leading to # an error message, or we have good output to collect. - with tempfile.TemporaryDirectory(prefix='tmp', dir=sandbox_root) as assembledir: - # Create artifact directory structure - filesdir = os.path.join(assembledir, 'files') - logsdir = os.path.join(assembledir, 'logs') - metadir = os.path.join(assembledir, 'meta') - os.mkdir(filesdir) - os.mkdir(logsdir) - os.mkdir(metadir) - - # Hard link files from collect dir to files directory - utils.link_files(collectdir, filesdir) - - # Copy build log - if self.__log_path: - shutil.copyfile(self.__log_path, os.path.join(logsdir, 'build.log')) - - # Store public data - _yaml.dump(_yaml.node_sanitize(self.__dynamic_public), os.path.join(metadir, 'public.yaml')) - - # Store artifact metadata - dependencies = { - e.name: e._get_cache_key_from_artifact() for e in self.dependencies(Scope.BUILD) - } - workspaced_dependencies = { - e.name: e._workspaced() for e in self.dependencies(Scope.BUILD) - } - meta = { - 'keys': { - 'strong': self._get_cache_key_for_build(), - 'weak': self._get_cache_key(_KeyStrength.WEAK), - 'dependencies': dependencies - }, - 'workspaced': self._workspaced(), - 'workspaced_dependencies': workspaced_dependencies - } - _yaml.dump(_yaml.node_sanitize(meta), os.path.join(metadir, 'artifact.yaml')) - - with self.timed_activity("Caching Artifact"): - try: - self.__artifacts.commit(self, assembledir) - except _ArtifactError: - # If we failed to commit, there is a good chance it is because - # of some permission error, we force remove it now because - # it may cause some annoyance to the user who might not be able - # to easily remove the directory without changing permissions. - utils._force_rmtree(assembledir) - raise + # Create artifact directory structure + assembledir = os.path.join(rootdir, 'artifact') + filesdir = os.path.join(assembledir, 'files') + logsdir = os.path.join(assembledir, 'logs') + metadir = os.path.join(assembledir, 'meta') + os.mkdir(assembledir) + os.mkdir(filesdir) + os.mkdir(logsdir) + os.mkdir(metadir) + + # Hard link files from collect dir to files directory + utils.link_files(collectdir, filesdir) + + # Copy build log + if self.__log_path: + shutil.copyfile(self.__log_path, os.path.join(logsdir, 'build.log')) + + # Store public data + _yaml.dump(_yaml.node_sanitize(self.__dynamic_public), os.path.join(metadir, 'public.yaml')) + + # Store artifact metadata + dependencies = { + e.name: e._get_cache_key_from_artifact() for e in self.dependencies(Scope.BUILD) + } + workspaced_dependencies = { + e.name: e._workspaced() for e in self.dependencies(Scope.BUILD) + } + meta = { + 'keys': { + 'strong': self._get_cache_key_for_build(), + 'weak': self._get_cache_key(_KeyStrength.WEAK), + 'dependencies': dependencies + }, + 'workspaced': self._workspaced(), + 'workspaced_dependencies': workspaced_dependencies + } + _yaml.dump(_yaml.node_sanitize(meta), os.path.join(metadir, 'artifact.yaml')) + + with self.timed_activity("Caching Artifact"): + self.__artifacts.commit(self, assembledir) # Finally cleanup the build dir cleanup_rootdir() -- cgit v1.2.1