diff --git a/CHANGELOG.md b/CHANGELOG.md index baf34781d2..571a927246 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/src/cmakeProject.ts b/src/cmakeProject.ts index 7adfb90f66..57d42db4a5 100644 --- a/src/cmakeProject.ts +++ b/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 { 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() { diff --git a/src/paths.ts b/src/paths.ts index dd1016babd..56ca8c35ef 100644 --- a/src/paths.ts +++ b/src/paths.ts @@ -189,9 +189,13 @@ class Paths { return this._ninjaPath; } - async which(name: string): Promise { + async which(name: string, searchPATH?: string): Promise { return new Promise(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 { + async getCTestPath(wsc: DirectoryContext, overWriteCMakePathSetting?: string, searchPATH?: string): Promise { 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 { + async getCPackPath(wsc: DirectoryContext, overWriteCMakePathSetting?: string, searchPATH?: string): Promise { 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 { + async getCMakePath(wsc: DirectoryContext, overWriteCMakePathSetting?: string, searchPATH?: string): Promise { 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; } diff --git a/src/workspace.ts b/src/workspace.ts index d5d46a564f..a477ce6f7d 100644 --- a/src/workspace.ts +++ b/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 { - return paths.getCMakePath(this, overWriteCMakePathSetting); + getCMakePath(overWriteCMakePathSetting?: string, searchPATH?: string): Promise { + return paths.getCMakePath(this, overWriteCMakePathSetting, searchPATH); } /** * The CTest executable for the directory. See `cmakePath` for more * information. */ - getCTestPath(overWriteCMakePathSetting?: string): Promise { - return paths.getCTestPath(this, overWriteCMakePathSetting); + getCTestPath(overWriteCMakePathSetting?: string, searchPATH?: string): Promise { + return paths.getCTestPath(this, overWriteCMakePathSetting, searchPATH); } /** * The CPack executable for the directory. See `cmakePath` for more * information. */ - getCPackPath(overWriteCMakePathSetting?: string): Promise { - return paths.getCPackPath(this, overWriteCMakePathSetting); + getCPackPath(overWriteCMakePathSetting?: string, searchPATH?: string): Promise { + return paths.getCPackPath(this, overWriteCMakePathSetting, searchPATH); } } diff --git a/test/unit-tests/backend/paths.test.ts b/test/unit-tests/backend/paths.test.ts new file mode 100644 index 0000000000..00bb18d478 --- /dev/null +++ b/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; + }); +});