Repository navigation
fix: Windows batch file line endings to avoid cmd parsing bug - #1222
Conversation
e90e6c4 to
e7c03d2
Compare
|
e7c03d2 to
bd509fd
Compare
fmeum
left a comment
There was a problem hiding this comment.
Wow, that's a subtle bug. Thanks for working on a workaround!
09bbce4 to
fd8897e
Compare
|
Anyone from the maintainers or recent merge authors (e.g., @alexeagle @hofbi) able to review this? |
Windows cmd has a known bug where GOTO/CALL to labels fails when batch files use LF-only line endings and the label crosses a 512-byte boundary during parsing. This causes "cannot find batch label" errors. References: - https://www.dostips.com/forum/viewtopic.php?t=8988 - rocq-prover/rocq#8610 This fix ensures all generated .bat files use CRLF line endings by converting templates and using \r\n in string replacements throughout: - diff_test: template and substitutions, - write_source_file: batch updater scripts, - windows_utils: native launcher scripts. To verify these changes work correctly across platforms, we add a Go binary (`check_newlines`) that validates line endings: - on Windows: verifies scripts have proper CRLF line endings, - otherwise: verifies \n to \r\n replacements don't affect POSIX scripts. Note: we initially considered `sh_test` with PowerShell/batch wrappers, but encountered issues with interpreter dependencies, script portability, and symlink handling across platforms. A single Go binary invoked via `native_test` proved more reliable and maintainable, working consistently across all platforms (no new external dependencies either).
Address review feedback to use `splitlines()` instead of `split("\n")`
and `replace("\n", "\r\n")` which could produce `\r\r\n` if input
already contains `\r\n`.
`splitlines` properly handles all line ending types (`\n`, `\r\n`, `\r`)
and avoids double-escaping issues.
fd8897e to
60a159e
Compare
This upgrade includes a Windows compatibility fix for batch file line endings (bazel-contrib/bazel-lib#1222), which resolves a cmd parsing bug where GOTO/CALL to labels fails when batch files use non CRLF-only line endings and a label crosses a 512-byte boundary: ```diff $ bazel test //deps/openssl3:module_consistency_test [...] ==================== Test output for //deps/openssl3:module_consistency_test: -The system cannot find the batch label specified - compare_files +compare_files +FAIL: files "C:\[...]\bazel-out\[...]\deps\openssl3\openssl3.MODULE.bazel.new" and "C:\[workspace]\deps\openssl3\openssl3.MODULE.bazel" differ. + + +@@//deps/openssl3:openssl3.MODULE.bazel is out of date. To update this file, run: + + bazel run //deps/openssl3:module_consistency +To see differences run: + + diff "C:\[...]\bazel-out\[...]\deps\openssl3\openssl3.MODULE.bazel.new" "C:\[workspace]\deps\openssl3\openssl3.MODULE.bazel" + ================================================================================ ``` Because this now surfaces we'd been generating `*.MODULE.bazel` with OS-specific newlines, the present change fixes it by always issuing UNIX-style newlines: ```sh $ bazel test //deps/openssl3:module_consistency_test [...] //deps/openssl3:module_consistency_test PASSED in 0.4s ``` `bazel_lib` 3.1.1 comes with further improvements, like: - bazel-contrib/bazel-lib#1217 - bazel-contrib/bazel-lib#1220 - bazel-contrib/bazel-lib#1232
### What does this PR do? This upgrade includes a Windows compatibility fix for batch file line endings (bazel-contrib/bazel-lib#1222), which resolves a cmd parsing bug where GOTO/CALL to labels fails when batch files use non CRLF-only line endings and a label crosses a 512-byte boundary: ```diff > bazel test //deps/openssl3:module_consistency_test [...] ==================== Test output for //deps/openssl3:module_consistency_test: -The system cannot find the batch label specified - compare_files +compare_files +FAIL: files "C:\[...]\bazel-out\[...]\deps\openssl3\openssl3.MODULE.bazel.new" and "C:\[workspace]\deps\openssl3\openssl3.MODULE.bazel" differ. + + +@@//deps/openssl3:openssl3.MODULE.bazel is out of date. To update this file, run: + + bazel run //deps/openssl3:module_consistency +To see differences run: + + diff "C:\[...]\bazel-out\[...]\deps\openssl3\openssl3.MODULE.bazel.new" "C:\[workspace]\deps\openssl3\openssl3.MODULE.bazel" + ================================================================================ ``` Because this now surfaces we'd been generating `*.MODULE.bazel` with OS-specific newlines, the present change fixes it by always issuing UNIX-style newlines: ``` > bazel test //deps/openssl3:module_consistency_test [...] //deps/openssl3:module_consistency_test PASSED in 0.4s ``` ### Motivation Keep up to date and better support colleagues working on Windows. ### Describe how you validated your changes Locally for the time being because we still have very limited test coverage due to #44455 being disputed. ### Additional Notes `bazel_lib` 3.1.0/3.1.1 comes with further improvements, like: - bazel-contrib/bazel-lib#1217 - bazel-contrib/bazel-lib#1220 - bazel-contrib/bazel-lib#1232 Co-authored-by: regis.desgroppes <regis.desgroppes@datadoghq.com>
bazel_lib 3.0.0 ships lib/private/diff_test_tmpl.bat with LF line
endings. cmd.exe mis-parses LF-only batch files, and whether a given
`goto :label` resolves depends on the byte offsets in the generated
script, so diff_test targets fail non-deterministically on Windows with:
The system cannot find the batch label specified - compare_files
That is what //yq/tests:case_yq_test_filters hits, while the otherwise
identical //yq/tests:case_yq_test_semantic_yaml passes purely because
its generated .bat has a different size.
bazel-contrib/bazel-lib#1222 ("fix: Windows batch file line endings to
avoid cmd parsing bug") emits CRLF and first shipped in bazel_lib 3.2.0,
so raise our floor to that version.
`main` is red on Windows: `//yq/tests:case_yq_test_filters` fails with ``` The system cannot find the batch label specified - compare_files ``` ## Root cause `bazel_lib` 3.0.0 (our declared floor) ships `lib/private/diff_test_tmpl.bat` with **LF** line endings. `cmd.exe` mis-parses LF-only batch files, and whether a given `goto :label` resolves depends on the byte offsets within the generated script. So `diff_test` fails non-deterministically on Windows depending on how long the target name, runfile paths, and `failure_message` happen to be. That's exactly the pattern here: `case_yq_test_filters` (generated .bat = 3932 bytes) fails, while the structurally identical `case_yq_test_semantic_yaml` (3986 bytes) passes. Both `yq` actions produce byte-identical output — `fc.exe` never even runs. Verified on the Windows runner: the generated `.bat` has `CRLF count=0`, `bare LF count=151`. bazel-lib fixed this in [#1222](bazel-contrib/bazel-lib#1222) ("fix: Windows batch file line endings to avoid cmd parsing bug"), which emits CRLF and first shipped in **bazel_lib 3.2.0**. ## Changes - Raise the `bazel_lib` floor in `MODULE.bazel` from 3.0.0 to 3.2.0 (the minimum with the fix — `renovate.json` deliberately excludes `MODULE.bazel`, so this is the module's supported-version declaration, not a routine bump). - CI: pass `--test_output=errors`. Without it a failing test only prints a path to a `test.log` that is never uploaded, which is why this failure was undiagnosable from CI logs. (`.github/workflows/ci.bazelrc` claims to be "referenced with a --bazelrc option in the call to bazel in ci.yaml", but it isn't, so its `test --test_output=errors` never applied.) 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Windows cmd has a known bug where GOTO/CALL to labels fails when batch files use LF-only line endings and the label crosses a 512-byte boundary during parsing. This causes "cannot find batch label" errors like, for
diff_test:References:
This fix ensures all generated .bat files use CRLF line endings by converting templates and using \r\n in string replacements throughout:
To verify these changes work correctly across platforms, we add a Go binary (
check_newlines) that validates line endings:Note: we initially considered
sh_testwith PowerShell/batch wrappers, but encountered issues with interpreter dependencies, script portability, and symlink handling across platforms.A single Go binary invoked via
native_testproved more reliable and maintainable, working consistently across all platforms (no new external dependencies either).