Conditionally preserve file permissions when archiving through pkg_tar (#951)

* preserve file permission through an option

* fix parsing and handling of new preserve_mode argument

* restore version

* add tests

* use an existing test file, as git does not preserve file perms

* delete test file

* cover windows testing

* record test subject file perms

* use pythonic elif

Co-authored-by: Chuck Grindel <[email protected]>

* buildifier

* trim trailing whitespaces

---------

Co-authored-by: rnascimento <[email protected]>
Co-authored-by: Chuck Grindel <[email protected]>

Author: 3 people

pkg/private/tar/build_tar.py

@@ -15,6 +15,7 @@
 
 import argparse
 import os
+import stat
 import tarfile
 import tempfile
 
@@ -43,7 +44,7 @@ class DebError(Exception):
     pass
 
   def __init__(self, output, directory, compression, compressor, create_parents,
-               allow_dups_from_deps, default_mtime, compression_level):
+               allow_dups_from_deps, default_mtime, compression_level, preserve_mode):
     # Directory prefix on all output paths
     d = directory.strip('/')
     self.directory = (d + '/') if d else None
@@ -54,6 +55,7 @@ def __init__(self, output, directory, compression, compressor, create_parents,
     self.create_parents = create_parents
     self.allow_dups_from_deps = allow_dups_from_deps
     self.compression_level = compression_level
+    self.preserve_mode = preserve_mode
 
   def __enter__(self):
     self.tarfile = tar_writer.TarFileWriter(
@@ -101,9 +103,13 @@ def add_file(self, f, destfile, mode=None, ids=None, names=None):
          copied to `self.directory/destfile` in the layer.
     """
     dest = self.normalize_path(destfile)
-    # If mode is unspecified, derive the mode from the file's mode.
-    if mode is None:
-      mode = 0o755 if os.access(f, os.X_OK) else 0o644
+    # If preserve_mode is enabled, set mode by extracting the permission bits
+    # from the file's mode attribute. Note: the mode argument is ignored.
+    # Otherwise; if mode is unspecified, derive the mode from the file's mode.
+    if self.preserve_mode is True:
+      mode = stat.S_IMODE(os.stat(f).st_mode)
+    elif mode is None:
+        mode = 0o755 if os.access(f, os.X_OK) else 0o644
     if ids is None:
       ids = (0, 0)
     if names is None:
@@ -402,6 +408,10 @@ def main():
   parser.add_argument('--allow_dups_from_deps',
                       action='store_true',
                       help='')
+  parser.add_argument(
+      '--preserve_mode', default='False',
+      action='store_true',
+      help='Preserve original file permissions in the archive. Mode argument is ignored.')
   parser.add_argument(
       '--compression_level', default=-1,
       help='Specify the numeric compress level in gzip mode; may be 0-9 or -1 (default to 6).')
@@ -461,7 +471,8 @@ def main():
       default_mtime=default_mtime,
       create_parents=options.create_parents,
       allow_dups_from_deps=options.allow_dups_from_deps,
-      compression_level = compression_level) as output:
+      compression_level = compression_level,
+      preserve_mode = options.preserve_mode) as output:
 
     def file_attributes(filename):
       if filename.startswith('/'):

pkg/private/tar/tar.bzl

@@ -181,6 +181,9 @@ def _pkg_tar_impl(ctx):
     if ctx.attr.allow_duplicates_from_deps:
         args.add("--allow_dups_from_deps")
 
+    if ctx.attr.preserve_mode:
+        args.add("--preserve_mode")
+
     inputs = depset(
         direct = ctx.files.deps + files,
         transitive = mapping_context.file_deps,
@@ -294,6 +297,10 @@ Such behaviour is always incorrect, but we provide a flag to support it in case
 builds were accidentally doing it. Never explicitly set this to true for new code.
 """,
         ),
+        "preserve_mode": attr.bool(
+            default = False,
+            doc = """If true, will add file to archive with preserved file permissions.""",
+        ),
         "stamp": attr.int(
             doc = """Enable file time stamping.  Possible values:
 <li>stamp = 1: Use the time of the build as the modification time of each file in the archive.

tests/tar/BUILD

@@ -474,6 +474,8 @@ py_test(
         ":test-tar-files_dict.tar",
         ":test-tar-long-filename",
         ":test-tar-mtime.tar",
+        ":test-tar-preserve_mode-False.tar",
+        ":test-tar-preserve_mode-True.tar",
         ":test-tar-repackaging-long-filename.tar",
         ":test-tar-strip_prefix-dot.tar",
         ":test-tar-strip_prefix-empty.tar",
@@ -801,3 +803,14 @@ verify_archive_test(
     6,
     9,
 ]]
+
+[pkg_tar(
+    name = "test-tar-preserve_mode-%s" % state,
+    srcs = [
+        "//tests:testdata/hello.txt",  # rw- r-- r--
+    ],
+    preserve_mode = state,
+) for state in [
+    True,
+    False,
+]]

tests/tar/pkg_tar_test.py

@@ -306,5 +306,23 @@ def test_compression_level(self):
       file_size = os.stat(file_path).st_size
       self.assertEqual(file_size, expected_size, 'size error for ' + file_name)
 
+  def test_preserve_mode(self):
+    if os.name == 'nt':
+      expected_mode = [
+        ('test-tar-preserve_mode-False.tar', "0o555"), # chmod 555 = r-x r-x r-x
+        ('test-tar-preserve_mode-True.tar', "0o666"),  # chmod 666 = rw- rw- rw-
+      ]
+    else:
+      expected_mode = [
+        ('test-tar-preserve_mode-False.tar', "0o555"), # chmod 555 = r-x r-x r-x
+        ('test-tar-preserve_mode-True.tar', "0o644"),  # chmod 644 = rw- r-- r--
+      ]
+    for file_name, expected_mode in expected_mode:
+      file_path = runfiles.Create().Rlocation('rules_pkg/tests/tar/' + file_name)
+      with tarfile.open(file_path, 'r') as f:
+        for member in f.getmembers():
+          self.assertEqual(member.name, "hello.txt", "unexpected file name for " + file_name)
+          self.assertEqual(member.mode, int(expected_mode, 0), 'file mode not preserved for ' + file_name)
+
 if __name__ == '__main__':
   unittest.main()