Get rid of the long deprecated archive_name and extension from pkg_zip.

- Make 'out' work in a reasonable way. Fixes bazelbuild#414 - Partial fix for bazelbuild#284
aiuto · Mar 1, 2022 · 27360d1 · 27360d1
1 parent 6396f9a
commit 27360d1
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 63 deletions.
diff --git a/docs/latest.md b/docs/latest.md
@@ -308,32 +308,8 @@ Rules for manipulation of various packaging.
 ## pkg_zip
 
 <pre>
-pkg_zip(<a href="#pkg_zip-name">name</a>, <a href="#pkg_zip-kwargs">kwargs</a>)
-</pre>
-
-Creates a .zip file. See pkg_zip_impl.
-
-**PARAMETERS**
-
-
-| Name  | Description | Default Value |
-| :------------- | :------------- | :------------- |
-| <a id="pkg_zip-name"></a>name |   -    |  none |
-| <a id="pkg_zip-kwargs"></a>kwargs |   -    |  none |
-
-
-
-<!-- Generated with Stardoc: http://skydoc.bazel.build -->
-
-Rules for manipulation of various packaging.
-
-<a id="#pkg_zip_impl"></a>
-
-## pkg_zip_impl
-
-<pre>
-pkg_zip_impl(<a href="#pkg_zip_impl-name">name</a>, <a href="#pkg_zip_impl-mode">mode</a>, <a href="#pkg_zip_impl-out">out</a>, <a href="#pkg_zip_impl-package_dir">package_dir</a>, <a href="#pkg_zip_impl-package_file_name">package_file_name</a>, <a href="#pkg_zip_impl-package_variables">package_variables</a>,
-             <a href="#pkg_zip_impl-private_stamp_detect">private_stamp_detect</a>, <a href="#pkg_zip_impl-srcs">srcs</a>, <a href="#pkg_zip_impl-stamp">stamp</a>, <a href="#pkg_zip_impl-strip_prefix">strip_prefix</a>, <a href="#pkg_zip_impl-timestamp">timestamp</a>)
+pkg_zip(<a href="#pkg_zip-name">name</a>, <a href="#pkg_zip-mode">mode</a>, <a href="#pkg_zip-out">out</a>, <a href="#pkg_zip-package_dir">package_dir</a>, <a href="#pkg_zip-package_file_name">package_file_name</a>, <a href="#pkg_zip-package_variables">package_variables</a>,
+             <a href="#pkg_zip-private_stamp_detect">private_stamp_detect</a>, <a href="#pkg_zip-srcs">srcs</a>, <a href="#pkg_zip-stamp">stamp</a>, <a href="#pkg_zip-strip_prefix">strip_prefix</a>, <a href="#pkg_zip-timestamp">timestamp</a>)
 </pre>
 
 
@@ -343,17 +319,17 @@ pkg_zip_impl(<a href="#pkg_zip_impl-name">name</a>, <a href="#pkg_zip_impl-mode"
 
 | Name  | Description | Type | Mandatory | Default |
 | :------------- | :------------- | :------------- | :------------- | :------------- |
-| <a id="pkg_zip_impl-name"></a>name |  A unique name for this target.   | <a href="https://bazel.build/docs/build-ref.html#name">Name</a> | required |  |
-| <a id="pkg_zip_impl-mode"></a>mode |  The default mode for all files in the archive.   | String | optional | "0555" |
-| <a id="pkg_zip_impl-out"></a>out |  -   | <a href="https://bazel.build/docs/build-ref.html#labels">Label</a> | required |  |
-| <a id="pkg_zip_impl-package_dir"></a>package_dir |  The prefix to add to all all paths in the archive.   | String | optional | "/" |
-| <a id="pkg_zip_impl-package_file_name"></a>package_file_name |  See Common Attributes   | String | optional | "" |
-| <a id="pkg_zip_impl-package_variables"></a>package_variables |  See Common Attributes   | <a href="https://bazel.build/docs/build-ref.html#labels">Label</a> | optional | None |
-| <a id="pkg_zip_impl-private_stamp_detect"></a>private_stamp_detect |  -   | Boolean | optional | False |
-| <a id="pkg_zip_impl-srcs"></a>srcs |  List of files that should be included in the archive.   | <a href="https://bazel.build/docs/build-ref.html#labels">List of labels</a> | optional | [] |
-| <a id="pkg_zip_impl-stamp"></a>stamp |  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. <li>stamp = 0: Use an "epoch" time for the modification time of each file. This gives good build result caching. <li>stamp = -1: Control the chosen modification time using the --[no]stamp flag.   | Integer | optional | 0 |
-| <a id="pkg_zip_impl-strip_prefix"></a>strip_prefix |  -   | String | optional | "" |
-| <a id="pkg_zip_impl-timestamp"></a>timestamp |  Time stamp to place on all files in the archive, expressed as seconds since the Unix Epoch, as per RFC 3339.  The default is January 01, 1980, 00:00 UTC.<br><br>Due to limitations in the format of zip files, values before Jan 1, 1980 will be rounded up and the precision in the zip file is limited to a granularity of 2 seconds.   | Integer | optional | 315532800 |
+| <a id="pkg_zip-name"></a>name |  A unique name for this target.   | <a href="https://bazel.build/docs/build-ref.html#name">Name</a> | required |  |
+| <a id="pkg_zip-mode"></a>mode |  The default mode for all files in the archive.   | String | optional | "0555" |
+| <a id="pkg_zip-out"></a>out |  output file name. Default: name + ".zip".   | <a href="https://bazel.build/docs/build-ref.html#labels">Label</a> | required |  |
+| <a id="pkg_zip-package_dir"></a>package_dir |  The prefix to add to all all paths in the archive.   | String | optional | "/" |
+| <a id="pkg_zip-package_file_name"></a>package_file_name |  See Common Attributes   | String | optional | "" |
+| <a id="pkg_zip-package_variables"></a>package_variables |  See Common Attributes   | <a href="https://bazel.build/docs/build-ref.html#labels">Label</a> | optional | None |
+| <a id="pkg_zip-private_stamp_detect"></a>private_stamp_detect |  -   | Boolean | optional | False |
+| <a id="pkg_zip-srcs"></a>srcs |  List of files that should be included in the archive.   | <a href="https://bazel.build/docs/build-ref.html#labels">List of labels</a> | optional | [] |
+| <a id="pkg_zip-stamp"></a>stamp |  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. <li>stamp = 0: Use an "epoch" time for the modification time of each file. This gives good build result caching. <li>stamp = -1: Control the chosen modification time using the --[no]stamp flag.   | Integer | optional | 0 |
+| <a id="pkg_zip-strip_prefix"></a>strip_prefix |  -   | String | optional | "" |
+| <a id="pkg_zip-timestamp"></a>timestamp |  Time stamp to place on all files in the archive, expressed as seconds since the Unix Epoch, as per RFC 3339.  The default is January 01, 1980, 00:00 UTC.<br><br>Due to limitations in the format of zip files, values before Jan 1, 1980 will be rounded up and the precision in the zip file is limited to a granularity of 2 seconds.   | Integer | optional | 315532800 |
 
 
 

diff --git a/pkg/private/zip/zip.bzl b/pkg/private/zip/zip.bzl
@@ -138,7 +138,10 @@ limited to a granularity of 2 seconds.""",
         ),
 
         # Common attributes
-        "out": attr.output(mandatory = True),
+        "out": attr.output(
+            doc = """output file name. Default: name + ".zip".""",
+            mandatory = True,
+        ),
         "package_file_name": attr.string(doc = "See Common Attributes"),
         "package_variables": attr.label(
             doc = "See Common Attributes",
@@ -168,27 +171,19 @@ limited to a granularity of 2 seconds.""",
     provides = [PackageArtifactInfo],
 )
 
