Add --stamp support to pkg_zip. (#365)

* Add --stamp to zip.

Advances #287   Now both pkg_tar and pkg_zip support --stamp behavior in the same manor as *_binary rules.

pkg/docs/reference.md

@@ -289,6 +289,18 @@ Creates a tar file from a list of inputs.
         </p>
       </td>
     </tr>
+    <tr>
+      <td><code>stamp</code></td>
+      <td>
+        <code>Integer; optional; default is -1</code>
+        <p>Enable file time stamping.  Possible values:<ul>
+          <li>stamp = 1: Use the time of the build as the modification time of each file in the archive.</li>
+          <li>stamp = 0: Use an "epoch" time for the modification time of each file. This gives good build result caching.</li>
+          <li>stamp = -1: Control the chosen modification time using the --[no]stamp flag.</li>
+          </ul>
+        </p>
+      </td>
+    </tr>
     <tr>
       <td><code>symlinks</code></td>
       <td>
@@ -389,6 +401,18 @@ Creates a zip file from a list of inputs.
         </p>
       </td>
     </tr>
+    <tr>
+      <td><code>stamp</code></td>
+      <td>
+        <code>Integer; optional; default is -1</code>
+        <p>Enable file time stamping.  Possible values:<ul>
+          <li>stamp = 1: Use the time of the build as the modification time of each file in the archive.</li>
+          <li>stamp = 0: Use an "epoch" time for the modification time of each file. This gives good build result caching.</li>
+          <li>stamp = -1: Control the chosen modification time using the --[no]stamp flag.</li>
+          </ul>
+        </p>
+      </td>
+    </tr>
     <tr>
       <td><code>timestamp</code></td>
       <td>

pkg/pkg.bzl

@@ -369,7 +369,13 @@ pkg_tar_impl = rule(
             doc = "See Common Attributes",
             providers = [PackageVariablesInfo],
         ),
-        "stamp": attr.int(default = 0),
+        "stamp": attr.int(
+            doc = """Enable file time stamping.  Possible values:<ul>
+<li>stamp = 1: Use the time of the build as the modification time of each file in the archive.</li>
+<li>stamp = 0: Use an "epoch" time for the modification time of each file. This gives good build result caching.</li>
+<li>stamp = -1: Control the chosen modification time using the --[no]stamp flag.</li>
+</ul>""",
+            default = 0),
         # Is --stamp set on the command line?
         # TODO(https://github.com/bazelbuild/rules_pkg/issues/340): Remove this.
         "private_stamp_detect": attr.bool(default = False),
@@ -509,6 +515,11 @@ def _pkg_zip_impl(ctx):
     args.add("-d", ctx.attr.package_dir)
     args.add("-t", ctx.attr.timestamp)
     args.add("-m", ctx.attr.mode)
+    inputs = []
+    if ctx.attr.stamp == 1 or (ctx.attr.stamp == -1 and
+                               ctx.attr.private_stamp_detect):
+        args.add("--stamp_from", ctx.version_file.path)
+        inputs.append(ctx.version_file)
 
     data_path = compute_data_path(ctx, ctx.attr.strip_prefix)
     for f in ctx.files.srcs:
@@ -520,8 +531,8 @@ def _pkg_zip_impl(ctx):
 
     ctx.actions.run(
         mnemonic = "PackageZip",
-        inputs = ctx.files.srcs,
-        executable = ctx.executable.build_zip,
+        inputs = ctx.files.srcs + inputs,
+        executable = ctx.executable._build_zip,
         arguments = [args],
         outputs = [output_file],
         env = {
@@ -559,9 +570,20 @@ pkg_zip_impl = rule(
             doc = "See Common Attributes",
             providers = [PackageVariablesInfo],
         ),
+        "stamp": attr.int(
+            doc = """Enable file time stamping.  Possible values:<ul>
+<li>stamp = 1: Use the time of the build as the modification time of each file in the archive.</li>
+<li>stamp = 0: Use an "epoch" time for the modification time of each file. This gives good build result caching.</li>
+<li>stamp = -1: Control the chosen modification time using the --[no]stamp flag.</li>
+</ul>""",
+            default = 0),
+
+        # Is --stamp set on the command line?
+        # TODO(https://github.com/bazelbuild/rules_pkg/issues/340): Remove this.
+        "private_stamp_detect": attr.bool(default = False),
 
         # Implicit dependencies.
-        "build_zip": attr.label(
+        "_build_zip": attr.label(
             default = Label("//private:build_zip"),
             cfg = "exec",
             executable = True,
@@ -592,5 +614,9 @@ def pkg_zip(name, **kwargs):
     pkg_zip_impl(
         name = name,
         out = archive_name + "." + extension,
+        private_stamp_detect = select({
+            _stamp_condition: True,
+            "//conditions:default": False,
+        }),
         **kwargs
     )

pkg/private/BUILD

@@ -102,6 +102,7 @@ py_binary(
     visibility = ["//visibility:public"],
     deps = [
         ":archive",
+        ":build_info",
         ":helpers",
     ],
 )

pkg/private/build_zip.py

@@ -17,6 +17,7 @@
 import datetime
 import zipfile
 
+from rules_pkg.private import build_info
 from rules_pkg.private import helpers
 
 ZIP_EPOCH = 315532800
@@ -26,27 +27,23 @@ def _create_argument_parser():
   """Creates the command line arg parser."""
   parser = argparse.ArgumentParser(description='create a zip file',
                                    fromfile_prefix_chars='@')
-
   parser.add_argument('-o', '--output', type=str,
                       help='The output zip file path.')
-
   parser.add_argument(
       '-d', '--directory', type=str, default='/',
       help='An absolute path to use as a prefix for all files in the zip.')
-
   parser.add_argument(
       '-t', '--timestamp', type=int, default=ZIP_EPOCH,
       help='The unix time to use for files added into the zip. values prior to'
            ' Jan 1, 1980 are ignored.')
-
+  parser.add_argument('--stamp_from', default='',
+                      help='File to find BUILD_STAMP in')
   parser.add_argument(
       '-m', '--mode',
       help='The file system mode to use for files added into the zip.')
-
   parser.add_argument(
       'files', type=str, nargs='*',
       help='Files to be added to the zip, in the form of {srcpath}={dstpath}.')
-
   return parser
 
 
@@ -66,6 +63,8 @@ def parse_date(ts):
 
 def main(args):
   unix_ts = max(ZIP_EPOCH, args.timestamp)
+  if args.stamp_from:
+    unix_ts = build_info.get_timestamp(args.stamp_from)
   ts = parse_date(unix_ts)
   default_mode = None
   if args.mode:

pkg/tests/BUILD

@@ -583,13 +583,22 @@ pkg_tar(
     stamp = 1,
 )
 
+pkg_zip(
+    name = "stamped_zip",
+    srcs = ["BUILD"],
+    stamp = 1,
+)
+
+# Note that this only tests that stamping works. Other tests cover the case
+# of archive members having the default, epoch, time stamp.
 py_test(
     name = "stamp_test",
     srcs = [
         "stamp_test.py",
     ],
     data = [
         "stamped_tar.tar",
+        "stamped_zip.zip",
     ],
     python_version = "PY3",
     deps = [

pkg/tests/stamp_test.py

@@ -13,17 +13,57 @@
 # limitations under the License.
 """Test time stamping in pkg_tar"""
 
+import datetime
 import tarfile
 import time
 import unittest
+import zipfile
 
 from bazel_tools.tools.python.runfiles import runfiles
 
 # keep in sync with archive.py
 PORTABLE_MTIME = 946684800  # 2000-01-01 00:00:00.000 UTC
 
-class PkgTarTest(unittest.TestCase):
-  """Testing for pkg_tar rule."""
+
+class StampTest(unittest.TestCase):
+  """Test for non-epoch time stamps in packages."""
+
+  target_mtime = int(time.time())
+  zip_epoch_dt = datetime.datetime(1980, 1, 1, 0, 0, 0, tzinfo=datetime.timezone.utc)
+  ZIP_EPOCH = int(zip_epoch_dt.timestamp())
+  # We allow the stamp on the build file to be within this delta back in
+  # time from now. That allows for the test data to be built early in a clean
+  # CI run and have the test run a few seconds later. Ideally, this would
+  # be no greater than the expected total CI run time, but the extra margin
+  # does not hurt.
+  ALLOWED_DELTA_FROM_NOW = 10000  # seconds
+
+  def check_mtime(self, mtime, file_path, file_name):
+    """Checks that a time stamp is reasonable.
+
+    This checks that a timestamp is not 0 or any of the well known EPOCH values,
+    and that it is within some delta from the current time.
+
+    Args:
+      mtime: timestamp in seconds
+      file_path: path to archive name
+      file_name: file within archive
+    """
+    if mtime == 0:
+       self.fail('Archive %s contains file %s with mtime == 0' % (
+           file_path, file_name))
+    if mtime == PORTABLE_MTIME:
+       self.fail('Archive %s contains file %s with portable mtime' % (
+           file_path, file_name))
+    if mtime == StampTest.ZIP_EPOCH:
+       self.fail('Archive %s contains file %s with ZIP epoch' % (
+           file_path, file_name))
+    if ((mtime < self.target_mtime - StampTest.ALLOWED_DELTA_FROM_NOW)
+        or (mtime > self.target_mtime)):
+       self.fail(
+           'Archive %s contains file %s with mtime:%d, expected:%d +/- %d' % (
+               file_path, file_name, mtime, self.target_mtime,
+               StampTest.ALLOWED_DELTA_FROM_NOW))
 
   def assertTarFilesAreAlmostNew(self, file_name):
     """Assert that tarfile contains files with an mtime of roughly now.
@@ -37,23 +77,35 @@ def assertTarFilesAreAlmostNew(self, file_name):
         file_name: the path to the TAR file to test.
     """
     file_path = runfiles.Create().Rlocation('rules_pkg/tests/' + file_name)
-    target_mtime = int(time.time())
     with tarfile.open(file_path, 'r:*') as f:
-      i = 0
       for info in f:
-        if info.mtime == PORTABLE_MTIME:
-           self.fail('Archive %s contains file %s with portable mtime' % (
-               file_path, info.name))
-        if ((info.mtime < target_mtime - 10000)
-            or (info.mtime > target_mtime + 10000)):
-           self.fail(
-               'Archive %s contains file %s with mtime:%d, expected:%d' % (
-                   file_path, info.name, info.mtime, target_mtime))
+        self.check_mtime(info.mtime, file_path, info.name)
 
+  def assertZipFilesAreAlmostNew(self, file_name):
+    """Assert that zipfile contains files with an mtime of roughly now.
 
-  def test_not_epoch_times(self):
+    This is used to prove that the test data was a file which was presumably:
+    built with 'stamp=1' or ('stamp=-1' and --stamp) contains files which
+    all have a fairly recent mtime, thus indicating they are "current" time
+    rather than the epoch or some other time.
+
+    Args:
+        file_name: the path to the ZIP file to test.
+    """
+    file_path = runfiles.Create().Rlocation('rules_pkg/tests/' + file_name)
+    target_mtime = int(time.time())
+    with zipfile.ZipFile(file_path, mode='r') as f:
+      for info in f.infolist():
+        d = info.date_time
+        dt = datetime.datetime(d[0], d[1], d[2], d[3], d[4], d[5], tzinfo=datetime.timezone.utc)
+        self.check_mtime(int(dt.timestamp()), file_path, info.filename)
+
+  def test_not_epoch_times_tar(self):
     self.assertTarFilesAreAlmostNew('stamped_tar.tar')
 
+  def test_not_epoch_times_zip(self):
+    self.assertZipFilesAreAlmostNew('stamped_zip.zip')
+
 
 if __name__ == '__main__':
   unittest.main()

pkg/tests/zip_test.py

@@ -12,18 +12,27 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import datetime
 import filecmp
 import unittest
 import zipfile
 
 from bazel_tools.tools.python.runfiles import runfiles
-from rules_pkg.private import build_zip
 
 HELLO_CRC = 2069210904
 LOREM_CRC = 2178844372
 EXECUTABLE_CRC = 342626072
 
 
+# The ZIP epoch date: (1980, 1, 1, 0, 0, 0)
+_ZIP_EPOCH_DT = datetime.datetime(1980, 1, 1, 0, 0, 0, tzinfo=datetime.timezone.utc)
+_ZIP_EPOCH_S = int(_ZIP_EPOCH_DT.timestamp())
+
+def seconds_to_ziptime(s):
+  dt = datetime.datetime.utcfromtimestamp(s)
+  return (dt.year, dt.month, dt.day, dt.hour, dt.minute, dt.second)
+
+
 class ZipTest(unittest.TestCase):
 
   def get_test_zip(self, zip_file):
@@ -57,8 +66,7 @@ def assertZipFileContent(self, zip_file, content):
         self.assertEqual(info.filename, expected["filename"])
         self.assertEqual(info.CRC, expected["crc"])
 
-        ts = build_zip.parse_date(
-            expected.get("timestamp", build_zip.ZIP_EPOCH))
+        ts = seconds_to_ziptime(expected.get("timestamp", _ZIP_EPOCH_S))
         self.assertEqual(info.date_time, ts)
         self.assertEqual(info.external_attr >> 16, expected.get("attr", 0o555))