feat: Evaluate environmentSetupScript before CMake executable search

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## 1.23
 
 Features:
+- Evaluate kit's `environmentSetupScript` before searching for the CMake executable, so that `$PATH` changes from the script are respected during cmake lookup. [#2301](https://github.com/microsoft/vscode-cmake-tools/issues/2301)
 - 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)
 - Add individual CTest test nodes to the Project Outline with inline run/debug buttons, and enable debugging tests from both the Outline and Test Explorer without requiring a launch.json. [#4721](https://github.com/microsoft/vscode-cmake-tools/pull/4721)

src/cmakeProject.ts

@@ -30,7 +30,7 @@ import { CMakeBuildConsumer } from '@cmt/diagnostics/build';
 import { CMakeOutputConsumer } from '@cmt/diagnostics/cmake';
 import { FileDiagnostic, populateCollection } from '@cmt/diagnostics/util';
 import { expandStrings, expandString, ExpansionOptions } from '@cmt/expand';
-import { CMakeGenerator, Kit, SpecialKits } from '@cmt/kits/kit';
+import { CMakeGenerator, Kit, SpecialKits, effectiveKitEnvironment } from '@cmt/kits/kit';
 import * as logging from '@cmt/logging';
 import { fs } from '@cmt/pr';
 import { buildCmdStr, DebuggerEnvironmentVariable, ExecutionResult, ExecutionOptions } from './proc';
@@ -1407,7 +1407,14 @@ export class CMakeProject {
 
     async getCMakePathofProject(): Promise<string> {
         const overWriteCMakePathSetting = this.useCMakePresets ? this.configurePreset?.cmakeExecutable : undefined;
-        return await this.workspaceContext.getCMakePath(overWriteCMakePathSetting) || '';
+        // Evaluate the kit's environment (including environmentSetupScript) before searching
+        // for the cmake executable, so that PATH changes from the script are respected.
+        let searchPATH: string | undefined;
+        if (this._activeKit?.environmentSetupScript) {
+            const kitEnv = await effectiveKitEnvironment(this._activeKit);
+            searchPATH = kitEnv['PATH'];
+        }
+        return await this.workspaceContext.getCMakePath(overWriteCMakePathSetting, searchPATH) || '';
     }
 
     async getCMakeExecutable() {

src/paths.ts

@@ -189,9 +189,13 @@ class Paths {
         return this._ninjaPath;
     }
 
-    async which(name: string): Promise<string | null> {
+    async which(name: string, searchPATH?: string): Promise<string | null> {
         return new Promise<string | null>(resolve => {
-            which(name, (err, resolved) => {
+            const options: which.AsyncOptions & which.OptionsFirst = { all: false };
+            if (searchPATH) {
+                options.path = searchPATH;
+            }
+            which(name, options, (err, resolved) => {
                 if (err) {
                     resolve(null);
                 } else {
@@ -202,10 +206,10 @@ class Paths {
         });
     }
 
-    async getCTestPath(wsc: DirectoryContext, overWriteCMakePathSetting?: string): Promise<string | null> {
+    async getCTestPath(wsc: DirectoryContext, overWriteCMakePathSetting?: string, searchPATH?: string): Promise<string | null> {
         const ctestPath = await this.expandStringPath(wsc.config.rawCTestPath, wsc);
         if (!ctestPath || ctestPath === 'auto' || overWriteCMakePathSetting) {
-            const cmake = await this.getCMakePath(wsc, overWriteCMakePathSetting);
+            const cmake = await this.getCMakePath(wsc, overWriteCMakePathSetting, searchPATH);
             if (cmake === null) {
                 return null;
             } else {
@@ -225,10 +229,10 @@ class Paths {
         }
     }
 
-    async getCPackPath(wsc: DirectoryContext, overWriteCMakePathSetting?: string): Promise<string | null> {
+    async getCPackPath(wsc: DirectoryContext, overWriteCMakePathSetting?: string, searchPATH?: string): Promise<string | null> {
         const cpackPath = await this.expandStringPath(wsc.config.rawCPackPath, wsc);
         if (!cpackPath || cpackPath === 'auto' || overWriteCMakePathSetting) {
-            const cmake = await this.getCMakePath(wsc, overWriteCMakePathSetting);
+            const cmake = await this.getCMakePath(wsc, overWriteCMakePathSetting, searchPATH);
             if (cmake === null) {
                 return null;
             } else {
@@ -248,7 +252,7 @@ class Paths {
         }
     }
 
-    async getCMakePath(wsc: DirectoryContext, overWriteCMakePathSetting?: string): Promise<string | null> {
+    async getCMakePath(wsc: DirectoryContext, overWriteCMakePathSetting?: string, searchPATH?: string): Promise<string | null> {
         this._ninjaPath = undefined;
 
         let raw = overWriteCMakePathSetting;
@@ -258,7 +262,7 @@ class Paths {
 
         if (raw === 'auto' || raw === 'cmake') {
             // We start by searching $PATH for cmake
-            const on_path = await this.which('cmake');
+            const on_path = await this.which('cmake', searchPATH);
             if (on_path) {
                 return on_path;
             }

src/workspace.ts

@@ -43,21 +43,21 @@ export class DirectoryContext {
      * be used over `ConfigurationReader.cmakePath` because it will do additional
      * path expansion and searching.
      */
-    getCMakePath(overWriteCMakePathSetting?: string): Promise<string | null> {
-        return paths.getCMakePath(this, overWriteCMakePathSetting);
+    getCMakePath(overWriteCMakePathSetting?: string, searchPATH?: string): Promise<string | null> {
+        return paths.getCMakePath(this, overWriteCMakePathSetting, searchPATH);
     }
     /**
      * The CTest executable for the directory. See `cmakePath` for more
      * information.
      */
-    getCTestPath(overWriteCMakePathSetting?: string): Promise<string | null> {
-        return paths.getCTestPath(this, overWriteCMakePathSetting);
+    getCTestPath(overWriteCMakePathSetting?: string, searchPATH?: string): Promise<string | null> {
+        return paths.getCTestPath(this, overWriteCMakePathSetting, searchPATH);
     }
     /**
      * The CPack executable for the directory. See `cmakePath` for more
      * information.
      */
-    getCPackPath(overWriteCMakePathSetting?: string): Promise<string | null> {
-        return paths.getCPackPath(this, overWriteCMakePathSetting);
+    getCPackPath(overWriteCMakePathSetting?: string, searchPATH?: string): Promise<string | null> {
+        return paths.getCPackPath(this, overWriteCMakePathSetting, searchPATH);
     }
 }

test/unit-tests/backend/paths.test.ts

@@ -0,0 +1,78 @@
+import { expect } from 'chai';
+import * as path from 'path';
+import * as fs from 'fs';
+import * as os from 'os';
+import * as which from 'which';
+
+/**
+ * Tests verifying that the `which` npm package respects the `path` option,
+ * which is the mechanism used by paths.ts to search for cmake using a
+ * kit's environmentSetupScript-modified PATH.
+ */
+suite('[Paths - which with custom PATH]', () => {
+    let tmpDir: string;
+
+    setup(() => {
+        tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cmt-paths-test-'));
+    });
+
+    teardown(() => {
+        fs.rmSync(tmpDir, { recursive: true, force: true });
+    });
+
+    test('which finds executable on custom search PATH', async () => {
+        // Create a fake executable in our temp directory.
+        // On Windows, which uses PATHEXT (.EXE;.CMD;.BAT;.COM) to find executables,
+        // so the file needs a recognized extension.
+        const isWindows = process.platform === 'win32';
+        const fakeName = isWindows ? 'cmt-test-fake-exe.cmd' : 'cmt-test-fake-exe';
+        const fakePath = path.join(tmpDir, fakeName);
+        const searchName = isWindows ? 'cmt-test-fake-exe' : fakeName;
+        if (isWindows) {
+            fs.writeFileSync(fakePath, '@echo off\r\necho hello\r\n');
+        } else {
+            fs.writeFileSync(fakePath, '#!/bin/sh\necho hello\n');
+            fs.chmodSync(fakePath, 0o755);
+        }
+
+        // which should find it when we pass our tmpDir as the search path
+        const result = await which(searchName, { path: tmpDir });
+        expect(result).to.equal(fakePath);
+    });
+
+    test('which does not find executable when custom PATH does not include its directory', async () => {
+        // Create a fake executable in our temp directory
+        const isWindows = process.platform === 'win32';
+        const fakeName = isWindows ? 'cmt-test-fake-exe-missing.cmd' : 'cmt-test-fake-exe-missing';
+        const fakePath = path.join(tmpDir, fakeName);
+        const searchName = isWindows ? 'cmt-test-fake-exe-missing' : fakeName;
+        if (isWindows) {
+            fs.writeFileSync(fakePath, '@echo off\r\necho hello\r\n');
+        } else {
+            fs.writeFileSync(fakePath, '#!/bin/sh\necho hello\n');
+            fs.chmodSync(fakePath, 0o755);
+        }
+
+        // Create a different empty directory that doesn't contain the executable
+        const emptyDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cmt-paths-empty-'));
+        try {
+            let found: string | null = null;
+            try {
+                found = await which(searchName, { path: emptyDir });
+            } catch {
+                found = null;
+            }
+            expect(found).to.be.null;
+        } finally {
+            fs.rmSync(emptyDir, { recursive: true, force: true });
+        }
+    });
+
+    test('which falls back to process.env.PATH when no custom path is given', async () => {
+        // When no path option is provided, which searches process.env.PATH
+        // This should find a common system executable on any platform
+        const exeName = process.platform === 'win32' ? 'cmd' : 'sh';
+        const result = await which(exeName);
+        expect(result).to.not.be.null;
+    });
+});