Skip to content

Commit 2ef56fd

Browse files
Use cmake.environment and cmake.configureEnvironment for $penv{} expansion in preset includes (#4713)
* feat: use cmake.environment and cmake.configureEnvironment for $penv{} expansion in preset includes Fixes #3578 When loading CMakePresets.json (v7+), $penv{} macros in the `include` field now resolve environment variables defined in `cmake.environment` and `cmake.configureEnvironment` VS Code settings, not just process.env. Changes: - PresetsParser: Add settingsEnvironment setter and merge with process.env as penvOverride in getExpandedInclude() for v7+ and v9+ presets - PresetsController: Pass merged cmake.environment + cmake.configureEnvironment to PresetsParser, and subscribe to setting changes to reapply presets - Add integration test validating $penv{} resolves settings environment vars - Add CHANGELOG entry under 1.23 Improvements Co-authored-by: hanniavalera <[email protected]> * docs: clarify penvOverride conditional comment per code review Co-authored-by: hanniavalera <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: hanniavalera <[email protected]>
1 parent bf93ad9 commit 2ef56fd

4 files changed

Lines changed: 110 additions & 2 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ Features:
88

99
Improvements:
1010
- Add MSVC linker error problem matching to the Problems pane. [#4675](https://github.com/microsoft/vscode-cmake-tools/pull/4675) [@bradphelan](https://github.com/bradphelan)
11+
- Use environment variables from `cmake.environment` and `cmake.configureEnvironment` when expanding `$penv{}` macros in CMake Presets `include` paths. [#3578](https://github.com/microsoft/vscode-cmake-tools/issues/3578)
1112

1213
Bug Fixes:
1314
- Fix extension not switching to preset mode after "CMake: Quick Start" generates a CMakePresets.json, causing variant selection to have no effect. [#4569](https://github.com/microsoft/vscode-cmake-tools/issues/4569)

src/presets/presetsController.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import rollbar from '@cmt/rollbar';
1313
import { ExpansionErrorHandler, ExpansionOptions } from '@cmt/expand';
1414
import paths from '@cmt/paths';
1515
import { KitsController } from '@cmt/kits/kitsController';
16+
import { EnvironmentUtils } from '@cmt/environmentVariables';
1617
import { descriptionForKit, Kit, SpecialKits } from '@cmt/kits/kit';
1718
import { getHostTargetArchString } from '@cmt/installs/visualStudio';
1819
import { Diagnostic, DiagnosticSeverity, Position, Range } from 'vscode';
@@ -65,6 +66,10 @@ export class PresetsController implements vscode.Disposable {
6566
);
6667
}, presetsController._presetsChangedEmitter.fire, presetsController._userPresetsChangedEmitter.fire);
6768

69+
// Pass cmake.environment and cmake.configureEnvironment settings so that $penv{} in preset
70+
// include paths can resolve variables defined in VS Code settings.
71+
presetsController.updateSettingsEnvironment();
72+
6873
// We explicitly read presets file here, instead of on the initialization of the file watcher. Otherwise
6974
// there might be timing issues, since listeners are invoked async.
7075
await presetsController.reapplyPresets();
@@ -92,11 +97,33 @@ export class PresetsController implements vscode.Disposable {
9297
await presetsController.reapplyPresets();
9398
});
9499

100+
// We need to reapply presets when environment settings change so that $penv{} expansions
101+
// in include paths are re-evaluated with the updated environment variables.
102+
project.workspaceContext.config.onChange('environment', async () => {
103+
presetsController.updateSettingsEnvironment();
104+
await presetsController.reapplyPresets();
105+
});
106+
project.workspaceContext.config.onChange('configureEnvironment', async () => {
107+
presetsController.updateSettingsEnvironment();
108+
await presetsController.reapplyPresets();
109+
});
110+
95111
return presetsController;
96112
}
97113

98114
private constructor(private readonly project: CMakeProject, private readonly _kitsController: KitsController, private isMultiProject: boolean) {}
99115

116+
/**
117+
* Merges cmake.environment and cmake.configureEnvironment settings into a single
118+
* environment object and passes it to the PresetsParser for $penv{} expansion.
119+
* cmake.configureEnvironment takes precedence over cmake.environment.
120+
*/
121+
private updateSettingsEnvironment(): void {
122+
const env = this.project.workspaceContext.config.environment;
123+
const configureEnv = this.project.workspaceContext.config.configureEnvironment;
124+
this._presetsParser.settingsEnvironment = EnvironmentUtils.merge([env, configureEnv]);
125+
}
126+
100127
get presetsPath() {
101128
return this._presetsParser.presetsPath;
102129
}

src/presets/presetsParser.ts

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { fs } from "@cmt/pr";
88
import * as lodash from "lodash";
99
import { expandString, ExpansionErrorHandler, MinimalPresetContextVars } from "@cmt/expand";
1010
import { loadSchema } from "@cmt/schema";
11+
import { Environment, EnvironmentUtils } from "@cmt/environmentVariables";
1112

