Merge branch 'shashwatdalal/docker-source-patch' into 'master'

Add default permissions when adding to CAS and when linking files

See merge request BuildStream/buildstream!1458

5 files changed, 49 insertions, 6 deletions

diff --git a/src/buildstream/plugins/sources/tar.py b/src/buildstream/plugins/sources/tar.py
index 3e9018bba..702b7ba56 100644
--- a/src/buildstream/plugins/sources/tar.py
+++ b/src/buildstream/plugins/sources/tar.py
@@ -66,6 +66,23 @@ from buildstream import utils
 from ._downloadablefilesource import DownloadableFileSource
 
 
+class ReadableTarInfo(tarfile.TarInfo):
+    """
+           The goal is to override `TarFile`'s `extractall` semantics by ensuring that on extraction, the
+           files are readable by the owner of the file. This is done by overriding the accessor for the
+           `mode` attribute in `TarInfo`, the class that encapsulates the internal meta-data of the tarball,
+           so that the owner-read bit is always set.
+    """
+    @property
+    def mode(self):
+        # ensure file is readable by owner
+        return self.__permission | 0o400
+
+    @mode.setter
+    def mode(self, permission):
+        self.__permission = permission
+
+
 class TarSource(DownloadableFileSource):
     # pylint: disable=attribute-defined-outside-init
 
@@ -99,10 +116,10 @@ class TarSource(DownloadableFileSource):
     def _get_tar(self):
         if self.url.endswith('.lz'):
             with self._run_lzip() as lzip_dec:
-                with tarfile.open(fileobj=lzip_dec, mode='r:') as tar:
+                with tarfile.open(fileobj=lzip_dec, mode='r:', tarinfo=ReadableTarInfo) as tar:
                     yield tar
         else:
-            with tarfile.open(self._get_mirror_file()) as tar:
+            with tarfile.open(self._get_mirror_file(), tarinfo=ReadableTarInfo) as tar:
                 yield tar
 
     def stage(self, directory):
diff --git a/src/buildstream/utils.py b/src/buildstream/utils.py
index c9b64aa52..32d23d27c 100644
--- a/src/buildstream/utils.py
+++ b/src/buildstream/utils.py
@@ -765,12 +765,12 @@ def _force_rmtree(rootpath, **kwargs):
 def _copy_directories(srcdir, destdir, target):
     this_dir = os.path.dirname(target)
     new_dir = os.path.join(destdir, this_dir)
+    old_dir = os.path.join(srcdir, this_dir)
 
     if not os.path.lexists(new_dir):
         if this_dir:
             yield from _copy_directories(srcdir, destdir, this_dir)
 
-        old_dir = os.path.join(srcdir, this_dir)
         if os.path.lexists(old_dir):
             dir_stat = os.lstat(old_dir)
             mode = dir_stat.st_mode
@@ -781,6 +781,16 @@ def _copy_directories(srcdir, destdir, target):
             else:
                 raise UtilError('Source directory tree has file where '
                                 'directory expected: {}'.format(old_dir))
+    else:
+        if not os.access(new_dir, os.W_OK):
+            # If the destination directory is not writable, change permissions to make it
+            # writable. Callers of this method (like `_process_list`) must
+            # restore the original permissions towards the end of their processing.
+            try:
+                os.chmod(new_dir, 0o755)
+                yield (new_dir, os.lstat(old_dir).st_mode)
+            except PermissionError:
+                raise UtilError("Directory {} is not writable".format(destdir))
 
 
 # _ensure_real_directory()
diff --git a/tests/integration/workspace.py b/tests/integration/workspace.py
index fe5e76ea3..9e74e574f 100644
--- a/tests/integration/workspace.py
+++ b/tests/integration/workspace.py
@@ -37,6 +37,20 @@ def test_workspace_mount(cli, datafiles):
 
 
 @pytest.mark.datafiles(DATA_DIR)
+def test_workspace_mount_on_read_only_directory(cli, datafiles):
+    project = str(datafiles)
+    workspace = os.path.join(cli.directory, 'workspace')
+    os.makedirs(workspace)
+    element_name = 'workspace/workspace-mount.bst'
+
+    # make directory RO
+    os.chmod(workspace, 0o555)
+
+    res = cli.run(project=project, args=['workspace', 'open', '--directory', workspace, element_name])
+    assert res.exit_code == 0
+
+
+@pytest.mark.datafiles(DATA_DIR)
 @pytest.mark.skipif(not HAVE_SANDBOX, reason='Only available with a functioning sandbox')
 @pytest.mark.xfail(HAVE_SANDBOX == 'buildbox', reason='Not working with BuildBox', strict=True)
 def test_workspace_commanddir(cli, datafiles):
diff --git a/tests/sources/tar.py b/tests/sources/tar.py
index 9a5559d47..fac6f3f8b 100644
--- a/tests/sources/tar.py
+++ b/tests/sources/tar.py
@@ -279,9 +279,10 @@ def test_stage_default_basedir_lzip(cli, tmpdir, datafiles, srcdir):
 # Test that tarballs with read-only files work
 # a - contains read-only files in a writable directory
 # b - root directory has read-only permission
+# c - contains one file that has no read nor write permissions. Base-dir set to '' to extract root of tarball
 @pytest.mark.datafiles(os.path.join(DATA_DIR, 'read-only'))
-@pytest.mark.parametrize("tar_name", ["a", "b"])
-def test_read_only_dir(cli, tmpdir, datafiles, tar_name):
+@pytest.mark.parametrize("tar_name, base_dir", [("a", "*"), ("b", '*'), ("c", '')])
+def test_read_only_dir(cli, tmpdir, datafiles, tar_name, base_dir):
     try:
         project = str(datafiles)
         generate_project(project, tmpdir)
@@ -295,7 +296,8 @@ def test_read_only_dir(cli, tmpdir, datafiles, tar_name):
                 {
                     'kind': 'tar',
                     'url': 'tmpdir:/{}'.format(tar_file),
-                    'ref': 'foo'
+                    'ref': 'foo',
+                    'base-dir': base_dir
                 }
             ]
         }, bst_path)
diff --git a/tests/sources/tar/read-only/content/c.tar.gz b/tests/sources/tar/read-only/content/c.tar.gz
new file mode 100644
index 000000000..016a5a218
--- /dev/null
+++ b/tests/sources/tar/read-only/content/c.tar.gz