Properly handle powershell and batch wrappers#732
Properly handle powershell and batch wrappers#732fweikert merged 4 commits intobazelbuild:masterfrom
Conversation
|
@fweikert, could you please review it? |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for executing Windows-specific wrapper scripts (.bat and .ps1 files) on Windows platforms. When bazelisk delegates to a wrapper script on Windows, it now properly invokes cmd.exe for batch files and powershell.exe for PowerShell scripts with appropriate argument escaping and execution policies.
Key Changes
- Added platform-specific wrapper helper functions to handle Windows batch and PowerShell scripts
- Modified
makeBazelCmdto detect wrapper script types and invoke them through their respective interpreters - Created build tag-separated implementations for Windows and non-Windows platforms
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| core/wrapper_helpers.go | Windows-specific implementations of batch and PowerShell script command builders |
| core/wrapper_helpers_nonwindows.go | Non-Windows stub implementations that return standard exec.Command |
| core/core.go | Updated makeBazelCmd to route Windows wrapper scripts through appropriate interpreters |
| core/BUILD | Added new source files to the build configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…d use the command line with proper escaping for them. Without this fix, PowerShell wrappers were not working at all and batch scripts were failing to start if the path contains spaces.
…as the first element in the args array (found by Copilot during automatic review)
|
@fweikert, @meteorcloudy could you, please, take a look at the proposed changes? It's quite impolite to ignore the pull request for such a long time. After all, it fixes quite annoying bugs in your product. |
|
@valco1994 thanks for the PR! Looks good to me, modulo one small comment. |
…per type, as was requested in the code review
### What does this PR do? Update bazelisk from v1.27.0 to v1.28.1 across all supported platform configurations (Linux amd64/arm64 and Windows amd64). ### Motivation Ability to use platform-specific `tools/bazel*` wrappers: - bazelbuild/bazelisk#696 (v1.28.0) Ability to switch our `tools/bazel.bat` to `tools/bazel.ps1` on Windows: - bazelbuild/bazelisk#732 (v1.28.0) - bazelbuild/bazelisk#762 (v1.28.1)
### What does this PR do? Update bazelisk from v1.27.0 to v1.28.1 across all supported platform configurations (Linux amd64/arm64 and Windows amd64). ### Motivation Ability to use platform-specific `tools/bazel*` wrappers: - bazelbuild/bazelisk#696 (v1.28.0) Ability to switch our `tools/bazel.bat` to `tools/bazel.ps1` on Windows: - bazelbuild/bazelisk#732 (v1.28.0) - bazelbuild/bazelisk#762 (v1.28.1)
PowerShell scripts should be started using
powershell.exe -File ...and batch scripts should be started usingcmd.exe /c "..."when they are started via CreateProcessW (it's used under the hood to start processes on Windows in Go, seesyscall.StartProcess()). Otherwise PowerShell scripts can't be started at all and batch scripts can't be started in some cases.It's also important to mention, that
cmd.exerequires quite an unusual command line, and regular command line preparation insyscall.StartProcess()(seesyscall.makeCmdLine()) is not able to handle it properly and breaks it completely. So, I'm usingexec.Cmd.SysProcAttr.CmdLineto prepare the command line myself and use it without changes insyscall.StartProcess()in the case of a batch wrapper.This pull request fixes #643 and #731.