Skip to content

Commit 24f6112

Browse files
authored
Honor environmentSetupScript output when expanding non-VS kit environmentVariables (#4091) (#4869)
* ensure kit env variables use script updated values * normalize path in test * use script path instead of process path * fix for PATH resolution error to use absolute paths instead
1 parent 7bf1f05 commit 24f6112

3 files changed

Lines changed: 64 additions & 6 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ Improvements:
4040
- Display info tooltip when hovering over CMake policy identifiers (e.g., `CMP0177`), showing the CMake version that introduced the policy, a short description, and a link to the official documentation. [#4544](https://github.com/microsoft/vscode-cmake-tools/issues/4544)
4141
- Append `cmake` to diagnostics that this extension contributes to the Problems Pane. [PR #4766](https://github.com/microsoft/vscode-cmake-tools/pull/4766)
4242
- Set the `VSCODE_CMAKE_TOOLS` environment variable for all spawned subprocesses so that `CMakeLists.txt` can detect when CMake is run from VS Code. [#4233](https://github.com/microsoft/vscode-cmake-tools/issues/4233)
43+
- Ensure kit `environmentVariables` expansions (for example, `${env:PATH}` or `${env.PATH}`) use the environment produced by `environmentSetupScript`, so script-updated values are preserved during expansion. [#4091](https://github.com/microsoft/vscode-cmake-tools/issues/4091)
4344
- Honor `debugger.workingDirectory` from the CMake File API when debugging a target, so that the `DEBUGGER_WORKING_DIRECTORY` target property is used as the debugger working directory. [#4595](https://github.com/microsoft/vscode-cmake-tools/issues/4595)
4445
- Add `cmake.removeStaleKitsOnScan` setting to optionally remove stale compiler kits from the kit picker after a "Scan for Kits" when they are no longer rediscovered. This is useful after compiler upgrades that leave older versions outside `PATH`. Set `"keep": true` in a kit entry to prevent automatic removal. [#3852](https://github.com/microsoft/vscode-cmake-tools/issues/3852)
4546
- Add `pr-readiness` Copilot skill to verify PRs have a descriptive title, meaningful description, and a properly formatted CHANGELOG entry. [#4862](https://github.com/microsoft/vscode-cmake-tools/pull/4862)

src/kits/kit.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -735,9 +735,12 @@ export async function getShellScriptEnvironment(kit: Kit, opts?: expand.Expansio
735735
// Quote the script file path before running it, in case there are spaces.
736736
run_command = `call "${script_path}"`;
737737
} else { // non-windows
738+
// Ensure a failing setup script aborts so we don't silently capture an unmodified environment.
739+
script += 'set -e\n';
738740
script += `source ${environmentSetupScript}\n`; // run the user shell script
739-
script += `printenv >> ${environment_path}`; // write env vars to temp file
740-
run_command = `/bin/bash -c "source ${script_path}"`; // run script in bash to enable bash-builtin commands like 'source'
741+
// Use an absolute path so PATH modifications in the setup script don't break env capture.
742+
script += `/usr/bin/printenv > "${environment_path}"`; // write env vars to temp file
743+
run_command = `/bin/bash "${script_path}"`;
741744
}
742745
try {
743746
await fs.unlink(environment_path); // delete the temp file if it exists
@@ -1089,12 +1092,13 @@ export async function effectiveKitEnvironment(kit: Kit, opts?: expand.ExpansionO
10891092
const kit_env = EnvironmentUtils.create(kit.environmentVariables);
10901093
const getVSKitEnv = process.platform === 'win32' && kit.visualStudio && kit.visualStudioArchitecture;
10911094
if (!getVSKitEnv) {
1092-
const expandOptions: expand.ExpansionOptions = {
1095+
const expandOptions: expand.ExpansionOptions = opts ? { ...opts, envOverride: host_env, penvOverride: host_env } : {
10931096
vars: {} as expand.KitContextVars,
1094-
envOverride: host_env
1097+
envOverride: host_env,
1098+
penvOverride: host_env
10951099
};
10961100
for (const env_var of Object.keys(kit_env)) {
1097-
env[env_var] = await expand.expandString(kit_env[env_var], opts ?? expandOptions);
1101+
env[env_var] = await expand.expandString(kit_env[env_var], expandOptions);
10981102
}
10991103

11001104
if (process.platform === 'win32') {

test/unit-tests/kitmanager.test.ts

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* eslint-disable no-unused-expressions */
2-
import { readKitsFile, getShellScriptEnvironment } from '@cmt/kits/kit';
2+
import { readKitsFile, getShellScriptEnvironment, effectiveKitEnvironment } from '@cmt/kits/kit';
33
import { expect } from '@test/util';
44
import * as path from 'path';
55
import paths from '@cmt/paths';
@@ -93,4 +93,57 @@ suite('Kits test', () => {
9393
expect(env_vars_arr).to.deep.include(['TESTVAR13', 'cde']);
9494
}
9595
});
96+
97+
test('Test environmentVariables expand using environmentSetupScript output', async () => {
98+
const fname_extension = process.platform === 'win32' ? 'bat' : 'sh';
99+
const fname = `cmake-kit-test-${Math.random().toString()}.${fname_extension}`;
100+
const script_path = path.join(paths.tmpDir, fname);
101+
const pathSeparator = process.platform === 'win32' ? ';' : ':';
102+
const basePath = process.platform === 'win32' ? 'C:\\base-path' : '/base-path';
103+
const scriptPath = process.platform === 'win32' ? 'C:\\script-path' : '/script-path';
104+
const appendedPath = process.platform === 'win32' ? 'C:\\env-var-path' : '/env-var-path';
105+
106+
process.env.PATH = basePath;
107+
108+
if (process.platform === 'win32') {
109+
await fs.writeFile(script_path, `set "PATH=${scriptPath};%PATH%"`);
110+
} else {
111+
await fs.writeFile(script_path, `export PATH="${scriptPath}:$PATH"`);
112+
}
113+
114+
const kit = {
115+
name: 'Test Kit 3',
116+
environmentSetupScript: script_path,
117+
environmentVariables: {
118+
PATH: `\${env:PATH}${pathSeparator}${appendedPath}`
119+
},
120+
isTrusted: true
121+
};
122+
123+
const setupOnlyKit = {
124+
name: 'Test Kit 3 setup-only',
125+
environmentSetupScript: script_path,
126+
isTrusted: true
127+
};
128+
129+
const setup_env_vars = await getShellScriptEnvironment(setupOnlyKit);
130+
expect(setup_env_vars).to.not.be.undefined;
131+
132+
const env_vars = await effectiveKitEnvironment(kit);
133+
await fs.unlink(script_path);
134+
135+
const normalizePathList = (value: string | undefined): string[] => {
136+
expect(value).to.not.eq(undefined);
137+
const parts = (value ?? '').split(pathSeparator);
138+
if (process.platform !== 'win32') {
139+
return parts;
140+
}
141+
142+
// cmd/set can emit PATH entries with different slash and drive-letter casing.
143+
return parts.map(part => path.win32.normalize(part).toLowerCase());
144+
};
145+
146+
expect(normalizePathList(setup_env_vars?.PATH)).to.deep.eq(normalizePathList(`${scriptPath}${pathSeparator}${basePath}`));
147+
expect(normalizePathList(env_vars.PATH)).to.deep.eq(normalizePathList(`${scriptPath}${pathSeparator}${basePath}${pathSeparator}${appendedPath}`));
148+
});
96149
});

0 commit comments

Comments
 (0)