Add support for Windows .cmd and .bat files

[tonka3000]

Summary

I use ruff for our embedded python interpreter. This interpreter needs to be bootstrapped (env-vars) before be able to get called (technical limit of the environment itself).
This is usually done with .cmd file (the modern version of .bat) on windows which is kind of similar to .sh on linux.
.cmd files are often use on Windows, even vscode itself use it to start vscode on windows.

Today I noticed that I get a crash when using the .cmd interpreter path.

It seems that node require shell mode (in windows calling cmd.exe as the shell) to be able to call .cmd files correctly.
I needed to quote the input filename as well to avoid whitespace issues which looks like a bug in node itself.

With that PR I got no crashes anymore when the extension try to run ruff 😄 .

Note: The shell mode only get activated when the platform is windows and the file extension is .cmd, so users with regular executables should not be affected at all.

Test Plan

Manual testing it locally works great and as expected.

I added a utils-test to check the require shell mode flag.

The PR is related to the changes of #539

[dhruvmanila]

I use ruff for our embedded python interpreter. This interpreter needs to be bootstrapped (env-vars) before be able to get called (technical limit of the environment itself).
This is usually done with .cmd file (the modern version of .bat) on windows which is kind of similar to .sh on linux.
.cmd files are often use on Windows, even vscode itself use it to start vscode on windows.

So, what I understand from this is that a wrapper script is used to call the Python interpreter instead of calling it directory, right? I'm curious as to what are you using the ruff.interpreter option for? Currently, it's only used to start the ruff-lsp server or to find the ruff executable (for native server). Your use-case will help me understand whether this change is required or not.

[tonka3000]

So, what I understand from this is that a wrapper script is used to call the Python interpreter instead of calling it directory, right?

Yes. Not all required dll files can be next to the executable so we need to set e.g. the PATH variable upfront, otherwise the extension call the python interpreter and will fail.
In this line the ruff-vscode extension expect a callable executable which is not the case when I would call our "bare metal" executable.

I'm curious as to what are you using the ruff.interpreter option for?

I don't use it because I expect that ruff-vscode extension will get it from the python-vscode extension.

In general the python-vscode extension support interpreters with a .cmd extension.

Currently, it's only used to start the ruff-lsp server or to find the ruff executable (for native server). Your use-case will help me understand whether this change is required or not.

Checkout the mentioned line from above

[dhruvmanila]

I don't use it because I expect that ruff-vscode extension will get it from the python-vscode extension.

In general the python-vscode extension support interpreters with a .cmd extension.

I see. Does this mean that the vscode-python extension is providing a Python interpreter with a .cmd extension?

Today I noticed that I get a crash when using the .cmd interpreter path.

Is it possible for you to provide some logs around this crash?

Sorry to ask these many questions, I just want to understand why this is required. Overall, I think this seems like a reasonable change, I'll review it today.

[tonka3000]

I see. Does this mean that the vscode-python extension is providing a Python interpreter with a .cmd extension?

Not directly. It support .cmd files as interpreter path, so they can be executed.

Is it possible for you to provide some logs around this crash?

2024-08-07 17:22:30.671 [info] Server: Stop requested
2024-08-07 17:22:30.676 [info] Using interpreter: c:\Program Files\LDS Studio\bin\LDSPython.cmd
2024-08-07 17:22:30.685 [error] Error while trying to find the Ruff binary: Error: spawn EINVAL  <======= Spawn error
2024-08-07 17:22:30.822 [info] Falling back to bundled executable: c:\Users\myusername\.vscode\extensions\charliermarsh.ruff-2024.36.0-win32-x64\bundled\libs\bin\ruff.exe
2024-08-07 17:22:30.861 [info] Resolved 'ruff.nativeServer: auto' to use the native server
2024-08-07 17:22:30.862 [info] Found Ruff 0.5.4 at c:\Users\myusername\.vscode\extensions\charliermarsh.ruff-2024.36.0-win32-x64\bundled\libs\bin\ruff.exe
2024-08-07 17:22:30.862 [info] Server run command: c:\Users\myusername\.vscode\extensions\charliermarsh.ruff-2024.36.0-win32-x64\bundled\libs\bin\ruff.exe server
2024-08-07 17:22:30.862 [info] Server: Start requested.

My config

{
    "python.defaultInterpreterPath": "C:\\Program Files\\LDS Studio\\bin\\LDSPython.cmd",
    "python.autoComplete.extraPaths": [
        "C:\\Users\\myusername\\AppData\\Local\\company\\LDS\\pkgmanaged\\DC97AD51-CBBD-5954-9BC2-5124B5C6D82B\\python\\site-packages"
    ],
    "python.analysis.extraPaths": [
        "C:\\Users\\myusername\\AppData\\Local\\company\\LDS\\pkgmanaged\\DC97AD51-CBBD-5954-9BC2-5124B5C6D82B\\python\\site-packages"
    ]
}

[dhruvmanila]

Thanks for the details. Sorry that I wasn't able to get to it today. I plan to review it first thing tomorrow morning.

[MichaReiser]

The change does make sense to me. The only thing that I find odd is the need for quoting the executable name. It would be nice to understand the reason why it is necessary before merging

[MichaReiser]

It's "not" a node bug nodejs/node#38490 🥲

[MichaReiser]

I think an alternative to this is to call execFile with cmd.exe (may require some arguments) and the bat file as the first argument so that we don't need a shell (and deal with escaping arguments where spaces are just one character that needs escaping)

[tonka3000]

@MichaReiser Yes, calling cmd.exe directly is an option, but would need more code in the end. shell:true use %COMSPEC% in the background which is just the absolute path to cmd.exe.

So basically we are talking about reimplementing the node implemention with auto-quoting, because we use the shell on Windows anyway 😀

[dhruvmanila]

For reference, https://nodejs.org/api/child_process.html#spawning-bat-and-cmd-files-on-windows provides details on what's required for running .bat and .cmd files.

[dhruvmanila]

Thank you!