Skip to content

Commit

Permalink
Fix for windows
Browse files Browse the repository at this point in the history
  • Loading branch information
Alex Eagle authored and alexeagle committed Jan 25, 2021
1 parent 0fde42b commit 1639ecf
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 14 deletions.
36 changes: 26 additions & 10 deletions internal/node/node.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -224,13 +224,34 @@ fi

is_builtin = ctx.attr._node.label.workspace_name in ["nodejs_%s" % p for p in BUILT_IN_NODE_PLATFORMS]

runfiles = []
runfiles.extend(node_tool_files)
runfiles.extend(ctx.files._bash_runfile_helper)
runfiles.append(ctx.outputs.loader_script)
runfiles.append(ctx.outputs.require_patch_script)
runfiles.append(ctx.file._repository_args)

# First replace any instances of "$(rlocation " with "$$(rlocation " to preserve
# legacy uses of "$(rlocation"
expanded_args = [preserve_legacy_templated_args(a) for a in ctx.attr.templated_args]

# Add the working dir argument before expansions
# chdir has to include rlocation lookup for windows
# that means we have to generate a script so there's an entry in the runfiles manifest
if ctx.attr.chdir:
expanded_args.append("--bazel_node_working_dir=" + ctx.attr.chdir)
# limitation of ctx.actions.declare_file - you have to chdir within the package
if ctx.attr.chdir == ctx.label.package:
relative_dir = None
elif ctx.attr.chdir.startswith(ctx.label.package + "/"):
relative_dir = ctx.attr.chdir[len(ctx.label.package) + 1:]
else:
fail("""nodejs_binary/nodejs_test only support chdir inside the current package
but %s is not a subfolder of %s""" % (ctx.attr.chdir, ctx.label.package))
chdir_script = ctx.actions.declare_file(_join(relative_dir, "__chdir.js__"))
ctx.actions.write(chdir_script, "process.chdir(__dirname)")
runfiles.append(chdir_script)

# this join is effectively a $(rootdir) expansion
expanded_args.append("--node_options=--require=$$(rlocation %s)" % _join(ctx.workspace_name, chdir_script.short_path))

# Next expand predefined source/output path variables:
# $(execpath), $(rootpath) & legacy $(location)
Expand Down Expand Up @@ -279,13 +300,6 @@ fi
is_executable = True,
)

runfiles = []
runfiles.extend(node_tool_files)
runfiles.extend(ctx.files._bash_runfile_helper)
runfiles.append(ctx.outputs.loader_script)
runfiles.append(ctx.outputs.require_patch_script)
runfiles.append(ctx.file._repository_args)

if is_windows(ctx):
runfiles.append(ctx.outputs.launcher_sh)
executable = create_windows_native_launcher_script(ctx, ctx.outputs.launcher_sh)
Expand Down Expand Up @@ -331,12 +345,14 @@ _NODEJS_EXECUTABLE_ATTRS = {
"chdir": attr.string(
doc = """Working directory to run the binary or test in, relative to the workspace.
By default, Bazel always runs in the workspace root.
Due to implementation details, this argument must be underneath this package directory.
To run in the directory containing the `nodejs_binary` / `nodejs_test` use
`chdir = package_name()`
(or if you're in a macro, use `native.package_name()`)
NOTE that this can affect other paths passed to the program, which are workspace-relative.
WARNING: this will affect other paths passed to the program, either as arguments or in configuration files,
which are workspace-relative.
You may need `../../` segments to re-relativize such paths to the new working directory.
""",
),
Expand Down
6 changes: 4 additions & 2 deletions internal/node/npm_package_bin.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -178,15 +178,17 @@ def npm_package_bin(tool = None, package = None, package_bin = None, data = [],
To run in the output directory where the npm_package_bin writes outputs, use
`chdir = "$(RULEDIR)"`
NOTE that this can affect other paths passed to the program, which are workspace-relative.
WARNING: this will affect other paths passed to the program, either as arguments or in configuration files,
which are workspace-relative.
You may need `../../` segments to re-relativize such paths to the new working directory.
In a BUILD file you could do something like this to point to the output path:
In a `BUILD` file you could do something like this to point to the output path:
```python
_package_segments = len(package_name().split("/"))
npm_package_bin(
...
chdir = package_name(),
# ../.. segments to re-relative paths from the chdir back to workspace
args = ["/".join([".."] * _package_segments + ["$@"])],
)
```
Expand Down
2 changes: 1 addition & 1 deletion internal/node/test/chdir/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ npm_package_bin(
nodejs_test(
name = "test",
# Also run a test in the output directory
chdir = package_name(),
chdir = package_name() + "/build",
data = ["build/app.js"],
entry_point = "test.js",
)
3 changes: 2 additions & 1 deletion internal/node/test/chdir/test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const {readFileSync} = require('fs');
const assert = require('assert');

console.error('Test running in working directory', process.cwd())
// this test should be run in a working directory with
// that file in it
assert.strictEqual('console.log("hello world")', readFileSync('build/app.js', 'utf-8'));
assert.strictEqual('console.log("hello world")', readFileSync('app.js', 'utf-8'));

0 comments on commit 1639ecf

Please sign in to comment.