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.

Sign up for GitHub

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

Jump to bottom

feat(core): patch bazel-skylib for running npm in core #3008

Merged
merged 1 commit into from
Oct 11, 2021
Merged

feat(core): patch bazel-skylib for running npm in core #3008

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
90 changes: 88 additions & 2 deletions e2e/core/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ write_file(
genrule(
name = "use_node_toolchain",
srcs = ["some.js"],
outs = ["genrule_out"],
outs = ["actual1"],
cmd = "$(NODE_PATH) $(execpath some.js) $@",
toolchains = ["@node16_toolchains//:resolved_toolchain"],
tools = ["@node16_toolchains//:resolved_toolchain"],
Expand All @@ -55,7 +55,7 @@ genrule(
diff_test(
name = "test_genrule",
file1 = "expected",
file2 = "genrule_out",
file2 = "actual1",
)

# Here, my_nodejs is a fake for something like nodejs_binary or
Expand All @@ -72,3 +72,89 @@ diff_test(
file1 = "expected",
file2 = "thing",
)

##########################################################
# Call a program from npm to transform inputs to bazel-out

# For using acorn as our test fixture, this is
# the serialized AST for the JS program just containing a literal "1"
write_file(
name = "write_expected_ast",
out = "expected_ast.json",
content = [
"""{"type":"Program","start":0,"end":1,"body":[{"type":"ExpressionStatement","start":0,"end":1,"expression":{"type":"Literal","start":0,"end":1,"value":1,"raw":"1"}}],"sourceType":"script"}""",
"",
],
)

write_file(
name = "write_one",
out = "one.js",
content = ["1"],
)

genrule(
name = "call_acorn",
srcs = ["one.js"],
outs = ["actual2"],
cmd = """
$(NODE_PATH) \\
./$(execpath @npm_acorn-8.5.0)/bin/acorn \\
--compact \\
$(execpath one.js) \\
> $@""",
toolchains = ["@node16_toolchains//:resolved_toolchain"],
tools = [
"@node16_toolchains//:resolved_toolchain",
"@npm_acorn-8.5.0",
],
)

diff_test(
name = "test_acorn",
file1 = "actual2",
file2 = "expected_ast.json",
)

################################################
# Run a program that requires a package from npm

write_file(
name = "write_program",
out = "require_acorn.js",
content = [
"const fs = require('fs')",
"const acorn = require('acorn')",
"fs.writeFileSync(process.argv[2], JSON.stringify(acorn.parse('1', {ecmaVersion: 2020})) + '\\n')",
],
)

genrule(
name = "require_acorn",
srcs = ["require_acorn.js"],
outs = ["actual3"],
# Note: confusingly, node uses an environment variable NODE_PATH as a "global"
# location for module resolutions, but we used the same name for the Make
# variable exposed by the nodejs tooling.
# One is interpreted by the bash shell, while the other is interpreted by
# bazel, so it doesn't cause any problems.
# Note, the trailing "/.." on the NODE_PATH variable is because our target
# points to the output directory we wrote, named "acorn", but node needs
# to start its module search in a directory *containing" one called "acorn"
cmd = """
NODE_PATH=./$(execpath @npm_acorn-8.5.0)/.. \\
$(NODE_PATH) \\
./$(execpath require_acorn.js) \\
$@""",
toolchains = ["@node16_toolchains//:resolved_toolchain"],
tools = [
"@node16_toolchains//:resolved_toolchain",
"@npm_acorn-8.5.0",
],
)

diff_test(
name = "test_require_acorn",
file1 = "actual3",
file2 = "expected_ast.json",
)
21 changes: 21 additions & 0 deletions e2e/core/WORKSPACE
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,24 @@ nodejs_register_toolchains(
name = "node16",
node_version = "16.9.0",
)

http_archive(
name = "npm_acorn-8.5.0",
build_file_content = """
load("@rules_nodejs//third_party/github.com/bazelbuild/bazel-skylib:rules/copy_file.bzl", "copy_file")

# Turn a source directory into a TreeArtifact for RBE-compat
copy_file(
name = "npm_acorn-8.5.0",
src = "package",
# This attribute comes from rules_nodejs patch of
# https://github.com/bazelbuild/bazel-skylib/pull/323
is_directory = True,
# We must give this as the directory in order for it to appear on NODE_PATH
out = "acorn",
visibility = ["//visibility:public"],
)
""",
sha256 = "d8f9d40c4656537a60bf0c6daae6f0553f54df5ff2518f86464b7c785f20376b",
urls = ["https://registry.npmjs.org/acorn/-/acorn-8.5.0.tgz"],
)
125 changes: 94 additions & 31 deletions third_party/github.com/bazelbuild/bazel-skylib/rules/private/copy_file_private.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,38 +12,58 @@
# See the License for the specific language governing permissions and
# limitations under the License.

# LOCAL MODIFICATIONS:
# this has two PRs patched in:
# https://github.com/bazelbuild/bazel-skylib/pull/323
# https://github.com/bazelbuild/bazel-skylib/pull/324

"""Implementation of copy_file macro and underlying rules.

These rules copy a file to another location using Bash (on Linux/macOS) or
cmd.exe (on Windows). '_copy_xfile' marks the resulting file executable,
'_copy_file' does not.
These rules copy a file or directory to another location using Bash (on Linux/macOS) or
cmd.exe (on Windows). `_copy_xfile` marks the resulting file executable,
`_copy_file` does not.
"""

# BEGIN LOCAL MOD
# Hints for Bazel spawn strategy
_execution_requirements = {
# Copying files is entirely IO-bound and there is no point doing this work remotely.
# Also, remote-execution does not allow source directory inputs, see
# https://github.com/bazelbuild/bazel/commit/c64421bc35214f0414e4f4226cc953e8c55fa0d2
# So we must not attempt to execute remotely in that case.
"no-remote-exec": "1",
}

def _hash_file(file):
return str(hash(file.path))

# END LOCAL MOD

# buildifier: disable=function-docstring
def copy_cmd(ctx, src, dst):
# Most Windows binaries built with MSVC use a certain argument quoting
# scheme. Bazel uses that scheme too to quote arguments. However,
# cmd.exe uses different semantics, so Bazel's quoting is wrong here.
# To fix that we write the command to a .bat file so no command line
# quoting or escaping is required.

# BEGIN LOCAL MOD
# Put a hash of the file name into the name of the generated batch file to
# make it unique within the pacakge since rules such as copy_to_bin and
# js_library will use copy_cmd multiple times for multiple files.
# make it unique within the package, so that users can define multiple copy_file's.
bat = ctx.actions.declare_file("%s-%s-cmd.bat" % (ctx.label.name, _hash_file(src)))
# END LOCAL MOD

# Flags are documented at
# https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/copy
# https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/xcopy
if dst.is_directory:
cmd_tmpl = "@xcopy \"%s\" \"%s\\\" /V /E /H /Y /Q >NUL"
mnemonic = "CopyDirectory"
progress_message = "Copying directory"
else:
cmd_tmpl = "@copy /Y \"%s\" \"%s\" >NUL"
mnemonic = "CopyFile"
progress_message = "Copying file"

ctx.actions.write(
output = bat,
# Do not use lib/shell.bzl's shell.quote() method, because that uses
# Bash quoting syntax, which is different from cmd.exe's syntax.
content = "@copy /Y \"%s\" \"%s\" >NUL" % (
content = cmd_tmpl % (
src.path.replace("/", "\\"),
dst.path.replace("/", "\\"),
),
Expand All @@ -55,75 +75,116 @@ def copy_cmd(ctx, src, dst):
outputs = [dst],
executable = "cmd.exe",
arguments = ["/C", bat.path.replace("/", "\\")],
mnemonic = "CopyFile",
progress_message = "Copying files",
mnemonic = mnemonic,
progress_message = progress_message,
use_default_shell_env = True,
execution_requirements = _execution_requirements,
)

# buildifier: disable=function-docstring
def copy_bash(ctx, src, dst):
if dst.is_directory:
cmd_tmpl = "rm -rf \"$2\" && cp -rf \"$1/\" \"$2\""
mnemonic = "CopyDirectory"
progress_message = "Copying directory"
else:
cmd_tmpl = "cp -f \"$1\" \"$2\""
mnemonic = "CopyFile"
progress_message = "Copying file"

ctx.actions.run_shell(
tools = [src],
outputs = [dst],
command = "cp -f \"$1\" \"$2\"",
command = cmd_tmpl,
arguments = [src.path, dst.path],
mnemonic = "CopyFile",
progress_message = "Copying files",
mnemonic = mnemonic,
progress_message = progress_message,
use_default_shell_env = True,
execution_requirements = _execution_requirements,
)

def _copy_file_impl(ctx):
# When creating a directory, declare that to Bazel so downstream rules
# see it as a TreeArtifact and handle correctly, e.g. for remote execution
if getattr(ctx.attr, "is_directory", False):
output = ctx.actions.declare_directory(ctx.attr.out)
else:
output = ctx.outputs.out
if ctx.attr.allow_symlink:
if output.is_directory:
fail("Cannot use both is_directory and allow_symlink")
ctx.actions.symlink(
output = ctx.outputs.out,
output = output,
target_file = ctx.file.src,
is_executable = ctx.attr.is_executable,
)
elif ctx.attr.is_windows:
copy_cmd(ctx, ctx.file.src, ctx.outputs.out)
copy_cmd(ctx, ctx.file.src, output)
else:
copy_bash(ctx, ctx.file.src, ctx.outputs.out)
copy_bash(ctx, ctx.file.src, output)

files = depset(direct = [ctx.outputs.out])
runfiles = ctx.runfiles(files = [ctx.outputs.out])
files = depset(direct = [output])
runfiles = ctx.runfiles(files = [output])
if ctx.attr.is_executable:
return [DefaultInfo(files = files, runfiles = runfiles, executable = ctx.outputs.out)]
return [DefaultInfo(files = files, runfiles = runfiles, executable = output)]
else:
return [DefaultInfo(files = files, runfiles = runfiles)]

# @unsorted-dict-items
_ATTRS = {
"src": attr.label(mandatory = True, allow_single_file = True),
"out": attr.output(mandatory = True),
"is_windows": attr.bool(mandatory = True),
"is_executable": attr.bool(mandatory = True),
"allow_symlink": attr.bool(mandatory = True),
}

_copy_directory = rule(
implementation = _copy_file_impl,
provides = [DefaultInfo],
attrs = dict(_ATTRS, **{
"is_directory": attr.bool(default = True),
# Cannot declare out as an output here, because there's no API for declaring
# TreeArtifact outputs.
"out": attr.string(mandatory = True),
}),
)

_copy_file = rule(
implementation = _copy_file_impl,
provides = [DefaultInfo],
attrs = _ATTRS,
attrs = dict(_ATTRS, **{
"out": attr.output(mandatory = True),
}),
)

_copy_xfile = rule(
implementation = _copy_file_impl,
executable = True,
provides = [DefaultInfo],
attrs = _ATTRS,
attrs = dict(_ATTRS, **{
"out": attr.output(mandatory = True),
}),
)

def copy_file(name, src, out, is_executable = False, allow_symlink = False, **kwargs):
"""Copies a file to another location.
def copy_file(name, src, out, is_directory = False, is_executable = False, allow_symlink = False, **kwargs):
"""Copies a file or directory to another location.

`native.genrule()` is sometimes used to copy files (often wishing to rename them). The 'copy_file' rule does this with a simpler interface than genrule.

This rule uses a Bash command on Linux/macOS/non-Windows, and a cmd.exe command on Windows (no Bash is required).

If using this rule with source directories, it is recommended that you use the
`--host_jvm_args=-DBAZEL_TRACK_SOURCE_DIRECTORIES=1` startup option so that changes
to files within source directories are detected. See
https://github.com/bazelbuild/bazel/commit/c64421bc35214f0414e4f4226cc953e8c55fa0d2
for more context.

Args:
name: Name of the rule.
src: A Label. The file to make a copy of. (Can also be the label of a rule
that generates a file.)
src: A Label. The file or directory to make a copy of.
(Can also be the label of a rule that generates a file or directory.)
out: Path of the output file, relative to this package.
is_directory: treat the source file as a directory
Workaround for https://github.com/bazelbuild/bazel/issues/12954
is_executable: A boolean. Whether to make the output file executable. When
True, the rule's output can be executed using `bazel run` and can be
in the srcs of binary and test rules that require executable sources.
Expand All @@ -140,6 +201,8 @@ def copy_file(name, src, out, is_executable = False, allow_symlink = False, **kw
copy_file_impl = _copy_file
if is_executable:
copy_file_impl = _copy_xfile
elif is_directory:
copy_file_impl = _copy_directory

copy_file_impl(
name = name,
Expand Down