Skip to content

Commit

Permalink
Always set shell to cmd.exe on Windows
Browse files Browse the repository at this point in the history
  • Loading branch information
vinistock committed Oct 25, 2024
1 parent 4afc68b commit c5243e9
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 36 deletions.
19 changes: 0 additions & 19 deletions vscode/src/ruby/rubyInstaller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ import os from "os";

import * as vscode from "vscode";

import { asyncExec } from "../common";

import { Chruby } from "./chruby";

interface RubyVersion {
Expand Down Expand Up @@ -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,
});
}
}
4 changes: 3 additions & 1 deletion vscode/src/ruby/versionManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion vscode/src/test/suite/ruby/custom.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion vscode/src/test/suite/ruby/none.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
25 changes: 11 additions & 14 deletions vscode/src/test/suite/ruby/rubyInstaller.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down Expand Up @@ -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(
Expand All @@ -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,
Expand Down

0 comments on commit c5243e9

Please sign in to comment.