From 6033e2ef9fec6cdfce4300e4ad0de417dd080977 Mon Sep 17 00:00:00 2001 From: Chris Brown <77508021+peakschris@users.noreply.github.com> Date: Mon, 8 Jun 2026 17:57:29 -0400 Subject: [PATCH 1/9] windows fixes --- .gitattributes | 1 + .gitignore | 1 + e2e/bzlmod/BUILD.bazel | 2 - examples/runfiles/BUILD.bazel | 5 + js/private/BUILD.bazel | 1 + js/private/bash.bzl | 30 +- js/private/bat.bzl | 142 +++++ js/private/js_binary.bat.tpl | 516 ++++++++++++++++++ js/private/js_binary.bzl | 68 ++- js/private/js_binary.sh.tpl | 9 +- js/private/js_image_layer.bzl | 40 +- js/private/js_image_layer.mjs | 22 +- js/private/test/image/BUILD.bazel | 6 + js/private/test/image/asserts.bzl | 21 +- js/private/test/image/non_ascii/BUILD.bazel | 8 + js/private/test/image/tar_listing.sh | 22 + js/private/test/js_binary_sh/BUILD.bazel | 5 + js/private/test/node-patches/BUILD.bazel | 5 + npm/private/test/BUILD.bazel | 5 + .../test/npm_package_publish/BUILD.bazel | 5 + 20 files changed, 841 insertions(+), 73 deletions(-) create mode 100644 js/private/bat.bzl create mode 100644 js/private/js_binary.bat.tpl create mode 100644 js/private/test/image/tar_listing.sh diff --git a/.gitattributes b/.gitattributes index c5f301826f..c542867fbe 100644 --- a/.gitattributes +++ b/.gitattributes @@ -6,4 +6,5 @@ js/private/watch/aspect_watch_protocol.mjs linguist-generated=true js/private/watch/aspect_watch_protocol.d.mts linguist-generated=true js/private/node-patches/fs.cjs linguist-generated=true js/private/js_image_layer.mjs linguist-generated=true +*.bat.tpl text eol=crlf **/*_pb.d.ts linguist-generated=true diff --git a/.gitignore b/.gitignore index 7112b718aa..f433ae9d6c 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,7 @@ node_modules/ .bazelbsp/ .vscode .DS_Store +.bazelrc.user # Bazel's MODULE lockfile isn't ready to check in yet as of Bazel 7.1. # Do allow for it to be created, however, since it gives a performance boost for local development. diff --git a/e2e/bzlmod/BUILD.bazel b/e2e/bzlmod/BUILD.bazel index f417c6eb14..977d92ab06 100644 --- a/e2e/bzlmod/BUILD.bazel +++ b/e2e/bzlmod/BUILD.bazel @@ -35,8 +35,6 @@ assert_contains( name = "check_styles", actual = "my.css", expected = ".box,\n.bar {\n width: 100px;", - # assert_contains currently requires runfiles; needs fixing upstream - target_compatible_with = not_windows, ) jasmine_bin.jasmine_test( diff --git a/examples/runfiles/BUILD.bazel b/examples/runfiles/BUILD.bazel index 6db1188ca7..6fa02192cc 100644 --- a/examples/runfiles/BUILD.bazel +++ b/examples/runfiles/BUILD.bazel @@ -21,6 +21,11 @@ js_test( "test_fixture.md.generated_file_suffix", ":node_modules/@bazel/runfiles", ], + # Error: could not resolve module aspect_rules_js/examples/runfiles/test_fixture.md + target_compatible_with = select({ + "@platforms//os:windows": ["@platforms//:incompatible"], + "//conditions:default": [], + }), entry_point = "module_name_test.js", ) diff --git a/js/private/BUILD.bazel b/js/private/BUILD.bazel index 47500fe10b..652b75b352 100644 --- a/js/private/BUILD.bazel +++ b/js/private/BUILD.bazel @@ -5,6 +5,7 @@ load("@bazel_skylib//:bzl_library.bzl", "bzl_library") package(default_visibility = ["//visibility:public"]) exports_files([ + "js_binary.bat.tpl", "js_binary.sh.tpl", "node_wrapper.bat", "node_wrapper.sh", diff --git a/js/private/bash.bzl b/js/private/bash.bzl index 53415652e8..04f624c742 100644 --- a/js/private/bash.bzl +++ b/js/private/bash.bzl @@ -5,30 +5,6 @@ # NB: If this can be generalized fully in the future and not depend on logf_fatal # then it could be hoisted to bazel-lib where we have other bash snippets. BASH_INITIALIZE_RUNFILES = r""" -# It helps to determine if we are running on a Windows environment (excludes WSL as it acts like Unix) -case "$(uname -s)" in -CYGWIN*) _IS_WINDOWS=1 ;; -MINGW*) _IS_WINDOWS=1 ;; -MSYS_NT*) _IS_WINDOWS=1 ;; -*) _IS_WINDOWS=0 ;; -esac - -# It helps to normalizes paths when running on Windows. -# -# Example: -# C:/Users/XUser/_bazel_XUser/7q7kkv32/execroot/A/b/C -> /c/Users/XUser/_bazel_XUser/7q7kkv32/execroot/A/b/C -function _normalize_path { - if [ "$_IS_WINDOWS" -eq "1" ]; then - # Apply the followings paths transformations to normalize paths on Windows - # -process driver letter - # -convert path separator - sed -e 's#^\(.\):#/\L\1#' -e 's#\\#/#g' <<<"$1" - else - echo "$1" - fi - return -} - # Set a RUNFILES environment variable to the root of the runfiles tree # since RUNFILES_DIR is not set by Bazel in all contexts. # For example, `RUNFILES=/path/to/my_js_binary.sh.runfiles`. @@ -54,9 +30,9 @@ function _normalize_path { # Case 6a is handled like case 3. if [ "${TEST_SRCDIR:-}" ]; then # Case 4, bazel has identified runfiles for us. - RUNFILES=$(_normalize_path "$TEST_SRCDIR") + RUNFILES=$TEST_SRCDIR elif [ "${RUNFILES_MANIFEST_FILE:-}" ]; then - RUNFILES=$(_normalize_path "$RUNFILES_MANIFEST_FILE") + RUNFILES=$RUNFILES_MANIFEST_FILE if [[ "${RUNFILES}" == *.runfiles_manifest ]]; then # Bazel puts the manifest besides the runfiles with the suffix .runfiles_manifest. # For example, the runfiles directory is named my_binary.runfiles then the manifest is beside the @@ -102,8 +78,6 @@ else logf_fatal "RUNFILES environment variable is not set" exit 1 fi - - RUNFILES=$(_normalize_path "$RUNFILES") fi if [ "${RUNFILES:0:1}" != "/" ]; then # Ensure RUNFILES set above is an absolute path. It may be a path relative diff --git a/js/private/bat.bzl b/js/private/bat.bzl new file mode 100644 index 0000000000..8202762ad3 --- /dev/null +++ b/js/private/bat.bzl @@ -0,0 +1,142 @@ +"Windows Batch snippets for js rules" + +# TODO: Instead of setting a new RUNFILES env; just set RUNFILES_DIR if it is not set; +# needs testing to know if RUNFILES_DIR is set always set to the same value as RUNFILES +# when it is set. +# Batch snippet to initialize the RUNFILES environment variable. +# Depends on there being a logf_fatal function defined. +# NB: If this can be generalized fully in the future and not depend on logf_fatal +# then it could be hoisted to bazel-lib where we have other batch snippets. +BAT_INITIALIZE_RUNFILES = r""" +rem Initialize RUNFILES environment variable for Windows batch +rem This is the Windows batch equivalent of the bash runfiles initialization + +:init_runfiles +rem Set a RUNFILES environment variable to the root of the runfiles tree +rem since RUNFILES_DIR is not set by Bazel in all contexts. +rem For example, `RUNFILES=C:\path\to\my_js_binary.bat.runfiles`. +rem +rem Call this program X. X was generated by a genrule and may be invoked +rem in many ways: +rem 1a) directly by a user, with %0 in the output tree +rem 1b) via 'bazel run' (similar to case 1a) +rem 2) directly by a user, with %0 in X's runfiles +rem 3) by another program Y which has a data dependency on X, with %0 in Y's +rem runfiles +rem 4a) via 'bazel test' +rem 4b) case 3 in the context of a test +rem 5a) by a genrule cmd, with %0 in the output tree +rem 6a) case 3 in the context of a genrule +rem +rem For case 1, %0 will be a regular file, and the runfiles will be +rem at %0.runfiles. +rem For case 2 or 3, %0 will be a symlink to the file seen in case 1. +rem For case 4, %TEST_SRCDIR% should already be set to the runfiles by +rem bazel. +rem Case 5a is handled like case 1. +rem Case 6a is handled like case 3. + +if defined TEST_SRCDIR ( + rem Case 4, bazel has identified runfiles for us. + set "RUNFILES=%TEST_SRCDIR%" + rem Convert forward slashes to backslashes + set "RUNFILES=!RUNFILES:/=\!" +) else if defined RUNFILES_MANIFEST_FILE ( + set "RUNFILES=%RUNFILES_MANIFEST_FILE%" + rem Convert forward slashes to backslashes + set "RUNFILES=!RUNFILES:/=\!" + rem Check if RUNFILES ends with .runfiles_manifest + if "!RUNFILES:~-17!"==".runfiles_manifest" ( + rem Newer versions of Bazel put the manifest besides the runfiles with the suffix .runfiles_manifest. + rem For example, the runfiles directory is named my_binary.runfiles then the manifest is beside the + rem runfiles directory and named my_binary.runfiles_manifest + set "RUNFILES=!RUNFILES:~0,-17!" + ) else ( + rem Check if RUNFILES ends with \MANIFEST + echo !RUNFILES! | findstr /c:"\MANIFEST" >nul + if !errorlevel! equ 0 ( + rem Older versions of Bazel put the manifest file named MANIFEST in the runfiles directory + set "RUNFILES=!RUNFILES:\MANIFEST=!" + ) else ( + call :logf_fatal "Unexpected RUNFILES_MANIFEST_FILE value %RUNFILES_MANIFEST_FILE%" + exit /b 1 + ) + ) +) else ( + rem Determine the full path of the current script + set "self=%~f0" + + :resolve_runfiles_loop + rem Check if .runfiles directory exists next to the script + if exist "!self!.runfiles" ( + set "RUNFILES=!self!.runfiles" + goto :runfiles_found + ) + + rem Check if we're inside a .runfiles directory + echo !self! | findstr /c:".runfiles\" >nul + if !errorlevel! equ 0 ( + rem Extract the runfiles directory path + for /f "tokens=1 delims=" %%a in ('echo !self! ^| findstr /o /c:".runfiles\"') do set "pos_info=%%a" + for /f "tokens=1 delims=:" %%b in ("!pos_info!") do set "pos=%%b" + set /a "end_pos=!pos!+9" + for /f %%c in ('echo !self! ^| findstr /c:".runfiles\"') do ( + for /f "tokens=1 delims=\" %%d in ("%%c") do ( + set "base_part=%%d" + ) + ) + rem Find the .runfiles directory by going back from current position + for /f "delims=" %%e in ('echo !self!') do ( + set "temp_path=%%e" + for /f "tokens=1,* delims=\" %%f in ("!temp_path!") do ( + if "%%g"=="" ( + if exist "%%f.runfiles" set "RUNFILES=%%f.runfiles" + ) else ( + call :extract_runfiles_path "%%e" + ) + ) + ) + rem This is a last resort for case 6b - don't exit the loop yet + ) + + rem For Windows, we don't need to resolve symlinks like in bash since + rem Windows batch files don't typically use symlinks in the same way + rem If we reach here and haven't found runfiles, we'll fail below + + :runfiles_found + if not defined RUNFILES ( + call :logf_fatal "RUNFILES environment variable is not set" + exit /b 1 + ) + + rem Convert forward slashes to backslashes + set "RUNFILES=!RUNFILES:/=\!" +) + +rem Ensure RUNFILES is an absolute path +echo !RUNFILES! | findstr "^[A-Za-z]:" >nul +if !errorlevel! neq 0 ( + rem RUNFILES is not absolute, make it relative to current directory + set "RUNFILES=%CD%\!RUNFILES!" +) + +goto :runfiles_init_done + +:extract_runfiles_path +set "input_path=%~1" +rem Simple approach to find .runfiles directory in path +for /f "tokens=*" %%a in ('echo %input_path% ^| findstr /c:".runfiles"') do ( + set "temp=%%a" + for /f "tokens=1 delims=\" %%b in ('echo !temp! ^| findstr /o /c:".runfiles"') do ( + set "match_info=%%b" + for /f "tokens=1 delims=:" %%c in ("!match_info!") do ( + set "match_pos=%%c" + set /a "runfiles_end=!match_pos!+8" + set "RUNFILES=!input_path:~0,%runfiles_end%!" + ) + ) +) +exit /b 0 + +:runfiles_init_done +""" \ No newline at end of file diff --git a/js/private/js_binary.bat.tpl b/js/private/js_binary.bat.tpl new file mode 100644 index 0000000000..79b87a148a --- /dev/null +++ b/js/private/js_binary.bat.tpl @@ -0,0 +1,516 @@ +@if not defined DEBUG_HELPER @ECHO OFF +setlocal enabledelayedexpansion + +rem This batch script is a wrapper around the NodeJS JavaScript file +rem entry point with the following bazel label: +rem {{entry_point_label}} +rem +rem The script's was generated to execute the js_binary target +rem {{target_label}} +rem +rem The template used to generate this script is +rem {{template_label}} + +{{envs}} + +rem ============================================================================== +rem Prepare stdout capture, stderr capture && logging +rem ============================================================================== + +if defined JS_BINARY__STDOUT_OUTPUT_FILE ( + set "STDOUT_CAPTURE=%TEMP%\stdout_%RANDOM%_%TIME:~-2%.tmp" +) else if defined JS_BINARY__SILENT_ON_SUCCESS ( + set "STDOUT_CAPTURE=%TEMP%\stdout_%RANDOM%_%TIME:~-2%.tmp" +) + +if defined JS_BINARY__STDERR_OUTPUT_FILE ( + set "STDERR_CAPTURE=%TEMP%\stderr_%RANDOM%_%TIME:~-2%.tmp" +) else if defined JS_BINARY__SILENT_ON_SUCCESS ( + set "STDERR_CAPTURE=%TEMP%\stderr_%RANDOM%_%TIME:~-2%.tmp" +) + +set "JS_BINARY__LOG_PREFIX={{log_prefix_rule_set}}[{{log_prefix_rule}}]" + +goto :main + +:logf_stderr +set "format_string=%~1" +shift +if defined STDERR_CAPTURE ( + echo !format_string! >> "!STDERR_CAPTURE!" +) else ( + echo !format_string! >&2 +) +exit /b 0 + +:logf_fatal +if defined JS_BINARY__LOG_FATAL ( + if defined STDERR_CAPTURE ( + echo FATAL: %JS_BINARY__LOG_PREFIX%: >> "!STDERR_CAPTURE!" + ) else ( + echo FATAL: %JS_BINARY__LOG_PREFIX%: >&2 + ) + call :logf_stderr %* +) +exit /b 0 + +:logf_error +if defined JS_BINARY__LOG_ERROR ( + if defined STDERR_CAPTURE ( + echo ERROR: %JS_BINARY__LOG_PREFIX%: >> "!STDERR_CAPTURE!" + ) else ( + echo ERROR: %JS_BINARY__LOG_PREFIX%: >&2 + ) + call :logf_stderr %* +) +exit /b 0 + +:logf_warn +if defined JS_BINARY__LOG_WARN ( + if defined STDERR_CAPTURE ( + echo WARN: %JS_BINARY__LOG_PREFIX%: >> "!STDERR_CAPTURE!" + ) else ( + echo WARN: %JS_BINARY__LOG_PREFIX%: >&2 + ) + call :logf_stderr %* +) +exit /b 0 + +:logf_info +if defined JS_BINARY__LOG_INFO ( + if defined STDERR_CAPTURE ( + echo INFO: %JS_BINARY__LOG_PREFIX%: >> "!STDERR_CAPTURE!" + ) else ( + echo INFO: %JS_BINARY__LOG_PREFIX%: >&2 + ) + call :logf_stderr %* +) +exit /b 0 + +:logf_debug +if defined JS_BINARY__LOG_DEBUG ( + if defined STDERR_CAPTURE ( + echo DEBUG: %JS_BINARY__LOG_PREFIX%: >> "!STDERR_CAPTURE!" + ) else ( + echo DEBUG: %JS_BINARY__LOG_PREFIX%: >&2 + ) + call :logf_stderr %* +) +exit /b 0 + +:resolve_execroot_bin_path +set "short_path=%~1" +if "!short_path:~0,3!"=="..\\" ( + set "RESULT=%JS_BINARY__EXECROOT%\%BAZEL_BINDIR%\external\!short_path:~3!" +) else ( + set "RESULT=%JS_BINARY__EXECROOT%\%BAZEL_BINDIR%\!short_path!" +) +if not defined BAZEL_BINDIR ( + set "RESULT=%JS_BINARY__EXECROOT%\%JS_BINARY__BINDIR%\!short_path!" + if "!short_path:~0,3!"=="..\\" ( + set "RESULT=%JS_BINARY__EXECROOT%\%JS_BINARY__BINDIR%\external\!short_path:~3!" + ) +) +exit /b 0 + +:resolve_execroot_src_path +set "short_path=%~1" +if "!short_path:~0,3!"=="..\\" ( + set "RESULT=%JS_BINARY__EXECROOT%\external\!short_path:~3!" +) else ( + set "RESULT=%JS_BINARY__EXECROOT%\!short_path!" +) +exit /b 0 + +:cleanup_and_exit +if defined STDERR_CAPTURE ( + if defined JS_BINARY__STDERR_OUTPUT_FILE ( + copy "!STDERR_CAPTURE!" "%JS_BINARY__STDERR_OUTPUT_FILE%" >nul + ) + if %ERRORLEVEL% neq 0 ( + type "!STDERR_CAPTURE!" >&2 + ) else if not defined JS_BINARY__SILENT_ON_SUCCESS ( + type "!STDERR_CAPTURE!" >&2 + ) + del "!STDERR_CAPTURE!" >nul 2>&1 +) + +if defined STDOUT_CAPTURE ( + if defined JS_BINARY__STDOUT_OUTPUT_FILE ( + copy "!STDOUT_CAPTURE!" "%JS_BINARY__STDOUT_OUTPUT_FILE%" >nul + ) + if %ERRORLEVEL% neq 0 ( + type "!STDOUT_CAPTURE!" + ) else if not defined JS_BINARY__SILENT_ON_SUCCESS ( + type "!STDOUT_CAPTURE!" + ) + del "!STDOUT_CAPTURE!" >nul 2>&1 +) + +if defined JS_BINARY__LOG_DEBUG ( + call :logf_debug "exit code: %ERRORLEVEL%" +) + +if defined JS_BINARY__PUSHD ( + popd +) + +endlocal +exit /b %ERRORLEVEL% + +:main + +rem ============================================================================== +rem Initialize RUNFILES environment variable +rem ============================================================================== +{{initialize_runfiles}} +set "JS_BINARY__RUNFILES=%RUNFILES%" + +rem ============================================================================== +rem Prepare to run main program +rem ============================================================================== + +rem Convert stdout, stderr and exit_code capture outputs paths to absolute paths +if defined JS_BINARY__STDOUT_OUTPUT_FILE ( + set "temp_path=%JS_BINARY__STDOUT_OUTPUT_FILE%" + if not "!temp_path:~1,1!"==":" ( + set "JS_BINARY__STDOUT_OUTPUT_FILE=%CD%\%JS_BINARY__STDOUT_OUTPUT_FILE%" + ) +) +if defined JS_BINARY__STDERR_OUTPUT_FILE ( + set "temp_path=%JS_BINARY__STDERR_OUTPUT_FILE%" + if not "!temp_path:~1,1!"==":" ( + set "JS_BINARY__STDERR_OUTPUT_FILE=%CD%\%JS_BINARY__STDERR_OUTPUT_FILE%" + ) +) +if defined JS_BINARY__EXIT_CODE_OUTPUT_FILE ( + set "temp_path=%JS_BINARY__EXIT_CODE_OUTPUT_FILE%" + if not "!temp_path:~1,1!"==":" ( + set "JS_BINARY__EXIT_CODE_OUTPUT_FILE=%CD%\%JS_BINARY__EXIT_CODE_OUTPUT_FILE%" + ) +) + +rem Detect bazel-out segment in current directory +set "bazel_out_segment=" +echo "%CD%" | findstr /c:"\bazel-out\" >nul && set "bazel_out_segment=\bazel-out\" +if not defined bazel_out_segment ( + echo "%CD%" | findstr /c:"\BAZEL-~1\" >nul && set "bazel_out_segment=\BAZEL-~1\" +) +if not defined bazel_out_segment ( + echo "%CD%" | findstr /c:"\bazel-~1\" >nul && set "bazel_out_segment=\bazel-~1\" +) + +if defined bazel_out_segment ( + if defined JS_BINARY__USE_EXECROOT_ENTRY_POINT ( + if defined JS_BINARY__EXECROOT ( + call :logf_debug "inheriting JS_BINARY__EXECROOT %JS_BINARY__EXECROOT% from parent js_binary process as JS_BINARY__USE_EXECROOT_ENTRY_POINT is set" + ) + ) else ( + rem We in runfiles and we don't yet know the execroot + rem Find the position of bazel_out_segment and extract execroot + for /f "tokens=1 delims=" %%i in ('echo "%CD%" ^| findstr /n /c:"!bazel_out_segment!"') do ( + set "temp_line=%%i" + ) + rem Extract everything before bazel-out segment + call :extract_execroot_from_path "%CD%" "!bazel_out_segment!" + ) +) else ( + if defined JS_BINARY__USE_EXECROOT_ENTRY_POINT ( + if defined JS_BINARY__EXECROOT ( + call :logf_debug "inheriting JS_BINARY__EXECROOT %JS_BINARY__EXECROOT% from parent js_binary process as JS_BINARY__USE_EXECROOT_ENTRY_POINT is set" + ) + ) else ( + rem We are in execroot or in some other context all together such as a nodejs_image or a manually run js_binary + set "JS_BINARY__EXECROOT=%~dp0" + ) + + if not defined JS_BINARY__NO_CD_BINDIR ( + if not defined BAZEL_BINDIR ( + call :logf_fatal "BAZEL_BINDIR must be set in environment to the makevar $(BINDIR) in js_binary build actions (which run in the execroot) so that build actions can change directories to always run out of the root of the Bazel output tree. See https://docs.bazel.build/versions/main/be/make-variables.html#predefined_variables. This is automatically set by 'js_run_binary' (https://github.com/aspect-build/rules_js/blob/main/docs/js_run_binary.md) which is the recommended rule to use for using a js_binary as the tool of a build action. If this is not a build action you can set the BAZEL_BINDIR to '.' instead to suppress this error. For more context on this design decision, please read the aspect_rules_js README https://github.com/aspect-build/rules_js/tree/dbb5af0d2a9a2bb50e4cf4a96dbc582b27567155#running-nodejs-programs." + goto :cleanup_and_exit + ) + + rem Since the process was launched in the execroot, we automatically change directory into the root of the + rem output tree (which we expect to be set in BAZEL_BIN). See + rem https://github.com/aspect-build/rules_js/tree/dbb5af0d2a9a2bb50e4cf4a96dbc582b27567155#running-nodejs-programs + rem for more context on why we do this. + call :logf_debug "changing directory to BAZEL_BINDIR (root of Bazel output tree) %BAZEL_BINDIR%" + set JS_BINARY__PUSHD=1 + pushd "%BAZEL_BINDIR%" + ) +) + +if defined JS_BINARY__USE_EXECROOT_ENTRY_POINT ( + if not defined BAZEL_BINDIR ( + call :logf_fatal "Expected BAZEL_BINDIR to be set when JS_BINARY__USE_EXECROOT_ENTRY_POINT is set" + goto :cleanup_and_exit + ) + if not defined JS_BINARY__COPY_DATA_TO_BIN ( + if not defined JS_BINARY__ALLOW_EXECROOT_ENTRY_POINT_WITH_NO_COPY_DATA_TO_BIN ( + call :logf_fatal "Expected js_binary copy_data_to_bin to be True when js_run_binary use_execroot_entry_point is True. To disable this validation you can set allow_execroot_entry_point_with_no_copy_data_to_bin to True in js_run_binary" + goto :cleanup_and_exit + ) + ) +) + +if defined JS_BINARY__NO_RUNFILES ( + if not defined JS_BINARY__COPY_DATA_TO_BIN ( + if not defined JS_BINARY__ALLOW_EXECROOT_ENTRY_POINT_WITH_NO_COPY_DATA_TO_BIN ( + call :logf_fatal "Expected js_binary copy_data_to_bin to be True when js_binary use_execroot_entry_point is True. To disable this validation you can set allow_execroot_entry_point_with_no_copy_data_to_bin to True in js_run_binary" + goto :cleanup_and_exit + ) + ) +) + +if defined JS_BINARY__USE_EXECROOT_ENTRY_POINT ( + call :resolve_execroot_bin_path "{{entry_point_path}}" + set "entry_point=%RESULT%" +) else if defined JS_BINARY__NO_RUNFILES ( + call :resolve_execroot_bin_path "{{entry_point_path}}" + set "entry_point=%RESULT%" +) else ( + set "entry_point=%JS_BINARY__RUNFILES%\{{workspace_name}}\{{entry_point_path}}" +) +if not exist "!entry_point!" ( + call :logf_fatal "the entry_point '%entry_point%' not found" + goto :cleanup_and_exit +) + +set "node={{node}}" +rem Check if node path is absolute (starts with drive letter) +echo "%node%" | findstr "^[A-Za-z]:" >nul +if %errorlevel% equ 0 ( + rem A user may specify an absolute path to node using target_tool_path in node_toolchain + set "JS_BINARY__NODE_BINARY=%node%" + if not exist "!JS_BINARY__NODE_BINARY!" ( + call :logf_fatal "node binary '%JS_BINARY__NODE_BINARY%' not found" + goto :cleanup_and_exit + ) +) else ( + if defined JS_BINARY__NO_RUNFILES ( + call :resolve_execroot_src_path "{{node}}" + set "JS_BINARY__NODE_BINARY=%RESULT%" + ) else ( + set "JS_BINARY__NODE_BINARY=%JS_BINARY__RUNFILES%\{{workspace_name}}\{{node}}" + ) + if not exist "!JS_BINARY__NODE_BINARY!" ( + call :logf_fatal "node binary '%JS_BINARY__NODE_BINARY%' not found" + goto :cleanup_and_exit + ) +) + +set "npm={{npm}}" +if defined npm ( + echo "%npm%" | findstr "^[A-Za-z]:" >nul + if !errorlevel! equ 0 ( + rem A user may specify an absolute path to npm using npm_path in node_toolchain + set "JS_BINARY__NPM_BINARY=%npm%" + if not exist "!JS_BINARY__NPM_BINARY!" ( + call :logf_fatal "npm binary '%JS_BINARY__NPM_BINARY%' not found" + goto :cleanup_and_exit + ) + ) else ( + if defined JS_BINARY__NO_RUNFILES ( + call :resolve_execroot_src_path "%npm%" + set "JS_BINARY__NPM_BINARY=%RESULT%" + ) else ( + set "JS_BINARY__NPM_BINARY=%JS_BINARY__RUNFILES%\{{workspace_name}}\%npm%" + ) + if not exist "!JS_BINARY__NPM_BINARY!" ( + call :logf_fatal "npm binary '%JS_BINARY__NPM_BINARY%' not found" + goto :cleanup_and_exit + ) + ) +) + +if defined JS_BINARY__NO_RUNFILES ( + call :resolve_execroot_bin_path "{{node_wrapper}}" + set "JS_BINARY__NODE_WRAPPER=%RESULT%" +) else ( + set "JS_BINARY__NODE_WRAPPER=%JS_BINARY__RUNFILES%\{{workspace_name}}\{{node_wrapper}}" +) +if not exist "%JS_BINARY__NODE_WRAPPER%" ( + call :logf_fatal "node wrapper '%JS_BINARY__NODE_WRAPPER%' not found" + goto :cleanup_and_exit +) + +if defined JS_BINARY__NO_RUNFILES ( + call :resolve_execroot_src_path "{{node_patches}}" + set "JS_BINARY__NODE_PATCHES=%RESULT%" +) else ( + set "JS_BINARY__NODE_PATCHES=%JS_BINARY__RUNFILES%\{{workspace_name}}\{{node_patches}}" +) +if not exist "%JS_BINARY__NODE_PATCHES%" ( + call :logf_fatal "node patches '%JS_BINARY__NODE_PATCHES%' not found" + goto :cleanup_and_exit +) + +rem Change directory to user specified package if set +if defined JS_BINARY__CHDIR ( + call :logf_debug "changing directory to user specified package %JS_BINARY__CHDIR%" + cd /d "%JS_BINARY__CHDIR%" +) + +rem Gather node options +set "JS_BINARY__NODE_OPTIONS=" +{{node_options}} + +rem Process command line arguments +set "ARGS=" +set "ALL_ARGS={{fixed_args}} %*" +for %%a in (%ALL_ARGS%) do ( + set "ARG=%%a" + if "!ARG:~0,15!"=="--node_options=" ( + set "JS_BINARY__NODE_OPTIONS=!JS_BINARY__NODE_OPTIONS! !ARG:~15!" + ) else ( + set "ARGS=!ARGS! %%a" + ) +) + +rem Configure JS_BINARY__FS_PATCH_ROOTS for node fs patches which are run via --require in the node wrapper. +rem Don't override JS_BINARY__FS_PATCH_ROOTS if already set by an outer js_binary in case a js_binary such +rem as js_run_devserver runs another js_binary tool. +if not defined JS_BINARY__FS_PATCH_ROOTS ( + set "JS_BINARY__FS_PATCH_ROOTS=%JS_BINARY__EXECROOT%;%JS_BINARY__RUNFILES%" +) + +rem Enable coverage if requested +if defined COVERAGE_DIR ( + call :logf_debug "enabling v8 coverage support %COVERAGE_DIR%" + set "NODE_V8_COVERAGE=%COVERAGE_DIR%" +) + +rem Put the node wrapper directory on the path so that child processes find it first +for %%i in ("%JS_BINARY__NODE_WRAPPER%") do set "NODE_WRAPPER_DIR=%%~dpi" +set "PATH=%NODE_WRAPPER_DIR%;%PATH%" + +rem Debug logs +if defined JS_BINARY__LOG_DEBUG ( + call :logf_debug "PATH %PATH%" + if defined BAZEL_BINDIR call :logf_debug "BAZEL_BINDIR %BAZEL_BINDIR%" + if defined BAZEL_BUILD_FILE_PATH call :logf_debug "BAZEL_BUILD_FILE_PATH %BAZEL_BUILD_FILE_PATH%" + if defined BAZEL_COMPILATION_MODE call :logf_debug "BAZEL_COMPILATION_MODE %BAZEL_COMPILATION_MODE%" + if defined BAZEL_INFO_FILE call :logf_debug "BAZEL_INFO_FILE %BAZEL_INFO_FILE%" + if defined BAZEL_PACKAGE call :logf_debug "BAZEL_PACKAGE %BAZEL_PACKAGE%" + if defined BAZEL_TARGET_CPU call :logf_debug "BAZEL_TARGET_CPU %BAZEL_TARGET_CPU%" + if defined BAZEL_TARGET_NAME call :logf_debug "BAZEL_TARGET_NAME %BAZEL_TARGET_NAME%" + if defined BAZEL_VERSION_FILE call :logf_debug "BAZEL_VERSION_FILE %BAZEL_VERSION_FILE%" + if defined BAZEL_WORKSPACE call :logf_debug "BAZEL_WORKSPACE %BAZEL_WORKSPACE%" + call :logf_debug "JS_BINARY__FS_PATCH_ROOTS %JS_BINARY__FS_PATCH_ROOTS%" + call :logf_debug "JS_BINARY__NODE_PATCHES %JS_BINARY__NODE_PATCHES%" + call :logf_debug "JS_BINARY__NODE_OPTIONS %JS_BINARY__NODE_OPTIONS%" + if defined JS_BINARY__BINDIR call :logf_debug "JS_BINARY__BINDIR %JS_BINARY__BINDIR%" + if defined JS_BINARY__BUILD_FILE_PATH call :logf_debug "JS_BINARY__BUILD_FILE_PATH %JS_BINARY__BUILD_FILE_PATH%" + if defined JS_BINARY__COMPILATION_MODE call :logf_debug "JS_BINARY__COMPILATION_MODE %JS_BINARY__COMPILATION_MODE%" + call :logf_debug "JS_BINARY__NODE_BINARY %JS_BINARY__NODE_BINARY%" + call :logf_debug "JS_BINARY__NODE_WRAPPER %JS_BINARY__NODE_WRAPPER%" + if defined JS_BINARY__NPM_BINARY call :logf_debug "JS_BINARY__NPM_BINARY %JS_BINARY__NPM_BINARY%" + if defined JS_BINARY__NO_RUNFILES call :logf_debug "JS_BINARY__NO_RUNFILES %JS_BINARY__NO_RUNFILES%" + if defined JS_BINARY__PACKAGE call :logf_debug "JS_BINARY__PACKAGE %JS_BINARY__PACKAGE%" + if defined JS_BINARY__TARGET_CPU call :logf_debug "JS_BINARY__TARGET_CPU %JS_BINARY__TARGET_CPU%" + if defined JS_BINARY__TARGET_NAME call :logf_debug "JS_BINARY__TARGET_NAME %JS_BINARY__TARGET_NAME%" + if defined JS_BINARY__WORKSPACE call :logf_debug "JS_BINARY__WORKSPACE %JS_BINARY__WORKSPACE%" + call :logf_debug "js_binary entry point %entry_point%" + if defined JS_BINARY__USE_EXECROOT_ENTRY_POINT call :logf_debug "JS_BINARY__USE_EXECROOT_ENTRY_POINT %JS_BINARY__USE_EXECROOT_ENTRY_POINT%" +) + +rem Info logs +if defined JS_BINARY__LOG_INFO ( + if defined BAZEL_TARGET call :logf_info "BAZEL_TARGET %BAZEL_TARGET%" + if defined JS_BINARY__TARGET call :logf_info "JS_BINARY__TARGET %JS_BINARY__TARGET%" + call :logf_info "JS_BINARY__RUNFILES %JS_BINARY__RUNFILES%" + call :logf_info "JS_BINARY__EXECROOT %JS_BINARY__EXECROOT%" + call :logf_info "PWD %CD%" +) + +rem ============================================================================== +rem Run the main program +rem ============================================================================== + +if defined JS_BINARY__LOG_INFO ( + call :logf_info "running %JS_BINARY__NODE_WRAPPER% %JS_BINARY__NODE_OPTIONS% -- %entry_point% %ARGS%" +) + +rem Execute the node wrapper with proper output redirection +if defined STDOUT_CAPTURE ( + if defined STDERR_CAPTURE ( + "%JS_BINARY__NODE_WRAPPER%" %JS_BINARY__NODE_OPTIONS% -- "%entry_point%" %ARGS% 1>>"%STDOUT_CAPTURE%" 2>>"%STDERR_CAPTURE%" + ) else ( + "%JS_BINARY__NODE_WRAPPER%" %JS_BINARY__NODE_OPTIONS% -- "%entry_point%" %ARGS% 1>>"%STDOUT_CAPTURE%" + ) +) else ( + if defined STDERR_CAPTURE ( + "%JS_BINARY__NODE_WRAPPER%" %JS_BINARY__NODE_OPTIONS% -- "%entry_point%" %ARGS% 2>>"%STDERR_CAPTURE%" + ) else ( + "%JS_BINARY__NODE_WRAPPER%" %JS_BINARY__NODE_OPTIONS% -- "%entry_point%" %ARGS% + ) +) + +set "RESULT=%ERRORLEVEL%" + +rem ============================================================================== +rem Mop up after main program +rem ============================================================================== + +if defined JS_BINARY__EXPECTED_EXIT_CODE ( + if %RESULT% neq %JS_BINARY__EXPECTED_EXIT_CODE% ( + call :logf_error "expected exit code to be '%JS_BINARY__EXPECTED_EXIT_CODE%', but got '%RESULT%'" + if %RESULT% equ 0 ( + rem This exit code is handled specially by Bazel: + rem https://github.com/bazelbuild/bazel/blob/486206012a664ecb20bdb196a681efc9a9825049/src/main/java/com/google/devtools/build/lib/util/ExitCode.java#L44 + set "BAZEL_EXIT_TESTS_FAILED=3" + call :cleanup_and_exit + exit /b 3 + ) + call :cleanup_and_exit + exit /b %RESULT% + ) else ( + call :cleanup_and_exit + exit /b 0 + ) +) + +if defined JS_BINARY__EXIT_CODE_OUTPUT_FILE ( + rem Exit zero if the exit code was captured + echo %RESULT%> "%JS_BINARY__EXIT_CODE_OUTPUT_FILE%" + call :cleanup_and_exit + exit /b 0 +) else ( + call :cleanup_and_exit + exit /b %RESULT% +) + +rem ============================================================================== +rem Helper functions +rem ============================================================================== + +:extract_execroot_from_path +set "full_path=%~1" +set "segment=%~2" +rem Check if segment is empty or undefined +if "%segment%"=="" ( + set "JS_BINARY__EXECROOT=%full_path%" + exit /b 0 +) +rem Simple approach: find the segment and take everything before it +for /f "tokens=1 delims=" %%a in ('echo "%full_path%" ^| findstr /o /c:"%segment%"') do ( + set "pos_info=%%a" + goto :found_segment +) +rem If segment not found, use the full path as execroot +set "JS_BINARY__EXECROOT=%full_path%" +exit /b 0 + +:found_segment +rem Extract position number (before the colon) +for /f "tokens=1 delims=:" %%b in ("!pos_info!") do ( + set "pos=%%b" +) +rem Calculate the position to extract (pos-1) +set /a "extract_pos=!pos!-1" +if !extract_pos! leq 0 ( + set "JS_BINARY__EXECROOT=%full_path%" +) else ( + set "JS_BINARY__EXECROOT=!full_path:~0,%extract_pos%!" +) +exit /b 0 diff --git a/js/private/js_binary.bzl b/js/private/js_binary.bzl index 3e864f8fde..332dd1141a 100644 --- a/js/private/js_binary.bzl +++ b/js/private/js_binary.bzl @@ -3,8 +3,8 @@ load("@bazel_lib//lib:copy_to_bin.bzl", "COPY_FILE_TO_BIN_TOOLCHAINS") load("@bazel_lib//lib:directory_path.bzl", "DirectoryPathInfo") load("@bazel_lib//lib:expand_make_vars.bzl", "expand_locations", "expand_variables") -load("@bazel_lib//lib:windows_utils.bzl", "create_windows_native_launcher_script") load(":bash.bzl", "BASH_INITIALIZE_RUNFILES") +load(":bat.bzl", "BAT_INITIALIZE_RUNFILES") load(":js_helpers.bzl", "LOG_LEVELS", "envs_for_log_level", "gather_files_from_js_infos", "gather_runfiles") _ATTRS = { @@ -240,10 +240,14 @@ _ATTRS = { for more information. """, ), - "_launcher_template": attr.label( + "_launcher_template_sh": attr.label( default = Label("//js/private:js_binary.sh.tpl"), allow_single_file = True, ), + "_launcher_template_bat": attr.label( + default = Label("//js/private:js_binary.bat.tpl"), + allow_single_file = True, + ), "_node_wrapper_sh": attr.label( default = Label("//js/private:node_wrapper.sh"), allow_single_file = True, @@ -260,7 +264,6 @@ _ATTRS = { default = Label("//js/private:npm_wrapper.bat"), allow_single_file = True, ), - "_windows_constraint": attr.label(default = "@platforms//os:windows"), "_node_patches_files": attr.label_list( allow_files = True, default = [Label("@aspect_rules_js//js/private/node-patches:fs.cjs")], @@ -271,24 +274,48 @@ _ATTRS = { ), } -_ENV_SET = """export {var}={quoted_value}""" -_ENV_SET_IFF_NOT_SET = """if [[ -z "${{{var}:-}}" ]]; then export {var}={quoted_value}; fi""" -_NODE_OPTION = """JS_BINARY__NODE_OPTIONS+=(\"{value}\")""" +# Unix shell constants +_ENV_SET_UNIX = """export {var}={quoted_value}""" +_ENV_SET_IFF_NOT_SET_UNIX = """if [[ -z "${{{var}:-}}" ]]; then export {var}={quoted_value}; fi""" +_NODE_OPTION_UNIX = """JS_BINARY__NODE_OPTIONS+=(\"{value}\")""" +# Windows batch constants +_ENV_SET_WINDOWS = """set "{var}={quoted_value}\"""" +_ENV_SET_IFF_NOT_SET_WINDOWS = """if not defined {var} set "{var}={quoted_value}\"""" +_NODE_OPTION_WINDOWS = """set "JS_BINARY__NODE_OPTIONS=!JS_BINARY__NODE_OPTIONS! {value}\"""" def _expand_env_if_needed(ctx, value): if ctx.attr.expand_env: return " ".join([expand_variables(ctx, exp, attribute_name = "env") for exp in expand_locations(ctx, value, ctx.attr.data).split(" ")]) return value +def _windows_host(ctx): + """Returns true if the host platform is windows. + + The typical approach using ctx.target_platform_has_constraint does not work for transitioned + build targets. We need to know the host platform, not the target platform. + """ + return ctx.configuration.host_path_separator == ";" + +def _windows_path(path): + """Convert a Unix-style path to a Windows-style path. + """ + return path.replace("/", "\\") + def _bash_quote(value): return json.encode(value) -def _bash_launcher(ctx, nodeinfo, entry_point_path, log_prefix_rule_set, log_prefix_rule, fixed_args, fixed_env, is_windows): +def _bash_launcher(ctx, nodeinfo, entry_point_path, log_prefix_rule_set, log_prefix_rule, fixed_args, fixed_env): # Explicitly disable node fs patches on Windows: # https://github.com/aspect-build/rules_js/issues/1137 + is_windows = _windows_host(ctx) if is_windows: fixed_env = dict(fixed_env, **{"JS_BINARY__PATCH_NODE_FS": "0"}) + # Use Windows batch syntax or Unix shell syntax based on platform + _ENV_SET = _ENV_SET_WINDOWS if is_windows else _ENV_SET_UNIX + _ENV_SET_IFF_NOT_SET = _ENV_SET_IFF_NOT_SET_WINDOWS if is_windows else _ENV_SET_IFF_NOT_SET_UNIX + _NODE_OPTION = _NODE_OPTION_WINDOWS if is_windows else _NODE_OPTION_UNIX + envs = [ _ENV_SET.format(var = key, quoted_value = _bash_quote(_expand_env_if_needed(ctx, value))) for key, value in fixed_env.items() @@ -418,30 +445,33 @@ def _bash_launcher(ctx, nodeinfo, entry_point_path, log_prefix_rule_set, log_pre node_path = nodeinfo.node.short_path if nodeinfo.node else nodeinfo.node_path + template = ctx.file._launcher_template_bat if is_windows else ctx.file._launcher_template_sh + template_label = ctx.attr._launcher_template_bat.label if is_windows else ctx.attr._launcher_template_sh.label + launcher_subst = { "{{target_label}}": str(ctx.label), - "{{template_label}}": str(ctx.attr._launcher_template.label), + "{{template_label}}": str(template_label), "{{entry_point_label}}": str(ctx.attr.entry_point.label), - "{{entry_point_path}}": entry_point_path, + "{{entry_point_path}}": _windows_path(entry_point_path) if is_windows else entry_point_path, "{{envs}}": "\n".join(envs), "{{fixed_args}}": " ".join(fixed_args), - "{{initialize_runfiles}}": BASH_INITIALIZE_RUNFILES, + "{{initialize_runfiles}}": BAT_INITIALIZE_RUNFILES if is_windows else BASH_INITIALIZE_RUNFILES, "{{log_prefix_rule_set}}": log_prefix_rule_set, "{{log_prefix_rule}}": log_prefix_rule, "{{node_options}}": "\n".join(node_options), - "{{node_patches}}": ctx.file._node_patches.short_path, - "{{node_wrapper}}": node_wrapper.short_path, - "{{node}}": node_path, - "{{npm}}": npm_path, + "{{node_patches}}": _windows_path(ctx.file._node_patches.short_path) if is_windows else ctx.file._node_patches.short_path, + "{{node_wrapper}}": _windows_path(node_wrapper.short_path) if is_windows else node_wrapper.short_path, + "{{node}}": _windows_path(node_path) if is_windows else node_path, + "{{npm}}": _windows_path(npm_path) if is_windows else npm_path, "{{workspace_name}}": ctx.workspace_name, } # The '_' avoids collisions with another file matching the label name. # For example, test and test/my.spec.ts. This naming scheme is borrowed from rules_go: # https://github.com/bazelbuild/rules_go/blob/f3cc8a2d670c7ccd5f45434ab226b25a76d44de1/go/private/context.bzl#L144 - launcher = ctx.actions.declare_file("{}_/{}".format(ctx.label.name, ctx.label.name)) + launcher = ctx.actions.declare_file("{}_/{}{}".format(ctx.label.name, ctx.label.name, ".bat" if is_windows else "")) ctx.actions.expand_template( - template = ctx.file._launcher_template, + template = template, output = launcher, substitutions = launcher_subst, is_executable = True, @@ -450,7 +480,6 @@ def _bash_launcher(ctx, nodeinfo, entry_point_path, log_prefix_rule_set, log_pre return launcher, toolchain_files def _create_launcher(ctx, log_prefix_rule_set, log_prefix_rule, fixed_args = [], fixed_env = {}): - is_windows = ctx.target_platform_has_constraint(ctx.attr._windows_constraint[platform_common.ConstraintValueInfo]) if ctx.attr.node_toolchain: nodeinfo = ctx.attr.node_toolchain[platform_common.ToolchainInfo].nodeinfo @@ -469,10 +498,9 @@ def _create_launcher(ctx, log_prefix_rule_set, log_prefix_rule, fixed_args = [], entry_point = ctx.files.entry_point[0] entry_point_path = entry_point.short_path - bash_launcher, toolchain_files = _bash_launcher(ctx, nodeinfo, entry_point_path, log_prefix_rule_set, log_prefix_rule, fixed_args, fixed_env, is_windows) - launcher = create_windows_native_launcher_script(ctx, bash_launcher) if is_windows else bash_launcher + launcher, toolchain_files = _bash_launcher(ctx, nodeinfo, entry_point_path, log_prefix_rule_set, log_prefix_rule, fixed_args, fixed_env) - launcher_files = [bash_launcher] + launcher_files = [launcher] launcher_files.extend(toolchain_files) if nodeinfo.node: launcher_files.append(nodeinfo.node) diff --git a/js/private/js_binary.sh.tpl b/js/private/js_binary.sh.tpl index 5d0234cb5c..dd0def5238 100644 --- a/js/private/js_binary.sh.tpl +++ b/js/private/js_binary.sh.tpl @@ -246,7 +246,7 @@ if [ ! -f "$entry_point" ]; then exit 1 fi -node="$(_normalize_path "{{node}}")" +node="{{node}}" if [ "${node:0:1}" = "/" ]; then # A user may specify an absolute path to node using target_tool_path in node_toolchain export JS_BINARY__NODE_BINARY="$node" @@ -266,14 +266,13 @@ else exit 1 fi fi -if [ "$_IS_WINDOWS" -ne "1" ] && [ ! -x "$JS_BINARY__NODE_BINARY" ]; then +if [ ! -x "$JS_BINARY__NODE_BINARY" ]; then logf_fatal "node binary '%s' is not executable" "$JS_BINARY__NODE_BINARY" exit 1 fi npm="{{npm}}" if [ "$npm" ]; then - npm="$(_normalize_path "$npm")" if [ "${npm:0:1}" = "/" ]; then # A user may specify an absolute path to npm using npm_path in node_toolchain export JS_BINARY__NPM_BINARY="$npm" @@ -293,7 +292,7 @@ if [ "$npm" ]; then exit 1 fi fi - if [ "$_IS_WINDOWS" -ne "1" ] && [ ! -x "$JS_BINARY__NPM_BINARY" ]; then + if [ ! -x "$JS_BINARY__NPM_BINARY" ]; then logf_fatal "npm binary '%s' is not executable" "$JS_BINARY__NPM_BINARY" exit 1 fi @@ -309,7 +308,7 @@ if [ ! -f "$JS_BINARY__NODE_WRAPPER" ]; then logf_fatal "node wrapper '%s' not found" "$JS_BINARY__NODE_WRAPPER" exit 1 fi -if [ "$_IS_WINDOWS" -ne "1" ] && [ ! -x "$JS_BINARY__NODE_WRAPPER" ]; then +if [ ! -x "$JS_BINARY__NODE_WRAPPER" ]; then logf_fatal "node wrapper '%s' is not executable" "$JS_BINARY__NODE_WRAPPER" exit 1 fi diff --git a/js/private/js_image_layer.bzl b/js/private/js_image_layer.bzl index e4e24ba9e5..f8096282d8 100644 --- a/js/private/js_image_layer.bzl +++ b/js/private/js_image_layer.bzl @@ -182,10 +182,19 @@ The default layer groups are as follows and always created. """ +def _windows_host(ctx): + """Returns true if the host platform is windows. + + The typical approach using ctx.target_platform_has_constraint does not work for transitioned + build targets. We need to know the host platform, not the target platform. + """ + return ctx.configuration.host_path_separator == ";" + + # BAZEL_BINDIR has to be set to '.' so that js_binary preserves the PWD when running inside container. # See https://github.com/aspect-build/rules_js/tree/dbb5af0d2a9a2bb50e4cf4a96dbc582b27567155#running-nodejs-programs # for why this is needed. -_LAUNCHER_PREABMLE = """\ +_LAUNCHER_PREAMBLE = """\ #!/usr/bin/env bash export BAZEL_BINDIR="." @@ -193,21 +202,36 @@ export BAZEL_BINDIR="." # patched by js_image_layer for hermeticity """ -def _write_laucher(ctx, real_binary): +_LAUNCHER_PREAMBLE_WINDOWS = """\ +setlocal enabledelayedexpansion + +set BAZEL_BINDIR=%~dp0 + +rem patched by js_image_layer for hermeticity +""" + +def _write_launcher(ctx, real_binary): "Creates a call-through shell entrypoint which sets BAZEL_BINDIR to '.' then immediately invokes the original entrypoint." launcher = ctx.actions.declare_file("%s_launcher" % ctx.label.name) - substitutions = { - "#!/usr/bin/env bash": _LAUNCHER_PREABMLE, + substitutions_unix = { + "#!/usr/bin/env bash": _LAUNCHER_PREAMBLE, 'export JS_BINARY__BINDIR="%s"' % real_binary.root.path: 'export JS_BINARY__BINDIR="$(pwd)"', 'export JS_BINARY__TARGET_CPU="%s"' % ctx.expand_make_variables("", "$(TARGET_CPU)", {}): 'export JS_BINARY__TARGET_CPU="$(uname -m)"', } - substitutions['export JS_BINARY__BINDIR="%s"' % ctx.bin_dir.path] = 'export JS_BINARY__BINDIR="$(pwd)"' + substitutions_unix['export JS_BINARY__BINDIR="%s"' % ctx.bin_dir.path] = 'export JS_BINARY__BINDIR="$(pwd)"' + + substitutions_windows = { + "setlocal enabledelayedexpansion": _LAUNCHER_PREAMBLE_WINDOWS, + 'set "JS_BINARY__BINDIR=%s"' % real_binary.root.path: 'set "JS_BINARY__BINDIR=%CD%"', + 'set "JS_BINARY__TARGET_CPU=%s"' % ctx.expand_make_variables("", "$(TARGET_CPU)", {}): 'set "JS_BINARY__TARGET_CPU=%PROCESSOR_ARCHITECTURE%"', + } + substitutions_windows['set "JS_BINARY__BINDIR=%s"' % ctx.bin_dir.path] = 'set "JS_BINARY__BINDIR=%CD%"' ctx.actions.expand_template( template = real_binary, output = launcher, - substitutions = substitutions, + substitutions = substitutions_windows if _windows_host(ctx) else substitutions_unix, is_executable = True, ) return launcher @@ -338,10 +362,10 @@ def _js_image_layer_impl(ctx): binary_default_info = ctx.attr.binary[0][DefaultInfo] binary_label = ctx.attr.binary[0].label - binary_path = "./" + paths.join(ctx.attr.root.lstrip("./").lstrip("/"), binary_label.package, binary_label.name) + binary_path = "./" + paths.join(ctx.attr.root.lstrip("./").lstrip("/"), binary_label.package, binary_label.name + (".bat" if _windows_host(ctx) else "")) runfiles_dir = binary_path + ".runfiles" - launcher = _write_laucher(ctx, binary_default_info.files_to_run.executable) + launcher = _write_launcher(ctx, binary_default_info.files_to_run.executable) repo_mapping = _repo_mapping_manifest(binary_default_info.files_to_run) diff --git a/js/private/js_image_layer.mjs b/js/private/js_image_layer.mjs index 4998223bf2..4c31247305 100644 --- a/js/private/js_image_layer.mjs +++ b/js/private/js_image_layer.mjs @@ -43,15 +43,19 @@ async function readlinkSafe(p) { // assume the file exists in another layer return p } + if (process.platform === 'win32' && e.code == 'UNKNOWN' && e.errno == -4094) { + // Windows returns 'UNKNOWN' when reading a link that is a file + return p + } throw e } } -const EXECROOT = process.cwd() +const EXECROOT = process.cwd().replace(/\\/g, '/') // Resolve symlinks while staying inside the sandbox. async function resolveSymlink(p) { - let prevHop = path.resolve(p) + let prevHop = path.posix.resolve(p) let hopped = false while (true) { // /output-base/sandbox/4/execroot/wksp/bazel-out @@ -60,6 +64,7 @@ async function resolveSymlink(p) { // if the next hop leads to out of execroot, that means // we hopped too far, return the previous hop. + nextHop = nextHop.replace(/\\/g, '/') if (!nextHop.startsWith(EXECROOT)) { return hopped ? prevHop : undefined } @@ -94,7 +99,7 @@ function add_parents(mtree, dest) { if (!part || part == '.') { continue } - prev = path.join(prev, part) + prev = path.posix.join(prev, part) mtree.add(_mtree_dir_line(prev)) } } @@ -110,6 +115,13 @@ function vis(str) { // Rust has this https://doc.rust-lang.org/std/string/struct.String.html#method.as_bytes // and the equivalent in nodejs is Buffer. for (const char of Buffer.from(str)) { + if (char == '\\') { + throw new Error( + `unexpected entry format. ${JSON.stringify( + str + )}. find the source of the errant backslash` + ) + } if (char < 33 || char > 126) { // Non-printable result += '\\' + char.toString(8).padStart(3, '0') @@ -128,8 +140,8 @@ function _mtree_dir_line(dir) { } function _mtree_link_line(key, linkname) { - const link_parent = path.dirname(key) - linkname = path.relative(link_parent, linkname) + const link_parent = path.posix.dirname(key) + linkname = path.posix.relative(link_parent, linkname) // interestingly, bazel 5 and 6 sets different mode bits on symlinks. // well use `0o755` to allow owner&group to `rwx` and others `rx` diff --git a/js/private/test/image/BUILD.bazel b/js/private/test/image/BUILD.bazel index 31d87d19e4..e7bc326696 100644 --- a/js/private/test/image/BUILD.bazel +++ b/js/private/test/image/BUILD.bazel @@ -22,6 +22,12 @@ platform( visibility = ["//js/private/test/image:__subpackages__"], ) +sh_binary( + name = "tar_listing", + srcs = ["tar_listing.sh"], + deps = ["@bazel_tools//tools/bash/runfiles"], +) + platform( name = "linux_arm64", constraint_values = [ diff --git a/js/private/test/image/asserts.bzl b/js/private/test/image/asserts.bzl index 118a4478cd..301a1c3ba1 100644 --- a/js/private/test/image/asserts.bzl +++ b/js/private/test/image/asserts.bzl @@ -3,6 +3,11 @@ load("@bazel_lib//lib:write_source_files.bzl", "write_source_file", "write_source_files") load("//js:defs.bzl", "js_image_layer") +NOT_WINDOWS = select({ + "@platforms//os:windows": ["@platforms//:incompatible"], + "//conditions:default": [], +}) + # js_binary launcher scripts have unstable sizes across Bazel versions. _UNSTABLE_SIZE_BASENAMES = ["bin", "bin2"] @@ -24,12 +29,17 @@ def assert_tar_listing(name, actual, expected): actual_listing = "_{}_listing".format(name) native.genrule( name = actual_listing, - srcs = [actual], + srcs = [ + actual, + ], testonly = True, outs = ["_{}.listing".format(name)], # TODO: now that app layer has repo_mapping file in it which is not stable between different operating systems - # we need to exlude it from checksums + # we need to exclude it from checksums # See: https://github.com/aspect-build/rules_js/actions/runs/11749187598/job/32734931009?pr=2011 + # cmd = "$(location :tar_listing) $(BSDTAR_BIN) $(location {}) > $@".format(actual), + # tools = [":tar_listing"], + # toolchains = ["@bsd_tar_toolchains//:resolved_toolchain", "@coreutils_toolchains//:resolved_toolchain"], cmd = 'TZ="UTC" LC_ALL="en_US.UTF-8" $(BSDTAR_BIN) -tvf $(execpath {}) --exclude "**/_repo_mapping" | {} >$@'.format(actual, sanitize_cmd), toolchains = ["@bsd_tar_toolchains//:resolved_toolchain"], ) @@ -58,7 +68,6 @@ def assert_js_image_layer_listings(name, js_image_layer, additional_layers = []) actual = "{}_{}".format(js_image_layer, layer), expected = "{}_{}.listing".format(name, layer), ) - write_source_files( name = name + "_update_all", additional_update_targets = [ @@ -97,13 +106,15 @@ def assert_checksum(name, image_layer): srcs = ["{}_{}".format(image_layer, layer) for layer in layers], outs = [name + ".checksums"], # TODO: now that app layer has repo_mapping file in it which is not stable between different operating systems - # we need to exlude it from checksums + # we need to exclude it from checksums # See: https://github.com/aspect-build/rules_js/actions/runs/11749187598/job/32734931009?pr=2011 + # TODO: also exclude node layer which is different between windows and linux + # and ignore sha256sum windows difference (it prints '*' before each filename) cmd = """ COREUTILS_BIN=$$(realpath $(COREUTILS_BIN)) && RESULT="$$($$COREUTILS_BIN sha256sum $(SRCS))" BINDIR="$(BINDIR)/" -echo "$${RESULT//$$BINDIR/}" | $$COREUTILS_BIN head -n -1 > $@ +echo "$${RESULT//$$BINDIR/}" | $$COREUTILS_BIN head -n -1 | $$COREUTILS_BIN tail -n -3 | tr '*' ' ' > $@ """, output_to_bindir = True, toolchains = ["@coreutils_toolchains//:resolved_toolchain"], diff --git a/js/private/test/image/non_ascii/BUILD.bazel b/js/private/test/image/non_ascii/BUILD.bazel index cfe3343877..e2061c8d6a 100644 --- a/js/private/test/image/non_ascii/BUILD.bazel +++ b/js/private/test/image/non_ascii/BUILD.bazel @@ -10,6 +10,10 @@ js_binary( "ㅑㅕㅣㅇ.ㄴㅅ", ], entry_point = "main.js", + target_compatible_with = select({ + "@platforms//os:windows": ["@platforms//:incompatible"], + "//conditions:default": [], + }), ) make_js_image_layer( @@ -20,6 +24,10 @@ make_js_image_layer( }, platform = "//js/private/test/image:linux_amd64", root = "/app", + target_compatible_with = select({ + "@platforms//os:windows": ["@platforms//:incompatible"], + "//conditions:default": [], + }), ) assert_js_image_layer_listings( diff --git a/js/private/test/image/tar_listing.sh b/js/private/test/image/tar_listing.sh new file mode 100644 index 0000000000..7a2552495a --- /dev/null +++ b/js/private/test/image/tar_listing.sh @@ -0,0 +1,22 @@ +#!/usr/bin/env bash +set -o errexit -o nounset -o pipefail +RUNFILES_MANIFEST_ONLY=1 +# --- begin runfiles.bash initialization v3 --- +# Copy-pasted from the Bazel Bash runfiles library v3. +# https://github.com/bazelbuild/bazel/blob/master/tools/bash/runfiles/runfiles.bash +set -uo pipefail; set +e; f=bazel_tools/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: runfiles.bash initializer cannot find $f. An executable rule may have forgotten to expose it in the runfiles, or the binary may require RUNFILES_DIR to be set."; exit 1; }; f=; set -e +# --- end runfiles.bash initialization v3 --- + +BSDTAR_BIN=$1 +FILE_PATH=$2 +TZ="UTC" +LC_ALL="en_US.UTF-8" +# https://github.com/libarchive/libarchive/issues/2726 +${BSDTAR_BIN} -tvf ${FILE_PATH} --exclude "**/_repo_mapping" | sed 's|Dec 31 1969|Jan 1 1970|g' | tr -d '\r' + diff --git a/js/private/test/js_binary_sh/BUILD.bazel b/js/private/test/js_binary_sh/BUILD.bazel index bfc047cf29..39b8c6389f 100644 --- a/js/private/test/js_binary_sh/BUILD.bazel +++ b/js/private/test/js_binary_sh/BUILD.bazel @@ -123,6 +123,11 @@ js_run_binary( log_level = "debug", silent_on_success = False, stdout = "regexy-stdout", + # escaping arguments passed via bat needs to be improved + target_compatible_with = select({ + "@platforms//os:windows": ["@platforms//:incompatible"], + "//conditions:default": [], + }), tool = ":regexy", ) diff --git a/js/private/test/node-patches/BUILD.bazel b/js/private/test/node-patches/BUILD.bazel index 902b182771..da8e0c02e2 100644 --- a/js/private/test/node-patches/BUILD.bazel +++ b/js/private/test/node-patches/BUILD.bazel @@ -149,6 +149,11 @@ babel_bin.babel( entry_point = "spawn.js", node_toolchain = toolchain, patch_node_fs = True, + # on windows: Error: spawnSync node ENOENT + target_compatible_with = select({ + "@platforms//os:windows": ["@platforms//:incompatible"], + "//conditions:default": [], + }), ) for toolchain_name, toolchain in zip( TOOLCHAINS_NAMES, diff --git a/npm/private/test/BUILD.bazel b/npm/private/test/BUILD.bazel index cedc245cf9..d7a3858b26 100644 --- a/npm/private/test/BUILD.bazel +++ b/npm/private/test/BUILD.bazel @@ -109,6 +109,11 @@ sh_test( "@nodejs_linux_amd64//:node_files", "@nodejs_linux_arm64//:node_files", ], + # on windows: bin_test: line 11: node: command not found + target_compatible_with = select({ + "@platforms//os:windows": ["@platforms//:incompatible"], + "//conditions:default": [], + }), toolchains = ["@rules_nodejs//nodejs:current_node_runtime"], ) diff --git a/npm/private/test/npm_package_publish/BUILD.bazel b/npm/private/test/npm_package_publish/BUILD.bazel index 18adfd3723..84f0b20c91 100644 --- a/npm/private/test/npm_package_publish/BUILD.bazel +++ b/npm/private/test/npm_package_publish/BUILD.bazel @@ -35,4 +35,9 @@ sh_test( "no-remote-exec", "no-sandbox", ], + # on windows: expected 'npm notice package: @mycorp/pkg-b@' error + target_compatible_with = select({ + "@platforms//os:windows": ["@platforms//:incompatible"], + "//conditions:default": [], + }), ) From 50174d9b754de7fd085fe1bcd74f024aaae65239 Mon Sep 17 00:00:00 2001 From: Chris Brown <77508021+peakschris@users.noreply.github.com> Date: Mon, 8 Jun 2026 22:00:49 -0400 Subject: [PATCH 2/9] tidy --- js/private/test/image/BUILD.bazel | 6 ------ js/private/test/image/asserts.bzl | 8 +------- js/private/test/image/tar_listing.sh | 22 ---------------------- 3 files changed, 1 insertion(+), 35 deletions(-) delete mode 100644 js/private/test/image/tar_listing.sh diff --git a/js/private/test/image/BUILD.bazel b/js/private/test/image/BUILD.bazel index e7bc326696..31d87d19e4 100644 --- a/js/private/test/image/BUILD.bazel +++ b/js/private/test/image/BUILD.bazel @@ -22,12 +22,6 @@ platform( visibility = ["//js/private/test/image:__subpackages__"], ) -sh_binary( - name = "tar_listing", - srcs = ["tar_listing.sh"], - deps = ["@bazel_tools//tools/bash/runfiles"], -) - platform( name = "linux_arm64", constraint_values = [ diff --git a/js/private/test/image/asserts.bzl b/js/private/test/image/asserts.bzl index 301a1c3ba1..db926a6548 100644 --- a/js/private/test/image/asserts.bzl +++ b/js/private/test/image/asserts.bzl @@ -34,13 +34,7 @@ def assert_tar_listing(name, actual, expected): ], testonly = True, outs = ["_{}.listing".format(name)], - # TODO: now that app layer has repo_mapping file in it which is not stable between different operating systems - # we need to exclude it from checksums - # See: https://github.com/aspect-build/rules_js/actions/runs/11749187598/job/32734931009?pr=2011 - # cmd = "$(location :tar_listing) $(BSDTAR_BIN) $(location {}) > $@".format(actual), - # tools = [":tar_listing"], - # toolchains = ["@bsd_tar_toolchains//:resolved_toolchain", "@coreutils_toolchains//:resolved_toolchain"], - cmd = 'TZ="UTC" LC_ALL="en_US.UTF-8" $(BSDTAR_BIN) -tvf $(execpath {}) --exclude "**/_repo_mapping" | {} >$@'.format(actual, sanitize_cmd), + cmd ='TZ="UTC" LC_ALL="en_US.UTF-8" $(BSDTAR_BIN) -tvf $(execpath {}) --exclude "**/_repo_mapping" | {} >$@'.format(actual, sanitize_cmd), toolchains = ["@bsd_tar_toolchains//:resolved_toolchain"], ) diff --git a/js/private/test/image/tar_listing.sh b/js/private/test/image/tar_listing.sh deleted file mode 100644 index 7a2552495a..0000000000 --- a/js/private/test/image/tar_listing.sh +++ /dev/null @@ -1,22 +0,0 @@ -#!/usr/bin/env bash -set -o errexit -o nounset -o pipefail -RUNFILES_MANIFEST_ONLY=1 -# --- begin runfiles.bash initialization v3 --- -# Copy-pasted from the Bazel Bash runfiles library v3. -# https://github.com/bazelbuild/bazel/blob/master/tools/bash/runfiles/runfiles.bash -set -uo pipefail; set +e; f=bazel_tools/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: runfiles.bash initializer cannot find $f. An executable rule may have forgotten to expose it in the runfiles, or the binary may require RUNFILES_DIR to be set."; exit 1; }; f=; set -e -# --- end runfiles.bash initialization v3 --- - -BSDTAR_BIN=$1 -FILE_PATH=$2 -TZ="UTC" -LC_ALL="en_US.UTF-8" -# https://github.com/libarchive/libarchive/issues/2726 -${BSDTAR_BIN} -tvf ${FILE_PATH} --exclude "**/_repo_mapping" | sed 's|Dec 31 1969|Jan 1 1970|g' | tr -d '\r' - From a504b7f9a11a9860910c233ff786c23668f83a5f Mon Sep 17 00:00:00 2001 From: Chris Brown <77508021+peakschris@users.noreply.github.com> Date: Mon, 8 Jun 2026 23:04:21 -0400 Subject: [PATCH 3/9] fix: Windows env var quoting and path separator in register.cjs --- js/private/bat.bzl | 19 +++++++++++++++++++ js/private/js_binary.bzl | 24 +++++++++++++++--------- js/private/node-patches/register.cjs | 2 +- 3 files changed, 35 insertions(+), 10 deletions(-) diff --git a/js/private/bat.bzl b/js/private/bat.bzl index 8202762ad3..28abb62f26 100644 --- a/js/private/bat.bzl +++ b/js/private/bat.bzl @@ -120,6 +120,25 @@ if !errorlevel! neq 0 ( set "RUNFILES=%CD%\!RUNFILES!" ) + +rem ============================================================================== +rem Configure NODE_PATH with forward-slash normalized paths for module resolution +rem ============================================================================== +rem This is essential for tools like jest-cli that use import-local/resolve-cwd. +rem Node.js on Windows returns backslash-separated paths by default, but our +rem symlink resolution code (js_image_layer.mjs) normalizes to forward slashes. +rem To ensure modules like resolve-cwd can find transitive dependencies, we need +rem to explicitly set NODE_PATH with forward-slash normalized paths. +rem See: https://github.com/aspect-build/rules_js/issues/2325 + +if not defined NODE_PATH ( + rem Initialize NODE_PATH to runfiles node_modules + set "NODE_PATH=!JS_BINARY__RUNFILES!\_main\node_modules" + + rem Convert backslashes to forward slashes for consistent Node module resolution + set "NODE_PATH=!NODE_PATH:\=/!" +) + goto :runfiles_init_done :extract_runfiles_path diff --git a/js/private/js_binary.bzl b/js/private/js_binary.bzl index 332dd1141a..9c4e0f651e 100644 --- a/js/private/js_binary.bzl +++ b/js/private/js_binary.bzl @@ -304,6 +304,11 @@ def _windows_path(path): def _bash_quote(value): return json.encode(value) +def _bat_quote(value): + # Windows batch: set "VAR=VALUE" already provides quoting via the template, + # so the value must NOT be wrapped in extra quotes (json.encode adds "...") + return value + def _bash_launcher(ctx, nodeinfo, entry_point_path, log_prefix_rule_set, log_prefix_rule, fixed_args, fixed_env): # Explicitly disable node fs patches on Windows: # https://github.com/aspect-build/rules_js/issues/1137 @@ -315,12 +320,13 @@ def _bash_launcher(ctx, nodeinfo, entry_point_path, log_prefix_rule_set, log_pre _ENV_SET = _ENV_SET_WINDOWS if is_windows else _ENV_SET_UNIX _ENV_SET_IFF_NOT_SET = _ENV_SET_IFF_NOT_SET_WINDOWS if is_windows else _ENV_SET_IFF_NOT_SET_UNIX _NODE_OPTION = _NODE_OPTION_WINDOWS if is_windows else _NODE_OPTION_UNIX + _quote = _bat_quote if is_windows else _bash_quote envs = [ - _ENV_SET.format(var = key, quoted_value = _bash_quote(_expand_env_if_needed(ctx, value))) + _ENV_SET.format(var = key, quoted_value = _quote(_expand_env_if_needed(ctx, value))) for key, value in fixed_env.items() ] + [ - _ENV_SET.format(var = key, quoted_value = _bash_quote(_expand_env_if_needed(ctx, value))) + _ENV_SET.format(var = key, quoted_value = _quote(_expand_env_if_needed(ctx, value))) for key, value in ctx.attr.env.items() ] @@ -333,7 +339,7 @@ def _bash_launcher(ctx, nodeinfo, entry_point_path, log_prefix_rule_set, log_pre for (key, value) in makevars.items(): envs.append(_ENV_SET.format( var = key, - quoted_value = _bash_quote(ctx.expand_make_variables("env", value, {})), + quoted_value = _quote(ctx.expand_make_variables("env", value, {})), )) # Add rule context variables to the environment @@ -351,24 +357,24 @@ def _bash_launcher(ctx, nodeinfo, entry_point_path, log_prefix_rule_set, log_pre if is_windows and not ctx.attr.enable_runfiles: builtins["JS_BINARY__NO_RUNFILES"] = "1" for (key, value) in builtins.items(): - envs.append(_ENV_SET.format(var = key, quoted_value = _bash_quote(value))) + envs.append(_ENV_SET.format(var = key, quoted_value = _quote(value))) if ctx.attr.patch_node_fs: # Set patch node fs API env if not already set to allow js_run_binary to override envs.append(_ENV_SET_IFF_NOT_SET.format( var = "JS_BINARY__PATCH_NODE_FS", - quoted_value = _bash_quote("1"), + quoted_value = _quote("1"), )) if ctx.attr.expected_exit_code: envs.append(_ENV_SET.format( var = "JS_BINARY__EXPECTED_EXIT_CODE", - quoted_value = _bash_quote(str(ctx.attr.expected_exit_code)), + quoted_value = _quote(str(ctx.attr.expected_exit_code)), )) if ctx.attr.copy_data_to_bin: # Set an environment variable to flag that we have copied js_binary data to bin - envs.append(_ENV_SET.format(var = "JS_BINARY__COPY_DATA_TO_BIN", quoted_value = _bash_quote("1"))) + envs.append(_ENV_SET.format(var = "JS_BINARY__COPY_DATA_TO_BIN", quoted_value = _quote("1"))) if ctx.attr.chdir: # Set chdir env if not already set to allow js_run_binary to override @@ -388,11 +394,11 @@ def _bash_launcher(ctx, nodeinfo, entry_point_path, log_prefix_rule_set, log_pre else: normalized_chdir = chdir_value - envs.append(_ENV_SET_IFF_NOT_SET.format(var = "JS_BINARY__CHDIR", quoted_value = _bash_quote(normalized_chdir))) + envs.append(_ENV_SET_IFF_NOT_SET.format(var = "JS_BINARY__CHDIR", quoted_value = _quote(normalized_chdir))) # Set log envs iff not already set to allow js_run_binary to override for env in envs_for_log_level(ctx.attr.log_level): - envs.append(_ENV_SET_IFF_NOT_SET.format(var = env, quoted_value = _bash_quote("1"))) + envs.append(_ENV_SET_IFF_NOT_SET.format(var = env, quoted_value = _quote("1"))) node_options = [] for node_option in ctx.attr.node_options: diff --git a/js/private/node-patches/register.cjs b/js/private/node-patches/register.cjs index 85d67ddf90..c100d3f583 100644 --- a/js/private/node-patches/register.cjs +++ b/js/private/node-patches/register.cjs @@ -35,7 +35,7 @@ if ( JS_BINARY__PATCH_NODE_FS != '0' && JS_BINARY__FS_PATCH_ROOTS ) { - const roots = JS_BINARY__FS_PATCH_ROOTS.split(':') + const roots = JS_BINARY__FS_PATCH_ROOTS.split(process.platform === 'win32' ? ';' : ':') if (JS_BINARY__LOG_DEBUG) { console.error( `DEBUG: ${JS_BINARY__LOG_PREFIX}: node fs patches will be applied with roots: ${roots}` From dca3d4788ffe48c85b3e26662ac7eaab1bdfb40d Mon Sep 17 00:00:00 2001 From: Chris Brown <77508021+peakschris@users.noreply.github.com> Date: Tue, 9 Jun 2026 06:30:26 -0400 Subject: [PATCH 4/9] fixes from AI review --- js/private/js_binary.bat.tpl | 42 ++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/js/private/js_binary.bat.tpl b/js/private/js_binary.bat.tpl index 79b87a148a..5d5cfb8d36 100644 --- a/js/private/js_binary.bat.tpl +++ b/js/private/js_binary.bat.tpl @@ -127,7 +127,7 @@ if defined STDERR_CAPTURE ( if defined JS_BINARY__STDERR_OUTPUT_FILE ( copy "!STDERR_CAPTURE!" "%JS_BINARY__STDERR_OUTPUT_FILE%" >nul ) - if %ERRORLEVEL% neq 0 ( + if %_exit_code% neq 0 ( type "!STDERR_CAPTURE!" >&2 ) else if not defined JS_BINARY__SILENT_ON_SUCCESS ( type "!STDERR_CAPTURE!" >&2 @@ -139,7 +139,7 @@ if defined STDOUT_CAPTURE ( if defined JS_BINARY__STDOUT_OUTPUT_FILE ( copy "!STDOUT_CAPTURE!" "%JS_BINARY__STDOUT_OUTPUT_FILE%" >nul ) - if %ERRORLEVEL% neq 0 ( + if %_exit_code% neq 0 ( type "!STDOUT_CAPTURE!" ) else if not defined JS_BINARY__SILENT_ON_SUCCESS ( type "!STDOUT_CAPTURE!" @@ -148,15 +148,14 @@ if defined STDOUT_CAPTURE ( ) if defined JS_BINARY__LOG_DEBUG ( - call :logf_debug "exit code: %ERRORLEVEL%" + call :logf_debug "exit code: %_exit_code%" ) if defined JS_BINARY__PUSHD ( popd ) -endlocal -exit /b %ERRORLEVEL% +endlocal & exit /b %_exit_code% :main @@ -221,12 +220,13 @@ if defined bazel_out_segment ( ) ) else ( rem We are in execroot or in some other context all together such as a nodejs_image or a manually run js_binary - set "JS_BINARY__EXECROOT=%~dp0" + set "JS_BINARY__EXECROOT=%CD%" ) if not defined JS_BINARY__NO_CD_BINDIR ( if not defined BAZEL_BINDIR ( call :logf_fatal "BAZEL_BINDIR must be set in environment to the makevar $(BINDIR) in js_binary build actions (which run in the execroot) so that build actions can change directories to always run out of the root of the Bazel output tree. See https://docs.bazel.build/versions/main/be/make-variables.html#predefined_variables. This is automatically set by 'js_run_binary' (https://github.com/aspect-build/rules_js/blob/main/docs/js_run_binary.md) which is the recommended rule to use for using a js_binary as the tool of a build action. If this is not a build action you can set the BAZEL_BINDIR to '.' instead to suppress this error. For more context on this design decision, please read the aspect_rules_js README https://github.com/aspect-build/rules_js/tree/dbb5af0d2a9a2bb50e4cf4a96dbc582b27567155#running-nodejs-programs." + set "_exit_code=1" goto :cleanup_and_exit ) @@ -243,11 +243,13 @@ if defined bazel_out_segment ( if defined JS_BINARY__USE_EXECROOT_ENTRY_POINT ( if not defined BAZEL_BINDIR ( call :logf_fatal "Expected BAZEL_BINDIR to be set when JS_BINARY__USE_EXECROOT_ENTRY_POINT is set" + set "_exit_code=1" goto :cleanup_and_exit ) if not defined JS_BINARY__COPY_DATA_TO_BIN ( if not defined JS_BINARY__ALLOW_EXECROOT_ENTRY_POINT_WITH_NO_COPY_DATA_TO_BIN ( call :logf_fatal "Expected js_binary copy_data_to_bin to be True when js_run_binary use_execroot_entry_point is True. To disable this validation you can set allow_execroot_entry_point_with_no_copy_data_to_bin to True in js_run_binary" + set "_exit_code=1" goto :cleanup_and_exit ) ) @@ -257,6 +259,7 @@ if defined JS_BINARY__NO_RUNFILES ( if not defined JS_BINARY__COPY_DATA_TO_BIN ( if not defined JS_BINARY__ALLOW_EXECROOT_ENTRY_POINT_WITH_NO_COPY_DATA_TO_BIN ( call :logf_fatal "Expected js_binary copy_data_to_bin to be True when js_binary use_execroot_entry_point is True. To disable this validation you can set allow_execroot_entry_point_with_no_copy_data_to_bin to True in js_run_binary" + set "_exit_code=1" goto :cleanup_and_exit ) ) @@ -273,6 +276,7 @@ if defined JS_BINARY__USE_EXECROOT_ENTRY_POINT ( ) if not exist "!entry_point!" ( call :logf_fatal "the entry_point '%entry_point%' not found" + set "_exit_code=1" goto :cleanup_and_exit ) @@ -284,6 +288,7 @@ if %errorlevel% equ 0 ( set "JS_BINARY__NODE_BINARY=%node%" if not exist "!JS_BINARY__NODE_BINARY!" ( call :logf_fatal "node binary '%JS_BINARY__NODE_BINARY%' not found" + set "_exit_code=1" goto :cleanup_and_exit ) ) else ( @@ -295,6 +300,7 @@ if %errorlevel% equ 0 ( ) if not exist "!JS_BINARY__NODE_BINARY!" ( call :logf_fatal "node binary '%JS_BINARY__NODE_BINARY%' not found" + set "_exit_code=1" goto :cleanup_and_exit ) ) @@ -307,6 +313,7 @@ if defined npm ( set "JS_BINARY__NPM_BINARY=%npm%" if not exist "!JS_BINARY__NPM_BINARY!" ( call :logf_fatal "npm binary '%JS_BINARY__NPM_BINARY%' not found" + set "_exit_code=1" goto :cleanup_and_exit ) ) else ( @@ -318,6 +325,7 @@ if defined npm ( ) if not exist "!JS_BINARY__NPM_BINARY!" ( call :logf_fatal "npm binary '%JS_BINARY__NPM_BINARY%' not found" + set "_exit_code=1" goto :cleanup_and_exit ) ) @@ -331,6 +339,7 @@ if defined JS_BINARY__NO_RUNFILES ( ) if not exist "%JS_BINARY__NODE_WRAPPER%" ( call :logf_fatal "node wrapper '%JS_BINARY__NODE_WRAPPER%' not found" + set "_exit_code=1" goto :cleanup_and_exit ) @@ -342,6 +351,7 @@ if defined JS_BINARY__NO_RUNFILES ( ) if not exist "%JS_BINARY__NODE_PATCHES%" ( call :logf_fatal "node patches '%JS_BINARY__NODE_PATCHES%' not found" + set "_exit_code=1" goto :cleanup_and_exit ) @@ -459,25 +469,25 @@ if defined JS_BINARY__EXPECTED_EXIT_CODE ( rem This exit code is handled specially by Bazel: rem https://github.com/bazelbuild/bazel/blob/486206012a664ecb20bdb196a681efc9a9825049/src/main/java/com/google/devtools/build/lib/util/ExitCode.java#L44 set "BAZEL_EXIT_TESTS_FAILED=3" - call :cleanup_and_exit - exit /b 3 + set "_exit_code=3" + goto :cleanup_and_exit ) - call :cleanup_and_exit - exit /b %RESULT% + set "_exit_code=%RESULT%" + goto :cleanup_and_exit ) else ( - call :cleanup_and_exit - exit /b 0 + set "_exit_code=0" + goto :cleanup_and_exit ) ) if defined JS_BINARY__EXIT_CODE_OUTPUT_FILE ( rem Exit zero if the exit code was captured echo %RESULT%> "%JS_BINARY__EXIT_CODE_OUTPUT_FILE%" - call :cleanup_and_exit - exit /b 0 + set "_exit_code=0" + goto :cleanup_and_exit ) else ( - call :cleanup_and_exit - exit /b %RESULT% + set "_exit_code=%RESULT%" + goto :cleanup_and_exit ) rem ============================================================================== From dc4db64279b941329c53b13f28d528549164869d Mon Sep 17 00:00:00 2001 From: Chris Brown <77508021+peakschris@users.noreply.github.com> Date: Tue, 9 Jun 2026 06:46:51 -0400 Subject: [PATCH 5/9] revert some lines to tidy diff --- js/private/test/image/asserts.bzl | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/js/private/test/image/asserts.bzl b/js/private/test/image/asserts.bzl index db926a6548..a909144457 100644 --- a/js/private/test/image/asserts.bzl +++ b/js/private/test/image/asserts.bzl @@ -29,15 +29,15 @@ def assert_tar_listing(name, actual, expected): actual_listing = "_{}_listing".format(name) native.genrule( name = actual_listing, - srcs = [ - actual, - ], + srcs = [actual], testonly = True, outs = ["_{}.listing".format(name)], + # TODO: now that app layer has repo_mapping file in it which is not stable between different operating systems + # we need to exclude it from checksums + # See: https://github.com/aspect-build/rules_js/actions/runs/11749187598/job/32734931009?pr=2011 cmd ='TZ="UTC" LC_ALL="en_US.UTF-8" $(BSDTAR_BIN) -tvf $(execpath {}) --exclude "**/_repo_mapping" | {} >$@'.format(actual, sanitize_cmd), toolchains = ["@bsd_tar_toolchains//:resolved_toolchain"], ) - write_source_file( name = name, in_file = actual_listing, @@ -99,10 +99,10 @@ def assert_checksum(name, image_layer): testonly = True, srcs = ["{}_{}".format(image_layer, layer) for layer in layers], outs = [name + ".checksums"], - # TODO: now that app layer has repo_mapping file in it which is not stable between different operating systems + # now that app layer has repo_mapping file in it which is not stable between different operating systems # we need to exclude it from checksums # See: https://github.com/aspect-build/rules_js/actions/runs/11749187598/job/32734931009?pr=2011 - # TODO: also exclude node layer which is different between windows and linux + # also exclude node layer which is different between windows and linux # and ignore sha256sum windows difference (it prints '*' before each filename) cmd = """ COREUTILS_BIN=$$(realpath $(COREUTILS_BIN)) && From 795c89f753c0a2d63d1ca8cce586b066649324e5 Mon Sep 17 00:00:00 2001 From: Chris Brown <77508021+peakschris@users.noreply.github.com> Date: Tue, 9 Jun 2026 06:48:27 -0400 Subject: [PATCH 6/9] tidy diff --- js/private/test/image/asserts.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/private/test/image/asserts.bzl b/js/private/test/image/asserts.bzl index a909144457..749fa3752b 100644 --- a/js/private/test/image/asserts.bzl +++ b/js/private/test/image/asserts.bzl @@ -35,7 +35,7 @@ def assert_tar_listing(name, actual, expected): # TODO: now that app layer has repo_mapping file in it which is not stable between different operating systems # we need to exclude it from checksums # See: https://github.com/aspect-build/rules_js/actions/runs/11749187598/job/32734931009?pr=2011 - cmd ='TZ="UTC" LC_ALL="en_US.UTF-8" $(BSDTAR_BIN) -tvf $(execpath {}) --exclude "**/_repo_mapping" | {} >$@'.format(actual, sanitize_cmd), + cmd = 'TZ="UTC" LC_ALL="en_US.UTF-8" $(BSDTAR_BIN) -tvf $(execpath {}) --exclude "**/_repo_mapping" | {} >$@'.format(actual, sanitize_cmd), toolchains = ["@bsd_tar_toolchains//:resolved_toolchain"], ) write_source_file( From 5140618a3f763f6e5c151b5f381dfc0dc9d44783 Mon Sep 17 00:00:00 2001 From: Chris Brown <77508021+peakschris@users.noreply.github.com> Date: Tue, 9 Jun 2026 11:24:58 -0400 Subject: [PATCH 7/9] support bazel run --- js/private/js_binary.bat.tpl | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/js/private/js_binary.bat.tpl b/js/private/js_binary.bat.tpl index 5d5cfb8d36..4307301f67 100644 --- a/js/private/js_binary.bat.tpl +++ b/js/private/js_binary.bat.tpl @@ -225,18 +225,26 @@ if defined bazel_out_segment ( if not defined JS_BINARY__NO_CD_BINDIR ( if not defined BAZEL_BINDIR ( - call :logf_fatal "BAZEL_BINDIR must be set in environment to the makevar $(BINDIR) in js_binary build actions (which run in the execroot) so that build actions can change directories to always run out of the root of the Bazel output tree. See https://docs.bazel.build/versions/main/be/make-variables.html#predefined_variables. This is automatically set by 'js_run_binary' (https://github.com/aspect-build/rules_js/blob/main/docs/js_run_binary.md) which is the recommended rule to use for using a js_binary as the tool of a build action. If this is not a build action you can set the BAZEL_BINDIR to '.' instead to suppress this error. For more context on this design decision, please read the aspect_rules_js README https://github.com/aspect-build/rules_js/tree/dbb5af0d2a9a2bb50e4cf4a96dbc582b27567155#running-nodejs-programs." - set "_exit_code=1" - goto :cleanup_and_exit + if defined BUILD_WORKSPACE_DIRECTORY ( + rem BUILD_WORKSPACE_DIRECTORY is set by Bazel during 'bazel run' but not during build actions. + rem On Windows, 'bazel run' does not set the working directory to the runfiles tree (which would + rem contain \bazel-out\ and skip this code path), unlike Linux/Mac. So we detect the 'bazel run' + rem context here and skip the BAZEL_BINDIR requirement. See https://github.com/aspect-build/rules_js/issues/481 + call :logf_debug "BUILD_WORKSPACE_DIRECTORY is set, skipping BAZEL_BINDIR requirement as this is a 'bazel run' context" + ) else ( + call :logf_fatal "BAZEL_BINDIR must be set in environment to the makevar $(BINDIR) in js_binary build actions (which run in the execroot) so that build actions can change directories to always run out of the root of the Bazel output tree. See https://docs.bazel.build/versions/main/be/make-variables.html#predefined_variables. This is automatically set by 'js_run_binary' (https://github.com/aspect-build/rules_js/blob/main/docs/js_run_binary.md) which is the recommended rule to use for using a js_binary as the tool of a build action. If this is not a build action you can set the BAZEL_BINDIR to '.' instead to suppress this error. For more context on this design decision, please read the aspect_rules_js README https://github.com/aspect-build/rules_js/tree/dbb5af0d2a9a2bb50e4cf4a96dbc582b27567155#running-nodejs-programs." + set "_exit_code=1" + goto :cleanup_and_exit + ) + ) else ( + rem Since the process was launched in the execroot, we automatically change directory into the root of the + rem output tree (which we expect to be set in BAZEL_BIN). See + rem https://github.com/aspect-build/rules_js/tree/dbb5af0d2a9a2bb50e4cf4a96dbc582b27567155#running-nodejs-programs + rem for more context on why we do this. + call :logf_debug "changing directory to BAZEL_BINDIR (root of Bazel output tree) %BAZEL_BINDIR%" + set JS_BINARY__PUSHD=1 + pushd "%BAZEL_BINDIR%" ) - - rem Since the process was launched in the execroot, we automatically change directory into the root of the - rem output tree (which we expect to be set in BAZEL_BIN). See - rem https://github.com/aspect-build/rules_js/tree/dbb5af0d2a9a2bb50e4cf4a96dbc582b27567155#running-nodejs-programs - rem for more context on why we do this. - call :logf_debug "changing directory to BAZEL_BINDIR (root of Bazel output tree) %BAZEL_BINDIR%" - set JS_BINARY__PUSHD=1 - pushd "%BAZEL_BINDIR%" ) ) From 3ed4cab298b4c8ec7a9c85b1635dc1e1fff5af86 Mon Sep 17 00:00:00 2001 From: Chris Brown <77508021+peakschris@users.noreply.github.com> Date: Tue, 9 Jun 2026 20:49:54 -0400 Subject: [PATCH 8/9] jest_test fixes --- js/private/js_binary.bat.tpl | 15 +++++++++------ js/private/js_binary.bzl | 34 +++++++++++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/js/private/js_binary.bat.tpl b/js/private/js_binary.bat.tpl index 4307301f67..6dd40d88dd 100644 --- a/js/private/js_binary.bat.tpl +++ b/js/private/js_binary.bat.tpl @@ -225,12 +225,15 @@ if defined bazel_out_segment ( if not defined JS_BINARY__NO_CD_BINDIR ( if not defined BAZEL_BINDIR ( - if defined BUILD_WORKSPACE_DIRECTORY ( - rem BUILD_WORKSPACE_DIRECTORY is set by Bazel during 'bazel run' but not during build actions. - rem On Windows, 'bazel run' does not set the working directory to the runfiles tree (which would - rem contain \bazel-out\ and skip this code path), unlike Linux/Mac. So we detect the 'bazel run' - rem context here and skip the BAZEL_BINDIR requirement. See https://github.com/aspect-build/rules_js/issues/481 - call :logf_debug "BUILD_WORKSPACE_DIRECTORY is set, skipping BAZEL_BINDIR requirement as this is a 'bazel run' context" + rem On Windows, 'bazel run' and 'bazel test' do not set the working directory inside the + rem runfiles tree (which would contain \bazel-out\ and skip this code path), unlike Linux/Mac. + rem Detect these contexts via env vars that Bazel sets only for run/test, not build actions. + rem See https://github.com/aspect-build/rules_js/issues/481 + set "_is_bazel_run_or_test=" + if defined BUILD_WORKSPACE_DIRECTORY set "_is_bazel_run_or_test=1" + if defined TEST_TMPDIR set "_is_bazel_run_or_test=1" + if defined _is_bazel_run_or_test ( + call :logf_debug "skipping BAZEL_BINDIR requirement as this is a 'bazel run' or 'bazel test' context" ) else ( call :logf_fatal "BAZEL_BINDIR must be set in environment to the makevar $(BINDIR) in js_binary build actions (which run in the execroot) so that build actions can change directories to always run out of the root of the Bazel output tree. See https://docs.bazel.build/versions/main/be/make-variables.html#predefined_variables. This is automatically set by 'js_run_binary' (https://github.com/aspect-build/rules_js/blob/main/docs/js_run_binary.md) which is the recommended rule to use for using a js_binary as the tool of a build action. If this is not a build action you can set the BAZEL_BINDIR to '.' instead to suppress this error. For more context on this design decision, please read the aspect_rules_js README https://github.com/aspect-build/rules_js/tree/dbb5af0d2a9a2bb50e4cf4a96dbc582b27567155#running-nodejs-programs." set "_exit_code=1" diff --git a/js/private/js_binary.bzl b/js/private/js_binary.bzl index 9c4e0f651e..c953942646 100644 --- a/js/private/js_binary.bzl +++ b/js/private/js_binary.bzl @@ -304,10 +304,42 @@ def _windows_path(path): def _bash_quote(value): return json.encode(value) +_ENV_VAR_CHARS = {c: True for c in "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789_".elems()} + def _bat_quote(value): # Windows batch: set "VAR=VALUE" already provides quoting via the template, # so the value must NOT be wrapped in extra quotes (json.encode adds "...") - return value + # + # Convert $VAR env-var references to Windows batch %VAR% syntax. + # The $$ convention (used by rules like jest_test for $$XML_OUTPUT_FILE) + # gets reduced to $VAR by expand_locations/expand_variables before reaching + # here. On bash $VAR is a valid env-var dereference; on batch it's a literal + # string. Convert $VAR and ${VAR} to %VAR% so batch expands them at runtime. + # + # Example: required for $XML_OUTPUT_FILE from rules_jest + result = value + for _i in range(20): + pos = result.find("$") + if pos < 0: + break + rest = result[pos + 1:] + if len(rest) > 0 and rest[0] == "{": + brace_end = rest.find("}") + if brace_end < 0: + break + var_name = rest[1:brace_end] + result = result[:pos] + "%" + var_name + "%" + rest[brace_end + 1:] + else: + var_name = "" + for ch in rest.elems(): + if ch in _ENV_VAR_CHARS: + var_name += ch + else: + break + if not var_name: + break + result = result[:pos] + "%" + var_name + "%" + rest[len(var_name):] + return result def _bash_launcher(ctx, nodeinfo, entry_point_path, log_prefix_rule_set, log_prefix_rule, fixed_args, fixed_env): # Explicitly disable node fs patches on Windows: From aa08ca441e048c8d76295da093a99a74d6fb1a22 Mon Sep 17 00:00:00 2001 From: Chris Brown <77508021+peakschris@users.noreply.github.com> Date: Wed, 10 Jun 2026 07:05:35 -0400 Subject: [PATCH 9/9] don't fail on unsafe characters in target names --- js/private/js_binary.bat.tpl | 23 ++++++++++++++--------- js/private/js_binary.bzl | 27 ++++++++++++++++++++++++--- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/js/private/js_binary.bat.tpl b/js/private/js_binary.bat.tpl index 6dd40d88dd..f1e4d0ebca 100644 --- a/js/private/js_binary.bat.tpl +++ b/js/private/js_binary.bat.tpl @@ -377,9 +377,12 @@ set "JS_BINARY__NODE_OPTIONS=" {{node_options}} rem Process command line arguments -set "ARGS=" -set "ALL_ARGS={{fixed_args}} %*" -for %%a in (%ALL_ARGS%) do ( +rem Fixed args are baked in at build time; put them directly into ARGS. +rem They may contain double-quotes or parentheses so we cannot expand them +rem inside a for-loop or a set "..." literal. +set ARGS={{fixed_args}} +rem Process runtime args to extract any --node_options= flags. +for %%a in (%*) do ( set "ARG=%%a" if "!ARG:~0,15!"=="--node_options=" ( set "JS_BINARY__NODE_OPTIONS=!JS_BINARY__NODE_OPTIONS! !ARG:~15!" @@ -449,21 +452,23 @@ rem Run the main program rem ============================================================================== if defined JS_BINARY__LOG_INFO ( - call :logf_info "running %JS_BINARY__NODE_WRAPPER% %JS_BINARY__NODE_OPTIONS% -- %entry_point% %ARGS%" + call :logf_info "running %JS_BINARY__NODE_WRAPPER% !JS_BINARY__NODE_OPTIONS! -- %entry_point% !ARGS!" ) -rem Execute the node wrapper with proper output redirection +rem Execute the node wrapper with proper output redirection. +rem Use delayed expansion (!ARGS!, !JS_BINARY__NODE_OPTIONS!) so that parentheses +rem in expanded values are not mis-parsed as CMD block delimiters. if defined STDOUT_CAPTURE ( if defined STDERR_CAPTURE ( - "%JS_BINARY__NODE_WRAPPER%" %JS_BINARY__NODE_OPTIONS% -- "%entry_point%" %ARGS% 1>>"%STDOUT_CAPTURE%" 2>>"%STDERR_CAPTURE%" + "%JS_BINARY__NODE_WRAPPER%" !JS_BINARY__NODE_OPTIONS! -- "%entry_point%" !ARGS! 1>>"%STDOUT_CAPTURE%" 2>>"%STDERR_CAPTURE%" ) else ( - "%JS_BINARY__NODE_WRAPPER%" %JS_BINARY__NODE_OPTIONS% -- "%entry_point%" %ARGS% 1>>"%STDOUT_CAPTURE%" + "%JS_BINARY__NODE_WRAPPER%" !JS_BINARY__NODE_OPTIONS! -- "%entry_point%" !ARGS! 1>>"%STDOUT_CAPTURE%" ) ) else ( if defined STDERR_CAPTURE ( - "%JS_BINARY__NODE_WRAPPER%" %JS_BINARY__NODE_OPTIONS% -- "%entry_point%" %ARGS% 2>>"%STDERR_CAPTURE%" + "%JS_BINARY__NODE_WRAPPER%" !JS_BINARY__NODE_OPTIONS! -- "%entry_point%" !ARGS! 2>>"%STDERR_CAPTURE%" ) else ( - "%JS_BINARY__NODE_WRAPPER%" %JS_BINARY__NODE_OPTIONS% -- "%entry_point%" %ARGS% + "%JS_BINARY__NODE_WRAPPER%" !JS_BINARY__NODE_OPTIONS! -- "%entry_point%" !ARGS! ) ) diff --git a/js/private/js_binary.bzl b/js/private/js_binary.bzl index c953942646..59c0101cbe 100644 --- a/js/private/js_binary.bzl +++ b/js/private/js_binary.bzl @@ -301,6 +301,23 @@ def _windows_path(path): """ return path.replace("/", "\\") +def _bat_safe_name(name): + """Return a CMD-safe filename by replacing characters that break CMD path parsing. + + CMD cannot invoke a .bat file whose path contains parentheses or spaces + because it treats them as special characters before quote processing. + Replace them with underscores so the generated .bat path is CMD-safe. + The target label name is still used inside the .bat content where it is + safely enclosed in set "..." or delayed-expansion !VAR! contexts. + """ + safe = "" + for ch in name.elems(): + if ch in ("(", ")", " "): + safe += "_" + else: + safe += ch + return safe + def _bash_quote(value): return json.encode(value) @@ -441,9 +458,13 @@ def _bash_launcher(ctx, nodeinfo, entry_point_path, log_prefix_rule_set, log_pre if ctx.attr.expand_args: fixed_args = [expand_variables(ctx, expand_locations(ctx, fixed_arg, ctx.attr.data)) for fixed_arg in fixed_args] + # Use a CMD-safe name for output file paths on Windows: CMD cannot invoke a .bat + # whose path contains parentheses or spaces (they are special CMD syntax characters). + launcher_name = _bat_safe_name(ctx.label.name) if is_windows else ctx.label.name + toolchain_files = [] if is_windows: - node_wrapper = ctx.actions.declare_file("%s_node_bin/node.bat" % ctx.label.name) + node_wrapper = ctx.actions.declare_file("%s_node_bin/node.bat" % launcher_name) ctx.actions.expand_template( template = ctx.file._node_wrapper_bat, output = node_wrapper, @@ -464,7 +485,7 @@ def _bash_launcher(ctx, nodeinfo, entry_point_path, log_prefix_rule_set, log_pre if ctx.attr.include_npm: npm_path = nodeinfo.npm.short_path if nodeinfo.npm else nodeinfo.npm_path if is_windows: - npm_wrapper = ctx.actions.declare_file("%s_node_bin/npm.bat" % ctx.label.name) + npm_wrapper = ctx.actions.declare_file("%s_node_bin/npm.bat" % launcher_name) ctx.actions.expand_template( template = ctx.file._npm_wrapper_bat, output = npm_wrapper, @@ -507,7 +528,7 @@ def _bash_launcher(ctx, nodeinfo, entry_point_path, log_prefix_rule_set, log_pre # The '_' avoids collisions with another file matching the label name. # For example, test and test/my.spec.ts. This naming scheme is borrowed from rules_go: # https://github.com/bazelbuild/rules_go/blob/f3cc8a2d670c7ccd5f45434ab226b25a76d44de1/go/private/context.bzl#L144 - launcher = ctx.actions.declare_file("{}_/{}{}".format(ctx.label.name, ctx.label.name, ".bat" if is_windows else "")) + launcher = ctx.actions.declare_file("{}_/{}{}".format(launcher_name, launcher_name, ".bat" if is_windows else "")) ctx.actions.expand_template( template = template, output = launcher,