Skip to content

Commit

Permalink
Merge branch 'main' into dev/gcampbell/FixIsUserPreset
Browse files Browse the repository at this point in the history
  • Loading branch information
gcampbell-msft authored Oct 9, 2024
2 parents fabdabd + 1fe1278 commit 30e3f11
Show file tree
Hide file tree
Showing 13 changed files with 400 additions and 278 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## 1.20

Features:

- Add support for Presets v9, which enables more macro expansion for the `include` field. [#3946](https://github.com/microsoft/vscode-cmake-tools/issues/3946)

Bug Fixes:

- Fix our setting of `isUserPreset` for presets, only set it to `true` if it's defined in a user presets file. [#4059](https://github.com/microsoft/vscode-cmake-tools/issues/4059)
Expand Down
2 changes: 2 additions & 0 deletions docs/cmake-settings.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,11 @@ Supported commands for substitution:
|`cmake.getLaunchTargetPath`|The full path to the target executable, including the filename. The existence of the target is not validated.|
|`cmake.getLaunchTargetDirectory`|The full path to the target executable's directory. The existence of the directory is not validated.|
|`cmake.getLaunchTargetFilename`|The name of the target executable file without any path information. The existence of the target is not validated.|
|`cmake.getLaunchTargetName`|The name to the target. The existence of the target is not validated.|
|`cmake.launchTargetPath`|The full path to the target executable, including the filename. If `cmake.buildBeforeRun` is true, invoking this substitution will also start a build.|
|`cmake.launchTargetDirectory`|The full path to the target executable's directory. If `cmake.buildBeforeRun` is true, invoking this substitution will also start a build.|
|`cmake.launchTargetFilename`|The name of the target executable file without any path information. If `cmake.buildBeforeRun` is true, invoking this substitution will also start a build.|
|`cmake.launchTargetName`|The name of the target. If `cmake.buildBeforeRun` is true, invoking this substitution will also start a build.|
|`cmake.buildTargetName`|The current target selected for build.|
|`cmake.buildType`|Same as `${buildType}`. The current CMake build type.|
|`cmake.buildKit`|Same as `${buildKit}`. The current CMake kit name.|
Expand Down
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,11 @@
"onCommand:cmake.launchTargetPath",
"onCommand:cmake.launchTargetDirectory",
"onCommand:cmake.launchTargetFilename",
"onCommand:cmake.launchTargetName",
"onCommand:cmake.getLaunchTargetPath",
"onCommand:cmake.getLaunchTargetDirectory",
"onCommand:cmake.getLaunchTargetFilename",
"onCommand:cmake.getlaunchTargetName",
"onCommand:cmake.buildType",
"onCommand:cmake.buildDirectory",
"onCommand:cmake.executableTargets",
Expand Down Expand Up @@ -3745,7 +3747,7 @@
"typescript": "^4.1.5",
"vscode-cmake-tools": "^1.2.0",
"vscode-nls-dev": "^3.3.2",
"webpack": "^5.76.0",
"webpack": "^5.94.0",
"webpack-cli": "^4.5.0"
},
"dependencies": {
Expand Down
22 changes: 22 additions & 0 deletions src/cmakeProject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2567,6 +2567,17 @@ export class CMakeProject {
return path.basename(targetPath);
}

/**
* Implementation of `cmake.launchTargetName`. This also ensures the target exists if `cmake.buildBeforeRun` is set.
*/
async launchTargetNameForSubstitution(): Promise<string | null> {
const targetPath = await this.launchTargetPath();
if (targetPath === null) {
return null;
}
return path.parse(targetPath).name;
}

/**
* Implementation of `cmake.getLaunchTargetPath`. This does not ensure the target exists.
*/
Expand Down Expand Up @@ -2613,6 +2624,17 @@ export class CMakeProject {
return path.basename(targetPath);
}

/**
* Implementation of `cmake.getLaunchTargetName`. This does not ensure the target exists.
*/
async getLaunchTargetName(): Promise<string | null> {
const targetPath = await this.getLaunchTargetPath();
if (targetPath === null) {
return null;
}
return path.parse(targetPath).name;
}

/**
* Implementation of `cmake.buildType`
*/
Expand Down
12 changes: 9 additions & 3 deletions src/drivers/cmakeServerClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,9 @@ export class CMakeServerClient {
break;
}
if (mat.length !== 3) {
debugger;
if (process.env.NODE_ENV === 'development') {
debugger;
}
throw new global.Error(localize('protocol.error.cmake', 'Protocol error talking to CMake! Got this input: {0}', input));
}
this.accInput = mat[2];
Expand Down Expand Up @@ -474,7 +476,9 @@ export class CMakeServerClient {
}
}
}
debugger;
if (process.env.NODE_ENV === 'development') {
debugger;
}
log.warning(localize('cant.yet.handle.message', 'Can\'t yet handle the {0} messages', some.type));
}

