Skip to content

Commit 0a1c3da

Browse files
Copilothanniavalera
andcommitted
Fix stale preset state on included file changes (#4777)
- Unify FileWatcher to use single debounced callback for all events (change/create/delete), fixing race condition when external tools regenerate included files via atomic write (delete + create) - Detect root presets file creation inside reapplyPresets() by comparing before/after presetsFileExists state instead of separate handler - Add reapplyPresets() call in configureInternal() so explicit user commands always read fresh preset state from disk - Update tests to reflect unified event handling - Add CHANGELOG entry Co-authored-by: hanniavalera <[email protected]>
1 parent c7ce061 commit 0a1c3da

5 files changed

Lines changed: 72 additions & 23 deletions

File tree

File renamed without changes.

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ Improvements:
2222
- 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)
2323

2424
Bug Fixes:
25+
- Fix changes to files referenced via `include` in CMakePresets.json or CMakeUserPresets.json not being detected until VS Code restart. The file watcher now uniformly handles create events (from atomic writes) on included files, and explicit reconfigure commands always refresh presets from disk. [#4777](https://github.com/microsoft/vscode-cmake-tools/issues/4777)
2526
- Fix configure/build sometimes using stale preset values when unsaved changes to included preset files are auto-saved before configure. The extension now explicitly refreshes presets from disk after saving, instead of relying solely on the asynchronous file watcher. [#4502](https://github.com/microsoft/vscode-cmake-tools/issues/4502)
2627
- Reduce overly verbose logging when CMake configure or build fails. The Output panel no longer floods with duplicated output, and the channel is only revealed on error rather than unconditionally. [#4749](https://github.com/microsoft/vscode-cmake-tools/issues/4749)
2728
- Fix Test Results panel not hyperlinking file paths for GoogleTest failures. The default `cmake.ctest.failurePatterns` now includes a pattern matching GoogleTest's `file:line: Failure` output format. [#4589](https://github.com/microsoft/vscode-cmake-tools/issues/4589)

src/cmakeProject.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1638,6 +1638,14 @@ export class CMakeProject {
16381638
}
16391639

16401640
async configureInternal(trigger: ConfigureTrigger = ConfigureTrigger.api, extraArgs: string[] = [], type: ConfigureType = ConfigureType.Normal, debuggerInformation?: DebuggerInformation, cancellationToken?: vscode.CancellationToken): Promise<ConfigureResult> {
1641+
// Explicitly refresh presets from disk so that explicit user commands
1642+
// (configure, clean-configure, etc.) always use up-to-date preset state,
1643+
// even if the asynchronous file watcher hasn't processed recent changes
1644+
// to included preset files yet. See issue #4777.
1645+
if (this.useCMakePresets) {
1646+
await this.presetsController.reapplyPresets();
1647+
}
1648+
16411649
const drv: CMakeDriver | null = await this.getCMakeDriverInstance();
16421650

16431651
// Don't show a progress bar when the extension is using Cache for configuration.

src/presets/presetsController.ts

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,9 @@ export class PresetsController implements vscode.Disposable {
181181
// in setConfigurePreset and to ensure consistent preset state.
182182
async reapplyPresets() {
183183
const doReapply = async () => {
184+
// Capture pre-reload state to detect root presets file creation.
185+
const existedBefore = this.presetsFileExist;
186+
184187
const referencedFiles: Map<string, preset.PresetsFile | undefined> =
185188
new Map();
186189

@@ -202,6 +205,13 @@ export class PresetsController implements vscode.Disposable {
202205
// Don't need to set build/test presets here since they are reapplied in setConfigurePreset
203206

204207
await this.watchPresetsChange();
208+
209+
// If a root presets file was freshly created (transitioned from absent to present),
210+
// update the active project so the UI reflects preset mode. This replaces
211+
// the previous onCreatePresetsFile() handler that was wired only to onDidCreate.
212+
if (!existedBefore && this.presetsFileExist) {
213+
await this.project.projectController?.updateActiveProject(this.workspaceFolder);
214+
}
205215
};
206216
this._reapplyInProgress = this._reapplyInProgress.then(doReapply, doReapply);
207217
return this._reapplyInProgress;
@@ -1706,23 +1716,14 @@ export class PresetsController implements vscode.Disposable {
17061716
return vscode.window.showTextDocument(vscode.Uri.file(presetsFilePath));
17071717
}
17081718

1709-
// this is used for the file watcher on adding a new presets file
1710-
async onCreatePresetsFile() {
1711-
await this.reapplyPresets();
1712-
await this.project.projectController?.updateActiveProject(this.workspaceFolder);
1713-
}
1714-
17151719
private async watchPresetsChange() {
17161720

17171721
const presetChangeHandler = () => {
17181722
void this.reapplyPresets();
17191723
};
1720-
const presetCreatedHandler = () => {
1721-
void this.onCreatePresetsFile();
1722-
};
17231724

17241725
this._presetsWatchers?.dispose();
1725-
this._presetsWatchers = new FileWatcher(this._referencedFiles, presetChangeHandler, presetCreatedHandler);
1726+
this._presetsWatchers = new FileWatcher(this._referencedFiles, presetChangeHandler);
17261727
};
17271728

17281729
dispose() {
@@ -1746,7 +1747,7 @@ class FileWatcher implements vscode.Disposable {
17461747
// Two change events are coming in rapid succession without this.
17471748
private canRunChangeHandler = true;
17481749

1749-
public constructor(filePaths: string[], changeHandler: () => void, createHandler: () => void) {
1750+
public constructor(filePaths: string[], changeHandler: () => void) {
17501751
const debouncedOnChange = () => {
17511752
if (this.canRunChangeHandler) {
17521753
changeHandler();
@@ -1762,7 +1763,7 @@ class FileWatcher implements vscode.Disposable {
17621763
const watcher = vscode.workspace.createFileSystemWatcher(pattern);
17631764

17641765
watcher.onDidChange(debouncedOnChange);
1765-
watcher.onDidCreate(createHandler);
1766+
watcher.onDidCreate(debouncedOnChange);
17661767
watcher.onDidDelete(debouncedOnChange);
17671768

17681769
this.watchers.push(watcher);

test/unit-tests/presets/presetsController.test.ts

Lines changed: 50 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -239,22 +239,61 @@ suite('PresetsController file watcher protection', () => {
239239
});
240240

241241
/**
242-
* Test that create events are not debounced and always fire immediately.
243-
* The FileWatcher treats create events differently from change events.
242+
* Test that all filesystem events (change, create, delete) are debounced uniformly.
243+
* After unifying the FileWatcher handler (fix for #4777), create events are
244+
* debounced the same way as change and delete events to prevent race conditions
245+
* when external tools regenerate included files via atomic write (delete + create).
244246
*/
245-
test('Create events are not debounced', () => {
246-
let createCount = 0;
247+
test('All events including create are debounced', () => {
248+
let canRunChangeHandler = true;
249+
let eventCount = 0;
250+
const debounceMs = 100;
251+
252+
// Simulates the unified debounced handler used for all events
253+
const handler = () => {
254+
if (canRunChangeHandler) {
255+
eventCount++;
256+
canRunChangeHandler = false;
257+
setTimeout(() => (canRunChangeHandler = true), debounceMs);
258+
}
259+
};
260+
261+
// Rapid create events should be debounced (only first fires)
262+
handler(); // create event
263+
handler(); // another create event
264+
handler(); // yet another create event
265+
266+
expect(eventCount).to.equal(1);
267+
});
268+
269+
/**
270+
* Test that a delete followed by a create (atomic write pattern) is handled
271+
* as a single reload. This simulates the scenario where an external tool like
272+
* Conan regenerates an included preset file. See issue #4777.
273+
*/
274+
test('Delete followed by create triggers single debounced reload', async () => {
275+
let canRunChangeHandler = true;
276+
let eventCount = 0;
277+
const debounceMs = 100;
247278

248-
const createHandler = () => {
249-
createCount++;
279+
const handler = () => {
280+
if (canRunChangeHandler) {
281+
eventCount++;
282+
canRunChangeHandler = false;
283+
setTimeout(() => (canRunChangeHandler = true), debounceMs);
284+
}
250285
};
251286

252-
// Multiple create events should all fire
253-
createHandler();
254-
createHandler();
255-
createHandler();
287+
// Simulate atomic write: delete event immediately followed by create event
288+
handler(); // delete event
289+
handler(); // create event (should be debounced)
290+
291+
expect(eventCount).to.equal(1);
256292

257-
expect(createCount).to.equal(3);
293+
// After debounce period, another event should fire
294+
await new Promise(resolve => setTimeout(resolve, debounceMs + 10));
295+
handler();
296+
expect(eventCount).to.equal(2);
258297
});
259298

260299
/**

0 commit comments

Comments
 (0)