Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure we correctly inherit preset from UserPreset #3958

Merged
merged 11 commits into from
Aug 6, 2024
Merged

Ensure we correctly inherit preset from UserPreset #3958

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
51b2dd3
fix
gcampbell-msft Aug 2, 2024
e2136a3
analyze user presets first because it's the base if it exists
gcampbell-msft Aug 2, 2024
2d1d637
more updates to fix include files and telemetry, still needs work and…
gcampbell-msft Aug 2, 2024
d54c6d9
ensure telemetry and other uses of 'all..Presets' is right
gcampbell-msft Aug 5, 2024
fd998a0
more fixes
gcampbell-msft Aug 5, 2024
d695ac6
ensure we don't duplicate cmakepresets in userpresets includes
gcampbell-msft Aug 5, 2024
00774fe
Merge branch 'main' into dev/gcampbell/UserPresetInheritPreset
gcampbell-msft Aug 5, 2024
d3662b2
remove unnecessary return
gcampbell-msft Aug 5, 2024
334bb4d
Merge branch 'dev/gcampbell/UserPresetInheritPreset' of https://githu…
gcampbell-msft Aug 5, 2024
91777e3
unionWith consistency
gcampbell-msft Aug 6, 2024
f30867b
add test
gcampbell-msft Aug 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions src/drivers/cmakeDriver.ts
View file
Open in desktop
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
Open in desktop
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
84 changes: 67 additions & 17 deletions src/presetsController.ts
View file
Open in desktop
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so smart!

// 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,27 @@ 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 => {
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 +1636,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.union(referencedIncludeFile.configurePresets, presetsFile.configurePresets || []);
}
if (referencedIncludeFile.buildPresets) {
presetsFile.buildPresets = lodash.union(referencedIncludeFile.buildPresets, presetsFile.buildPresets || []);
}
if (referencedIncludeFile.testPresets) {
presetsFile.testPresets = lodash.union(referencedIncludeFile.testPresets, presetsFile.testPresets || []);
}
if (referencedIncludeFile.packagePresets) {
presetsFile.packagePresets = lodash.union(referencedIncludeFile.packagePresets, presetsFile.packagePresets || []);
}
if (referencedIncludeFile.workflowPresets) {
presetsFile.workflowPresets = lodash.union(referencedIncludeFile.workflowPresets, presetsFile.workflowPresets || []);
}
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 +1671,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 +1684,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.union(includeFile.configurePresets, presetsFile.configurePresets || []);
}
if (includeFile.buildPresets) {
presetsFile.buildPresets = includeFile.buildPresets.concat(presetsFile.buildPresets || []);
presetsFile.buildPresets = lodash.union(includeFile.buildPresets, presetsFile.buildPresets || []);
}
if (includeFile.testPresets) {
presetsFile.testPresets = includeFile.testPresets.concat(presetsFile.testPresets || []);
presetsFile.testPresets = lodash.union(includeFile.testPresets, presetsFile.testPresets || []);
}
if (includeFile.packagePresets) {
presetsFile.packagePresets = includeFile.packagePresets.concat(presetsFile.packagePresets || []);
presetsFile.packagePresets = lodash.union(includeFile.packagePresets, presetsFile.packagePresets || []);
}
if (includeFile.workflowPresets) {
presetsFile.workflowPresets = includeFile.workflowPresets.concat(presetsFile.workflowPresets || []);
presetsFile.workflowPresets = lodash.union(includeFile.workflowPresets, presetsFile.workflowPresets || []);
}
if (includeFile.cmakeMinimumRequired) {
if (!presetsFile.cmakeMinimumRequired || util.versionLess(presetsFile.cmakeMinimumRequired, includeFile.cmakeMinimumRequired)) {
Expand Down
Loading