Expand Down Expand Up @@ -587,7 +591,9 @@ export class CMakeServerClient {
pipe.on('error', e => {
pipe.end();
if (!this.shutDownFlag) {
debugger;
if (process.env.NODE_ENV === 'development') {
debugger;
}
rollbar.takePromise(localize('pipe.error.from.cmake-server', 'Pipe error from cmake-server'),
{ pipe: pipeFile },
params.onPipeError(e));
Expand Down
12 changes: 9 additions & 3 deletions src/expand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,19 @@ export interface KitContextVars extends RequiredExpansionContextVars {
buildKitVersionMinor: string;
}

export interface PresetContextVars extends RequiredExpansionContextVars {
export interface PresetContextVars extends PresetContextNotPresetSpecificVars, RequiredExpansionContextVars {
[key: string]: string;
presetName: string;
}

export interface PresetContextNotPresetSpecificVars {
[key: string]: string;
sourceDir: string;
sourceParentDir: string;
sourceDirName: string;
presetName: string;
fileDir: string;
hostSystemName: string;
pathListSep: string;
}

export interface MinimalPresetContextVars extends RequiredExpansionContextVars {
Expand All @@ -68,7 +74,7 @@ export interface ExpansionOptions {
/**
* Plain `${variable}` style expansions.
*/
vars: KitContextVars | PresetContextVars | MinimalPresetContextVars;
vars: KitContextVars | PresetContextVars | MinimalPresetContextVars | PresetContextNotPresetSpecificVars;
/**
* Override the values used in `${env:var}`-style and `${env.var}`-style expansions.
*
Expand Down
28 changes: 28 additions & 0 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1601,6 +1601,19 @@ export class ExtensionManager implements vscode.Disposable {
}, folder);
}

launchTargetName(args?: FolderTargetNameArgsType) {
const [folder, targetName] = this.resolveFolderTargetNameArgs(args);

telemetry.logEvent("substitution", { command: "launchTargetName" });
return this.queryCMakeProject(async cmakeProject => {
if (targetName !== undefined && targetName !== null) {
await cmakeProject.setLaunchTargetByName(targetName);
}
const targetFilename = await cmakeProject.launchTargetNameForSubstitution();
return targetFilename;
}, folder);
}

getLaunchTargetPath(args?: FolderTargetNameArgsType) {
const [folder, targetName] = this.resolveFolderTargetNameArgs(args);

Expand Down Expand Up @@ -1640,6 +1653,19 @@ export class ExtensionManager implements vscode.Disposable {
}, folder);
}

getLaunchTargetName(args?: FolderTargetNameArgsType) {
const [folder, targetName] = this.resolveFolderTargetNameArgs(args);

telemetry.logEvent("substitution", { command: "getLaunchTargetName" });
return this.queryCMakeProject(async cmakeProject => {
if (targetName !== undefined && targetName !== null) {
await cmakeProject.setLaunchTargetByName(targetName);
}
const targetFilename = await cmakeProject.getLaunchTargetName();
return targetFilename;
}, folder);
}

buildTargetName(folder?: vscode.WorkspaceFolder | string) {
telemetry.logEvent("substitution", { command: "buildTargetName" });
return this.queryCMakeProject(cmakeProject => cmakeProject.buildTargetName(), folder);
Expand Down Expand Up @@ -2203,9 +2229,11 @@ async function setup(context: vscode.ExtensionContext, progress?: ProgressHandle
'launchTargetPath',
'launchTargetDirectory',
'launchTargetFilename',
'launchTargetName',
'getLaunchTargetPath',
'getLaunchTargetDirectory',
'getLaunchTargetFilename',
'getLaunchTargetName',
'buildTargetName',
'buildKit',
'buildType',
Expand Down
6 changes: 3 additions & 3 deletions src/preset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -822,13 +822,13 @@ async function getExpansionOptions(workspaceFolder: string, sourceDir: string, p
};

if (preset.__file && preset.__file.version >= 3) {
expansionOpts.vars['hostSystemName'] = await util.getHostSystemNameMemo();
expansionOpts.vars.hostSystemName = await util.getHostSystemNameMemo();
}
if (preset.__file && preset.__file.version >= 4) {
expansionOpts.vars['fileDir'] = path.dirname(preset.__file!.__path!);
expansionOpts.vars.fileDir = path.dirname(preset.__file!.__path!);
}
if (preset.__file && preset.__file.version >= 5) {
expansionOpts.vars['pathListSep'] = path.delimiter;
expansionOpts.vars.pathListSep = path.delimiter;
}

return expansionOpts;
Expand Down
88 changes: 62 additions & 26 deletions src/presetsController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { fs } from '@cmt/pr';
import * as preset from '@cmt/preset';
import * as util from '@cmt/util';
import rollbar from '@cmt/rollbar';
import { ExpansionErrorHandler, ExpansionOptions, getParentEnvSubstitutions, substituteAll } from '@cmt/expand';
import { expandString, ExpansionErrorHandler, ExpansionOptions, MinimalPresetContextVars } from '@cmt/expand';
import paths from '@cmt/paths';
import { KitsController } from '@cmt/kitsController';
import { descriptionForKit, Kit, SpecialKits } from '@cmt/kit';
Expand Down Expand Up @@ -187,18 +187,24 @@ export class PresetsController implements vscode.Disposable {
setOriginalPresetsFile(this.folderPath, undefined);
}

const expansionErrors: ExpansionErrorHandler = { errorList: [], tempErrorList: []};

presetsFile = await this.validatePresetsFile(presetsFile, file);
if (presetsFile) {
// Private fields must be set after validation, otherwise validation would fail.
this.populatePrivatePresetsFields(presetsFile, file);
await this.mergeIncludeFiles(presetsFile, file, referencedFiles);
await this.mergeIncludeFiles(presetsFile, file, referencedFiles, expansionErrors);

// add the include files to the original presets file
setPresetsPlusIncluded(this.folderPath, presetsFile);
if (expansionErrors.errorList.length > 0 || expansionErrors.tempErrorList.length > 0) {
presetsFile = undefined;
} else {
// add the include files to the original presets file
setPresetsPlusIncluded(this.folderPath, presetsFile);

// set the pre-expanded version so we can call expandPresetsFile on it
setExpandedPresets(this.folderPath, presetsFile);
presetsFile = await this.expandPresetsFile(presetsFile);
// set the pre-expanded version so we can call expandPresetsFile on it
setExpandedPresets(this.folderPath, presetsFile);
presetsFile = await this.expandPresetsFile(presetsFile, expansionErrors);
}
}

setExpandedPresets(this.folderPath, presetsFile);
Expand Down Expand Up @@ -1605,22 +1611,47 @@ export class PresetsController implements vscode.Disposable {
setFile(presetsFile.packagePresets);
}

private async mergeIncludeFiles(presetsFile: preset.PresetsFile | undefined, file: string, referencedFiles: Map<string, preset.PresetsFile | undefined>): Promise<void> {
private async getExpandedInclude(presetsFile: preset.PresetsFile, include: string, file: string, hostSystemName: string, expansionErrors: ExpansionErrorHandler): Promise<string> {
return presetsFile.version >= 9
? expandString(include, {
vars: {
sourceDir: this.folderPath,
sourceParentDir: path.dirname(this.folderPath),
sourceDirName: path.basename(this.folderPath),
hostSystemName: hostSystemName,
fileDir: path.dirname(file),
pathListSep: path.delimiter
},
envOverride: {} // $env{} expansions are not supported in `include` v9
}, expansionErrors)
: presetsFile.version >= 7
? // Version 7 and later support $penv{} expansions in include paths
expandString(include, {
// No vars are supported in Version 7 for include paths.
vars: {} as MinimalPresetContextVars,
envOverride: {} // $env{} expansions are not supported in `include` v9
}, expansionErrors)
: include;
}

private async mergeIncludeFiles(presetsFile: preset.PresetsFile | undefined, file: string, referencedFiles: Map<string, preset.PresetsFile | undefined>, expansionErrors: ExpansionErrorHandler): Promise<void> {
if (!presetsFile) {
return;
}

const hostSystemName = await util.getHostSystemNameMemo();

// CMakeUserPresets.json file should include CMakePresets.json file, by default.
if (this.presetsFileExist && file === this.userPresetsPath) {
presetsFile.include = presetsFile.include || [];
const filteredIncludes = presetsFile.include.filter(include => {
// Ensuring that we handle expansions. Duplicated from loop below.
const includePath = presetsFile.version >= 7 ?
// Version 7 and later support $penv{} expansions in include paths
substituteAll(include, getParentEnvSubstitutions(include, new Map<string, string>())).result :
include;
path.normalize(path.resolve(path.dirname(file), includePath)) === this.presetsPath;
});
const filteredIncludes = [];
for (const include of presetsFile.include) {
const expandedInclude = await this.getExpandedInclude(presetsFile, include, file, hostSystemName, expansionErrors);
if (path.normalize(path.resolve(path.dirname(file), expandedInclude)) === this.presetsPath) {
filteredIncludes.push(include);
}
}

if (filteredIncludes.length === 0) {
presetsFile.include.push(this.presetsPath);
}
Expand All @@ -1633,10 +1664,7 @@ export class PresetsController implements vscode.Disposable {
// Merge the includes in reverse order so that the final presets order is correct
for (let i = presetsFile.include.length - 1; i >= 0; i--) {
const rawInclude = presetsFile.include[i];
const includePath = presetsFile.version >= 7 ?
// Version 7 and later support $penv{} expansions in include paths
substituteAll(rawInclude, getParentEnvSubstitutions(rawInclude, new Map<string, string>())).result :
rawInclude;
const includePath = await this.getExpandedInclude(presetsFile, rawInclude, file, hostSystemName, expansionErrors);
const fullIncludePath = path.normalize(path.resolve(path.dirname(file), includePath));

// Do not include files more than once
Expand Down Expand Up @@ -1672,6 +1700,7 @@ export class PresetsController implements vscode.Disposable {
const includeFileBuffer = await this.readPresetsFile(fullIncludePath);
if (!includeFileBuffer) {
log.error(localize('included.presets.file.not.found', 'Included presets file {0} cannot be found', fullIncludePath));
expansionErrors.errorList.push([localize('included.presets.file.not.found', 'Included presets file {0} cannot be found', fullIncludePath), file]);
continue;
}

Expand All @@ -1686,7 +1715,7 @@ export class PresetsController implements vscode.Disposable {
this.populatePrivatePresetsFields(includeFile, fullIncludePath);

// Recursively merge included files
await this.mergeIncludeFiles(includeFile, fullIncludePath, referencedFiles);
await this.mergeIncludeFiles(includeFile, fullIncludePath, referencedFiles, expansionErrors);

if (includeFile.configurePresets) {
presetsFile.configurePresets = lodash.unionWith(includeFile.configurePresets, presetsFile.configurePresets || [], (a, b) => a.name === b.name);
Expand All @@ -1709,22 +1738,28 @@ export class PresetsController implements vscode.Disposable {
}
}
}

if (expansionErrors.errorList.length > 0 || expansionErrors.tempErrorList.length > 0) {
expansionErrors.tempErrorList.forEach((error) => expansionErrors.errorList.unshift(error));
log.error(localize('expansion.errors', 'Expansion errors found in the presets file.'));
await this.reportPresetsFileErrors(presetsFile.__path, expansionErrors);
} else {
collections.presets.set(vscode.Uri.file(presetsFile.__path || ""), undefined);
}
}

/**
* Returns the expanded presets file if there are no errors, otherwise returns undefined
* Does not apply vsdevenv to the presets file
*/
private async expandPresetsFile(presetsFile: preset.PresetsFile | undefined): Promise<preset.PresetsFile | undefined> {
private async expandPresetsFile(presetsFile: preset.PresetsFile | undefined, expansionErrors: ExpansionErrorHandler): Promise<preset.PresetsFile | undefined> {

if (!presetsFile) {
return undefined;
}

log.info(localize('expanding.presets.file', 'Expanding presets file {0}', presetsFile?.__path || ''));

const expansionErrors: ExpansionErrorHandler = { errorList: [], tempErrorList: []};

const expandedConfigurePresets: preset.ConfigurePreset[] = [];
for (const configurePreset of presetsFile?.configurePresets || []) {
const inheritedPreset = await preset.getConfigurePresetInherits(
Expand Down Expand Up @@ -1884,7 +1919,7 @@ export class PresetsController implements vscode.Disposable {

log.info(localize('validating.presets.file', 'Reading and validating the presets "file {0}"', file));
let schemaFile;
const maxSupportedVersion = 8;
const maxSupportedVersion = 9;
const validationErrorsAreWarnings = presetsFile.version > maxSupportedVersion && this.project.workspaceContext.config.allowUnsupportedPresetsVersions;
if (presetsFile.version < 2) {
await this.showPresetsFileVersionError(file);
Expand All @@ -1902,7 +1937,8 @@ export class PresetsController implements vscode.Disposable {
} else if (presetsFile.version === 7) {
schemaFile = './schemas/CMakePresets-v7-schema.json';
} else {
schemaFile = './schemas/CMakePresets-v8-schema.json';
// This can be used for v9 as well, there is no schema difference.
schemaFile = "./schemas/CMakePresets-v8-schema.json";
}

const validator = await loadSchema(schemaFile);
Expand Down
Loading

0 comments on commit 30e3f11

Please sign in to comment.