Configure precondition handler to return boolean to signal a successf…

…ul recovery case (#3776)

* Making configure precondition handler return a boolean helps to differentiate the successful recovery cases

* Fix pre-condition handler prototype errors in tests

* Experiment with prototypes to fix linter errors

* Add setting for building whole build dir when clean configure

* Remove anything related to deleteBuildDirOnCleanConfigure since it's in a different PR. Keep proper promise<bool> prototype for pre-configure handler return and fix appropriately compilation and lint errors in tests.

* Forgot one file to remove deleteBuildDirOnCleanConfigure from

* Remove forgotten comment

* Add changelog entry

* remove changes not needed

* removing more bad changes

* add logic to regenerate the driver after selecting cmakelists

* remove unused imports

* keep sanity check in _beforeConfigureOrBuild

* only ensure we set the variable so that on retry, it works

* switch return

* erroneous update

* fix

---------

Co-authored-by: Garrett Campbell <gcampbell@microsoft.com>
Co-authored-by: Garrett Campbell <86264750+gcampbell-msft@users.noreply.github.com>

CHANGELOG.md

@@ -32,6 +32,7 @@ Bug Fixes:
 - Disable annoy and invalid extenion message about fix windows sdk for MSVC 2022. [#3837](https://github.com/microsoft/vscode-cmake-tools/pull/3837)
 - Fix re-using a terminal for launching even when the environment has changed. [#3478](https://github.com/microsoft/vscode-cmake-tools/issues/3478)
 - Fix our keybindings for debug and run without debugging to better match VS Code. [#3507](https://github.com/microsoft/vscode-cmake-tools/issues/3507)
+- Allow success recovery in the configure precondition handler. [#3554](https://github.com/microsoft/vscode-cmake-tools/issues/3554)
 - Prevent second configure after `QuickStart` if the `automaticReconfigure` setting is enabled. [#3910](https://github.com/microsoft/vscode-cmake-tools/issues/3910)
 
 ## 1.18.43

src/cmakeProject.ts

@@ -863,7 +863,7 @@ export class CMakeProject {
      * configure. This should be called by a derived driver before any
      * configuration tasks are run
      */
-    public async cmakePreConditionProblemHandler(e: CMakePreconditionProblems, isConfiguring: boolean, config?: ConfigurationReader): Promise<void> {
+    public async cmakePreConditionProblemHandler(e: CMakePreconditionProblems, isConfiguring: boolean, config?: ConfigurationReader): Promise<boolean> {
         let telemetryEvent: string | undefined;
         const telemetryProperties: telemetry.Properties = {};
 
@@ -956,9 +956,14 @@ export class CMakeProject {
 
                             if (!isConfiguring) {
                                 telemetry.logEvent(telemetryEvent, telemetryProperties);
-                                return vscode.commands.executeCommand('cmake.configure');
+                                await vscode.commands.executeCommand('cmake.configure');
+                                return true;
+                            } else {
+                                await this.reloadCMakeDriver();
                             }
                         }
+
+                        return true;
                     } else {
                         telemetryProperties["missingCMakeListsUserAction"] = "cancel-browse";
                     }
@@ -974,7 +979,8 @@ export class CMakeProject {
         // This project folder can go through various changes while executing this function
         // that could be relevant to the partial/full feature set view.
         // This is a good place for an update.
-        return updateFullFeatureSet();
+        await updateFullFeatureSet();
+        return false;
     }
 
     /**
@@ -1551,6 +1557,7 @@ export class CMakeProject {
 
     async configureInternal(trigger: ConfigureTrigger = ConfigureTrigger.api, extraArgs: string[] = [], type: ConfigureType = ConfigureType.Normal, debuggerInformation?: DebuggerInformation): Promise<ConfigureResult> {
         const drv: CMakeDriver | null = await this.getCMakeDriverInstance();
+
         // Don't show a progress bar when the extension is using Cache for configuration.
         // Using cache for configuration happens only one time.
         if (drv && drv.shouldUseCachedConfiguration(trigger)) {

src/cmakeTaskProvider.ts

@@ -433,6 +433,7 @@ export class CustomBuildTaskTerminal implements vscode.Pseudoterminal, proc.Outp
         telemetry.logEvent("task", {taskType: "configure", useCMakePresets: String(project.useCMakePresets)});
         await this.correctTargets(project, CommandType.config);
         const cmakeDriver: CMakeDriver | undefined = (await project?.getCMakeDriverInstance()) || undefined;
+
         if (cmakeDriver) {
             if (project.useCMakePresets && cmakeDriver.config.configureOnEdit) {
                 log.debug(localize("configure.on.edit", 'When running configure tasks using presets, setting configureOnEdit to true can potentially overwrite the task configurations.'));

src/drivers/cmakeDriver.ts

@@ -71,7 +71,7 @@ export interface ConfigureResult {
     resultType: ConfigureResultType;
 }
 
-export type CMakePreconditionProblemSolver = (e: CMakePreconditionProblems, config?: ConfigurationReader) => Promise<void>;
+export type CMakePreconditionProblemSolver = (e: CMakePreconditionProblems, config?: ConfigurationReader) => Promise<boolean>;
 
 function nullableValueToString(arg: any | null | undefined): string {
     return arg === null ? 'empty' : arg;

test/unit-tests/driver/driver-codemodel-tests.ts

@@ -80,7 +80,7 @@ export function makeCodeModelDriverTestsuite(driverName: string, driver_generato
             const config = ConfigurationReader.create();
             const executable = await getCMakeExecutableInformation(cmakePath);
 
-            driver = await driver_generator(executable, config, kitDefault, workspaceFolder, async () => {}, []);
+            driver = await driver_generator(executable, config, kitDefault, workspaceFolder, async () => true, []);
             let code_model: null | CodeModelContent = null;
             if (driver && !(driver instanceof CMakeLegacyDriver)) {
                 driver.onCodeModelChanged(cm => {

test/unit-tests/driver/driver-test.ts

@@ -85,7 +85,7 @@ export function makeDriverTestsuite(driverName: string, driver_generator: (cmake
             const config = ConfigurationReader.create();
             const executable = await getCMakeExecutableInformation(cmakePath);
 
-            driver = await driver_generator(executable, config, ninjaKitDefault, defaultWorkspaceFolder, async () => {}, []);
+            driver = await driver_generator(executable, config, ninjaKitDefault, defaultWorkspaceFolder, async () => true, []);
             const allTargetName = driver.allTargetName;
 
             expect(allTargetName).to.eq('all');
@@ -96,23 +96,23 @@ export function makeDriverTestsuite(driverName: string, driver_generator: (cmake
             const config = ConfigurationReader.create();
             const executable = await getCMakeExecutableInformation(cmakePath);
 
-            driver = await driver_generator(executable, config, ninjaKitDefault, defaultWorkspaceFolder, async () => {}, []);
+            driver = await driver_generator(executable, config, ninjaKitDefault, defaultWorkspaceFolder, async () => true, []);
             expect(driver.binaryDir).to.endsWith('test/unit-tests/driver/workspace/test_project/build');
         }).timeout(60000);
 
         test('Configure fails', async () => {
             const config = ConfigurationReader.create();
             const executable = await getCMakeExecutableInformation(cmakePath);
 
-            driver = await driver_generator(executable, config, ninjaKitDefault, badCommandWorkspaceFolder, async () => {}, []);
+            driver = await driver_generator(executable, config, ninjaKitDefault, badCommandWorkspaceFolder, async () => true, []);
             expect((await driver.cleanConfigure(ConfigureTrigger.runTests, [])).result).to.be.eq(1);
         }).timeout(90000);
 
         test('Build', async () => {
             const config = ConfigurationReader.create();
             const executable = await getCMakeExecutableInformation(cmakePath);
 
-            driver = await driver_generator(executable, config, ninjaKitDefault, defaultWorkspaceFolder, async () => {}, []);
+            driver = await driver_generator(executable, config, ninjaKitDefault, defaultWorkspaceFolder, async () => true, []);
             expect((await driver.cleanConfigure(ConfigureTrigger.runTests, [])).result).to.be.eq(0);
             expect(await driver.build([driver.allTargetName])).to.be.eq(0);
 
@@ -133,7 +133,7 @@ export function makeDriverTestsuite(driverName: string, driver_generator: (cmake
             const kit = { name: 'GCC', preferredGenerator: { name: 'invalid Name' } } as Kit;
 
             try {
-                await driver_generator(executable, config, kit, defaultWorkspaceFolder, async () => {}, []);
+                await driver_generator(executable, config, kit, defaultWorkspaceFolder, async () => true, []);
                 expect(false, 'configure did not detect the invalid generator').to.be.true;
             } catch (e) {
                 if (!(e instanceof NoGeneratorError)) {
@@ -145,7 +145,7 @@ export function makeDriverTestsuite(driverName: string, driver_generator: (cmake
         test('Test compiler name reporting for telemetry', async () => {
             const config = ConfigurationReader.create();
             const executable = await getCMakeExecutableInformation(cmakePath);
-            driver = await driver_generator(executable, config, ninjaKitDefault, defaultWorkspaceFolder, async () => {}, []);
+            driver = await driver_generator(executable, config, ninjaKitDefault, defaultWorkspaceFolder, async () => true, []);
 
             // A few path examples that would exercise through the telemetry reporting rules:
             // - any name that is not recognized is reported as "other"
@@ -172,7 +172,7 @@ export function makeDriverTestsuite(driverName: string, driver_generator: (cmake
             const config = ConfigurationReader.create();
             const executable = await getCMakeExecutableInformation(cmakePath);
 
-            driver = await driver_generator(executable, config, ninjaKitDefault, defaultWorkspaceFolder, async () => {}, []);
+            driver = await driver_generator(executable, config, ninjaKitDefault, defaultWorkspaceFolder, async () => true, []);
 
             // Set kit without a preferred generator
             await driver.setKit({ name: 'GCC', isTrusted: true }, []);
@@ -195,6 +195,7 @@ export function makeDriverTestsuite(driverName: string, driver_generator: (cmake
             const checkPreconditionHelper = async (e: CMakePreconditionProblems) => {
                 expect(e).to.be.eq(CMakePreconditionProblems.MissingCMakeListsFile);
                 called = true;
+                return false;
             };
             driver = await driver_generator(executable, config, ninjaKitDefault, emptyWorkspaceFolder, checkPreconditionHelper, []);
             expect((await driver.cleanConfigure(ConfigureTrigger.runTests, [])).result).to.be.eq(-2);
@@ -209,6 +210,7 @@ export function makeDriverTestsuite(driverName: string, driver_generator: (cmake
             const checkPreconditionHelper = async (e: CMakePreconditionProblems) => {
                 expect(e).to.be.eq(CMakePreconditionProblems.ConfigureIsAlreadyRunning);
                 called = true;
+                return false;
             };
             driver = await driver_generator(executable, config, ninjaKitDefault, defaultWorkspaceFolder, checkPreconditionHelper, []);
             const configure1 = driver.configure(ConfigureTrigger.runTests, []);
@@ -227,6 +229,7 @@ export function makeDriverTestsuite(driverName: string, driver_generator: (cmake
             const checkPreconditionHelper = async (e: CMakePreconditionProblems) => {
                 expect(e).to.be.eq(CMakePreconditionProblems.ConfigureIsAlreadyRunning);
                 called = true;
+                return false;
             };
             driver
                 = await driver_generator(executable, config, ninjaKitDefault, defaultWorkspaceFolder, checkPreconditionHelper, []);
@@ -247,6 +250,7 @@ export function makeDriverTestsuite(driverName: string, driver_generator: (cmake
                 if (e === CMakePreconditionProblems.BuildIsAlreadyRunning) {
                     called = true;
                 }
+                return false;
             };
             driver
                 = await driver_generator(executable, config, ninjaKitDefault, defaultWorkspaceFolder, checkPreconditionHelper, []);
@@ -268,6 +272,7 @@ export function makeDriverTestsuite(driverName: string, driver_generator: (cmake
                 if (e === CMakePreconditionProblems.ConfigureIsAlreadyRunning) {
                     called = true;
                 }
+                return false;
             };
             driver
                 = await driver_generator(executable, config, ninjaKitDefault, defaultWorkspaceFolder, checkPreconditionHelper, []);
@@ -289,6 +294,7 @@ export function makeDriverTestsuite(driverName: string, driver_generator: (cmake
                 if (e === CMakePreconditionProblems.BuildIsAlreadyRunning) {
                     called = true;
                 }
+                return false;
             };
             driver
                 = await driver_generator(executable, config, ninjaKitDefault, defaultWorkspaceFolder, checkPreconditionHelper, []);
@@ -310,6 +316,7 @@ export function makeDriverTestsuite(driverName: string, driver_generator: (cmake
                 if (e === CMakePreconditionProblems.ConfigureIsAlreadyRunning) {
                     called = true;
                 }
+                return false;
             };
             driver
                 = await driver_generator(executable, config, ninjaKitDefault, defaultWorkspaceFolder, checkPreconditionHelper, []);
@@ -330,6 +337,7 @@ export function makeDriverTestsuite(driverName: string, driver_generator: (cmake
                 if (e === CMakePreconditionProblems.BuildIsAlreadyRunning) {
                     called = true;
                 }
+                return false;
             };
             driver = await driver_generator(executable, config, ninjaKitDefault, defaultWorkspaceFolder, checkPreconditionHelper, []);
             expect((await driver.configure(ConfigureTrigger.runTests, [])).result).to.be.equal(0);
@@ -345,12 +353,12 @@ export function makeDriverTestsuite(driverName: string, driver_generator: (cmake
             const config = ConfigurationReader.create();
             const executable = await getCMakeExecutableInformation(cmakePath);
 
-            driver = await driver_generator(executable, config, secondaryKit, defaultWorkspaceFolder, async () => {}, []);
+            driver = await driver_generator(executable, config, secondaryKit, defaultWorkspaceFolder, async () => true, []);
             await driver.cleanConfigure(ConfigureTrigger.runTests, []);
             await driver.asyncDispose();
 
             driver = null;
-            driver = await driver_generator(executable, config, ninjaKitDefault, defaultWorkspaceFolder, async () => {}, []);
+            driver = await driver_generator(executable, config, ninjaKitDefault, defaultWorkspaceFolder, async () => true, []);
             expect((await driver.configure(ConfigureTrigger.runTests, [])).result).to.be.eq(0);
 
             const expFileApi = driver instanceof CMakeFileApiDriver;
@@ -368,7 +376,7 @@ export function makeDriverTestsuite(driverName: string, driver_generator: (cmake
             const config = ConfigurationReader.create();
             const executable = await getCMakeExecutableInformation(cmakePath);
 
-            driver = await driver_generator(executable, config, ninjaKitDefault, defaultWorkspaceFolder, async () => {}, []);
+            driver = await driver_generator(executable, config, ninjaKitDefault, defaultWorkspaceFolder, async () => true, []);
             await driver.cleanConfigure(ConfigureTrigger.runTests, []);
             expect(driver.cmakeCacheEntries.get('CMAKE_GENERATOR')!.value).to.be.eq('Ninja');
 
@@ -397,7 +405,7 @@ export function makeDriverTestsuite(driverName: string, driver_generator: (cmake
             const config = ConfigurationReader.create();
             const executable = await getCMakeExecutableInformation(cmakePath);
 
-            driver = await driver_generator(executable, config, secondaryKit, defaultWorkspaceFolder, async () => {}, []);
+            driver = await driver_generator(executable, config, secondaryKit, defaultWorkspaceFolder, async () => true, []);
             await driver.cleanConfigure(ConfigureTrigger.runTests, []);
             expect(await driver.build(['all'])).to.be.eq(0, 'Automatic correction of all target failed');
         }).timeout(90000);
@@ -409,7 +417,7 @@ export function makeDriverTestsuite(driverName: string, driver_generator: (cmake
             const config = ConfigurationReader.create();
             const executable = await getCMakeExecutableInformation(cmakePath);
 
-            driver = await driver_generator(executable, config, ninjaKitDefault, defaultWorkspaceFolder, async () => {}, []);
+            driver = await driver_generator(executable, config, ninjaKitDefault, defaultWorkspaceFolder, async () => true, []);
             await driver.cleanConfigure(ConfigureTrigger.runTests, []);
             expect(await driver.build(['ALL_BUILD'])).to.be.eq(0, 'Automatic correction of ALL_BUILD target failed');
         }).timeout(90000);
@@ -418,7 +426,7 @@ export function makeDriverTestsuite(driverName: string, driver_generator: (cmake
             const config = ConfigurationReader.create();
             const executable = await getCMakeExecutableInformation(cmakePath);
 
-            driver = await driver_generator(executable, config, ninjaKitDefault, defaultWorkspaceFolder, async () => {}, []);
+            driver = await driver_generator(executable, config, ninjaKitDefault, defaultWorkspaceFolder, async () => true, []);
             await driver.configure(ConfigureTrigger.runTests, ['-DEXTRA_ARGS_TEST=Hallo']);
             expect(driver.cmakeCacheEntries.get('extraArgsEnvironment')?.value).to.be.eq('Hallo');
         }).timeout(90000);
@@ -427,7 +435,7 @@ export function makeDriverTestsuite(driverName: string, driver_generator: (cmake
             const config = ConfigurationReader.create();
             const executable = await getCMakeExecutableInformation(cmakePath);
 
-            driver = await driver_generator(executable, config, ninjaKitDefault, defaultWorkspaceFolder, async () => {}, []);
+            driver = await driver_generator(executable, config, ninjaKitDefault, defaultWorkspaceFolder, async () => true, []);
             await driver.cleanConfigure(ConfigureTrigger.runTests, ['-DEXTRA_ARGS_TEST=Hallo']);
             expect(driver.cmakeCacheEntries.get('extraArgsEnvironment')?.value).to.be.eq('Hallo');
         }).timeout(90000);