diff --git a/src/test/fakeSuite/Repros/test_real_make_nonWin_baseline.out b/src/test/fakeSuite/Repros/test_real_make_nonWin_baseline.out index 9c2cffa3..029e0f54 100644 --- a/src/test/fakeSuite/Repros/test_real_make_nonWin_baseline.out +++ b/src/test/fakeSuite/Repros/test_real_make_nonWin_baseline.out @@ -45,6 +45,7 @@ Spawning child process with: process args: -f,"./SubDir/makefile with space",--dry-run,--always-make,--keep-going,--print-directory working directory: {REPO:VSCODE-MAKEFILE-TOOLS}/src/test/fakeSuite/Repros shell type: default + shell args: none Generating dry-run elapsed time: 0 The make dry-run command failed. IntelliSense may work only partially or not at all. @@ -79,6 +80,7 @@ Spawning child process with: process args: all,-f,"./SubDir/makefile with space",--print-data-base,--no-builtin-variables,--no-builtin-rules,--question working directory: {REPO:VSCODE-MAKEFILE-TOOLS}/src/test/fakeSuite/Repros shell type: default + shell args: none Generating dry-run elapsed time: 0 Parsing for build targets from: "{REPO:VSCODE-MAKEFILE-TOOLS}/src/test/fakeSuite/Repros/.vscode/targets.log" No build targets have been detected. diff --git a/src/test/fakeSuite/Repros/test_real_make_windows_baseline.out b/src/test/fakeSuite/Repros/test_real_make_windows_baseline.out index 2adc5877..998a01e1 100644 --- a/src/test/fakeSuite/Repros/test_real_make_windows_baseline.out +++ b/src/test/fakeSuite/Repros/test_real_make_windows_baseline.out @@ -45,6 +45,7 @@ Spawning child process with: process args: -f,"./SubDir/makefile with space",--dry-run,--always-make,--keep-going,--print-directory working directory: {REPO:VSCODE-MAKEFILE-TOOLS}\src\test\fakeSuite\Repros shell type: default + shell args: none Generating dry-run elapsed time: 0 The make dry-run command failed. IntelliSense may work only partially or not at all. @@ -79,6 +80,7 @@ Spawning child process with: process args: all,-f,"./SubDir/makefile with space",--print-data-base,--no-builtin-variables,--no-builtin-rules,--question working directory: {REPO:VSCODE-MAKEFILE-TOOLS}\src\test\fakeSuite\Repros shell type: default + shell args: none Generating dry-run elapsed time: 0 Parsing for build targets from: "{REPO:VSCODE-MAKEFILE-TOOLS}\src\test\fakeSuite\Repros\.vscode\targets.log" No build targets have been detected. diff --git a/src/test/fakeSuite/extension.test.ts b/src/test/fakeSuite/extension.test.ts index 5dfcb038..95e9d4a6 100644 --- a/src/test/fakeSuite/extension.test.ts +++ b/src/test/fakeSuite/extension.test.ts @@ -36,6 +36,86 @@ import * as fs from "fs"; import * as path from "path"; import * as vscode from "vscode"; +suite("Unit testing parseShellSetting", () => { + suiteSetup(async function (this: Mocha.Context) { + this.timeout(100000); + }); + + setup(async function (this: Mocha.Context) { + this.timeout(100000); + }); + + test("parseShellSetting with undefined returns all undefined", () => { + const result = util.parseShellSetting(undefined); + expect(result.shellPath).to.be.undefined; + expect(result.shellArgs).to.be.undefined; + expect(result.shellEnv).to.be.undefined; + }); + + test("parseShellSetting with string path returns path only", () => { + const result = util.parseShellSetting("/usr/bin/bash"); + expect(result.shellPath).to.equal("/usr/bin/bash"); + expect(result.shellArgs).to.be.undefined; + expect(result.shellEnv).to.be.undefined; + }); + + test("parseShellSetting with object containing path only", () => { + const result = util.parseShellSetting({ + path: "C:/msys64/usr/bin/bash.exe", + }); + expect(result.shellPath).to.equal("C:/msys64/usr/bin/bash.exe"); + expect(result.shellArgs).to.be.undefined; + expect(result.shellEnv).to.be.undefined; + }); + + test("parseShellSetting with object containing path and args", () => { + const result = util.parseShellSetting({ + path: "C:/msys64/usr/bin/bash.exe", + args: ["--login", "-i"], + }); + expect(result.shellPath).to.equal("C:/msys64/usr/bin/bash.exe"); + expect(result.shellArgs).to.deep.equal(["--login", "-i"]); + expect(result.shellEnv).to.be.undefined; + }); + + test("parseShellSetting with full automation profile object", () => { + const result = util.parseShellSetting({ + path: "C:/msys64/usr/bin/bash.exe", + args: ["--login", "-i"], + env: { + MSYSTEM: "UCRT64", + CHERE_INVOKING: "1", + MSYS2_PATH_TYPE: "inherit", + }, + }); + expect(result.shellPath).to.equal("C:/msys64/usr/bin/bash.exe"); + expect(result.shellArgs).to.deep.equal(["--login", "-i"]); + expect(result.shellEnv).to.deep.equal({ + MSYSTEM: "UCRT64", + CHERE_INVOKING: "1", + MSYS2_PATH_TYPE: "inherit", + }); + }); + + test("parseShellSetting with empty args array", () => { + const result = util.parseShellSetting({ + path: "/bin/bash", + args: [], + }); + expect(result.shellPath).to.equal("/bin/bash"); + expect(result.shellArgs).to.deep.equal([]); + expect(result.shellEnv).to.be.undefined; + }); + + test("parseShellSetting with null returns all undefined", () => { + // This tests the edge case where the setting value might be null + const result = util.parseShellSetting(null as unknown as undefined); + expect(result.shellPath).to.be.undefined; + expect(result.shellArgs).to.be.undefined; + expect(result.shellEnv).to.be.undefined; + }); +}); + suite("Unit testing replacing characters in and outside of quotes", () => { suiteSetup(async function (this: Mocha.Context) { this.timeout(100000); diff --git a/src/util.ts b/src/util.ts index fa362979..82a1ef4e 100644 --- a/src/util.ts +++ b/src/util.ts @@ -354,6 +354,42 @@ export interface SpawnProcessResult { signal: string; } +// Interface for VS Code's terminal.integrated.automationProfile setting +// This matches the schema defined by VS Code for automationProfile settings +export interface AutomationProfileSetting { + path?: string; + args?: string[]; + env?: EnvironmentVariables; +} + +// Result type for parsed shell configuration +export interface ParsedShellConfiguration { + shellPath: string | undefined; + shellArgs: string[] | undefined; + shellEnv: EnvironmentVariables | undefined; +} + +// Helper function to parse automation profile settings into a structured shell configuration +// Exported for testing purposes +export function parseShellSetting( + shellSetting: string | AutomationProfileSetting | undefined +): ParsedShellConfiguration { + let shellPath: string | undefined; + let shellArgs: string[] | undefined; + let shellEnv: EnvironmentVariables | undefined; + + // Extract shell path, args, and env from the setting + if (typeof shellSetting === "object" && shellSetting !== null) { + shellPath = shellSetting.path; + shellArgs = shellSetting.args; + shellEnv = shellSetting.env; + } else if (typeof shellSetting === "string") { + shellPath = shellSetting; + } + + return { shellPath, shellArgs, shellEnv }; +} + // Helper to spawn a child process, hooked to callbacks that are processing stdout/stderr // forceEnglish is true when the caller relies on parsing english words from the output. export function spawnChildProcess( @@ -376,11 +412,10 @@ export function spawnChildProcess( ); return new Promise((resolve, reject) => { - // Honor the "terminal.integrated.automationShell." setting. + // Honor the "terminal.integrated.automationProfile." or deprecated "automationShell." setting. // According to documentation (and settings.json schema), the three allowed values for are "windows", "linux" and "osx". // child_process.SpawnOptions accepts a string (which can be read from the above setting) or the boolean true to let VSCode pick a default // based on where it is running. - let shellType: string | undefined; let shellPlatform: string = process.platform === "win32" ? "windows" @@ -389,16 +424,23 @@ export function spawnChildProcess( : "osx"; let workspaceConfiguration: vscode.WorkspaceConfiguration = vscode.workspace.getConfiguration("terminal"); - shellType = - workspaceConfiguration.get( + + // First try to get automationProfile (preferred), then fall back to deprecated automationShell + let shellSetting: string | AutomationProfileSetting | undefined = + workspaceConfiguration.get( `integrated.automationProfile.${shellPlatform}` - ) || // automationShell is deprecated + ) || workspaceConfiguration.get( `integrated.automationShell.${shellPlatform}` - ); // and replaced with automationProfile + ); // automationShell is deprecated and replaced with automationProfile + + // Parse shell configuration from the setting + const { shellPath, shellArgs, shellEnv } = parseShellSetting(shellSetting); - if (typeof shellType === "object") { - shellType = shellType["path"]; + // Merge shell environment from automationProfile with existing environment + let spawnEnvironment = finalEnvironment; + if (shellEnv) { + spawnEnvironment = mergeEnvironment(finalEnvironment, shellEnv); } // Final quoting decisions for process name and args before being executed. @@ -415,25 +457,39 @@ export function spawnChildProcess( logger.message( localize( "utils.quoting", - "Spawning child process with:\n process name: {0}\n process args: {1}\n working directory: {2}\n shell type: {3}", + "Spawning child process with:\n process name: {0}\n process args: {1}\n working directory: {2}\n shell type: {3}\n shell args: {4}", qProcessName, qArgs.join(","), options.workingDirectory, - shellType || "default" + shellPath || "default", + shellArgs?.join(",") || "none" ), "Debug" ); } - const child: child_process.ChildProcess = child_process.spawn( - qProcessName, - qArgs, - { + // Determine how to spawn the process based on shell configuration + let child: child_process.ChildProcess; + if (shellPath && shellArgs && shellArgs.length > 0) { + // When shell args are specified (e.g., ["--login", "-i"]), we need to spawn the shell directly + // and pass the command via -c flag, as child_process.spawn's shell option doesn't support shell args. + // Note: This uses -c which is the standard flag for POSIX-compatible shells (bash, sh, zsh). + // This is the expected use case when using automationProfile with args for shells like msys2, git-bash, etc. + // The qProcessName and qArgs are already processed by quoteStringIfNeeded which handles spaces and ampersands. + const commandLine = `${qProcessName} ${qArgs.join(" ")}`; + child = child_process.spawn(shellPath, [...shellArgs, "-c", commandLine], { cwd: options.workingDirectory, - shell: shellType || true, - env: finalEnvironment, - } - ); + shell: false, + env: spawnEnvironment, + }); + } else { + // Standard spawn with shell option (either a path or true for default shell) + child = child_process.spawn(qProcessName, qArgs, { + cwd: options.workingDirectory, + shell: shellPath || true, + env: spawnEnvironment, + }); + } if (child.pid) { make.setCurPID(child.pid); }