CMake executable cannot be a script

[JVApen]

Brief Issue Summary

The option "cmake.cmakePath" cannot refer to a script.
For example: (cmake.bat)

C:\path\to\cmake.exe $*

(In the real case, this is more complex as this triggers some remote execution)

Output:

[cmakeExecutable] CMake executable not found in cache. Checking again.
[proc] Executing command: C:/path/to/cmake.cmd --version
[proc] Executing command: C:/path/to/cmake.cmd -E capabilities

CMake Tools Diagnostics

No response

Debug Log

No response

Additional Information

I already did some reverse engineering on the code.
proc.ts has a method called execute which accepts some options. If options.shell is set to true, it should enable the shell option as described in https://nodejs.org/api/child_process.html#spawning-bat-and-cmd-files-on-windows
However cmakeExecutable.ts in the method getCMakeExecutableInformation doesn't pass this option. const execOpt: proc.ExecutionOptions = { showOutputOnError: true };

[emmenlau]

This is a relevant issue for us too, and a regression over some previous version (unsure which version).

[gcampbell-msft]

@JVApen @emmenlau If you could provide some more information about this issue, that would be great. For example, repro steps to reproduce this, or a small exacmple of a script you're using.

Looking at the code, nothing between version 1.18.44 and 1.19.50 was changed when checking the cmake.exe, so I wouldn't expect that it's causing an issue. Therefore, more investigation may be needed and any repro steps can help with that. Thanks!

[JVApen]

@gcampbell-msft : see my first event, simple CMD file with c:\path\to\cmake.exe $* should be sufficient to reproduce

[emmenlau]

Dear @gcampbell-msft ,
indeded, @JVApen is correct. A reproducer can be achieved with a simple CMD script like:

@ECHO OFF

REM We do other steps here as required, for example modify cmake settings, or change PATH, or change directory, ...
REM Then execute cmake:

C:\path\to\cmake.exe %*

[gcampbell-msft]

@JVApen When trying to reproduce with these steps: #4037 (comment), I notice that the "Bad CMake executable" popup appears in 1.18.44 as well. Do you also see this?

[gcampbell-msft]

@emmenlau @JVApen I actually see this issue as far back as 1.14.34. Is there something I'm missing in the repro? Or is this correct?

[emmenlau]

Dear @gcampbell-msft , sadly I can not say which version worked for me before. We have an isolated system that is not updated too frequently. I would think that the last working version was from about 6 months ago (both VSCode and all extensions), but I may be off a bit there.

In case that helps, we're using the same scripting "trick" on Linus and MacOS every day, and there it works flawless. These systems are updated very frequently, and used more than our Windows environment. So the problem is Windows-only, and I would think it has to do with not allowing nodejs to spawn bat files. I can only say this worked at some point, for at least a few months, maybe the better part of a year.

Does the suggestion from @JVApen help, about https://nodejs.org/api/child_process.html#spawning-bat-and-cmd-files-on-windows?

[gcampbell-msft]

@JVApen @emmenlau I've tracked the issue down. This didn't come from a CMake Tools update, but rather a VS Code update. In July, VS Code updated to 1.92 which now uses Node 20. This contains a breaking change for spawn, which we weren't aware of.

Could you please test the vsix (after modifying the file extension from .zip to .vsix) from here?
#4044 (comment)

[emmenlau]

Dear @gcampbell-msft , awesome!!! This indeed fixes the issue!!!

Just to confirm, the linked extension will identify itself as v1.13.0 (pre-release) which is a bit surprising because VSCode will complain that a newer version was already installed. But aside from this small detail, the linked cmake tools extension works and successfully executes by wrapper bat script on Windows!

Regression fixed!

[kuch3n]

This fix would allow converting windows paths to cygwins posix paths, too: #4038 Or even load shells for Cygwin, MSys etc.

[dbaeumer]

Small hint in case it help. There was a breaking change in NodeJS that it requires to pass shell: true if you want to execute a command script. If not pass the execution fails.

[emmenlau]

Small hint in case it help. There was a breaking change in NodeJS that it requires to pass shell: true if you want to execute a command script. If not pass the execution fails.

Thanks @dbaeumer for the hint! However the regression has already been fixed. Do you think that the fix requires more attention?

[dbaeumer]

@emmenlauif it works you are fine. The shell: true results in an exception which you would be able to observe.

[emmenlau]

Dear @dbaeumer , so just to be double sure: You are saying that the fix is fine as it is, and nothing else needs to be done, correct?

[dbaeumer]

Correct.