fix: suppress CMake file auto-reconfigure during command-initiated saves#4803
fix: suppress CMake file auto-reconfigure during command-initiated saves#4803
Conversation
…d saves (#4794) When a command (build, test, etc.) calls maybeAutoSaveAll() which triggers vscode.workspace.saveAll(), the save event for CMakeLists.txt would fire doCMakeFileChangeReconfigure() and start an automatic reconfigure. This raced with the command's own configure, causing "Configuration is already in progress" errors. Add _suppressCMakeListsReconfigure flag (similar to the existing suppressWatcherReapply pattern for preset files) that is set around the saveAll() call in maybeAutoSaveAll() and checked in doCMakeFileChangeReconfigure() to skip the redundant automatic reconfigure. Co-authored-by: hanniavalera <90047725+hanniavalera@users.noreply.github.com>
Co-authored-by: hanniavalera <90047725+hanniavalera@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a race where command-initiated saveAll() (triggered before build/test/etc.) causes doCMakeFileChangeReconfigure() to start an automatic reconfigure, which then conflicts with the command’s own configure and produces “Configuration is already in progress” (issue #4794).
Changes:
- Add a
_suppressCMakeListsReconfigureflag onCMakeProjectto suppress watcher-triggered reconfigure during command-initiated saves. - Set/clear the suppression flag around
vscode.workspace.saveAll()inmaybeAutoSaveAll(). - Gate the auto-reconfigure path in
doCMakeFileChangeReconfigure()on the new suppression flag and emit a debug log when suppressed.
Comments suppressed due to low confidence (1)
src/cmakeProject.ts:1899
- _suppressCMakeListsReconfigure is set true before awaiting saveAll(), but it’s only cleared on normal control-flow paths. If saveAll() (or the subsequent UI prompt) throws/rejects, the flag can remain true and permanently suppress future configure-on-edit reactions. Consider wrapping the saveAll()/prompt sequence in a try/finally (or a small helper) to guarantee the flag is restored.
// Suppress the automatic configure-on-edit that would otherwise be
// triggered by the save event for CMakeLists.txt files. The caller
// (build, test, etc.) will perform its own configure if needed, so
// the watcher-triggered configure would be redundant and racy (#4794).
this._suppressCMakeListsReconfigure = true;
const cmakeConfiguration = vscode.workspace.getConfiguration('cmake');
const showSaveFailedNotificationString = "showNotAllDocumentsSavedQuestion";
const saveGood = await vscode.workspace.saveAll();
if (!saveGood && cmakeConfiguration.get(showSaveFailedNotificationString)) {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/cmakeProject.ts
Outdated
| if (this._suppressCMakeListsReconfigure) { | ||
| // A command-initiated save (build, test, etc.) is in progress. | ||
| // Skip the automatic reconfigure — the calling command will | ||
| // configure if needed, avoiding the "Configuration is already | ||
| // in progress" race condition (see #4794). | ||
| log.debug(localize('cmakelists.save.suppressed', "CMakeLists.txt saved by command-initiated save; skipping automatic reconfigure.")); |
There was a problem hiding this comment.
The new flag/logging are named as if they only apply to CMakeLists.txt, but doCMakeFileChangeReconfigure() treats any file in driver.cmakeFiles as a “cmake file” (including included .cmake files). With suppression enabled, saving an included .cmake will also skip reconfigure, and the debug message would be misleading. Consider renaming the flag/localize key/message to cover all CMake files, or tightening the suppression check to only match CMakeLists.txt paths.
There was a problem hiding this comment.
@copilot apply changes based on this feedback and this feedback - ensure to abide by the conventions of this codebase, conduct a code review before you commit the changes, and do not change the PR description
There was a problem hiding this comment.
Renamed _suppressCMakeListsReconfigure → _suppressCMakeFileReconfigure and updated all comments and the localize key/message to cover all CMake files (CMakeLists.txt and included .cmake files), since doCMakeFileChangeReconfigure() handles both. See commit 33e6a5f.
| // Suppress the automatic configure-on-edit that would otherwise be | ||
| // triggered by the save event for CMakeLists.txt files. The caller | ||
| // (build, test, etc.) will perform its own configure if needed, so | ||
| // the watcher-triggered configure would be redundant and racy (#4794). | ||
| this._suppressCMakeListsReconfigure = true; | ||
|
|
There was a problem hiding this comment.
This change is a user-visible regression fix (avoids “Configuration is already in progress” when a command triggers saveAll with dirty CMake files), but there doesn’t appear to be coverage for it in the existing VS Code extension end-to-end tests. Adding a test that dirties CMakeLists.txt (or an included .cmake), triggers a command that calls maybeAutoSaveAll() (e.g., runTests/build), and asserts no overlapping configure is started would help prevent regressions.
The flag and log messages now accurately reflect that doCMakeFileChangeReconfigure() handles all CMake files (CMakeLists.txt and included .cmake files), not just CMakeLists.txt. Co-authored-by: hanniavalera <90047725+hanniavalera@users.noreply.github.com>
This change addresses item #4794
This changes visible behavior
The following changes are proposed:
_suppressCMakeFileReconfigureflag toCMakeProject, following the existingsuppressWatcherReapplypattern used for preset filesmaybeAutoSaveAll()around thesaveAll()call to prevent the save event from triggering a redundantdoCMakeFileChangeReconfigure()doCMakeFileChangeReconfigure()and skip automatic reconfigure when a command-initiated save is in progressThe purpose of this change
When clicking test/build with an unsaved CMakeLists.txt (or any included
.cmakefile),maybeAutoSaveAll()→saveAll()firesonDidSaveTextDocument, which triggersdoCMakeFileChangeReconfigure()and starts an automatic reconfigure. The command's own subsequent configure then hitsisConfigInProgress === trueand fails with "Configuration is already in progress". A second click always works because the watcher-triggered configure has completed.The fix suppresses the watcher-triggered reconfigure during command-initiated saves for all CMake files (CMakeLists.txt and included
.cmakefiles), sincedoCMakeFileChangeReconfigure()handles both. The calling command (build, test, launch, etc.) will configure on its own if needed. NormalconfigureOnEditbehavior for manual user saves is unaffected.Other Notes/Information
All four
maybeAutoSaveAll()call sites are covered:ensureConfigured(),doConfigure(), and the launch/debug path — so this handles build, test, ctest, cpack, and debug flows uniformly.Original prompt
This section details on the original issue you should resolve
<issue_title>[Bug] Clicking test button with an unsaved CMakeLists.txt fails due to automatic configure on save.</issue_title>
<issue_description>### Brief Issue Summary
If you click the "test" button while a CMakeLists.txt is unsaved, it will first save the CMakeLists.txt, but this automatically triggers a reconfigure which then conflicts with the test, causing an error "Configuration is already in progress". Clicking the test button again then works properly.
CMake Tools Diagnostics
{ "os": "linux", "vscodeVersion": "1.109.0", "cmtVersion": "1.22.28", "configurations": [ { "folder": "/workspace", "cmakeVersion": "3.28.3", "configured": true, "generator": "Ninja", "usesPresets": false, "compilers": { "C": "/var/yocto-sdk/sysroots/x86_64-pokysdk-linux/usr/bin/aarch64-poky-linux/aarch64-poky-linux-gcc", "CXX": "/var/yocto-sdk/sysroots/x86_64-pokysdk-linux/usr/bin/aarch64-poky-linux/aarch64-poky-linux-g++" } } ], "cpptoolsIntegration": { "isReady": true, "hasCodeModel": true, "activeBuildType": "Debug", "buildTypesSeen": [ "Debug" ], "requests": [ "file:///workspace/system/monitor/src/monitor_unit.cpp", "file:///workspace/system/monitor/src/monitor_controller.cpp" ], "responses": [ { "uri": "file:///workspace/system/monitor/src/monitor_unit.cpp", "configuration": { "includePath": [ "/workspace/system/monitor/include", "/root/build/great-lakes/_deps/boost-src/libs/mp11/include", "/workspace/common/asio/include", "/root/build/great-lakes/_deps/boost-src/libs/asio/include", "/root/build/great-lakes/_deps/boost-src/libs/align/include", "/root/build/great-lakes/_deps/boost-src/libs/assert/include", "/root/build/great-lakes/_deps/boost-src/libs/config/include", "/root/build/great-lakes/_deps/boost-src/libs/core/include", "/root/build/great-lakes/_deps/boost-src/libs/static_assert/include", "/root/build/great-lakes/_deps/boost-src/libs/throw_exception/include", "/root/build/great-lakes/_deps/boost-src/libs/system/include", "/root/build/great-lakes/_deps/boost-src/libs/variant2/include", "/root/build/great-lakes/_deps/boost-src/libs/winapi/include", "/root/build/great-lakes/_deps/boost-src/libs/predef/include", "/root/build/great-lakes/_deps/boost-src/libs/date_time/include", "/root/build/great-lakes/_deps/boost-src/libs/algorithm/include", "/root/build/great-lakes/_deps/boost-src/libs/array/include", "/root/build/great-lakes/_deps/boost-src/libs/bind/include", "/root/build/great-lakes/_deps/boost-src/libs/concept_check/include", "/root/build/great-lakes/_deps/boost-src/libs/preprocessor/include", "/root/build/great-lakes/_deps/boost-src/libs/type_traits/include", "/root/build/great-lakes/_deps/boost-src/libs/exception/include", "/root/build/great-lakes/_deps/boost-src/libs/smart_ptr/include", "/root/build/great-lakes/_deps/boost-src/libs/tuple/include", "/root/build/great-lakes/_deps/boost-src/libs/function/include", "/root/build/great-lakes/_deps/boost-src/libs/iterator/include", "/root/build/great-lakes/_deps/boost-src/libs/detail/include", "/root/build/great-lakes/_deps/boost-src/libs/fusion/include", "/root/build/great-lakes/_deps/boost-src/libs/container_hash/include", "/root/build/great-lakes/_deps/boost-src/libs/describe/include", "/root/build/great-lakes/_deps/boost-src/libs/function_types/include", "/root/build/great-lakes/_deps/boost-src/libs/mpl/include", "/root/build/great-lakes/_deps/boost-src/libs/utility/include", "/root/build/great-lakes/_deps/boost-src/libs/io/include", "/root/build/great-lakes/_deps/boost-src/libs/typeof/include", "/root/build/great-lakes/_deps/boost-src/libs/functional/include", "/root/build/great-lakes/_deps/boost-src/libs/optional/include", "/root/build/great-lakes/_deps/boost-src/libs/range/include", "/root/build/great-lakes/_deps/boost-src/libs/conversion/include", "/root/build/great-lakes/_deps/boost-src/libs/regex/include", "/root/build/great-lakes/_deps/boost-src/libs/unordered/include", "/root/build/great-lakes/_deps/boost-src/libs/lexical_cast/include", "/root/build/great-lakes/_deps/boost-src/libs/container/include", "/root/build/great-lakes/_deps/boost-src/libs/intrusive/include", "/root/build/great-lakes/_deps/boost-src/libs/move/include", "/root/build/great-lakes/_deps/boost-src... </details> <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes microsoft/vscode-cmake-tools#4794 <!-- START COPILOT CODING AGENT TIPS --> --- 🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. [Learn more about Advanced Security.](https://gh.io/cca-advanced-security)