Merge branch 'juerg/export-to-tar' into 'master'

Improve Directory.export_to_tar()

See merge request BuildStream/buildstream!1858

3 files changed, 35 insertions, 9 deletions

diff --git a/src/buildstream/storage/_casbaseddirectory.py b/src/buildstream/storage/_casbaseddirectory.py
index d6c4a7813..b8b5ca09c 100644
--- a/src/buildstream/storage/_casbaseddirectory.py
+++ b/src/buildstream/storage/_casbaseddirectory.py
@@ -536,7 +536,7 @@ class CasBasedDirectory(Directory):
         self.cas_cache.checkout(to_directory, self._get_digest(), can_link=can_link)
 
     def export_to_tar(self, tarfile, destination_dir, mtime=BST_ARBITRARY_TIMESTAMP):
-        for filename, entry in self.index.items():
+        for filename, entry in sorted(self.index.items()):
             arcname = os.path.join(destination_dir, filename)
             if entry.type == _FileType.DIRECTORY:
                 tarinfo = tarfilelib.TarInfo(arcname)
@@ -549,14 +549,15 @@ class CasBasedDirectory(Directory):
                 source_name = self.cas_cache.objpath(entry.digest)
                 tarinfo = tarfilelib.TarInfo(arcname)
                 tarinfo.mtime = mtime
-                tarinfo.mode |= entry.is_executable & stat.S_IXUSR
+                if entry.is_executable:
+                    tarinfo.mode |= stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH
                 tarinfo.size = os.path.getsize(source_name)
                 with open(source_name, "rb") as f:
                     tarfile.addfile(tarinfo, f)
             elif entry.type == _FileType.SYMLINK:
                 tarinfo = tarfilelib.TarInfo(arcname)
                 tarinfo.mtime = mtime
-                tarinfo.mode |= entry.is_executable & stat.S_IXUSR
+                tarinfo.mode = 0o777
                 tarinfo.linkname = entry.target
                 tarinfo.type = tarfilelib.SYMTYPE
                 f = StringIO(entry.target)
@@ -736,18 +737,23 @@ class CasBasedDirectory(Directory):
         if mode not in ["r", "rb", "w", "wb", "x", "xb"]:
             raise ValueError("Unsupported mode: `{}`".format(mode))
 
+        if "b" in mode:
+            encoding = None
+        else:
+            encoding = "utf-8"
+
         if "r" in mode:
             if not entry:
                 raise FileNotFoundError("{} not found in {}".format(path[-1], str(subdir)))
 
             # Read-only access, allow direct access to CAS object
-            with open(self.cas_cache.objpath(entry.digest), mode, encoding="utf-8") as f:
+            with open(self.cas_cache.objpath(entry.digest), mode, encoding=encoding) as f:
                 yield f
         else:
             if "x" in mode and entry:
                 raise FileExistsError("{} already exists in {}".format(path[-1], str(subdir)))
 
-            with utils._tempnamedfile(mode, encoding="utf-8", dir=self.cas_cache.tmpdir) as f:
+            with utils._tempnamedfile(mode, encoding=encoding, dir=self.cas_cache.tmpdir) as f:
                 yield f
                 # Import written temporary file into CAS
                 f.flush()
diff --git a/src/buildstream/storage/_filebaseddirectory.py b/src/buildstream/storage/_filebaseddirectory.py
index 53a87b64c..c492c41cd 100644
--- a/src/buildstream/storage/_filebaseddirectory.py
+++ b/src/buildstream/storage/_filebaseddirectory.py
@@ -190,6 +190,10 @@ class FileBasedDirectory(Directory):
             arcname = os.path.join(destination_dir, filename)
             tarinfo = tarfile.gettarinfo(source_name, arcname)
             tarinfo.mtime = mtime
+            tarinfo.uid = 0
+            tarinfo.gid = 0
+            tarinfo.uname = ""
+            tarinfo.gname = ""
 
             if tarinfo.isreg():
                 with open(source_name, "rb") as f:
@@ -256,10 +260,15 @@ class FileBasedDirectory(Directory):
         if mode not in ["r", "rb", "w", "wb", "x", "xb"]:
             raise ValueError("Unsupported mode: `{}`".format(mode))
 
+        if "b" in mode:
+            encoding = None
+        else:
+            encoding = "utf-8"
+
         if "r" in mode:
-            return open(newpath, mode=mode, encoding="utf-8")
+            return open(newpath, mode=mode, encoding=encoding)
         else:
-            return utils.save_file_atomic(newpath, mode=mode, encoding="utf-8")
+            return utils.save_file_atomic(newpath, mode=mode, encoding=encoding)
 
     def __str__(self):
         # This returns the whole path (since we don't know where the directory started)
diff --git a/tests/frontend/buildcheckout.py b/tests/frontend/buildcheckout.py
index 512c6c151..52cf031ad 100644
--- a/tests/frontend/buildcheckout.py
+++ b/tests/frontend/buildcheckout.py
@@ -254,6 +254,9 @@ def test_build_checkout_tarball(datafiles, cli):
     project = str(datafiles)
     checkout = os.path.join(cli.directory, "checkout.tar")
 
+    # Work-around datafiles not preserving mode
+    os.chmod(os.path.join(project, "files/bin-files/usr/bin/hello"), 0o0755)
+
     result = cli.run(project=project, args=["build", "target.bst"])
     result.assert_success()
 
@@ -267,8 +270,16 @@ def test_build_checkout_tarball(datafiles, cli):
     result.assert_success()
 
     tar = tarfile.TarFile(checkout)
-    assert os.path.join(".", "usr", "bin", "hello") in tar.getnames()
-    assert os.path.join(".", "usr", "include", "pony.h") in tar.getnames()
+
+    tarinfo = tar.getmember(os.path.join(".", "usr", "bin", "hello"))
+    assert tarinfo.mode == 0o755
+    assert tarinfo.uid == 0 and tarinfo.gid == 0
+    assert tarinfo.uname == "" and tarinfo.gname == ""
+
+    tarinfo = tar.getmember(os.path.join(".", "usr", "include", "pony.h"))
+    assert tarinfo.mode == 0o644
+    assert tarinfo.uid == 0 and tarinfo.gid == 0
+    assert tarinfo.uname == "" and tarinfo.gname == ""
 
 
 @pytest.mark.datafiles(DATA_DIR)