1213
nls.config({ messageFormat: nls.MessageFormat.bundle, bundleFormat: nls.BundleFormat.standalone })();
1314
const localize: nls.LocalizeFunc = nls.loadMessageBundle();
@@ -36,6 +37,7 @@ export class PresetsParser {
3637
private collectionsModifier: (filePath: string) => void;
3738
private presetsChangedHandler: (presets: preset.PresetsFile | undefined) => void;
3839
private userPresetsChangedHandler: (presets: preset.PresetsFile | undefined) => void;
40+
private _settingsEnvironment: Environment = {};
3941

4042
/**
4143
* Constructs the PresetsParser object
@@ -75,6 +77,14 @@ export class PresetsParser {
7577
this._sourceDir = sourceDir;
7678
}
7779

80+
/**
81+
* Sets additional environment variables from VS Code settings (cmake.environment, cmake.configureEnvironment)
82+
* that should be available for $penv{} macro expansion in preset include paths.
83+
*/
84+
set settingsEnvironment(env: Environment) {
85+
this._settingsEnvironment = env;
86+
}
87+
7888
get presetsFileExists(): boolean {
7989
return this._presetsFileExists || this._userPresetsFileExists;
8090
}
@@ -324,6 +334,14 @@ export class PresetsParser {
324334
hostSystemName: string,
325335
expansionErrors: ExpansionErrorHandler
326336
): Promise<string> {
337+
// Merge process.env with settings environment (cmake.environment, cmake.configureEnvironment)
338+
// so that $penv{} can resolve variables from VS Code settings.
339+
// When penvOverride is set, it fully replaces process.env in getParentEnvSubstitutions(),
340+
// so we only set it when there are settings to merge — otherwise leave undefined to preserve
341+
// the default process.env fallback behavior.
342+
const penvOverride = Object.keys(this._settingsEnvironment).length > 0
343+
? EnvironmentUtils.merge([process.env, this._settingsEnvironment])
344+
: undefined;
327345
return presetsFile.version >= 9
328346
? expandString(
329347
include,
@@ -336,7 +354,8 @@ export class PresetsParser {
336354
fileDir: path.dirname(file),
337355
pathListSep: path.delimiter
338356
},
339-
envOverride: {} // $env{} expansions are not supported in `include` v9
357+
envOverride: {}, // $env{} expansions are not supported in `include` v9
358+
penvOverride
340359
},
341360
expansionErrors
342361
)
@@ -347,7 +366,8 @@ export class PresetsParser {
347366
{
348367
// No vars are supported in Version 7 for include paths.
349368
vars: {} as MinimalPresetContextVars,
350-
envOverride: {} // $env{} expansions are not supported in `include` v9
369+
envOverride: {}, // $env{} expansions are not supported in `include` v9
370+
penvOverride
351371
},
352372
expansionErrors
353373
)

test/integration-tests/presets/validation.test.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -721,6 +721,66 @@ suite('Presets validation, inclusion, and expansion tests', () => {
721721
fs.rmSync(path.join(presetsParser.presetsPath, "..", "test.json"));
722722
}).timeout(100000);
723723

724+
/**
725+
* Confirm that $penv{} in include paths resolves environment variables
726+
* set via settingsEnvironment (cmake.environment / cmake.configureEnvironment).
727+
* Also verify that settingsEnvironment takes precedence over process.env for $penv{}.
728+
*/
729+
test('Validate `include` field supporting penv macro expansion with settings environment in v7', async () => {
730+
// Remove any process.env.SETTINGS_TEST that might exist.
731+
const origSettingsTest = process.env.SETTINGS_TEST;
732+
delete process.env.SETTINGS_TEST;
733+
734+
// Set the settings environment to provide SETTINGS_TEST variable.
735+
presetsParser.settingsEnvironment = { SETTINGS_TEST: sourceDirectory };
736+
737+
const v7WithInclude: any = lodash.cloneDeep(version7SupportedPresets);
738+
v7WithInclude.include = ["$penv{SETTINGS_TEST}/test.json"];
739+
v7WithInclude.configurePresets[0].name = "testSettingsName";
740+
741+
fs.writeFileSync(presetsParser.presetsPath, JSON.stringify(v7WithInclude));
742+
743+
// Create the include file.
744+
fs.writeFileSync(path.join(presetsParser.presetsPath, "..", "test.json"),
745+
JSON.stringify(v3SupportedPresets));
746+
747+
await presetsParser.resetPresetsFiles(
748+
new Map<string, PresetsFile>(),
749+
false,
750+
false
751+
);
752+
753+
// Should succeed: SETTINGS_TEST is resolved from settingsEnvironment.
754+
expect(presetsFileErrors).to.have.lengthOf(0);
755+
expect(preset.configurePresets(sourceDirectory).length).to.be.equal(2);
756+
757+
// Verify settingsEnvironment takes precedence over process.env for the same variable.
758+
presetsFileErrors = [];
759+
process.env.SETTINGS_TEST = "/nonexistent/path";
760+
presetsParser.settingsEnvironment = { SETTINGS_TEST: sourceDirectory };
761+
762+
fs.writeFileSync(presetsParser.presetsPath, JSON.stringify(v7WithInclude));
763+
764+
await presetsParser.resetPresetsFiles(
765+
new Map<string, PresetsFile>(),
766+
false,
767+
false
768+
);
769+
770+
// Should succeed: settingsEnvironment value (sourceDirectory) takes precedence.
771+
expect(presetsFileErrors).to.have.lengthOf(0);
772+
expect(preset.configurePresets(sourceDirectory).length).to.be.equal(2);
773+
774+
// Clean up.
775+
fs.rmSync(path.join(presetsParser.presetsPath, "..", "test.json"));
776+
presetsParser.settingsEnvironment = {};
777+
if (origSettingsTest !== undefined) {
778+
process.env.SETTINGS_TEST = origSettingsTest;
779+
} else {
780+
delete process.env.SETTINGS_TEST;
781+
}
782+
}).timeout(100000);
783+
724784
/**
725785
* Validate the v8 supports `$schema` field.
726786
*/

0 commit comments

Comments
 (0)