Skip to content

Commit d9d048b

Browse files
DonJayamanneDonJayamanne
committed
Revert "Revert "Move .env file and environment variable parser into the extension (#6773)" (#6794)"
This reverts commit 1499a8f.
1 parent 3cf181a commit d9d048b

8 files changed

Lines changed: 67 additions & 53 deletions

File tree

‎news/3 Code Health/6770.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Move .env file handling into the extension. This is in preparation to switch to the out-of-proc debug adapter from ptvsd.

‎src/client/debugger/debugAdapter/DebugClients/LocalDebugClient.ts‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,14 @@ import * as path from 'path';
33
import { DebugSession, OutputEvent } from 'vscode-debugadapter';
44
import { DebugProtocol } from 'vscode-debugprotocol';
55
import { open } from '../../../common/open';
6-
import { IS_WINDOWS } from '../../../common/platform/constants';
7-
import { PathUtils } from '../../../common/platform/pathUtils';
8-
import { CurrentProcess } from '../../../common/process/currentProcess';
96
import { noop } from '../../../common/utils/misc';
10-
import { EnvironmentVariablesService } from '../../../common/variables/environment';
117
import { IServiceContainer } from '../../../ioc/types';
128
import { LaunchRequestArguments } from '../../types';
139
import { IDebugServer } from '../Common/Contracts';
1410
import { BaseDebugServer } from '../DebugServers/BaseDebugServer';
1511
import { LocalDebugServerV2 } from '../DebugServers/LocalDebugServerV2';
1612
import { ILocalDebugLauncherScriptProvider } from '../types';
1713
import { DebugClient, DebugType } from './DebugClient';
18-
import { DebugClientHelper } from './helper';
1914

2015
enum DebugServerStatus {
2116
Unknown = 1,
@@ -67,11 +62,6 @@ export class LocalDebugClient extends DebugClient<LaunchRequestArguments> {
6762
}
6863
// tslint:disable-next-line:max-func-body-length member-ordering no-any
6964
public async LaunchApplicationToDebug(dbgServer: IDebugServer): Promise<any> {
70-
const pathUtils = new PathUtils(IS_WINDOWS);
71-
const currentProcess = new CurrentProcess();
72-
const environmentVariablesService = new EnvironmentVariablesService(pathUtils);
73-
const helper = new DebugClientHelper(environmentVariablesService, pathUtils, currentProcess);
74-
const environmentVariables = await helper.getEnvironmentVariables(this.args);
7565
// tslint:disable-next-line:max-func-body-length cyclomatic-complexity no-any
7666
return new Promise<any>((resolve, reject) => {
7767
const fileDir = this.args && this.args.program ? path.dirname(this.args.program) : '';
@@ -84,15 +74,16 @@ export class LocalDebugClient extends DebugClient<LaunchRequestArguments> {
8474
pythonPath = this.args.pythonPath;
8575
}
8676
const args = this.buildLaunchArguments(processCwd, dbgServer.port);
77+
const envVars = this.args.env ? { ...this.args.env } : {};
8778
switch (this.args.console) {
8879
case 'externalTerminal':
8980
case 'integratedTerminal': {
9081
const isSudo = Array.isArray(this.args.debugOptions) && this.args.debugOptions.some(opt => opt === 'Sudo');
91-
this.launchExternalTerminal(isSudo, processCwd, pythonPath, args, environmentVariables).then(resolve).catch(reject);
82+
this.launchExternalTerminal(isSudo, processCwd, pythonPath, args, envVars).then(resolve).catch(reject);
9283
break;
9384
}
9485
default: {
95-
this.pyProc = spawn(pythonPath, args, { cwd: processCwd, env: environmentVariables });
86+
this.pyProc = spawn(pythonPath, args, { cwd: processCwd, env: envVars });
9687
this.handleProcessOutput(this.pyProc!, reject);
9788

9889
// Here we wait for the application to connect to the socket server.

src/client/debugger/debugAdapter/DebugClients/helper.ts renamed to src/client/debugger/extension/configuration/resolvers/helper.ts

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,25 @@
1-
import { ICurrentProcess, IPathUtils } from '../../../common/types';
2-
import { EnvironmentVariables, IEnvironmentVariablesService } from '../../../common/variables/types';
3-
import { LaunchRequestArguments } from '../../types';
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
43

5-
export class DebugClientHelper {
6-
constructor(private envParser: IEnvironmentVariablesService, private pathUtils: IPathUtils,
7-
private process: ICurrentProcess) { }
4+
'use strict';
5+
6+
import { inject, injectable } from 'inversify';
7+
import { ICurrentProcess, IPathUtils } from '../../../../common/types';
8+
import { EnvironmentVariables, IEnvironmentVariablesService } from '../../../../common/variables/types';
9+
import { LaunchRequestArguments } from '../../../types';
10+
11+
export const IDebugEnvironmentVariablesService = Symbol('IDebugEnvironmentVariablesService');
12+
export interface IDebugEnvironmentVariablesService {
13+
getEnvironmentVariables(args: LaunchRequestArguments): Promise<EnvironmentVariables>;
14+
}
15+
16+
@injectable()
17+
export class DebugEnvironmentVariablesHelper implements IDebugEnvironmentVariablesService {
18+
constructor(
19+
@inject(IEnvironmentVariablesService) private envParser: IEnvironmentVariablesService,
20+
@inject(IPathUtils) private pathUtils: IPathUtils,
21+
@inject(ICurrentProcess) private process: ICurrentProcess
22+
) { }
823
public async getEnvironmentVariables(args: LaunchRequestArguments): Promise<EnvironmentVariables> {
924
const pathVariableName = this.pathUtils.getPathVariableName();
1025

‎src/client/debugger/extension/configuration/resolvers/launch.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { IConfigurationService } from '../../../../common/types';
1313
import { DebuggerTypeName } from '../../../constants';
1414
import { DebugOptions, LaunchRequestArguments } from '../../../types';
1515
import { BaseConfigurationResolver } from './base';
16+
import { IDebugEnvironmentVariablesService } from './helper';
1617

1718
@injectable()
1819
export class LaunchConfigurationResolver extends BaseConfigurationResolver<LaunchRequestArguments> {
@@ -21,7 +22,8 @@ export class LaunchConfigurationResolver extends BaseConfigurationResolver<Launc
2122
@inject(IDocumentManager) documentManager: IDocumentManager,
2223
@inject(IDiagnosticsService) @named(InvalidPythonPathInDebuggerServiceId) private readonly invalidPythonPathInDebuggerService: IInvalidPythonPathInDebuggerService,
2324
@inject(IPlatformService) private readonly platformService: IPlatformService,
24-
@inject(IConfigurationService) configurationService: IConfigurationService
25+
@inject(IConfigurationService) configurationService: IConfigurationService,
26+
@inject(IDebugEnvironmentVariablesService) private readonly debugEnvHelper: IDebugEnvironmentVariablesService
2527
) {
2628
super(workspaceService, documentManager, configurationService);
2729
}
@@ -63,6 +65,10 @@ export class LaunchConfigurationResolver extends BaseConfigurationResolver<Launc
6365
const settings = this.configurationService.getSettings(workspaceFolder);
6466
debugConfiguration.envFile = settings.envFile;
6567
}
68+
// Extract environment variables from .env file in the vscode context and
69+
// set the "env" debug configuration argument. This expansion should be
70+
// done here before handing of the environment settings to the debug adapter
71+
debugConfiguration.env = await this.debugEnvHelper.getEnvironmentVariables(debugConfiguration);
6672
if (typeof debugConfiguration.stopOnEntry !== 'boolean') {
6773
debugConfiguration.stopOnEntry = false;
6874
}

‎src/client/debugger/extension/serviceRegistry.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { DebugConfigurationProviderFactory } from './configuration/providers/pro
1818
import { PyramidLaunchDebugConfigurationProvider } from './configuration/providers/pyramidLaunch';
1919
import { RemoteAttachDebugConfigurationProvider } from './configuration/providers/remoteAttach';
2020
import { AttachConfigurationResolver } from './configuration/resolvers/attach';
21+
import { DebugEnvironmentVariablesHelper, IDebugEnvironmentVariablesService } from './configuration/resolvers/helper';
2122
import { LaunchConfigurationResolver } from './configuration/resolvers/launch';
2223
import { IDebugConfigurationProviderFactory, IDebugConfigurationResolver } from './configuration/types';
2324
import { ChildProcessAttachEventHandler } from './hooks/childProcessAttachHandler';
@@ -41,4 +42,5 @@ export function registerTypes(serviceManager: IServiceManager) {
4142
serviceManager.addSingleton<IDebugConfigurationProvider>(IDebugConfigurationProvider, RemoteAttachDebugConfigurationProvider, DebugConfigurationType.remoteAttach);
4243
serviceManager.addSingleton<IDebugConfigurationProvider>(IDebugConfigurationProvider, ModuleLaunchDebugConfigurationProvider, DebugConfigurationType.launchModule);
4344
serviceManager.addSingleton<IDebugConfigurationProvider>(IDebugConfigurationProvider, PyramidLaunchDebugConfigurationProvider, DebugConfigurationType.launchPyramid);
45+
serviceManager.addSingleton<IDebugEnvironmentVariablesService>(IDebugEnvironmentVariablesService, DebugEnvironmentVariablesHelper);
4446
}

‎src/test/debugger/envVars.test.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import * as path from 'path';
99
import * as shortid from 'shortid';
1010
import { ICurrentProcess, IPathUtils } from '../../client/common/types';
1111
import { IEnvironmentVariablesService } from '../../client/common/variables/types';
12-
import { DebugClientHelper } from '../../client/debugger/debugAdapter/DebugClients/helper';
12+
import { DebugEnvironmentVariablesHelper, IDebugEnvironmentVariablesService } from '../../client/debugger/extension/configuration/resolvers/helper';
1313
import { ConsoleType, LaunchRequestArguments } from '../../client/debugger/types';
1414
import { isOs, OSType } from '../common';
1515
import { closeActiveWindows, initialize, initializeTest, IS_MULTI_ROOT_TEST, TEST_DEBUGGER } from '../initialize';
@@ -19,7 +19,7 @@ use(chaiAsPromised);
1919

2020
suite('Resolving Environment Variables when Debugging', () => {
2121
let ioc: UnitTestIocContainer;
22-
let helper: DebugClientHelper;
22+
let debugEnvParser: IDebugEnvironmentVariablesService;
2323
let pathVariableName: string;
2424
let mockProcess: ICurrentProcess;
2525

@@ -37,7 +37,7 @@ suite('Resolving Environment Variables when Debugging', () => {
3737
const envParser = ioc.serviceContainer.get<IEnvironmentVariablesService>(IEnvironmentVariablesService);
3838
const pathUtils = ioc.serviceContainer.get<IPathUtils>(IPathUtils);
3939
mockProcess = ioc.serviceContainer.get<ICurrentProcess>(ICurrentProcess);
40-
helper = new DebugClientHelper(envParser, pathUtils, mockProcess);
40+
debugEnvParser = new DebugEnvironmentVariablesHelper(envParser, pathUtils, mockProcess);
4141
pathVariableName = pathUtils.getPathVariableName();
4242
});
4343
suiteTeardown(closeActiveWindows);
@@ -60,7 +60,7 @@ suite('Resolving Environment Variables when Debugging', () => {
6060
// tslint:disable-next-line:no-any
6161
} as any as LaunchRequestArguments;
6262

63-
const envVars = await helper.getEnvironmentVariables(args);
63+
const envVars = await debugEnvParser.getEnvironmentVariables(args);
6464
expect(envVars).not.be.undefined;
6565
expect(Object.keys(envVars)).lengthOf(expectedNumberOfVariables, 'Incorrect number of variables');
6666
expect(envVars).to.have.property('PYTHONUNBUFFERED', '1', 'Property not found');
@@ -97,7 +97,7 @@ suite('Resolving Environment Variables when Debugging', () => {
9797
// tslint:disable-next-line:no-any
9898
} as any as LaunchRequestArguments;
9999

100-
const envVars = await helper.getEnvironmentVariables(args);
100+
const envVars = await debugEnvParser.getEnvironmentVariables(args);
101101

102102
// tslint:disable-next-line:no-unused-expression chai-vague-errors
103103
expect(envVars).not.be.undefined;
@@ -154,7 +154,7 @@ suite('Resolving Environment Variables when Debugging', () => {
154154
console, env
155155
} as any as LaunchRequestArguments;
156156

157-
const envVars = await helper.getEnvironmentVariables(args);
157+
const envVars = await debugEnvParser.getEnvironmentVariables(args);
158158
expect(envVars).not.be.undefined;
159159
expect(Object.keys(envVars)).lengthOf(expectedNumberOfVariables, 'Incorrect number of variables');
160160
expect(envVars).to.have.property('PYTHONPATH');

‎src/test/debugger/extension/configuration/resolvers/launch.unit.test.ts‎

Lines changed: 16 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -9,31 +9,27 @@ import { expect } from 'chai';
99
import * as path from 'path';
1010
import * as TypeMoq from 'typemoq';
1111
import { DebugConfiguration, DebugConfigurationProvider, TextDocument, TextEditor, Uri, WorkspaceFolder } from 'vscode';
12-
import { InvalidPythonPathInDebuggerServiceId } from '../../../../../client/application/diagnostics/checks/invalidPythonPathInDebugger';
13-
import { IDiagnosticsService, IInvalidPythonPathInDebuggerService } from '../../../../../client/application/diagnostics/types';
14-
import { IApplicationShell, IDocumentManager, IWorkspaceService } from '../../../../../client/common/application/types';
12+
import { IInvalidPythonPathInDebuggerService } from '../../../../../client/application/diagnostics/types';
13+
import { IDocumentManager, IWorkspaceService } from '../../../../../client/common/application/types';
1514
import { PYTHON_LANGUAGE } from '../../../../../client/common/constants';
16-
import { IFileSystem, IPlatformService } from '../../../../../client/common/platform/types';
15+
import { IPlatformService } from '../../../../../client/common/platform/types';
1716
import { IPythonExecutionFactory, IPythonExecutionService } from '../../../../../client/common/process/types';
18-
import { IConfigurationService, ILogger, IPythonSettings } from '../../../../../client/common/types';
17+
import { IConfigurationService, IPythonSettings } from '../../../../../client/common/types';
1918
import { DebuggerTypeName } from '../../../../../client/debugger/constants';
19+
import { IDebugEnvironmentVariablesService } from '../../../../../client/debugger/extension/configuration/resolvers/helper';
2020
import { LaunchConfigurationResolver } from '../../../../../client/debugger/extension/configuration/resolvers/launch';
2121
import { DebugOptions, LaunchRequestArguments } from '../../../../../client/debugger/types';
2222
import { IInterpreterHelper } from '../../../../../client/interpreter/contracts';
23-
import { IServiceContainer } from '../../../../../client/ioc/types';
2423

2524
suite('Debugging - Config Resolver Launch', () => {
26-
let serviceContainer: TypeMoq.IMock<IServiceContainer>;
2725
let debugProvider: DebugConfigurationProvider;
2826
let platformService: TypeMoq.IMock<IPlatformService>;
29-
let fileSystem: TypeMoq.IMock<IFileSystem>;
30-
let appShell: TypeMoq.IMock<IApplicationShell>;
3127
let pythonExecutionService: TypeMoq.IMock<IPythonExecutionService>;
32-
let logger: TypeMoq.IMock<ILogger>;
3328
let helper: TypeMoq.IMock<IInterpreterHelper>;
3429
let workspaceService: TypeMoq.IMock<IWorkspaceService>;
3530
let documentManager: TypeMoq.IMock<IDocumentManager>;
3631
let diagnosticsService: TypeMoq.IMock<IInvalidPythonPathInDebuggerService>;
32+
let debugEnvHelper: TypeMoq.IMock<IDebugEnvironmentVariablesService>;
3733
function createMoqWorkspaceFolder(folderPath: string) {
3834
const folder = TypeMoq.Mock.ofType<WorkspaceFolder>();
3935
folder.setup(f => f.uri).returns(() => Uri.file(folderPath));
@@ -43,13 +39,10 @@ suite('Debugging - Config Resolver Launch', () => {
4339
const confgService = TypeMoq.Mock.ofType<IConfigurationService>();
4440
workspaceService = TypeMoq.Mock.ofType<IWorkspaceService>();
4541
documentManager = TypeMoq.Mock.ofType<IDocumentManager>();
46-
serviceContainer = TypeMoq.Mock.ofType<IServiceContainer>();
4742

4843
platformService = TypeMoq.Mock.ofType<IPlatformService>();
49-
fileSystem = TypeMoq.Mock.ofType<IFileSystem>();
50-
appShell = TypeMoq.Mock.ofType<IApplicationShell>();
51-
logger = TypeMoq.Mock.ofType<ILogger>();
5244
diagnosticsService = TypeMoq.Mock.ofType<IInvalidPythonPathInDebuggerService>();
45+
debugEnvHelper = TypeMoq.Mock.ofType<IDebugEnvironmentVariablesService>();
5346

5447
pythonExecutionService = TypeMoq.Mock.ofType<IPythonExecutionService>();
5548
helper = TypeMoq.Mock.ofType<IInterpreterHelper>();
@@ -61,24 +54,22 @@ suite('Debugging - Config Resolver Launch', () => {
6154
.setup(h => h.validatePythonPath(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
6255
.returns(() => Promise.resolve(true));
6356

64-
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IPythonExecutionFactory))).returns(() => factory.object);
65-
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IConfigurationService))).returns(() => confgService.object);
66-
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IPlatformService))).returns(() => platformService.object);
67-
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IFileSystem))).returns(() => fileSystem.object);
68-
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IApplicationShell))).returns(() => appShell.object);
69-
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(ILogger))).returns(() => logger.object);
70-
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IInterpreterHelper))).returns(() => helper.object);
71-
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IDiagnosticsService), TypeMoq.It.isValue(InvalidPythonPathInDebuggerServiceId))).returns(() => diagnosticsService.object);
72-
7357
const settings = TypeMoq.Mock.ofType<IPythonSettings>();
7458
settings.setup(s => s.pythonPath).returns(() => pythonPath);
7559
if (workspaceFolder) {
7660
settings.setup(s => s.envFile).returns(() => path.join(workspaceFolder!.uri.fsPath, '.env2'));
7761
}
7862
confgService.setup(c => c.getSettings(TypeMoq.It.isAny())).returns(() => settings.object);
7963
setupOs(isWindows, isMac, isLinux);
80-
81-
debugProvider = new LaunchConfigurationResolver(workspaceService.object, documentManager.object, diagnosticsService.object, platformService.object, confgService.object);
64+
debugEnvHelper.setup(x => x.getEnvironmentVariables(TypeMoq.It.isAny())).returns(() => Promise.resolve({}));
65+
66+
debugProvider = new LaunchConfigurationResolver(
67+
workspaceService.object,
68+
documentManager.object,
69+
diagnosticsService.object,
70+
platformService.object,
71+
confgService.object,
72+
debugEnvHelper.object);
8273
}
8374
function setupActiveEditor(fileName: string | undefined, languageId: string) {
8475
if (fileName) {
@@ -91,12 +82,10 @@ suite('Debugging - Config Resolver Launch', () => {
9182
} else {
9283
documentManager.setup(d => d.activeTextEditor).returns(() => undefined);
9384
}
94-
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IDocumentManager))).returns(() => documentManager.object);
9585
}
9686
function setupWorkspaces(folders: string[]) {
9787
const workspaceFolders = folders.map(createMoqWorkspaceFolder);
9888
workspaceService.setup(w => w.workspaceFolders).returns(() => workspaceFolders);
99-
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IWorkspaceService))).returns(() => workspaceService.object);
10089
}
10190
function setupOs(isWindows: boolean, isMac: boolean, isLinux: boolean) {
10291
platformService.setup(p => p.isWindows).returns(() => isWindows);

0 commit comments

Comments
 (0)