Enhance config, install, clean tasks (#2716)

Elaheh Rashedi · web-flow · commit ffabd1ec058b · 2022-08-29T16:23:02.000-07:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,7 +5,7 @@ Improvements:
 - Support for presets version 4. [#2492](https://github.com/microsoft/vscode-cmake-tools/issues/2492) [@chausner](https://github.com/chausner)
 - Triggering reconfigure after changes are made to included files. [#2526](https://github.com/microsoft/vscode-cmake-tools/issues/2526) [@chausner](https://github.com/chausner)
 - Add target name to terminal window name for launch. [#2613](https://github.com/microsoft/vscode-cmake-tools/issues/2613)
-- Add support for "preset" and "env" in task provider. [#2636](https://github.com/microsoft/vscode-cmake-tools/issues/2636) [#2553](https://github.com/microsoft/vscode-cmake-tools/issues/2553)
+- Add support for "preset" and "env" in task provider. [#2636](https://github.com/microsoft/vscode-cmake-tools/issues/2636) [#2553](https://github.com/microsoft/vscode-cmake-tools/issues/2553) [#2714](https://github.com/microsoft/vscode-cmake-tools/issues/2714) [#2706](https://github.com/microsoft/vscode-cmake-tools/issues/2706)
 - Add Craig Scott's "Professional CMake" book to the list of resources in doc/faq.md for learning CMake. [#2679](https://github.com/microsoft/vscode-cmake-tools/pull/2679) [@david-fong](https://github.com/david-fong)
 
 Bug Fixes:
diff --git a/src/cmakeTaskProvider.ts b/src/cmakeTaskProvider.ts
@@ -182,16 +182,16 @@ export class CustomBuildTaskTerminal implements vscode.Pseudoterminal, proc.Outp
                 await this.runConfigTask();
                 break;
             case CommandType.build:
-                await this.runBuildTask();
+                await this.runBuildTask(CommandType.build);
                 break;
             case CommandType.install:
-                await this.runInstallTask();
+                await this.runBuildTask(CommandType.install);
                 break;
             case CommandType.test:
                 await this.runTestTask();
                 break;
             case CommandType.clean:
-                await this.runCleanTask();
+                await this.runBuildTask(CommandType.clean);
                 break;
             case CommandType.cleanRebuild:
                 await this.runCleanRebuildTask();
@@ -272,7 +272,7 @@ export class CustomBuildTaskTerminal implements vscode.Pseudoterminal, proc.Outp
 
             this.preset = await this.resolvePresetName(this.preset, cmakeTools.useCMakePresets, CommandType.config);
             const configPreset: preset.ConfigurePreset | undefined = await cmakeTools?.expandConfigPresetbyName(this.preset);
-            const result = await cmakeDriver.configure(ConfigureTrigger.taskProvider, [], this, false, false, configPreset);
+            const result = await cmakeDriver.configure(ConfigureTrigger.taskProvider, [], this, false, false, configPreset, this.options);
             if (result === undefined || result === null) {
                 this.writeEmitter.fire(localize('configure.terminated', 'Configure was terminated') + endOfLine);
                 this.closeEmitter.fire(-1);
@@ -287,7 +287,16 @@ export class CustomBuildTaskTerminal implements vscode.Pseudoterminal, proc.Outp
         }
     }
 
-    private async runBuildTask(): Promise<any> {
+    private async runBuildTask(commandType: CommandType): Promise<any> {
+        let targets = this.targets;
+        const taskName: string = localizeCommandType(commandType);
+        if (commandType === CommandType.install) {
+            this.checkTargets(true);
+            targets = ['install'];
+        } else if (commandType === CommandType.clean) {
+            this.checkTargets(true);
+            targets = ['clean'];
+        }
         let fullCommand: proc.BuildCommand | null;
         let args: string[] = [];
         const cmakeTools: CMakeTools | undefined = this.getCMakeTools();
@@ -306,18 +315,18 @@ export class CustomBuildTaskTerminal implements vscode.Pseudoterminal, proc.Outp
                 const buildPreset: preset.BuildPreset | undefined = await cmakeTools?.expandBuildPresetbyName(this.preset);
                 if (!buildPreset) {
                     log.debug(localize("build.preset.not.found", 'Build preset not found.'));
-                    this.writeEmitter.fire(localize("build.failed", "Build preset {0} not found. Build failed.", this.preset) + endOfLine);
+                    this.writeEmitter.fire(localize("build.failed", "Build preset {0} not found. {1} failed.", this.preset, taskName) + endOfLine);
                     this.closeEmitter.fire(-1);
                     return;
                 }
-                fullCommand = await cmakeDriver.generateBuildCommandFromPreset(buildPreset, this.targets);
+                fullCommand = await cmakeDriver.generateBuildCommandFromPreset(buildPreset, targets);
                 if (fullCommand) {
                     cmakePath = fullCommand.command;
                     args = fullCommand.args || [];
                     this.options.environment = EnvironmentUtils.merge([ fullCommand.build_env, this.options.environment], {preserveNull: true});
                 }
             } else {
-                fullCommand = await cmakeDriver.generateBuildCommandFromSettings(this.targets);
+                fullCommand = await cmakeDriver.generateBuildCommandFromSettings(targets);
                 if (fullCommand) {
                     cmakePath = fullCommand.command;
                     args = fullCommand.args ? fullCommand.args : [];
@@ -326,44 +335,27 @@ export class CustomBuildTaskTerminal implements vscode.Pseudoterminal, proc.Outp
             }
         } else {
             log.debug(localize("cmake.driver.not.found", 'CMake driver not found.'));
-            this.writeEmitter.fire(localize("build.failed", "Build failed.") + endOfLine);
+            this.writeEmitter.fire(localize("build.failed", "{0} failed.", taskName) + endOfLine);
             this.closeEmitter.fire(-1);
             return;
         }
-        this.writeEmitter.fire(localize("build.started", "Build task started....") + endOfLine);
+        this.writeEmitter.fire(localize("build.started", "{0} task started....", taskName) + endOfLine);
         this.writeEmitter.fire(proc.buildCmdStr(cmakePath, args) + endOfLine);
         try {
             const result: proc.ExecutionResult = await proc.execute(cmakePath, args, this, this.options).result;
             if (result.retc) {
-                this.writeEmitter.fire(localize("build.finished.with.error", "Build finished with error(s).") + endOfLine);
+                this.writeEmitter.fire(localize("build.finished.with.error", "{0} finished with error(s).", taskName) + endOfLine);
             } else if (result.stderr && !result.stdout) {
-                this.writeEmitter.fire(localize("build.finished.with.warnings", "Build finished with warning(s).") + endOfLine);
+                this.writeEmitter.fire(localize("build.finished.with.warnings", "{0} finished with warning(s).", taskName) + endOfLine);
             } else if (result.stdout && result.stdout.includes("warning")) {
-                this.writeEmitter.fire(localize("build.finished.with.warnings", "Build finished with warning(s).") + endOfLine);
+                this.writeEmitter.fire(localize("build.finished.with.warnings", "{0} finished with warning(s).", taskName) + endOfLine);
             } else {
-                this.writeEmitter.fire(localize("build.finished.successfully", "Build finished successfully.") + endOfLine);
+                this.writeEmitter.fire(localize("build.finished.successfully", "{0} finished successfully.", taskName) + endOfLine);
             }
             this.closeEmitter.fire(0);
         } catch {
-            this.writeEmitter.fire(localize("build.finished.with.error", "Build finished with error(s).") + endOfLine);
-            this.closeEmitter.fire(-1);
-        }
-    }
-
-    private async runInstallTask(): Promise<any> {
-        this.writeEmitter.fire(localize("install.started", "Install task started...") + endOfLine);
-        this.checkTargets(true);
-        const cmakeTools: CMakeTools | undefined = this.getCMakeTools();
-        if (!cmakeTools) {
-            return;
-        }
-        const result: number | undefined =  await cmakeTools.runBuild(['install'], false, this);
-        if (result === undefined) {
-            this.writeEmitter.fire(localize('install.terminated', 'Install was terminated') + endOfLine);
+            this.writeEmitter.fire(localize("build.finished.with.error", "{0} finished with error(s).", taskName) + endOfLine);
             this.closeEmitter.fire(-1);
-        } else {
-            this.writeEmitter.fire(localize('install.finished.with.code', 'Install finished with return code {0}', result) + endOfLine);
-            this.closeEmitter.fire(result);
         }
     }
 
@@ -402,28 +394,10 @@ export class CustomBuildTaskTerminal implements vscode.Pseudoterminal, proc.Outp
         }
     }
 
-    private async runCleanTask(ignoreTargets: boolean = true): Promise<any> {
-        this.writeEmitter.fire(localize("clean.started", "Clean task started...") + endOfLine);
-        this.checkTargets(ignoreTargets);
-        const cmakeTools: CMakeTools | undefined = this.getCMakeTools();
-        if (!cmakeTools) {
-            return;
-        }
-        const result: number | undefined =  await cmakeTools.runBuild(['clean'], false, this);
-        if (result === undefined || result !== 0) {
-            this.writeEmitter.fire(localize("clean.failed", "Clean task failed.") + endOfLine);
-            this.closeEmitter.fire(-1);
-        } else {
-            this.writeEmitter.fire(localize("clean.finished.with.code", "Clean finished with return code {0}", result) + endOfLine);
-            this.closeEmitter.fire(result);
-        }
-        return result;
-    }
-
     private async runCleanRebuildTask(): Promise<any> {
-        const cleanResult = await this.runCleanTask(false);
+        const cleanResult = await this.runBuildTask(CommandType.clean);
         if (cleanResult === 0) {
-            await this.runBuildTask();
+            await this.runBuildTask(CommandType.build);
         }
     }
 }
diff --git a/src/drivers/cmakeDriver.ts b/src/drivers/cmakeDriver.ts
@@ -73,7 +73,7 @@ export abstract class CMakeDriver implements vscode.Disposable {
      *
      * @returns The exit code from CMake
      */
-    protected abstract doConfigure(extra_args: string[], consumer?: proc.OutputConsumer, showCommandOnly?: boolean, configurePreset?: preset.ConfigurePreset | null): Promise<number>;
+    protected abstract doConfigure(extra_args: string[], consumer?: proc.OutputConsumer, showCommandOnly?: boolean, configurePreset?: preset.ConfigurePreset | null, options?: proc.ExecutionOptions): Promise<number>;
     protected abstract doCacheConfigure(): Promise<number>;
 
     private _isConfiguredAtLeastOnce = false;
@@ -199,18 +199,22 @@ export abstract class CMakeDriver implements vscode.Disposable {
     /**
      * Get the environment variables that should be set at CMake-configure time.
      */
-    async getConfigureEnvironment(configurePreset?: preset.ConfigurePreset | null): Promise<Environment> {
+    async getConfigureEnvironment(configurePreset?: preset.ConfigurePreset | null, extraEnvironmentVariables?: Environment): Promise<Environment> {
+        let envs;
         if (this.useCMakePresets) {
-            return EnvironmentUtils.create(configurePreset ? configurePreset.environment : this._configurePreset?.environment);
+            envs = EnvironmentUtils.create(configurePreset ? configurePreset.environment : this._configurePreset?.environment);
         } else {
-            let envs = this._kitEnvironmentVariables;
+            envs = this._kitEnvironmentVariables;
             /* NOTE: By mergeEnvironment one by one to enable expanding self containd variable such as PATH properly */
             /* If configureEnvironment and environment both configured different PATH, doing this will preserve them all */
             envs = EnvironmentUtils.merge([envs, await this.computeExpandedEnvironment(this.config.environment, envs)]);
             envs = EnvironmentUtils.merge([envs, await this.computeExpandedEnvironment(this.config.configureEnvironment, envs)]);
             envs = EnvironmentUtils.merge([envs, await this.computeExpandedEnvironment(this._variantEnv, envs)]);
-            return envs;
         }
+        if (extraEnvironmentVariables) {
+            envs = EnvironmentUtils.merge([envs, await this.computeExpandedEnvironment(extraEnvironmentVariables, envs)]);
+        }
+        return envs;
     }
 
     /**
@@ -1219,7 +1223,7 @@ export abstract class CMakeDriver implements vscode.Disposable {
         return Promise.all(expanded_flags_promises);
     }
 
-    async configure(trigger: ConfigureTrigger, extra_args: string[], consumer?: proc.OutputConsumer, withoutCmakeSettings: boolean = false, showCommandOnly?: boolean, presetOverride?: preset.ConfigurePreset): Promise<number> {
+    async configure(trigger: ConfigureTrigger, extra_args: string[], consumer?: proc.OutputConsumer, withoutCmakeSettings: boolean = false, showCommandOnly?: boolean, presetOverride?: preset.ConfigurePreset, options?: proc.ExecutionOptions): Promise<number> {
         // Check if the configuration is using cache in the first configuration and adjust the logging messages based on that.
         const shouldUseCachedConfiguration: boolean = this.shouldUseCachedConfiguration(trigger);
 
@@ -1279,7 +1283,7 @@ export abstract class CMakeDriver implements vscode.Disposable {
                 this._isConfiguredAtLeastOnce = true;
                 return retc;
             } else {
-                retc = await this.doConfigure(expanded_flags, consumer, showCommandOnly, presetOverride);
+                retc = await this.doConfigure(expanded_flags, consumer, showCommandOnly, presetOverride, options);
                 this._isConfiguredAtLeastOnce = true;
             }
             const timeEnd: number = new Date().getTime();
diff --git a/src/drivers/cmakeFileApiDriver.ts b/src/drivers/cmakeFileApiDriver.ts
@@ -221,7 +221,7 @@ export class CMakeFileApiDriver extends CMakeDriver {
         return 0;
     }
 
-    async doConfigure(args_: string[], outputConsumer?: proc.OutputConsumer, showCommandOnly?: boolean, configurePreset?: ConfigurePreset | null): Promise<number> {
+    async doConfigure(args_: string[], outputConsumer?: proc.OutputConsumer, showCommandOnly?: boolean, configurePreset?: ConfigurePreset | null, options?: proc.ExecutionOptions): Promise<number> {
         const api_path = this.getCMakeFileApiPath(configurePreset?.binaryDir);
         await createQueryFileForApi(api_path);
 
@@ -269,17 +269,20 @@ export class CMakeFileApiDriver extends CMakeDriver {
         } else {
             log.debug(`Configuring using ${this.useCMakePresets ? 'preset' : 'kit'}`);
             log.debug('Invoking CMake', cmake, 'with arguments', JSON.stringify(args));
-            const env = await this.getConfigureEnvironment(configurePreset);
-            const res = await this.executeCommand(cmake, args, outputConsumer, { environment: env, cwd: binaryDir }).result;
-            log.trace(res.stderr);
-            log.trace(res.stdout);
-            if (res.retc === 0) {
+            const env = await this.getConfigureEnvironment(configurePreset, options?.environment);
+            const result = await this.executeCommand(cmake, args, outputConsumer, {
+                environment: env,
+                cwd: options?.cwd ?? binaryDir
+            }).result;
+            log.trace(result.stderr);
+            log.trace(result.stdout);
+            if (result.retc === 0) {
                 if (!configurePreset) {
                     this._needsReconfigure = false;
                 }
                 await this.updateCodeModel(configurePreset?.binaryDir);
             }
-            return res.retc === null ? -1 : res.retc;
+            return result.retc === null ? -1 : result.retc;
         }
     }
 
diff --git a/src/drivers/cmakeLegacyDriver.ts b/src/drivers/cmakeLegacyDriver.ts
@@ -76,7 +76,7 @@ export class CMakeLegacyDriver extends CMakeDriver {
         this._cacheWatcher.dispose();
     }
 
-    async doConfigure(args_: string[], outputConsumer?: proc.OutputConsumer, showCommandOnly?: boolean, configurePreset?: ConfigurePreset | null): Promise<number> {
+    async doConfigure(args_: string[], outputConsumer?: proc.OutputConsumer, showCommandOnly?: boolean, configurePreset?: ConfigurePreset | null, options?: proc.ExecutionOptions): Promise<number> {
         // Ensure the binary directory exists
         const binaryDir = configurePreset?.binaryDir ?? this.binaryDir;
         await fs.mkdir_p(binaryDir);
@@ -113,19 +113,19 @@ export class CMakeLegacyDriver extends CMakeDriver {
             return 0;
         } else {
             log.debug(localize('invoking.cmake.with.arguments', 'Invoking CMake {0} with arguments {1}', cmake, JSON.stringify(args)));
-            const res = await this.executeCommand(cmake, args, outputConsumer, {
-                environment: await this.getConfigureEnvironment(configurePreset),
-                cwd: binaryDir
+            const result = await this.executeCommand(cmake, args, outputConsumer, {
+                environment: await this.getConfigureEnvironment(configurePreset, options?.environment),
+                cwd: options?.cwd ?? binaryDir
             }).result;
-            log.trace(res.stderr);
-            log.trace(res.stdout);
-            if (res.retc === 0 && !configurePreset) {
+            log.trace(result.stderr);
+            log.trace(result.stdout);
+            if (result.retc === 0 && !configurePreset) {
                 this._needsReconfigure = false;
             }
             if (!configurePreset) {
                 await this._reloadPostConfigure();
             }
-            return res.retc === null ? -1 : res.retc;
+            return result.retc === null ? -1 : result.retc;
         }
     }
 
diff --git a/src/drivers/cmakeServerDriver.ts b/src/drivers/cmakeServerDriver.ts
@@ -154,7 +154,7 @@ export class CMakeServerDriver extends CMakeDriver {
         })();
     }
 
-    protected async doConfigure(args: string[], consumer?: proc.OutputConsumer, showCommandOnly?: boolean, configurePreset?: ConfigurePreset | null) {
+    protected async doConfigure(args: string[], consumer?: proc.OutputConsumer, showCommandOnly?: boolean, configurePreset?: ConfigurePreset | null, _options?: proc.ExecutionOptions) {
         await this._clientChangeInProgress;
         const cl = await this.getClient();
         const sub = this.onMessage(msg => {

Original file line number	Diff line number	Diff line change
`@@ -154,7 +154,7 @@ export class CMakeServerDriver extends CMakeDriver {`
`154`	`154`	`})();`
`155`	`155`	`}`
`156`	`156`
`157`		`- protected async doConfigure(args: string[], consumer?: proc.OutputConsumer, showCommandOnly?: boolean, configurePreset?: ConfigurePreset \| null) {`
	`157`	`+ protected async doConfigure(args: string[], consumer?: proc.OutputConsumer, showCommandOnly?: boolean, configurePreset?: ConfigurePreset \| null, _options?: proc.ExecutionOptions) {`
`158`	`158`	`await this._clientChangeInProgress;`
`159`	`159`	`const cl = await this.getClient();`
`160`	`160`	`const sub = this.onMessage(msg => {`