WIP: Import unreadable files
Description
Certain artifacts stage things which have no read permission. See #684 for an example.
Changes proposed in this merge request:
-
Alter _process_list in utils.py
so it can read files with no read permission.
There were other previously proposed changes which aren't necessary:
-
Alter _process_list so it can do the same job for directories, symlinks and other node types
- Directories, devices, and fifos are created at the destination without needing to read the source.
- The permissions set on symlinks are irrelevant under a unix-based OS, and do not need to have their contents read anyway (as above)
-
Alter
cascache.py
so it can hash unreadable files -
Allow
CasBasedDirectory
to import unreadable files-
#429 is expected to correct CASCache so it can hash unreadable files. CasBasedDirectory doesn't need to open any files; it just asks the CASCache to hash them with
add_object
.
-
#429 is expected to correct CASCache so it can hash unreadable files. CasBasedDirectory doesn't need to open any files; it just asks the CASCache to hash them with
This merge request, when approved, will close issue/bug: #684
Merge request reports
Activity
added 6 commits
-
5ca97364...fafa8136 - 5 commits from branch
master
- 76c8070b - _process_list: Make files readable before copying/linking
-
5ca97364...fafa8136 - 5 commits from branch
added 1 commit
- c8677af0 - _process_list: Make files readable before copying/linking
mentioned in issue #684
831 831 result.ignored.append(path) 832 832 continue 833 833 834 if not file_stat.st_mode & stat.S_IRUSR: 835 os.chmod(srcpath, file_stat.st_mode | stat.S_IRUSR) 836 834 837 actionfunc(srcpath, destpath, result=result) I agree, especially having to do it in such a generic utility function. Also, based on my analysis and testing, this doesn't even fix the exceptions reported in #684.
The permission error is triggered by the fallback of
shutil.move
because the destination directory for staging sources is typically no longer on the same filesystem (~/.cache vs. /tmp). The fallback ofshutil.move
is not related to BuildStream's_process_list
utility function, so patching that up won't fix anything.I propose that we change the temporary directory in
Element._stage_sources_at
to be within ~/.cache/buildstream as it used to be. This will provide a massive speedup (shutil.move
can useos.rename
instead of having to copy the whole tree) and fix the exceptions reported in #684.However, I expect that with this temporary directory change we will still trigger an exception in
import_files
that this MR would fix. But even if we applied this change, this would still leave #429.I'm tempted to say that it's the source's responsibility to stage readable files and fix up source plugins that don't always do that (I suspect ostree and tar, the others might be ok). How do others feel about this?
!856 (merged) moved the temporary storage to
~/.cache
.assigned to @jmacarthur