Skip to content

Commit 5cc9092

Browse files
author
Kartik Raj
authored
Revert to using conda activate for fetching activated environment variables (#19819)
Because of conda/conda#11814, `conda activate` is faster. Closes #19347 For #19101
1 parent 00316f3 commit 5cc9092

File tree

2 files changed

+26
-47
lines changed

2 files changed

+26
-47
lines changed

src/client/interpreter/activation/service.ts

Lines changed: 23 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { sleep } from '../../common/utils/async';
1616
import { InMemoryCache } from '../../common/utils/cacheUtils';
1717
import { OSType } from '../../common/utils/platform';
1818
import { IEnvironmentVariablesProvider } from '../../common/variables/types';
19-
import { EnvironmentType, PythonEnvironment } from '../../pythonEnvironments/info';
19+
import { PythonEnvironment } from '../../pythonEnvironments/info';
2020
import { captureTelemetry, sendTelemetryEvent } from '../../telemetry';
2121
import { EventName } from '../../telemetry/constants';
2222
import { IInterpreterService } from '../contracts';
@@ -30,7 +30,6 @@ import {
3030
traceVerbose,
3131
traceWarn,
3232
} from '../../logging';
33-
import { Conda } from '../../pythonEnvironments/common/environmentManagers/conda';
3433

3534
const ENVIRONMENT_PREFIX = 'e8b39361-0157-4923-80e1-22d70d46dee6';
3635
const CACHE_DURATION = 10 * 60 * 1000;
@@ -170,41 +169,20 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi
170169
if (!shellInfo) {
171170
return;
172171
}
172+
let isPossiblyCondaEnv = false;
173173
try {
174-
let command: string | undefined;
175-
let [args, parse] = internalScripts.printEnvVariables();
176-
args.forEach((arg, i) => {
177-
args[i] = arg.toCommandArgumentForPythonExt();
178-
});
179-
interpreter = interpreter ?? (await this.interpreterService.getActiveInterpreter(resource));
180-
if (interpreter?.envType === EnvironmentType.Conda) {
181-
const conda = await Conda.getConda();
182-
const pythonArgv = await conda?.getRunPythonArgs({
183-
name: interpreter.envName,
184-
prefix: interpreter.envPath ?? '',
185-
});
186-
if (pythonArgv) {
187-
// Using environment prefix isn't needed as the marker script already takes care of it.
188-
command = [...pythonArgv, ...args].map((arg) => arg.toCommandArgumentForPythonExt()).join(' ');
189-
}
190-
}
191-
if (!command) {
192-
const activationCommands = await this.helper.getEnvironmentActivationShellCommands(
193-
resource,
194-
shellInfo.shellType,
195-
interpreter,
196-
);
197-
traceVerbose(`Activation Commands received ${activationCommands} for shell ${shellInfo.shell}`);
198-
if (!activationCommands || !Array.isArray(activationCommands) || activationCommands.length === 0) {
199-
return;
200-
}
201-
// Run the activate command collect the environment from it.
202-
const activationCommand = this.fixActivationCommands(activationCommands).join(' && ');
203-
// In order to make sure we know where the environment output is,
204-
// put in a dummy echo we can look for
205-
command = `${activationCommand} && echo '${ENVIRONMENT_PREFIX}' && python ${args.join(' ')}`;
174+
const activationCommands = await this.helper.getEnvironmentActivationShellCommands(
175+
resource,
176+
shellInfo.shellType,
177+
interpreter,
178+
);
179+
traceVerbose(`Activation Commands received ${activationCommands} for shell ${shellInfo.shell}`);
180+
if (!activationCommands || !Array.isArray(activationCommands) || activationCommands.length === 0) {
181+
return;
206182
}
207-
183+
isPossiblyCondaEnv = activationCommands.join(' ').toLowerCase().includes('conda');
184+
// Run the activate command collect the environment from it.
185+
const activationCommand = this.fixActivationCommands(activationCommands).join(' && ');
208186
const processService = await this.processServiceFactory.create(resource);
209187
const customEnvVars = await this.envVarsService.getEnvironmentVariables(resource);
210188
const hasCustomEnvVars = Object.keys(customEnvVars).length;
@@ -216,6 +194,14 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi
216194
env[PYTHON_WARNINGS] = 'ignore';
217195

218196
traceVerbose(`${hasCustomEnvVars ? 'Has' : 'No'} Custom Env Vars`);
197+
198+
// In order to make sure we know where the environment output is,
199+
// put in a dummy echo we can look for
200+
const [args, parse] = internalScripts.printEnvVariables();
201+
args.forEach((arg, i) => {
202+
args[i] = arg.toCommandArgumentForPythonExt();
203+
});
204+
const command = `${activationCommand} && echo '${ENVIRONMENT_PREFIX}' && python ${args.join(' ')}`;
219205
traceVerbose(`Activating Environment to capture Environment variables, ${command}`);
220206

221207
// Do some wrapping of the call. For two reasons:
@@ -233,10 +219,7 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi
233219
result = await processService.shellExec(command, {
234220
env,
235221
shell: shellInfo.shell,
236-
timeout:
237-
interpreter?.envType === EnvironmentType.Conda
238-
? CONDA_ENVIRONMENT_TIMEOUT
239-
: ENVIRONMENT_TIMEOUT,
222+
timeout: isPossiblyCondaEnv ? CONDA_ENVIRONMENT_TIMEOUT : ENVIRONMENT_TIMEOUT,
240223
maxBuffer: 1000 * 1000,
241224
throwOnStdErr: false,
242225
});
@@ -282,7 +265,7 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi
282265
} catch (e) {
283266
traceError('getActivatedEnvironmentVariables', e);
284267
sendTelemetryEvent(EventName.ACTIVATE_ENV_TO_GET_ENV_VARS_FAILED, undefined, {
285-
isPossiblyCondaEnv: interpreter?.envType === EnvironmentType.Conda,
268+
isPossiblyCondaEnv,
286269
terminal: shellInfo.shellType,
287270
});
288271

@@ -300,9 +283,6 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi
300283
@traceDecoratorError('Failed to parse Environment variables')
301284
@traceDecoratorVerbose('parseEnvironmentOutput', TraceOptions.None)
302285
protected parseEnvironmentOutput(output: string, parse: (out: string) => NodeJS.ProcessEnv | undefined) {
303-
if (output.indexOf(ENVIRONMENT_PREFIX) === -1) {
304-
return parse(output);
305-
}
306286
output = output.substring(output.indexOf(ENVIRONMENT_PREFIX) + ENVIRONMENT_PREFIX.length);
307287
const js = output.substring(output.indexOf('{')).trim();
308288
return parse(js);

src/test/interpreters/activation/service.unit.test.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ suite('Interpreters Activation - Python Environment Variables', () => {
5858
architecture: Architecture.x64,
5959
};
6060

61-
function initSetup(interpreter: PythonEnvironment | undefined) {
61+
function initSetup() {
6262
helper = mock(TerminalHelper);
6363
platform = mock(PlatformService);
6464
processServiceFactory = mock(ProcessServiceFactory);
@@ -71,7 +71,6 @@ suite('Interpreters Activation - Python Environment Variables', () => {
7171
onDidChangeInterpreter = new EventEmitter<void>();
7272
when(envVarsService.onDidEnvironmentVariablesChange).thenReturn(onDidChangeEnvVariables.event);
7373
when(interpreterService.onDidChangeInterpreter).thenReturn(onDidChangeInterpreter.event);
74-
when(interpreterService.getActiveInterpreter(anything())).thenResolve(interpreter);
7574
service = new EnvironmentActivationService(
7675
instance(helper),
7776
instance(platform),
@@ -90,7 +89,7 @@ suite('Interpreters Activation - Python Environment Variables', () => {
9089
[undefined, Uri.parse('a')].forEach((resource) =>
9190
[undefined, pythonInterpreter].forEach((interpreter) => {
9291
suite(title(resource, interpreter), () => {
93-
setup(() => initSetup(interpreter));
92+
setup(initSetup);
9493
test('Unknown os will return empty variables', async () => {
9594
when(platform.osType).thenReturn(OSType.Unknown);
9695
const env = await service.getActivatedEnvironmentVariables(resource);
@@ -103,7 +102,7 @@ suite('Interpreters Activation - Python Environment Variables', () => {
103102

104103
osTypes.forEach((osType) => {
105104
suite(osType.name, () => {
106-
setup(() => initSetup(interpreter));
105+
setup(initSetup);
107106
test('getEnvironmentActivationShellCommands will be invoked', async () => {
108107
when(platform.osType).thenReturn(osType.value);
109108
when(

0 commit comments

Comments
 (0)