[rules_pkg] pkg_zip: initial tree artifact implementation

Notes:
- Rather than tear `_add_manifest_entry` apart, I took the lazy way out
  by constructing dummy `ManifestEntry` objects and recursively calling
  `_add_manifest_entry`.
- vim footers added to build_zip.py and zip_test.py since those srcs
  use 2-space indents.
- I don't know whether I need to file separate issues for the "food
  for thought" code comments or if those could just be tracked in the
  discussion of bazelbuild#309.

Advances bazelbuild#309.

Testing Done: `bazelisk test //tests:zip_test`
Bug Number: VCBAZ-2726
Reviewed by: apsaltis
Review URL: https://reviewboard.eng.vmware.com/r/1842746/

pkg/private/build_zip.py

@@ -16,6 +16,7 @@
 import argparse
 import datetime
 import json
+import os
 import zipfile
 
 from pkg.private import build_info
@@ -108,6 +109,32 @@ def _add_manifest_entry(options, zip_file, entry, default_mode, ts):
     # Set directory bits
     entry_info.external_attr |= (UNIX_DIR_BIT << 16) | MSDOS_DIR_BIT
     zip_file.writestr(entry_info, '')
+  elif entry_type == manifest.ENTRY_IS_TREE:
+    # TODO(#440) Collision detection for paths from tree artifacts?
+    # (Starlark rules prevent duplicate destination entries from appearing
+    # in the manifest.)
+    # TODO(#440) Do we need to include (empty) directory entries?
+    # TODO(#440) Do we want to cross-reference any directories against
+    # ENTRY_IS_DIR items so that permissions are applied from the latter?
+    for walk_root, dirs, files in os.walk(src):
+      # "[...] the caller can modify the dirnames list in-place [...];
+      # subdirectories whose names remain in dirnames; this can be used to
+      # [...] impose a specific order of visiting"
+      # -- https://docs.python.org/3/library/os.html#os.walk
+      dirs.sort()  # sort for determinism
+      rel_root = os.path.relpath(walk_root, src).replace(os.path.sep, '/')
+      if rel_root == ".":  # store members as `a` rather than `./a`
+        rel_root = ""
+      for f in sorted(files):  # sort for determinism
+        fake_entry = manifest.ManifestEntry(
+          entry_type = manifest.ENTRY_IS_FILE,
+          dest = os.path.join(dst_path, rel_root, f).replace(os.path.sep, '/'),
+          src = os.path.join(walk_root, f),
+          mode = mode,
+          user = user,
+          group = group,
+        )
+        _add_manifest_entry(options, zip_file, fake_entry, default_mode, ts)
   # TODO(#309): All the rest
 
 def main(args):
@@ -130,3 +157,5 @@ def main(args):
 if __name__ == '__main__':
   arg_parser = _create_argument_parser()
   main(arg_parser.parse_args())
+
+# vim: set sw=2 sw=2:

tests/BUILD

@@ -323,6 +323,11 @@ filegroup(
     "/abc/def/",
 ])]
 
+pkg_zip(
+    name = "test_zip_tree",
+    srcs = [":generate_tree"],
+)
+
 py_test(
     name = "zip_test",
     srcs = [
@@ -338,6 +343,7 @@ py_test(
         "test-zip-strip_prefix-none.zip",
         "test-zip-strip_prefix-zipcontent.zip",
         "test-zip-strip_prefix-dot.zip",
+        "test_zip_tree.zip",
 
         # these could be replaced with diff_test() rules (from skylib)
         "test_zip_basic_timestamp_before_epoch.zip",

tests/zip_test.py

@@ -20,6 +20,7 @@
 from bazel_tools.tools.python.runfiles import runfiles
 
 HELLO_CRC = 2069210904
+HELLOTHERE_CRC = 1829955920
 LOREM_CRC = 2178844372
 EXECUTABLE_CRC = 342626072
 
@@ -130,6 +131,15 @@ def test_zip_strip_prefix_dot(self):
         {"filename": "zipcontent/loremipsum.txt", "crc": LOREM_CRC},
     ])
 
+  def test_zip_tree_artifact(self):
+    self.assertZipFileContent("test_zip_tree.zip", [
+        {"filename": "a/a", "crc": HELLOTHERE_CRC},
+        {"filename": "a/b/c", "crc": HELLOTHERE_CRC},
+        {"filename": "b/d", "crc": HELLOTHERE_CRC},
+        {"filename": "b/e", "crc": HELLOTHERE_CRC},
+        {"filename": "b/c/d", "crc": HELLOTHERE_CRC},
+    ])
+
 
 class ZipEquivalency(ZipTest):
   """Check that some generated zip files are equivalent to each-other."""
@@ -169,3 +179,5 @@ def test_package_dir2(self):
 
 if __name__ == "__main__":
   unittest.main()
+
+# vim: set sw=2 sw=2: