Skip to content

Commit f10e4fa

Browse files
authored
fix: correctly handle an empty chdir argument on js_run_binary (#2817)
The `js_run_binary` macro has a `chdir` parameter for determining the working directory where the target should run, and this should take precedence over the `chdir` attribute on the underlying `js_binary`. Currently this does not work correctly if you set `chdir = package_name()` on a `js_run_binary` at the top level of a repo, because in that case `chdir` is the empty string and it is ignored. This change fixes that problem by having the macro make a distinction between `None` and the empty string, and if `chdir` is empty then we replace it with a dot so that it represents the top-level package. This does for `js_run_binary` what 7d28f8e recently did for `js_binary` and `js_test`. --- ### Changes are visible to end-users: yes - Searched for relevant documentation and updated as needed: yes - Breaking change (forces users to change their own code or config): no - Suggested release notes appear below: yes The `js_run_binary` macro now correctly handles an empty string for `chdir`. ### Test plan - Covered by existing test cases - New test cases added
1 parent 19955f0 commit f10e4fa

2 files changed

Lines changed: 27 additions & 2 deletions

File tree

e2e/js_binary_workspace/BUILD.bazel

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,37 @@
11
load("@aspect_rules_js//js:defs.bzl", "js_run_binary")
22
load("@bazel_skylib//rules:build_test.bzl", "build_test")
3+
load("@rules_shell//shell:sh_test.bzl", "sh_test")
34

45
js_run_binary(
56
name = "run_workspace_bin",
7+
chdir = package_name(),
68
stdout = "out.txt",
79
tool = "@workspace//:bin",
810
)
911

12+
# This generated shell script verifies that the contents of the file passed as
13+
# the first argument end with the output directory for this package.
14+
genrule(
15+
name = "working_directory_test_sh",
16+
outs = ["working_directory_test.sh"],
17+
cmd = """cat > $@ << 'EOF'
18+
#!/usr/bin/env bash
19+
actual="$$1"
20+
expected="$(@D)"
21+
expected_len=$$(printf '%s\\n' "$$expected" | wc -c)
22+
diff -u <(tail -c "$$expected_len" "$$actual") <(printf '%s\\n' "$$expected")
23+
EOF""",
24+
)
25+
26+
# Assert that the js_run_binary target above runs in this package's output
27+
# directory, overriding the chdir argument on the underlying js_binary.
28+
sh_test(
29+
name = "js_run_binary_chdir_test",
30+
srcs = [":working_directory_test.sh"],
31+
args = ["$(rootpath :out.txt)"],
32+
data = [":out.txt"],
33+
)
34+
1035
build_test(
1136
name = "test",
1237
targets = [

js/private/js_run_binary.bzl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -294,8 +294,8 @@ def js_run_binary(
294294
}
295295

296296
# Configure working directory to `chdir` is set
297-
if chdir:
298-
normalized_chdir = chdir
297+
if chdir != None:
298+
normalized_chdir = "." if chdir == "" else chdir
299299
repo = native.repo_name()
300300

301301
# Normalize workspace-relative chdir for external repositories so callers can pass

0 commit comments

Comments
 (0)