📦 Conditionally preserve file mtime when archiving through pkg_tar (#974)

* preserve file mtime through an option

* add tests

* delete debug comment

* wrap long line

* buildifier

---------

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

Author: ronodnn

pkg/private/tar/build_tar.py

@@ -44,7 +44,8 @@ class DebError(Exception):
     pass
 
   def __init__(self, output, directory, compression, compressor, create_parents,
-               allow_dups_from_deps, default_mtime, compression_level, preserve_mode):
+               allow_dups_from_deps, default_mtime, compression_level, preserve_mode,
+               preserve_mtime):
     # Directory prefix on all output paths
     d = directory.strip('/')
     self.directory = (d + '/') if d else None
@@ -56,6 +57,7 @@ def __init__(self, output, directory, compression, compressor, create_parents,
     self.allow_dups_from_deps = allow_dups_from_deps
     self.compression_level = compression_level
     self.preserve_mode = preserve_mode
+    self.preserve_mtime = preserve_mtime
 
   def __enter__(self):
     self.tarfile = tar_writer.TarFileWriter(
@@ -90,7 +92,7 @@ def normalize_path(self, path: str) -> str:
       dest = self.directory + dest
     return dest
 
-  def add_file(self, f, destfile, mode=None, ids=None, names=None):
+  def add_file(self, f, destfile, mode=None, ids=None, names=None, mtime=None):
     """Add a file to the tar file.
 
     Args:
@@ -110,6 +112,8 @@ def add_file(self, f, destfile, mode=None, ids=None, names=None):
       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 self.preserve_mtime is True:
+      mtime = os.stat(f).st_mtime
     if ids is None:
       ids = (0, 0)
     if names is None:
@@ -121,7 +125,8 @@ def add_file(self, f, destfile, mode=None, ids=None, names=None):
         uid=ids[0],
         gid=ids[1],
         uname=names[0],
-        gname=names[1])
+        gname=names[1],
+        mtime=mtime)
 
   def add_empty_file(self,
                      destfile,
@@ -412,6 +417,10 @@ def main():
       '--preserve_mode', default='False',
       action='store_true',
       help='Preserve original file permissions in the archive. Mode argument is ignored.')
+  parser.add_argument(
+      '--preserve_mtime', default='False',
+      action='store_true',
+      help='Preserve original file mtime in the archive. mtime 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).')
@@ -472,7 +481,8 @@ def main():
       create_parents=options.create_parents,
       allow_dups_from_deps=options.allow_dups_from_deps,
       compression_level = compression_level,
-      preserve_mode = options.preserve_mode) as output:
+      preserve_mode = options.preserve_mode,
+      preserve_mtime = options.preserve_mtime) as output:
 
     def file_attributes(filename):
       if filename.startswith('/'):

pkg/private/tar/tar.bzl

@@ -184,6 +184,9 @@ def _pkg_tar_impl(ctx):
     if ctx.attr.preserve_mode:
         args.add("--preserve_mode")
 
+    if ctx.attr.preserve_mtime:
+        args.add("--preserve_mtime")
+
     inputs = depset(
         direct = ctx.files.deps + files,
         transitive = mapping_context.file_deps,
@@ -301,6 +304,10 @@ builds were accidentally doing it. Never explicitly set this to true for new cod
             default = False,
             doc = """If true, will add file to archive with preserved file permissions.""",
         ),
+        "preserve_mtime": attr.bool(
+            default = False,
+            doc = """If true, will add file to archive with preserved file mtime.""",
+        ),
         "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

@@ -476,6 +476,8 @@ py_test(
         ":test-tar-mtime.tar",
         ":test-tar-preserve_mode-False.tar",
         ":test-tar-preserve_mode-True.tar",
+        ":test-tar-preserve_mtime-False.tar",
+        ":test-tar-preserve_mtime-True.tar",
         ":test-tar-repackaging-long-filename.tar",
         ":test-tar-strip_prefix-dot.tar",
         ":test-tar-strip_prefix-empty.tar",
@@ -814,3 +816,14 @@ verify_archive_test(
     True,
     False,
 ]]
+
+[pkg_tar(
+    name = "test-tar-preserve_mtime-%s" % state,
+    srcs = [
+        "//tests:testdata/hello.txt",
+    ],
+    preserve_mtime = state,
+) for state in [
+    True,
+    False,
+]]

tests/tar/pkg_tar_test.py

@@ -324,5 +324,21 @@ def test_preserve_mode(self):
           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)
 
+  def test_preserve_mtime(self):
+    test_cases = [
+      # tar file name, mtime should be equal to PORTABLE_MTIME?
+      ('test-tar-preserve_mtime-False.tar', True),
+      ('test-tar-preserve_mtime-True.tar', False),
+    ]
+    for file_name, should_be_equal_to_portable_mtime in test_cases:
+      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)
+          if should_be_equal_to_portable_mtime:
+            self.assertEqual(member.mtime, PORTABLE_MTIME, "unexpected mtime for " + file_name)
+          else:
+            self.assertNotEqual(member.mtime, PORTABLE_MTIME, "file mtime not preserved for " + file_name)
+
 if __name__ == '__main__':
   unittest.main()