Skip to content

Commit

Permalink
Ensure we correctly inherit preset from UserPreset (#3958)
Browse files Browse the repository at this point in the history 
* fix

* analyze user presets first because it's the base if it exists

* more updates to fix include files and telemetry, still needs work and testing

* ensure telemetry and other uses of 'all..Presets' is right

* more fixes

* ensure we don't duplicate cmakepresets in userpresets includes

* remove unnecessary return

* unionWith consistency

* add test
  • Loading branch information
@gcampbell-msft
gcampbell-msft authored Aug 6, 2024
1 parent ec03fc1 commit c779bf1
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 28 deletions.
11 changes: 6 additions & 5 deletions src/drivers/cmakeDriver.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import * as path from 'path';
import * as vscode from 'vscode';
import * as lodash from "lodash";

import { CMakeExecutable } from '@cmt/cmake/cmakeExecutable';
import * as codepages from '@cmt/codePageTable';
Expand Down Expand Up @@ -1568,15 +1569,15 @@ export abstract class CMakeDriver implements vscode.Disposable {
};
if (this.useCMakePresets && this.workspaceFolder) {
const configurePresets = preset.configurePresets(this.workspaceFolder);
const userConfigurePresets = preset.userConfigurePresets(this.workspaceFolder);
const userConfigurePresets = lodash.differenceWith(preset.userConfigurePresets(this.workspaceFolder), configurePresets, (a, b) => a.name === b.name);
const buildPresets = preset.buildPresets(this.workspaceFolder);
const userBuildPresets = preset.userBuildPresets(this.workspaceFolder);
const userBuildPresets = lodash.differenceWith(preset.userBuildPresets(this.workspaceFolder), buildPresets, (a, b) => a.name === b.name);
const testPresets = preset.testPresets(this.workspaceFolder);
const userTestPresets = preset.userTestPresets(this.workspaceFolder);
const userTestPresets = lodash.differenceWith(preset.userTestPresets(this.workspaceFolder), testPresets, (a, b) => a.name === b.name);
const packagePresets = preset.packagePresets(this.workspaceFolder);
const userPackagePresets = preset.userPackagePresets(this.workspaceFolder);
const userPackagePresets = lodash.differenceWith(preset.userPackagePresets(this.workspaceFolder), packagePresets, (a, b) => a.name === b.name);
const workflowPresets = preset.workflowPresets(this.workspaceFolder);
const userWorkflowPresets = preset.userWorkflowPresets(this.workspaceFolder);
const userWorkflowPresets = lodash.differenceWith(preset.userWorkflowPresets(this.workspaceFolder), workflowPresets, (a, b) => a.name === b.name);
telemetryMeasures['ConfigurePresets'] = configurePresets.length;
telemetryMeasures['HiddenConfigurePresets'] = this.countHiddenPresets(configurePresets);
telemetryMeasures['UserConfigurePresets'] = userConfigurePresets.length;
Expand Down
11 changes: 6 additions & 5 deletions src/preset.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import * as nls from 'vscode-nls';
import * as path from 'path';
import * as vscode from "vscode";
import * as lodash from "lodash";

import * as util from '@cmt/util';
import * as logging from '@cmt/logging';
Expand Down Expand Up @@ -540,7 +541,7 @@ export function userConfigurePresets(folder: string, usePresetsPlusIncluded: boo
* Don't use this function if you need to keep any changes in the presets
*/
export function allConfigurePresets(folder: string, usePresetsPlusIncluded: boolean = false) {
return configurePresets(folder, usePresetsPlusIncluded).concat(userConfigurePresets(folder, usePresetsPlusIncluded));
return lodash.unionWith(configurePresets(folder, usePresetsPlusIncluded).concat(userConfigurePresets(folder, usePresetsPlusIncluded)), (a, b) => a.name === b.name);
}

export function buildPresets(folder: string) {
Expand All @@ -555,7 +556,7 @@ export function userBuildPresets(folder: string) {
* Don't use this function if you need to keep any changes in the presets
*/
export function allBuildPresets(folder: string) {
return buildPresets(folder).concat(userBuildPresets(folder));
return lodash.unionWith(buildPresets(folder).concat(userBuildPresets(folder)), (a, b) => a.name === b.name);
}

export function testPresets(folder: string) {
Expand All @@ -570,7 +571,7 @@ export function userTestPresets(folder: string) {
* Don't use this function if you need to keep any changes in the presets
*/
export function allTestPresets(folder: string) {
return testPresets(folder).concat(userTestPresets(folder));
return lodash.unionWith(testPresets(folder).concat(userTestPresets(folder)), (a, b) => a.name === b.name);
}

export function packagePresets(folder: string) {
Expand All @@ -585,7 +586,7 @@ export function userPackagePresets(folder: string) {
* Don't use this function if you need to keep any changes in the presets
*/
export function allPackagePresets(folder: string) {
return packagePresets(folder).concat(userPackagePresets(folder));
return lodash.unionWith(packagePresets(folder).concat(userPackagePresets(folder)), (a, b) => a.name === b.name);
}

export function workflowPresets(folder: string) {
Expand All @@ -600,7 +601,7 @@ export function userWorkflowPresets(folder: string) {
* Don't use this function if you need to keep any changes in the presets
*/
export function allWorkflowPresets(folder: string) {
return workflowPresets(folder).concat(userWorkflowPresets(folder));
return lodash.unionWith(workflowPresets(folder).concat(userWorkflowPresets(folder)), (a, b) => a.name === b.name);
}

export function getPresetByName<T extends Preset>(presets: T[], name: string): T | null {
Expand Down
85 changes: 68 additions & 17 deletions src/presetsController.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,15 +166,15 @@ export class PresetsController implements vscode.Disposable {
preset.setOriginalUserPresetsFile(folder, presetsFile);
};

private async resetPresetsFile(file: string, setExpandedPresets: SetPresetsFileFunc, setPresetsPlusIncluded: SetPresetsFileFunc, setOriginalPresetsFile: SetPresetsFileFunc, fileExistCallback: (fileExists: boolean) => void, referencedFiles: Set<string>) {
private async resetPresetsFile(file: string, setExpandedPresets: SetPresetsFileFunc, setPresetsPlusIncluded: SetPresetsFileFunc, setOriginalPresetsFile: SetPresetsFileFunc, fileExistCallback: (fileExists: boolean) => void, referencedFiles: Map<string, preset.PresetsFile | undefined>) {
const presetsFileBuffer = await this.readPresetsFile(file);

// There might be a better location for this, but for now this is the best one...
fileExistCallback(Boolean(presetsFileBuffer));

// Record the file as referenced, even if the file does not exist.
referencedFiles.add(file);
let presetsFile = await this.parsePresetsFile(presetsFileBuffer, file);
referencedFiles.set(file, presetsFile);
if (presetsFile) {
// Parse again so we automatically have a copy by value
setOriginalPresetsFile(this.folderPath, await this.parsePresetsFile(presetsFileBuffer, file));
Expand Down Expand Up @@ -203,14 +203,14 @@ export class PresetsController implements vscode.Disposable {
// Need to reapply presets every time presets changed since the binary dir or cmake path could change
// (need to clean or reload driver)
async reapplyPresets() {
const referencedFiles: Set<string> = new Set();
const referencedFiles: Map<string, preset.PresetsFile | undefined> = new Map();

// Reset all changes due to expansion since parents could change
await this.resetPresetsFile(this.presetsPath, this._setExpandedPresets, this._setPresetsPlusIncluded, this._setOriginalPresetsFile, exists => this._presetsFileExists = exists, referencedFiles);
await this.resetPresetsFile(this.userPresetsPath, this._setExpandedUserPresetsFile, this._setUserPresetsPlusIncluded, this._setOriginalUserPresetsFile, exists => this._userPresetsFileExists = exists, referencedFiles);

// reset all expanded presets storage.
this._referencedFiles = Array.from(referencedFiles);
this._referencedFiles = Array.from(referencedFiles.keys());

this.project.minCMakeVersion = preset.minCMakeVersion(this.folderPath);

Expand Down Expand Up @@ -837,24 +837,31 @@ export class PresetsController implements vscode.Disposable {
return chosenPresets?.preset;
}

// For all of the `getAll` methods, we now can safely grab only the user presets (if present), because they inherently include
// the presets.
async getAllConfigurePresets(): Promise<preset.ConfigurePreset[]> {
return preset.configurePresets(this.folderPath).concat(preset.userConfigurePresets(this.folderPath));
const userPresets = preset.userConfigurePresets(this.folderPath);
return userPresets.length > 0 ? userPresets : preset.configurePresets(this.folderPath);
}

async getAllBuildPresets(): Promise<preset.BuildPreset[]> {
return preset.buildPresets(this.folderPath).concat(preset.userBuildPresets(this.folderPath));
const userPresets = preset.userBuildPresets(this.folderPath);
return userPresets.length > 0 ? userPresets : preset.buildPresets(this.folderPath);
}

async getAllTestPresets(): Promise<preset.TestPreset[]> {
return preset.testPresets(this.folderPath).concat(preset.userTestPresets(this.folderPath));
const userPresets = preset.userTestPresets(this.folderPath);
return userPresets.length > 0 ? userPresets : preset.testPresets(this.folderPath);
}

async getAllPackagePresets(): Promise<preset.PackagePreset[]> {
return preset.packagePresets(this.folderPath).concat(preset.userPackagePresets(this.folderPath));
const userPresets = preset.userPackagePresets(this.folderPath);
return userPresets.length > 0 ? userPresets : preset.packagePresets(this.folderPath);
}

async getAllWorkflowPresets(): Promise<preset.WorkflowPreset[]> {
return preset.workflowPresets(this.folderPath).concat(preset.userWorkflowPresets(this.folderPath));
const userPresets = preset.userWorkflowPresets(this.folderPath);
return userPresets.length > 0 ? userPresets : preset.workflowPresets(this.folderPath);
}

async selectConfigurePreset(quickStart?: boolean): Promise<boolean> {
Expand Down Expand Up @@ -1594,8 +1601,28 @@ export class PresetsController implements vscode.Disposable {
setFile(presetsFile.packagePresets);
}

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

// 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;
});
if (filteredIncludes.length === 0) {
presetsFile.include.push(this.presetsPath);
}
}

if (!presetsFile.include) {
return;
}

Expand All @@ -1610,10 +1637,33 @@ export class PresetsController implements vscode.Disposable {

// Do not include files more than once
if (referencedFiles.has(fullIncludePath)) {
const referencedIncludeFile = referencedFiles.get(fullIncludePath);
if (referencedIncludeFile) {
if (referencedIncludeFile.configurePresets) {
presetsFile.configurePresets = lodash.unionWith(referencedIncludeFile.configurePresets, presetsFile.configurePresets || [], (a, b) => a.name === b.name);
}
if (referencedIncludeFile.buildPresets) {
presetsFile.buildPresets = lodash.unionWith(referencedIncludeFile.buildPresets, presetsFile.buildPresets || [], (a, b) => a.name === b.name);
}
if (referencedIncludeFile.testPresets) {
presetsFile.testPresets = lodash.unionWith(referencedIncludeFile.testPresets, presetsFile.testPresets || [], (a, b) => a.name === b.name);
}
if (referencedIncludeFile.packagePresets) {
presetsFile.packagePresets = lodash.unionWith(referencedIncludeFile.packagePresets, presetsFile.packagePresets || [], (a, b) => a.name === b.name);
}
if (referencedIncludeFile.workflowPresets) {
presetsFile.workflowPresets = lodash.unionWith(referencedIncludeFile.workflowPresets, presetsFile.workflowPresets || [], (a, b) => a.name === b.name);
}
if (referencedIncludeFile.cmakeMinimumRequired) {
if (!presetsFile.cmakeMinimumRequired || util.versionLess(presetsFile.cmakeMinimumRequired, referencedIncludeFile.cmakeMinimumRequired)) {
presetsFile.cmakeMinimumRequired = referencedIncludeFile.cmakeMinimumRequired;
}
}
}
continue;
}
// Record the file as referenced, even if the file does not exist.
referencedFiles.add(fullIncludePath);
referencedFiles.set(fullIncludePath, undefined);

const includeFileBuffer = await this.readPresetsFile(fullIncludePath);
if (!includeFileBuffer) {
Expand All @@ -1622,6 +1672,7 @@ export class PresetsController implements vscode.Disposable {
}

let includeFile = await this.parsePresetsFile(includeFileBuffer, fullIncludePath);
referencedFiles.set(fullIncludePath, includeFile);
includeFile = await this.validatePresetsFile(includeFile, fullIncludePath);
if (!includeFile) {
continue;
Expand All @@ -1634,19 +1685,19 @@ export class PresetsController implements vscode.Disposable {
await this.mergeIncludeFiles(includeFile, fullIncludePath, referencedFiles);

if (includeFile.configurePresets) {
presetsFile.configurePresets = includeFile.configurePresets.concat(presetsFile.configurePresets || []);
presetsFile.configurePresets = lodash.unionWith(includeFile.configurePresets, presetsFile.configurePresets || [], (a, b) => a.name === b.name);
}
if (includeFile.buildPresets) {
presetsFile.buildPresets = includeFile.buildPresets.concat(presetsFile.buildPresets || []);
presetsFile.buildPresets = lodash.unionWith(includeFile.buildPresets, presetsFile.buildPresets || [], (a, b) => a.name === b.name);
}
if (includeFile.testPresets) {
presetsFile.testPresets = includeFile.testPresets.concat(presetsFile.testPresets || []);
presetsFile.testPresets = lodash.unionWith(includeFile.testPresets, presetsFile.testPresets || [], (a, b) => a.name === b.name);
}
if (includeFile.packagePresets) {
presetsFile.packagePresets = includeFile.packagePresets.concat(presetsFile.packagePresets || []);
presetsFile.packagePresets = lodash.unionWith(includeFile.packagePresets, presetsFile.packagePresets || [], (a, b) => a.name === b.name);
}
if (includeFile.workflowPresets) {
presetsFile.workflowPresets = includeFile.workflowPresets.concat(presetsFile.workflowPresets || []);
presetsFile.workflowPresets = lodash.unionWith(includeFile.workflowPresets, presetsFile.workflowPresets || [], (a, b) => a.name === b.name);
}
if (includeFile.cmakeMinimumRequired) {
if (!presetsFile.cmakeMinimumRequired || util.versionLess(presetsFile.cmakeMinimumRequired, includeFile.cmakeMinimumRequired)) {
Expand Down
6 changes: 5 additions & 1 deletion test/extension-tests/single-root-UI/project-folder/CMakeUserPresets.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@
"environment": {
"TEST_VARIANT_ENV": "0cbfb6ae-f2ec-4017-8ded-89df8759c502"
}
},
{
"name": "TestInheritFromPreset",
"inherits": "Linux1"
}
]
}
}
5 changes: 5 additions & 0 deletions test/extension-tests/single-root-UI/test/include-presets.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,9 @@ suite('Preset include functionality', () => {
const result = await testEnv.result.getResultAsJson();
expect(result['cookie']).to.eq('passed-cookie');
}).timeout(100000);

test('Configure CMakeUserPreset inheriting from CMakePreset', async function (this: Mocha.Context) {
await vscode.commands.executeCommand('cmake.setConfigurePreset', 'TestInheritFromPreset');
expect(await vscode.commands.executeCommand('cmake.showConfigureCommand')).to.be.eq(0);
}).timeout(100000);
});

0 comments on commit c779bf1

Please sign in to comment.