Ensure microvenv is added to path after selection.

karthiknadig · karthiknadig · commit f52049d4400a · 2023-04-26T12:53:30.000-07:00
diff --git a/src/client/interpreter/activation/service.ts b/src/client/interpreter/activation/service.ts
@@ -6,6 +6,7 @@
 
 import '../../common/extensions';
 
+import * as path from 'path';
 import { inject, injectable } from 'inversify';
 
 import { IWorkspaceService } from '../../common/application/types';
@@ -25,10 +26,18 @@ import { EventName } from '../../telemetry/constants';
 import { IInterpreterService } from '../contracts';
 import { IEnvironmentActivationService } from './types';
 import { TraceOptions } from '../../logging/types';
-import { traceDecoratorError, traceDecoratorVerbose, traceError, traceVerbose, traceWarn } from '../../logging';
+import {
+    traceDecoratorError,
+    traceDecoratorVerbose,
+    traceError,
+    traceInfo,
+    traceVerbose,
+    traceWarn,
+} from '../../logging';
 import { Conda } from '../../pythonEnvironments/common/environmentManagers/conda';
 import { StopWatch } from '../../common/utils/stopWatch';
 import { identifyShellFromShellPath } from '../../common/terminal/shellDetectors/baseShellDetector';
+import { getSearchPathEnvVarNames } from '../../common/utils/exec';
 
 const ENVIRONMENT_PREFIX = 'e8b39361-0157-4923-80e1-22d70d46dee6';
 const CACHE_DURATION = 10 * 60 * 1000;
