fix(builtin): only generate a .tar pkg_npm output when requested (#2428)

Author: mattem

internal/pkg_npm/pkg_npm.bzl

@@ -71,9 +71,18 @@ You can pass arguments to npm by escaping them from Bazel using a double-hyphen,
 
 `bazel run my_package.publish -- --tag=next`
 
-It is also possible to use the resulting tar file file from the `.pack` as an action input via the `.tar` label:
+It is also possible to use the resulting tar file file from the `.pack` as an action input via the `.tar` label.
+To make use of this label, the `tgz` attribute must be set, and the generating `pkg_npm` rule must have a valid `package.json` file
+as part of its sources:
 
 ```python
+pkg_npm(
+    name = "my_package",
+    srcs = ["package.json"],
+    deps = [":my_typescript_lib"],
+    tgz = "my_package.tgz",
+)
+
 my_rule(
     name = "foo",
     srcs = [
@@ -109,6 +118,12 @@ You can use values from the workspace status command using curly braces, for exa
 See the section on stamping in the [README](stamping)
 """,
     ),
+    "tgz": attr.string(
+        doc = """If set, will create a `.tgz` file that can be used as an input to another rule, the tar will be given the name assigned to this attribute.
+
+        NOTE: If this attribute is set, a valid `package.json` file must be included in the sources of this target
+        """,
+    ),
     "vendor_external": attr.string_list(
         doc = """External workspaces whose contents should be vendored into this workspace.
         Avoids `external/foo` path segments in the resulting package.""",
@@ -313,7 +328,14 @@ pkg_npm = rule(
     outputs = PKG_NPM_OUTPUTS,
 )
 
-def pkg_npm_macro(name, **kwargs):
+def pkg_npm_macro(name, tgz = None, **kwargs):
+    """Wrapper macro around pkg_npm
+
+    Args:
+        name: Unique name for this target
+        tgz: If provided, creates a `.tar` target that can be used as an action input version of `.pack`
+        **kwargs: All other args forwarded to pkg_npm
+    """
     pkg_npm(
         name = name,
         **kwargs
@@ -335,20 +357,25 @@ def pkg_npm_macro(name, **kwargs):
         }),
     )
 
-    native.genrule(
-        name = "%s.tar" % name,
-        outs = ["%s.tgz" % name],
-        cmd = "$(location :%s.pack) | xargs -I {} cp {} $@" % name,
-        # NOTE(mattem): on windows, it seems to output a buch of other stuff on stdout when piping, so pipe to tail
-        # and grab the last line
-        cmd_bat = "$(location :%s.pack) | tail -1 | xargs -I {} cp {} $@" % name,
-        tools = [
-            ":%s.pack" % name,
-        ],
-        # tagged as manual so this doesn't case two actions for each input with builds for "host" (as used as a tool)
-        tags = [
-            "local",
-            "manual",
-        ],
-        visibility = kwargs.get("visibility"),
-    )
+    if tgz != None:
+        if not tgz.endswith(".tgz"):
+            fail("tgz output for pkg_npm %s must produce a .tgz file" % name)
+
+        native.genrule(
+            name = "%s.tar" % name,
+            outs = [tgz],
+            cmd = "$(location :%s.pack) | xargs -I {} cp {} $@" % name,
+            # NOTE(mattem): on windows, it seems to output a buch of other stuff on stdout when piping, so pipe to tail
+            # and grab the last line
+            cmd_bat = "$(location :%s.pack) | tail -1 | xargs -I {} cp {} $@" % name,
+            srcs = [
+                name,
+            ],
+            tools = [
+                ":%s.pack" % name,
+            ],
+            tags = [
+                "local",
+            ],
+            visibility = kwargs.get("visibility"),
+        )

internal/pkg_npm/test/tgz_out/BUILD.bazel

@@ -0,0 +1,31 @@
+load("@build_bazel_rules_nodejs//:index.bzl", "pkg_npm")
+
+pkg_npm(
+    name = "pkg",
+    srcs = [
+        "main.js",
+        "package.json",
+    ],
+    tgz = "my_tar.tgz",
+)
+
+sh_test(
+    name = "pkg_test",
+    size = "small",
+    srcs = [
+        "test.sh",
+    ],
+    data = [
+        "my_tar.tgz",
+        "//third_party/github.com/bazelbuild/bazel-skylib:tests/unittest.bash",
+    ],
+    # Disabled on windows due to how tar interprets the colons in file paths as remotes, and the --force-local
+    # only being an option in GNU tar...
+    # This feature has additional coverage as it used by all bazel tests for examples
+    tags = [
+        "fix-windows",
+    ],
+    deps = [
+        "@build_bazel_rules_nodejs//third_party/github.com/bazelbuild/bazel/tools/bash/runfiles",
+    ],
+)

internal/pkg_npm/test/tgz_out/main.js

@@ -0,0 +1 @@
+export const MAIN = 'MAIN';

internal/pkg_npm/test/tgz_out/package.json

@@ -0,0 +1,5 @@
+{
+    "name": "awesome-package",
+    "private": true,
+    "version": "0.0.0-PLACEHOLDER"
+}

internal/pkg_npm/test/tgz_out/test.sh

@@ -0,0 +1,23 @@
+# --- begin runfiles.bash initialization v2 ---
+# Copy-pasted from the Bazel Bash runfiles library v2.
+set -uo pipefail; f=build_bazel_rules_nodejs/third_party/github.com/bazelbuild/bazel/tools/bash/runfiles/runfiles.bash
+source "${RUNFILES_DIR:-/dev/null}/$f" 2>/dev/null || \
+  source "$(grep -sm1 "^$f " "${RUNFILES_MANIFEST_FILE:-/dev/null}" | cut -f2- -d' ')" 2>/dev/null || \
+  source "$0.runfiles/$f" 2>/dev/null || \
+  source "$(grep -sm1 "^$f " "$0.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \
+  source "$(grep -sm1 "^$f " "$0.exe.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \
+  { echo>&2 "ERROR: cannot find $f"; exit 1; }; f=; set -e
+# --- end runfiles.bash initialization v2 ---
+
+source "$(rlocation build_bazel_rules_nodejs/third_party/github.com/bazelbuild/bazel-skylib/tests/unittest.bash)" \
+  || { echo "Could not source build_bazel_rules_nodejs/third_party/github.com/bazelbuild/bazel-skylib/tests/unittest.bash" >&2; exit 1; }
+
+function test_tgz_package_json() {
+    TGZ=$(rlocation build_bazel_rules_nodejs/internal/pkg_npm/test/tgz_out/my_tar.tgz)
+    tar -vxf "${TGZ}"
+    
+    assert_contains "awesome-package" "./package/package.json"
+    assert_contains "MAIN" "./package/main.js"
+}
+
+run_suite "test_tgz_package_json"

tools/defaults.bzl

@@ -55,8 +55,12 @@ def pkg_npm(**kwargs):
         "0.0.0-PLACEHOLDER": "{STABLE_BUILD_SCM_VERSION}",
     })
 
+    name = kwargs.pop("name")
+
     # Call through to the rule with our defaults set
     _pkg_npm(
+        name = name,
+        tgz = "%s.tgz" % name,
         deps = deps,
         substitutions = select({
             "@build_bazel_rules_nodejs//internal:stamp": stamped_substitutions,