Introducing new cmake.shell setting for custom shell executable for CMake subprocesses (#4762)

Copilot · hanniavalera · web-flow · commit 206c6ee1b12b · 2026-03-02T16:30:39.000-05:00
* Initial plan * feat: Add cmake.shell setting for custom shell execution (#1750) Add a `cmake.shell` setting (string|null, default null) that routes all cmake/ctest/cpack subprocess invocations through a user-specified shell (e.g., Git Bash, MSYS2). This enables embedded devs using GNU toolchains with Unix Makefiles on Windows where POSIX path translation is required. Root cause fix: `!!options.shell` in proc.ts coerced string shell paths to boolean `true`, discarding the actual path. Changed to `options.shell ?? false` to preserve string values that Node.js child_process.spawn() natively supports. Fixes #1750 Co-authored-by: hanniavalera <90047725+hanniavalera@users.noreply.github.com> * fix: resolve bad merge in config.ts and config.test.ts Fix three merge conflicts between the shell and setBuildTargetSameAsLaunchTarget features: 1. config.ts: Add missing closing brace for shell getter 2. config.ts: Remove duplicate additionalBuildProblemMatchers emitter, add missing comma 3. config.test.ts: Remove duplicate additionalBuildProblemMatchers entry, add missing comma Co-authored-by: hanniavalera <90047725+hanniavalera@users.noreply.github.com> * fix: determineShell precedence, wire shell onChange, audit call sites 1. cmakeDriver.ts executeCommand(): On Windows, determineShell's command-type-specific detection (.cmd→cmd, .ps1→powershell) now takes precedence over config.shell so .cmd/.bat commands aren't routed through Git Bash. 2. cmakeProject.ts: Wire shell onChange emitter to trigger driver reload when cmake.shell changes at runtime. 3. cmakeTaskProvider.ts: Apply same determineShell precedence logic for direct proc.execute calls. 4. debuggerScriptDriver.ts: Propagate config.shell with determineShell precedence for CMake script debugging. 5. shell-propagation.test.ts: Add 9 tests for determineShell detection and precedence logic. Co-authored-by: hanniavalera <90047725+hanniavalera@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: hanniavalera <90047725+hanniavalera@users.noreply.github.com>
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,7 @@
 ## 1.23
 
 Features:
+- Add `cmake.shell` setting to route CMake/CTest/CPack subprocess invocations through a custom shell (e.g., Git Bash, MSYS2), enabling embedded toolchains that require POSIX path translation on Windows. [#1750](https://github.com/microsoft/vscode-cmake-tools/issues/1750)
 - triple: Add riscv32be riscv64be support. [#4648](https://github.com/microsoft/vscode-cmake-tools/pull/4648) [@lygstate](https://github.com/lygstate)
 - Add command to clear build diagnostics from the Problems pane. [#4691](https://github.com/microsoft/vscode-cmake-tools/pull/4691)
 - Clear build diagnostics from the Problems pane when a new build starts and populate them incrementally during the build. [#4608](https://github.com/microsoft/vscode-cmake-tools/issues/4608)
diff --git a/docs/cmake-settings.md b/docs/cmake-settings.md
@@ -78,6 +78,7 @@ Options that support substitution, in the table below, allow variable references
 | `cmake.saveBeforeBuild` | If `true` (the default), saves open text documents when build or configure is invoked before running CMake. | `true` | no |
 | `cmake.setBuildTargetSameAsLaunchTarget` | If `true`, setting the launch/debug target automatically sets the build target to match. | `false` | no |
 | `cmake.setBuildTypeOnMultiConfig` | If `true`, set build type on multi-config generators. | `false` | no |
+| `cmake.shell` | Path to a shell executable to route all CMake/CTest/CPack subprocess invocations through (e.g., Git Bash or MSYS2). Useful for embedded toolchains that require POSIX path translation on Windows. When `null`, the default system shell behavior is used. | `null` | no |
 | `cmake.showConfigureWithDebuggerNotification` | If `true`, show notification when configure with debugger. | `true` | no |
 | `cmake.showNotAllDocumentsSavedQuestion` | If `true`, show not all documents saved question. | `true` | no |
 | `cmake.showSystemKits` | If `true`, show system kits in kit selection. | `true` | no |
diff --git a/package.json b/package.json
@@ -2394,6 +2394,15 @@
           "description": "%cmake-tools.configuration.cmake.platform.description%",
           "scope": "resource"
         },
+        "cmake.shell": {
+          "type": [
+            "string",
+            "null"
+          ],
+          "default": null,
+          "description": "%cmake-tools.configuration.cmake.shell.description%",
+          "scope": "resource"
+        },
         "cmake.configureArgs": {
           "type": "array",
           "description": "%cmake-tools.configuration.cmake.configureArgs.description%",
diff --git a/package.nls.json b/package.nls.json
@@ -121,6 +121,7 @@
     "cmake-tools.configuration.cmake.generator.description": "The CMake generator to use.",
     "cmake-tools.configuration.cmake.toolset.description": "The CMake toolset to use when configuring.",
     "cmake-tools.configuration.cmake.platform.description": "The CMake platform to use when configuring.",
+    "cmake-tools.configuration.cmake.shell.description": "Path to a shell executable to use when running CMake, CTest, and CPack commands (e.g., Git Bash or MSYS2). When set, all subprocess invocations are routed through this shell. Useful for embedded toolchains that require POSIX path translation. When null, the default system shell behavior is used.",
     "cmake-tools.configuration.cmake.configureArgs.description": "Additional arguments to pass to CMake when configuring. When using CMake Presets, these arguments are temporarily appended to the arguments provided by the active configure preset.",
     "cmake-tools.configuration.cmake.buildArgs.description": "Additional arguments to pass to CMake when building. When using CMake Presets, these arguments are temporarily appended to the arguments provided by the active build preset.",
     "cmake-tools.configuration.cmake.buildToolArgs.description": "Additional arguments to pass to the underlying build tool when building. When using CMake Presets, these arguments are temporarily appended to the arguments provided by the active build preset to invoke the build tool.",
diff --git a/src/cmakeProject.ts b/src/cmakeProject.ts
@@ -847,6 +847,11 @@ export class CMakeProject {
         await this.reloadCMakeDriver();
     });
 
+    private readonly shellSub = this.workspaceContext.config.onChange('shell', async () => {
+        log.info(localize('shell.changed.restart.driver', "Restarting the CMake driver after a shell change."));
+        await this.reloadCMakeDriver();
+    });
+
     /**
      * The variant manager keeps track of build variants. Has two-phase init.
      */
@@ -890,7 +895,8 @@ export class CMakeProject {
             this.generatorSub,
             this.preferredGeneratorsSub,
             this.communicationModeSub,
-            this.cmakePathSub
+            this.cmakePathSub,
+            this.shellSub
         ]) {
             sub.dispose();
         }
diff --git a/src/cmakeTaskProvider.ts b/src/cmakeTaskProvider.ts
@@ -587,7 +587,10 @@ export class CustomBuildTaskTerminal extends proc.CommandConsumer implements vsc
         this.writeEmitter.fire(localize("build.started", "{0} task started....", taskName) + endOfLine);
         this.writeEmitter.fire(proc.buildCmdStr(cmakePath, args) + endOfLine);
         try {
-            this._process = proc.execute(cmakePath, args, this, this.options);
+            // On Windows, command-type-specific detection takes precedence over config.shell
+            const commandShell = process.platform === 'win32' ? proc.determineShell(cmakePath) : false;
+            const shell = (commandShell || undefined) ?? cmakeDriver.config.shell ?? undefined;
+            this._process = proc.execute(cmakePath, args, this, { ...this.options, shell });
             const result: proc.ExecutionResult = await this._process.result;
             this._process = undefined;
             if (result.retc) {
diff --git a/src/config.ts b/src/config.ts
@@ -237,6 +237,7 @@ export interface ExtensionConfigurationSettings {
     useFolderPropertyInBuildTargetDropdown: boolean;
     setBuildTargetSameAsLaunchTarget: boolean;
     additionalBuildProblemMatchers: BuildProblemMatcherConfig[];
+    shell: string | null;
 }
 
 type EmittersOf<T> = {
@@ -628,6 +629,10 @@ export class ConfigurationReader implements vscode.Disposable {
         return this.configData.useFolderPropertyInBuildTargetDropdown;
     }
 
+    get shell(): string | null {
+        return this.configData.shell;
+    }
+
     get setBuildTargetSameAsLaunchTarget(): boolean {
         return this.configData.setBuildTargetSameAsLaunchTarget;
     }
@@ -703,8 +708,9 @@ export class ConfigurationReader implements vscode.Disposable {
         postRunCoverageTarget: new vscode.EventEmitter<string | null>(),
         coverageInfoFiles: new vscode.EventEmitter<string[]>(),
         useFolderPropertyInBuildTargetDropdown: new vscode.EventEmitter<boolean>(),
-        setBuildTargetSameAsLaunchTarget: new vscode.EventEmitter<boolean>(),
-        additionalBuildProblemMatchers: new vscode.EventEmitter<BuildProblemMatcherConfig[]>()
+        additionalBuildProblemMatchers: new vscode.EventEmitter<BuildProblemMatcherConfig[]>(),
+        shell: new vscode.EventEmitter<string | null>(),
+        setBuildTargetSameAsLaunchTarget: new vscode.EventEmitter<boolean>()
     };
 
     /**
diff --git a/src/debug/cmakeDebugger/debuggerScriptDriver.ts b/src/debug/cmakeDebugger/debuggerScriptDriver.ts
@@ -46,7 +46,10 @@ export async function executeScriptWithDebugger(scriptPath: string, scriptArgs:
             cmakeLogger.info(localize('run.script', "Executing CMake script: {0}", scriptPath));
 
             const env = EnvironmentUtils.merge([process.env, EnvironmentUtils.create(scriptEnv)]);
-            const child = proc.execute(cmakeExe.path, concreteArgs, outputConsumer, { environment: env});
+            const commandShell = process.platform === 'win32' ? proc.determineShell(cmakeExe.path) : false;
+            const configShell = cmakeProject.workspaceContext.config.shell;
+            const shell = (commandShell || undefined) ?? configShell ?? undefined;
+            const child = proc.execute(cmakeExe.path, concreteArgs, outputConsumer, { environment: env, shell });
 
             while (
                 !outputConsumer.stateMessages.includes(
diff --git a/src/drivers/cmakeDriver.ts b/src/drivers/cmakeDriver.ts
@@ -519,7 +519,12 @@ export abstract class CMakeDriver implements vscode.Disposable {
 
     executeCommand(command: string, args?: string[], consumer?: proc.OutputConsumer, options?: proc.ExecutionOptions): proc.Subprocess {
         const environment = this.getEffectiveSubprocessEnvironment(options);
-        const exec_options = { ...options, environment };
+        // On Windows, command-type-specific detection (e.g. .cmd → cmd, .ps1 → powershell)
+        // must take precedence over config.shell to avoid routing commands through
+        // an incompatible shell (e.g. .cmd files through Git Bash).
+        const commandShell = process.platform === 'win32' ? proc.determineShell(command) : false;
+        const shell = options?.shell ?? (commandShell || undefined) ?? this.config.shell ?? undefined;
+        const exec_options = { ...options, environment, shell };
         return proc.execute(command, args, consumer, exec_options);
     }
 
@@ -554,7 +559,7 @@ export abstract class CMakeDriver implements vscode.Disposable {
             existing = undefined;
         }
         if (!existing) {
-            const shellPath = process.platform === 'win32' ? 'cmd.exe' : undefined;
+            const shellPath = this.config.shell ?? (process.platform === 'win32' ? 'cmd.exe' : undefined);
             const term = vscode.window.createTerminal({
                 name: localize('file.compilation', 'File Compilation'),
                 cwd: cmd.directory,
diff --git a/src/proc.ts b/src/proc.ts
@@ -189,7 +189,7 @@ export function execute(command: string, args?: string[], outputConsumer?: Outpu
 
     const spawn_opts: proc.SpawnOptions = {
         env: final_env,
-        shell: !!options.shell
+        shell: options.shell ?? false
     };
     if (options?.cwd !== undefined) {
         util.createDirIfNotExistsSync(options.cwd);
diff --git a/test/unit-tests/backend/shell-propagation.test.ts b/test/unit-tests/backend/shell-propagation.test.ts
@@ -0,0 +1,151 @@
+import { expect } from 'chai';
+
+/**
+ * Tests for the shell propagation logic used in proc.execute().
+ *
+ * The core fix is changing `shell: !!options.shell` to `shell: options.shell ?? false`
+ * in the spawn options. This preserves string shell paths instead of coercing them
+ * to boolean `true`, which is required for Node.js child_process.spawn() to use
+ * a specific shell executable.
+ */
+
+// Mirror of proc.determineShell for backend tests (cannot import proc.ts directly
+// because it transitively depends on 'vscode').
+function determineShell(command: string): string | boolean {
+    if (command.endsWith('.cmd') || command.endsWith('.bat')) {
+        return 'cmd';
+    }
+    if (command.endsWith('.ps1')) {
+        return 'powershell';
+    }
+    return false;
+}
+
+suite('Shell propagation logic', () => {
+    // Simulates the old (broken) behavior: `!!options.shell`
+    function oldShellCoercion(shell: boolean | string | undefined): boolean {
+        return !!shell;
+    }
+
+    // Simulates the new (fixed) behavior: `options.shell ?? false`
+    function newShellCoercion(shell: boolean | string | undefined): boolean | string {
+        return shell ?? false;
+    }
+
+    test('String shell path is preserved with new logic', () => {
+        const shellPath = 'C:\\Program Files\\Git\\bin\\bash.exe';
+        const result = newShellCoercion(shellPath);
+        expect(result).to.eq(shellPath);
+    });
+
+    test('String shell path was coerced to true with old logic', () => {
+        const shellPath = 'C:\\Program Files\\Git\\bin\\bash.exe';
+        const result = oldShellCoercion(shellPath);
+        // This was the bug: string was coerced to boolean true
+        expect(result).to.eq(true);
+    });
+
+    test('Boolean true is preserved with new logic', () => {
+        expect(newShellCoercion(true)).to.eq(true);
+    });
+
+    test('Boolean false is preserved with new logic', () => {
+        expect(newShellCoercion(false)).to.eq(false);
+    });
+
+    test('Undefined defaults to false with new logic', () => {
+        expect(newShellCoercion(undefined)).to.eq(false);
+    });
+
+    test('POSIX shell path is preserved with new logic', () => {
+        const shellPath = '/usr/bin/bash';
+        const result = newShellCoercion(shellPath);
+        expect(result).to.eq(shellPath);
+    });
+
+    // Simulates the executeCommand shell resolution with determineShell precedence:
+    // options?.shell ?? (commandShell || undefined) ?? config.shell ?? undefined
+    function resolveShell(optionsShell: boolean | string | undefined, configShell: string | null, command?: string): boolean | string | undefined {
+        const commandShell = command ? determineShell(command) : false;
+        return optionsShell ?? (commandShell || undefined) ?? configShell ?? undefined;
+    }
+
+    test('Explicit shell option takes precedence over config', () => {
+        expect(resolveShell(true, '/usr/bin/bash')).to.eq(true);
+    });
+
+    test('Config shell is used when no explicit option', () => {
+        expect(resolveShell(undefined, '/usr/bin/bash')).to.eq('/usr/bin/bash');
+    });
+
+    test('Undefined when neither option nor config is set', () => {
+        expect(resolveShell(undefined, null)).to.eq(undefined);
+    });
+
+    test('Explicit false option takes precedence over config', () => {
+        expect(resolveShell(false, '/usr/bin/bash')).to.eq(false);
+    });
+
+    test('Explicit string option takes precedence over config', () => {
+        expect(resolveShell('C:\\msys64\\usr\\bin\\bash.exe', '/usr/bin/bash')).to.eq('C:\\msys64\\usr\\bin\\bash.exe');
+    });
+});
+
+suite('determineShell command-type detection', () => {
+    test('.cmd commands require cmd shell', () => {
+        expect(determineShell('cmake.cmd')).to.eq('cmd');
+    });
+
+    test('.bat commands require cmd shell', () => {
+        expect(determineShell('build.bat')).to.eq('cmd');
+    });
+
+    test('.ps1 commands require powershell', () => {
+        expect(determineShell('setup.ps1')).to.eq('powershell');
+    });
+
+    test('Regular executables return false', () => {
+        expect(determineShell('cmake')).to.eq(false);
+        expect(determineShell('cmake.exe')).to.eq(false);
+        expect(determineShell('/usr/bin/cmake')).to.eq(false);
+    });
+
+    test('.cmd takes precedence over config.shell (Git Bash)', () => {
+        // Simulates the executeCommand resolution for a .cmd command
+        // when config.shell is set to Git Bash
+        const commandShell = determineShell('cmake.cmd');
+        const configShell = 'C:\\Program Files\\Git\\bin\\bash.exe';
+        // commandShell should win because .cmd requires cmd.exe
+        const resolved = (commandShell || undefined) ?? configShell ?? undefined;
+        expect(resolved).to.eq('cmd');
+    });
+
+    test('.bat takes precedence over config.shell (Git Bash)', () => {
+        const commandShell = determineShell('build.bat');
+        const configShell = 'C:\\Program Files\\Git\\bin\\bash.exe';
+        const resolved = (commandShell || undefined) ?? configShell ?? undefined;
+        expect(resolved).to.eq('cmd');
+    });
+
+    test('.ps1 takes precedence over config.shell (Git Bash)', () => {
+        const commandShell = determineShell('setup.ps1');
+        const configShell = 'C:\\Program Files\\Git\\bin\\bash.exe';
+        const resolved = (commandShell || undefined) ?? configShell ?? undefined;
+        expect(resolved).to.eq('powershell');
+    });
+
+    test('Regular command falls through to config.shell', () => {
+        const commandShell = determineShell('cmake');
+        const configShell = 'C:\\Program Files\\Git\\bin\\bash.exe';
+        // commandShell is false, so config.shell should be used
+        const resolved = (commandShell || undefined) ?? configShell ?? undefined;
+        expect(resolved).to.eq(configShell);
+    });
+
+    test('Regular command with no config.shell returns undefined', () => {
+        const commandShell = determineShell('cmake');
+        const configShell: string | null = null;
+        const resolved = (commandShell || undefined) ?? configShell ?? undefined;
+        expect(resolved).to.eq(undefined);
+    });
+});
diff --git a/test/unit-tests/config.test.ts b/test/unit-tests/config.test.ts
@@ -86,8 +86,9 @@ function createConfig(conf: Partial<ExtensionConfigurationSettings>): Configurat
         postRunCoverageTarget: null,
         coverageInfoFiles: [],
         useFolderPropertyInBuildTargetDropdown: true,
-        setBuildTargetSameAsLaunchTarget: false,
-        additionalBuildProblemMatchers: []
+        additionalBuildProblemMatchers: [],
+        shell: null,
+        setBuildTargetSameAsLaunchTarget: false
     });
     ret.updatePartial(conf);
     return ret;
@@ -156,4 +157,21 @@ suite('Configuration', () => {
         conf.updatePartial({ buildDirectory: 'Foo' });
         expect(conf.parallelJobs).to.eq(5);
     });
+
+    test('Read shell as null by default', () => {
+        const conf = createConfig({});
+        expect(conf.shell).to.be.null;
+    });
+
+    test('Read shell as string when set', () => {
+        const conf = createConfig({ shell: 'C:\\Program Files\\Git\\bin\\bash.exe' });
+        expect(conf.shell).to.eq('C:\\Program Files\\Git\\bin\\bash.exe');
+    });
+
+    test('Update shell setting', () => {
+        const conf = createConfig({ shell: null });
+        expect(conf.shell).to.be.null;
+        conf.updatePartial({ shell: '/usr/bin/bash' });
+        expect(conf.shell).to.eq('/usr/bin/bash');
+    });
 });