@@ -193,6 +202,11 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi
             shellInfo = { shellType: customShellType, shell };
         }
         try {
+            const processService = await this.processServiceFactory.create(resource);
+            const customEnvVars = (await this.envVarsService.getEnvironmentVariables(resource)) ?? {};
+            const hasCustomEnvVars = Object.keys(customEnvVars).length;
+            const env = hasCustomEnvVars ? customEnvVars : { ...this.currentProcess.env };
+
             let command: string | undefined;
             const [args, parse] = internalScripts.printEnvVariables();
             args.forEach((arg, i) => {
@@ -217,6 +231,16 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi
                 );
                 traceVerbose(`Activation Commands received ${activationCommands} for shell ${shellInfo.shell}`);
                 if (!activationCommands || !Array.isArray(activationCommands) || activationCommands.length === 0) {
+                    if (interpreter?.envType === EnvironmentType.Venv) {
+                        const key = getSearchPathEnvVarNames()[0];
+                        if (env[key]) {
+                            env[key] = `${path.dirname(interpreter.path)}${path.delimiter}${env[key]}`;
+                        } else {
+                            env[key] = `${path.dirname(interpreter.path)}`;
+                        }
+
+                        return env;
+                    }
                     return undefined;
                 }
                 // Run the activate command collect the environment from it.
@@ -226,11 +250,6 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi
                 command = `${activationCommand} && echo '${ENVIRONMENT_PREFIX}' && python ${args.join(' ')}`;
             }
 
-            const processService = await this.processServiceFactory.create(resource);
-            const customEnvVars = await this.envVarsService.getEnvironmentVariables(resource);
-            const hasCustomEnvVars = Object.keys(customEnvVars).length;
-            const env = hasCustomEnvVars ? customEnvVars : { ...this.currentProcess.env };
-
             // Make sure python warnings don't interfere with getting the environment. However
             // respect the warning in the returned values
             const oldWarnings = env[PYTHON_WARNINGS];
@@ -283,7 +302,7 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi
                     // that's the case, wait and try again. This happens especially on AzDo
                     const excString = (exc as Error).toString();
                     if (condaRetryMessages.find((m) => excString.includes(m)) && tryCount < 10) {
-                        traceVerbose(`Conda is busy, attempting to retry ...`);
+                        traceInfo(`Conda is busy, attempting to retry ...`);
                         result = undefined;
                         tryCount += 1;
                         await sleep(500);
diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts
@@ -1,8 +1,10 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
+import * as path from 'path';
 import { inject, injectable } from 'inversify';
-import { ProgressOptions, ProgressLocation, MarkdownString } from 'vscode';
+import { ProgressOptions, ProgressLocation, MarkdownString, WorkspaceFolder } from 'vscode';
+import { pathExists } from 'fs-extra';
 import { IExtensionActivationService } from '../../activation/types';
 import { IApplicationShell, IApplicationEnvironment, IWorkspaceService } from '../../common/application/types';
 import { inTerminalEnvVarExperiment } from '../../common/experiments/helpers';
@@ -22,6 +24,7 @@ import { traceDecoratorVerbose, traceVerbose } from '../../logging';
 import { IInterpreterService } from '../contracts';
 import { defaultShells } from './service';
 import { IEnvironmentActivationService } from './types';
+import { EnvironmentType } from '../../pythonEnvironments/info';
 
 @injectable()
 export class TerminalEnvVarCollectionService implements IExtensionActivationService {
@@ -53,6 +56,17 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
     public async activate(resource: Resource): Promise<void> {
         if (!inTerminalEnvVarExperiment(this.experimentService)) {
             this.context.environmentVariableCollection.clear();
+            await this.handleMicroVenv(resource);
+            if (!this.registeredOnce) {
+                this.interpreterService.onDidChangeInterpreter(
+                    async (r) => {
+                        await this.handleMicroVenv(r);
+                    },
+                    this,
+                    this.disposables,
+                );
+                this.registeredOnce = true;
+            }
             return;
         }
         if (!this.registeredOnce) {
@@ -82,14 +96,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
     }
 
     public async _applyCollection(resource: Resource, shell = this.applicationEnvironment.shell): Promise<void> {
-        let workspaceFolder = this.workspaceService.getWorkspaceFolder(resource);
-        if (
-            !workspaceFolder &&
-            Array.isArray(this.workspaceService.workspaceFolders) &&
-            this.workspaceService.workspaceFolders.length > 0
-        ) {
-            [workspaceFolder] = this.workspaceService.workspaceFolders;
-        }
+        const workspaceFolder = this.getWorkspaceFolder(resource);
         const settings = this.configurationService.getSettings(resource);
         if (!settings.terminal.activateEnvironment) {
             traceVerbose('Activating environments in terminal is disabled for', resource?.fsPath);
@@ -143,6 +150,37 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
         });
     }
 
+    private async handleMicroVenv(resource: Resource) {
+        const workspaceFolder = this.getWorkspaceFolder(resource);
+        const interpreter = await this.interpreterService.getActiveInterpreter(resource);
+        if (interpreter?.envType === EnvironmentType.Venv) {
+            const activatePath = path.join(path.dirname(interpreter.path), 'activate');
+            if (!(await pathExists(activatePath))) {
+                this.context.environmentVariableCollection.replace(
+                    'PATH',
+                    `${path.dirname(interpreter.path)}${path.delimiter}${process.env.Path}`,
+                    {
+                        workspaceFolder,
+                    },
+                );
+                return;
+            }
+        }
+        this.context.environmentVariableCollection.clear();
+    }
+
+    private getWorkspaceFolder(resource: Resource): WorkspaceFolder | undefined {
+        let workspaceFolder = this.workspaceService.getWorkspaceFolder(resource);
+        if (
+            !workspaceFolder &&
+            Array.isArray(this.workspaceService.workspaceFolders) &&
+            this.workspaceService.workspaceFolders.length > 0
+        ) {
+            [workspaceFolder] = this.workspaceService.workspaceFolders;
+        }
+        return workspaceFolder;
+    }
+
     @traceDecoratorVerbose('Display activating terminals')
     private showProgress(): void {
         if (!this.deferred) {
diff --git a/src/test/interpreters/activation/service.unit.test.ts b/src/test/interpreters/activation/service.unit.test.ts
@@ -28,6 +28,7 @@ import { EnvironmentActivationService } from '../../../client/interpreter/activa
 import { IInterpreterService } from '../../../client/interpreter/contracts';
 import { InterpreterService } from '../../../client/interpreter/interpreterService';
 import { EnvironmentType, PythonEnvironment } from '../../../client/pythonEnvironments/info';
+import { getSearchPathEnvVarNames } from '../../../client/common/utils/exec';
 
 const getEnvironmentPrefix = 'e8b39361-0157-4923-80e1-22d70d46dee6';
 const defaultShells = {
@@ -118,6 +119,25 @@ suite('Interpreters Activation - Python Environment Variables', () => {
                                 helper.getEnvironmentActivationShellCommands(resource, anything(), interpreter),
                             ).once();
                         });
+                        test('Env variables returned for microvenv', async () => {
+                            when(platform.osType).thenReturn(osType.value);
+
+                            const microVenv = { ...pythonInterpreter, envType: EnvironmentType.Venv };
+                            const key = getSearchPathEnvVarNames()[0];
+                            const varsFromEnv = { [key]: '/foo/bar' };
+
+                            when(
+                                helper.getEnvironmentActivationShellCommands(resource, anything(), microVenv),
+                            ).thenResolve();
+
+                            const env = await service.getActivatedEnvironmentVariables(resource, microVenv);
+
+                            verify(platform.osType).once();
+                            expect(env).to.deep.equal(varsFromEnv);
+                            verify(
+                                helper.getEnvironmentActivationShellCommands(resource, anything(), microVenv),
+                            ).once();
+                        });
                         test('Validate command used to activation and printing env vars', async () => {
                             const cmd = ['1', '2'];
                             const envVars = { one: '1', two: '2' };
diff --git a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts
@@ -117,11 +117,11 @@ suite('Terminal Environment Variable Collection Service', () => {
 
         await terminalEnvVarCollectionService.activate(undefined);
 
-        verify(interpreterService.onDidChangeInterpreter(anything(), anything(), anything())).never();
+        verify(interpreterService.onDidChangeInterpreter(anything(), anything(), anything())).once();
         verify(applicationEnvironment.onDidChangeShell(anything(), anything(), anything())).never();
         assert(applyCollectionStub.notCalled, 'Collection should not be applied on activation');
 
-        verify(collection.clear()).once();
+        verify(collection.clear()).atLeast(1);
     });
 
     test('When interpreter changes, apply new activated variables to the collection', async () => {
