Initial support for pkg_files* in pkg_zip

- This is the minimal PR to switch to the newer style. That is, only plain files.
- Followup CLs will add things previously not supported, such as links.

pkg/pkg.bzl

@@ -579,10 +579,43 @@ def _pkg_zip_impl(ctx):
         inputs.append(ctx.version_file)
 
     data_path = compute_data_path(ctx, ctx.attr.strip_prefix)
-    for f in ctx.files.srcs:
-        arg = "%s=%s" % (_quote(f.path), dest_path(f, data_path))
-        args.add(arg)
+    data_path_without_prefix = compute_data_path(ctx, ".")
 
+    content_map = {}  # content handled in the manifest
+    for src in ctx.attr.srcs:
+        # Gather the files for every srcs entry here, even if it is not from
+        # a pkg_* rule.
+        if DefaultInfo in src:
+            inputs.extend(src[DefaultInfo].files.to_list())
+        if not process_src(
+            content_map,
+            src,
+            src.label,
+            default_mode = None,
+            default_user = None,
+            default_group = None,
+        ):
+            # Add in the files of srcs which are not pkg_* types
+            for f in src.files.to_list():
+                d_path = dest_path(f, data_path, data_path_without_prefix)
+                if f.is_directory:
+                    # Tree artifacts need a name, but the name is never really
+                    # the important part. The likely behavior people want is
+                    # just the content, so we strip the directory name.
+                    dest = '/'.join(d_path.split('/')[0:-1])
+                    add_tree_artifact(content_map, dest, f, src.label)
+                else:
+                    # Note: This extra remap is the bottleneck preventing this
+                    # large block from being a utility method as shown below.
+                    # Should we disallow mixing pkg_files in srcs with remap?
+                    # I am fine with that if it makes the code more readable.
+                    # dest = _remap(remap_paths, d_path)
+                    add_single_file(content_map, d_path, f, src.label)
+
+    manifest_file = ctx.actions.declare_file(ctx.label.name + ".manifest")
+    inputs.append(manifest_file)
+    write_manifest(ctx, manifest_file, content_map)
+    args.add("--manifest", manifest_file.path)
     args.set_param_file_format("multiline")
     args.use_param_file("@%s")
 

pkg/private/build_zip.py

@@ -15,13 +15,20 @@
 
 import argparse
 import datetime
+import json
 import zipfile
 
 from rules_pkg.private import build_info
 from rules_pkg.private import helpers
 
 ZIP_EPOCH = 315532800
 
+# These must be kept in sync with the values from private/pkg_files.bzl
+ENTRY_IS_FILE = 0  # Entry is a file: take content from <src>
+ENTRY_IS_LINK = 1  # Entry is a symlink: dest -> <src>
+ENTRY_IS_DIR = 2  # Entry is an empty dir
+ENTRY_IS_TREE = 3  # Entry is a tree artifact: take tree from <src>
+
 
 def _create_argument_parser():
   """Creates the command line arg parser."""
@@ -41,6 +48,8 @@ def _create_argument_parser():
   parser.add_argument(
       '-m', '--mode',
       help='The file system mode to use for files added into the zip.')
+  parser.add_argument('--manifest',
+                      help='manifest of contents to add to the layer.')
   parser.add_argument(
       'files', type=str, nargs='*',
       help='Files to be added to the zip, in the form of {srcpath}={dstpath}.')
@@ -60,6 +69,31 @@ def parse_date(ts):
   ts = datetime.datetime.utcfromtimestamp(ts)
   return (ts.year, ts.month, ts.day, ts.hour, ts.minute, ts.second)
 
+def _add_manifest_entry(options, zip_file, entry, default_mode, ts):
+  """Add an entry to the zip file.
+
+  Args:
+    options: parsed options
+    zip_file: ZipFile to write to
+    entry: manifest entry
+    default_mode: (int) file mode to use if not specified in the entry.
+    ts: (int) time stamp to add to files
+  """
+
+  entry_type, dest, src, mode, user, group = entry
+
+  # Use the pkg_tar mode/owner remaping as a fallback
+  non_abs_path = dest.strip('/')
+  dst_path = _combine_paths(options.directory, non_abs_path)
+  entry_info = zipfile.ZipInfo(filename=dst_path, date_time=ts)
+  if default_mode:
+    entry_info.external_attr = default_mode << 16
+  entry_info.compress_type = zipfile.ZIP_DEFLATED
+
+  if entry_type == ENTRY_IS_FILE:
+    with open(src, 'rb') as src:
+      zip_file.writestr(entry_info, src.read())
+  # TODO(aiuto): All the rest
 
 def main(args):
   unix_ts = max(ZIP_EPOCH, args.timestamp)
@@ -71,23 +105,12 @@ def main(args):
     default_mode = int(args.mode, 8)
 
   with zipfile.ZipFile(args.output, 'w') as zip_file:
-    for f in args.files or []:
-      (src_path, dst_path) = helpers.SplitNameValuePairAtSeparator(f, '=')
-
-      dst_path = _combine_paths(args.directory, dst_path)
-
-      entry_info = zipfile.ZipInfo(filename=dst_path, date_time=ts)
-
-      if default_mode:
-        entry_info.external_attr = default_mode << 16
-
-      entry_info.compress_type = zipfile.ZIP_DEFLATED
+    if args.manifest:
+      with open(args.manifest, 'r') as manifest_fp:
+        manifest = json.load(manifest_fp)
+        for entry in manifest:
+          _add_manifest_entry(args, zip_file, entry, default_mode, ts)
 
-      # the zipfile library doesn't support adding a file by path with write()
-      # and specifying a ZipInfo at the same time.
-      with open(src_path, 'rb') as src:
-        data = src.read()
-        zip_file.writestr(entry_info, data)
 
 if __name__ == '__main__':
   arg_parser = _create_argument_parser()