-def pkg_zip(name, **kwargs):
-    """Creates a .zip file. See pkg_zip_impl."""
-    extension = kwargs.pop("extension", None)
-    if extension:
-        # buildifier: disable=print
-        print("'extension' is deprecated. Use 'package_file_name' or 'out' to name the output.")
-    else:
-        extension = "zip"
-    archive_name = kwargs.pop("archive_name", None)
-    if archive_name:
-        if kwargs.get("package_file_name"):
-            fail("You may not set both 'archive_name' and 'package_file_name'.")
-
-        # buildifier: disable=print
-        print("archive_name is deprecated. Use package_file_name instead.")
-        kwargs["package_file_name"] = archive_name + "." + extension
-    else:
-        archive_name = name
+def pkg_zip(name, out = None, **kwargs):
+    """Creates a .zip file.
+
+    @wraps(pkg_zip_impl)
+
+    Args:
+      out: output file name. Default: name + ".zip".
+    """
+    if not out:
+        out = name + ".zip"
     pkg_zip_impl(
         name = name,
-        out = archive_name + "." + extension,
+        out = out,
         private_stamp_detect = select({
             _stamp_condition: True,
             "//conditions:default": False,

diff --git a/tests/zip/BUILD b/tests/zip/BUILD
@@ -79,9 +79,14 @@ pkg_zip(
 
 # The contents of this file should equal test_zip_empty
 pkg_zip(
-    name = "test_zip_empty_different_extension",
-    srcs = [],
-    extension = "otherkindofzip",
+    name = "test_zip_basic_renamed",
+    srcs = [
+        ":dirs",
+        ":link",
+        "//tests:testdata/hello.txt",
+        "//tests:testdata/loremipsum.txt",
+    ],
+    package_file_name = "test_zip_basic_renamed.foo",
 )
 
 pkg_zip(
@@ -187,6 +192,7 @@ py_test(
     data = [
         "test_zip_empty.zip",
         "test_zip_basic.zip",
+        "test_zip_basic_renamed",
         "test_zip_package_dir0.zip",
         "test_zip_timestamp.zip",
         "test_zip_permissions.zip",
@@ -197,7 +203,6 @@ py_test(
 
         # these could be replaced with diff_test() rules (from skylib)
         "test_zip_basic_timestamp_before_epoch.zip",
-        "test_zip_empty_different_extension.otherkindofzip",
         "test_zip_package_dir1.zip",
         "test_zip_package_dir2.zip",
     ],

diff --git a/tests/zip/zip_test.py b/tests/zip/zip_test.py
@@ -153,8 +153,8 @@ def test_small_timestamp(self):
 
   def test_extension(self):
     self.assertFilesEqual(
-        "test_zip_empty_different_extension.otherkindofzip",
-        "test_zip_empty.zip",
+        "test_zip_basic_renamed.foo",
+        "test_zip_basic.zip",
     )
 
   def test_package_dir1(self):