From c5243e94a40699b45ed1678333245ca9485adb44 Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Fri, 25 Oct 2024 15:44:15 -0400 Subject: [PATCH] Always set shell to cmd.exe on Windows --- vscode/src/ruby/rubyInstaller.ts | 19 -------------- vscode/src/ruby/versionManager.ts | 4 ++- vscode/src/test/suite/ruby/custom.test.ts | 2 +- vscode/src/test/suite/ruby/none.test.ts | 2 +- .../src/test/suite/ruby/rubyInstaller.test.ts | 25 ++++++++----------- 5 files changed, 16 insertions(+), 36 deletions(-) diff --git a/vscode/src/ruby/rubyInstaller.ts b/vscode/src/ruby/rubyInstaller.ts index 33f4912ab..043e9bf23 100644 --- a/vscode/src/ruby/rubyInstaller.ts +++ b/vscode/src/ruby/rubyInstaller.ts @@ -3,8 +3,6 @@ import os from "os"; import * as vscode from "vscode"; -import { asyncExec } from "../common"; - import { Chruby } from "./chruby"; interface RubyVersion { @@ -54,21 +52,4 @@ export class RubyInstaller extends Chruby { Searched in ${possibleInstallationUris.map((uri) => uri.fsPath).join(", ")}`, ); } - - // Override the `runScript` method to ensure that we do not pass any `shell` to `asyncExec`. The activation script is - // only compatible with `cmd.exe`, and not Powershell, due to escaping of quotes. We need to ensure to always run the - // script on `cmd.exe`. - protected runScript(command: string) { - this.outputChannel.info( - `Running command: \`${command}\` in ${this.bundleUri.fsPath}`, - ); - this.outputChannel.debug( - `Environment used for command: ${JSON.stringify(process.env)}`, - ); - - return asyncExec(command, { - cwd: this.bundleUri.fsPath, - env: process.env, - }); - } } diff --git a/vscode/src/ruby/versionManager.ts b/vscode/src/ruby/versionManager.ts index 428fb5c82..51d411357 100644 --- a/vscode/src/ruby/versionManager.ts +++ b/vscode/src/ruby/versionManager.ts @@ -88,7 +88,9 @@ export abstract class VersionManager { // If the user has configured a default shell, we use that one since they are probably sourcing their version // manager scripts in that shell's configuration files. On Windows, we never set the shell no matter what to ensure // that activation runs on `cmd.exe` and not PowerShell, which avoids complex quoting and escaping issues. - if (vscode.env.shell.length > 0 && os.platform() !== "win32") { + if (os.platform() === "win32") { + shell = "cmd.exe"; + } else if (vscode.env.shell.length > 0) { shell = vscode.env.shell; } diff --git a/vscode/src/test/suite/ruby/custom.test.ts b/vscode/src/test/suite/ruby/custom.test.ts index 27bc824f1..9c48ac99f 100644 --- a/vscode/src/test/suite/ruby/custom.test.ts +++ b/vscode/src/test/suite/ruby/custom.test.ts @@ -42,7 +42,7 @@ suite("Custom", () => { const { env, version, yjit } = await custom.activate(); // We must not set the shell on Windows - const shell = os.platform() === "win32" ? undefined : vscode.env.shell; + const shell = os.platform() === "win32" ? "cmd.exe" : vscode.env.shell; assert.ok( execStub.calledOnceWithExactly( diff --git a/vscode/src/test/suite/ruby/none.test.ts b/vscode/src/test/suite/ruby/none.test.ts index 1b5bbaf3e..ad5796860 100644 --- a/vscode/src/test/suite/ruby/none.test.ts +++ b/vscode/src/test/suite/ruby/none.test.ts @@ -39,7 +39,7 @@ suite("None", () => { const { env, version, yjit } = await none.activate(); // We must not set the shell on Windows - const shell = os.platform() === "win32" ? undefined : vscode.env.shell; + const shell = os.platform() === "win32" ? "cmd.exe" : vscode.env.shell; assert.ok( execStub.calledOnceWithExactly( diff --git a/vscode/src/test/suite/ruby/rubyInstaller.test.ts b/vscode/src/test/suite/ruby/rubyInstaller.test.ts index 6e478da36..eec0e6c69 100644 --- a/vscode/src/test/suite/ruby/rubyInstaller.test.ts +++ b/vscode/src/test/suite/ruby/rubyInstaller.test.ts @@ -12,7 +12,6 @@ import { RubyInstaller } from "../../../ruby/rubyInstaller"; import { WorkspaceChannel } from "../../../workspaceChannel"; import { LOG_CHANNEL } from "../../../common"; import { RUBY_VERSION } from "../../rubyVersion"; -import { ACTIVATION_SEPARATOR } from "../../../ruby/versionManager"; suite("RubyInstaller", () => { if (os.platform() !== "win32") { @@ -104,7 +103,7 @@ suite("RubyInstaller", () => { }); }); - test("Doesn't set the shell when invoking activation script", async () => { + test("Sets shell to cmd.exe when invoking activation script", async () => { const [major, minor, _patch] = RUBY_VERSION.split(".").map(Number); fs.symlinkSync( path.join( @@ -121,20 +120,18 @@ suite("RubyInstaller", () => { fs.writeFileSync(path.join(workspacePath, ".ruby-version"), RUBY_VERSION); const windows = new RubyInstaller(workspaceFolder, outputChannel); - const result = ["/fake/dir", "/other/fake/dir", true, RUBY_VERSION].join( - ACTIVATION_SEPARATOR, - ); - const execStub = sinon.stub(common, "asyncExec").resolves({ - stdout: "", - stderr: result, - }); + const execSpy = sinon.spy(common, "asyncExec"); + const { env, version, yjit } = await windows.activate(); + execSpy.restore(); - await windows.activate(); - execStub.restore(); + assert.strictEqual(execSpy.callCount, 1); + const callArgs = execSpy.getCall(0).args; + assert.strictEqual(callArgs[1]?.shell, "cmd.exe"); - assert.strictEqual(execStub.callCount, 1); - const callArgs = execStub.getCall(0).args; - assert.strictEqual(callArgs[1]?.shell, undefined); + assert.match(env.GEM_PATH!, /ruby\/3\.3\.0/); + assert.match(env.GEM_PATH!, /lib\/ruby\/gems\/3\.3\.0/); + assert.strictEqual(version, RUBY_VERSION); + assert.notStrictEqual(yjit, undefined); fs.rmSync(path.join(os.homedir(), `Ruby${major}${minor}-${os.arch()}`), { recursive: true,