Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jan 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 46 additions & 19 deletions internal/pkg_npm/pkg_npm.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -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(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can it be an attr.output ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh wait I see, this attribute isn't actually used? this is just here to document how the macro works? smelly

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👃 Yeah, perhaps we refactor it like you suggested to be like ts_project macro, for another PR as that seems an independent change

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.""",
Expand Down Expand Up @@ -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
Expand All @@ -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"),
)
31 changes: 31 additions & 0 deletions internal/pkg_npm/test/tgz_out/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -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",
],
)
1 change: 1 addition & 0 deletions internal/pkg_npm/test/tgz_out/main.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const MAIN = 'MAIN';
5 changes: 5 additions & 0 deletions internal/pkg_npm/test/tgz_out/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"name": "awesome-package",
"private": true,
"version": "0.0.0-PLACEHOLDER"
}
23 changes: 23 additions & 0 deletions internal/pkg_npm/test/tgz_out/test.sh
Original file line number Diff line number Diff line change
@@ -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"
4 changes: 4 additions & 0 deletions tools/defaults.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down