From 8a6c9db4a72c5c3b1216d4cfc8b8976ac7661d7f Mon Sep 17 00:00:00 2001 From: Michael Aigner Date: Thu, 8 Aug 2024 18:01:11 +0200 Subject: [PATCH] Add support for Windows `.cmd` and `.bat` files (#570) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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 --------- Co-authored-by: Dhruv Manilawala --- src/common/server.ts | 16 +++++++++++++++- src/test/helper.ts | 5 +++++ src/test/utils.test.ts | 28 ++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 src/test/utils.test.ts diff --git a/src/common/server.ts b/src/common/server.ts index 9d07109..e29d538 100644 --- a/src/common/server.ts +++ b/src/common/server.ts @@ -1,5 +1,6 @@ import * as fsapi from "fs-extra"; import * as vscode from "vscode"; +import { platform } from "os"; import { Disposable, l10n, LanguageStatusSeverity, LogOutputChannel } from "vscode"; import { State } from "vscode-languageclient"; import { @@ -44,12 +45,25 @@ export type IInitializationOptions = { globalSettings: ISettings; }; +/** + * Check if shell mode is required for `execFile`. + * + * The conditions are: + * - Windows OS + * - File extension is `.cmd` or `.bat` + */ +export function execFileShellModeRequired(file: string) { + file = file.toLowerCase(); + return platform() === "win32" && (file.endsWith(".cmd") || file.endsWith(".bat")); +} + /** * Function to execute a command and return the stdout. */ function executeFile(file: string, args: string[] = []): Promise { + const shell = execFileShellModeRequired(file); return new Promise((resolve, reject) => { - execFile(file, args, (error, stdout, stderr) => { + execFile(shell ? `"${file}"` : file, args, { shell }, (error, stdout, stderr) => { if (error) { reject(new Error(stderr || error.message)); } else { diff --git a/src/test/helper.ts b/src/test/helper.ts index 94526c7..7dd5dde 100644 --- a/src/test/helper.ts +++ b/src/test/helper.ts @@ -1,5 +1,6 @@ import * as vscode from "vscode"; import * as path from "path"; +import { platform } from "os"; const EXTENSION_ID = "charliermarsh.ruff"; @@ -26,3 +27,7 @@ export const getDocumentPath = (p: string) => { export const getDocumentUri = (p: string) => { return vscode.Uri.file(getDocumentPath(p)); }; + +export const isWindows = () => { + return platform() === "win32"; +}; diff --git a/src/test/utils.test.ts b/src/test/utils.test.ts new file mode 100644 index 0000000..e5954e3 --- /dev/null +++ b/src/test/utils.test.ts @@ -0,0 +1,28 @@ +import * as assert from "assert"; +import { execFileShellModeRequired } from "../common/server"; +import { isWindows } from "./helper"; + +suite("Utils tests", () => { + test("Check execFile shell mode", () => { + assert.strictEqual( + execFileShellModeRequired("/use/random/python"), + false, + "Shell mode should not be required for unix paths", + ); + assert.strictEqual( + execFileShellModeRequired("C:\\random\\python.exe"), + false, + "Shell mode should not be required for .exe files", + ); + assert.strictEqual( + execFileShellModeRequired("C:\\random\\python.cmd"), + isWindows(), + "Shell mode should be required for .cmd files", + ); + assert.strictEqual( + execFileShellModeRequired("C:\\random\\python.bat"), + isWindows(), + "Shell mode should be required for .bat files", + ); + }); +});