diff --git a/src/client/common/variables/environmentVariablesProvider.ts b/src/client/common/variables/environmentVariablesProvider.ts index 0a6b7a962f4a..20ddff4b9fc0 100644 --- a/src/client/common/variables/environmentVariablesProvider.ts +++ b/src/client/common/variables/environmentVariablesProvider.ts @@ -5,10 +5,12 @@ import { inject, injectable, optional } from 'inversify'; import { ConfigurationChangeEvent, Disposable, Event, EventEmitter, FileSystemWatcher, Uri } from 'vscode'; import { sendFileCreationTelemetry } from '../../telemetry/envFileTelemetry'; import { IWorkspaceService } from '../application/types'; +import { PythonSettings } from '../configSettings'; import { traceVerbose } from '../logger'; import { IPlatformService } from '../platform/types'; -import { IConfigurationService, ICurrentProcess, IDisposableRegistry } from '../types'; +import { ICurrentProcess, IDisposableRegistry } from '../types'; import { InMemoryCache } from '../utils/cacheUtils'; +import { SystemVariables } from './systemVariables'; import { EnvironmentVariables, IEnvironmentVariablesProvider, IEnvironmentVariablesService } from './types'; const CACHE_DURATION = 60 * 60 * 1000; @@ -25,7 +27,6 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid @inject(IDisposableRegistry) disposableRegistry: Disposable[], @inject(IPlatformService) private platformService: IPlatformService, @inject(IWorkspaceService) private workspaceService: IWorkspaceService, - @inject(IConfigurationService) private readonly configurationService: IConfigurationService, @inject(ICurrentProcess) private process: ICurrentProcess, @optional() private cacheDuration: number = CACHE_DURATION, ) { @@ -86,11 +87,17 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid return mergedVars; } public async getCustomEnvironmentVariables(resource?: Uri): Promise { - const settings = this.configurationService.getSettings(resource); + const systemVariables: SystemVariables = new SystemVariables( + undefined, + PythonSettings.getSettingsUriAndTarget(resource, this.workspaceService).uri?.fsPath, + this.workspaceService, + ); + const envFileSetting = this.workspaceService.getConfiguration('python', resource).get('envFile'); + const envFile = systemVariables.resolveAny(envFileSetting)!; const workspaceFolderUri = this.getWorkspaceFolderUri(resource); this.trackedWorkspaceFolders.add(workspaceFolderUri ? workspaceFolderUri.fsPath : ''); - this.createFileWatcher(settings.envFile, workspaceFolderUri); - return this.envVarsService.parseFile(settings.envFile, this.process.env); + this.createFileWatcher(envFile, workspaceFolderUri); + return this.envVarsService.parseFile(envFile, this.process.env); } public configurationChanged(e: ConfigurationChangeEvent) { this.trackedWorkspaceFolders.forEach((item) => { diff --git a/src/client/extensionActivation.ts b/src/client/extensionActivation.ts index b0d0686de25c..816a0d400333 100644 --- a/src/client/extensionActivation.ts +++ b/src/client/extensionActivation.ts @@ -30,7 +30,6 @@ import { IOutputChannel, } from './common/types'; import { noop } from './common/utils/misc'; -import { registerTypes as variableRegisterTypes } from './common/variables/serviceRegistry'; import { DebuggerTypeName } from './debugger/constants'; import { DebugSessionEventDispatcher } from './debugger/extension/hooks/eventHandlerDispatcher'; import { IDebugSessionEventHandlers } from './debugger/extension/hooks/types'; @@ -112,7 +111,6 @@ async function activateLegacy(ext: ExtensionState): Promise { const { enableProposedApi } = applicationEnv.packageJson; serviceManager.addSingletonInstance(UseProposedApi, enableProposedApi); // Feature specific registrations. - variableRegisterTypes(serviceManager); unitTestsRegisterTypes(serviceManager); lintersRegisterTypes(serviceManager); interpretersRegisterTypes(serviceManager); diff --git a/src/client/extensionInit.ts b/src/client/extensionInit.ts index 60caffbb1617..1f825d9210b6 100644 --- a/src/client/extensionInit.ts +++ b/src/client/extensionInit.ts @@ -17,6 +17,7 @@ import { IOutputChannel, WORKSPACE_MEMENTO, } from './common/types'; +import { registerTypes as variableRegisterTypes } from './common/variables/serviceRegistry'; import { OutputChannelNames } from './common/utils/localize'; import { ExtensionState } from './components'; import { ServiceContainer } from './ioc/container'; @@ -69,6 +70,7 @@ export function initializeStandard(ext: ExtensionState): void { const { serviceManager } = ext.legacyIOC; // Core registrations (non-feature specific). commonRegisterTypes(serviceManager); + variableRegisterTypes(serviceManager); platformRegisterTypes(serviceManager); processRegisterTypes(serviceManager); diff --git a/src/test/common/variables/envVarsProvider.multiroot.test.ts b/src/test/common/variables/envVarsProvider.multiroot.test.ts index 56509cd4761a..a62dff0cb8af 100644 --- a/src/test/common/variables/envVarsProvider.multiroot.test.ts +++ b/src/test/common/variables/envVarsProvider.multiroot.test.ts @@ -7,7 +7,6 @@ import * as path from 'path'; import { anything, instance, mock, when } from 'ts-mockito'; import { ConfigurationTarget, Disposable, Uri, workspace } from 'vscode'; import { WorkspaceService } from '../../../client/common/application/workspace'; -import { ConfigurationService } from '../../../client/common/configuration/service'; import { PlatformService } from '../../../client/common/platform/platformService'; import { IFileSystem } from '../../../client/common/platform/types'; import { IDisposableRegistry, IPathUtils } from '../../../client/common/types'; @@ -76,14 +75,12 @@ suite('Multiroot Environment Variables Provider', () => { const variablesService = new EnvironmentVariablesService(pathUtils, fs); const disposables = ioc.serviceContainer.get(IDisposableRegistry); ioc.serviceManager.addSingletonInstance(IInterpreterAutoSelectionService, new MockAutoSelectionService()); - const cfgService = new ConfigurationService(ioc.serviceContainer); const workspaceService = new WorkspaceService(); return new EnvironmentVariablesProvider( variablesService, disposables, new PlatformService(), workspaceService, - cfgService, mockProcess, ); } diff --git a/src/test/common/variables/environmentVariablesProvider.unit.test.ts b/src/test/common/variables/environmentVariablesProvider.unit.test.ts index 44cfabe35c62..f55d128e48fc 100644 --- a/src/test/common/variables/environmentVariablesProvider.unit.test.ts +++ b/src/test/common/variables/environmentVariablesProvider.unit.test.ts @@ -11,12 +11,10 @@ import * as typemoq from 'typemoq'; import { ConfigurationChangeEvent, FileSystemWatcher, Uri } from 'vscode'; import { IWorkspaceService } from '../../../client/common/application/types'; import { WorkspaceService } from '../../../client/common/application/workspace'; -import { PythonSettings } from '../../../client/common/configSettings'; -import { ConfigurationService } from '../../../client/common/configuration/service'; import { PlatformService } from '../../../client/common/platform/platformService'; import { IPlatformService } from '../../../client/common/platform/types'; import { CurrentProcess } from '../../../client/common/process/currentProcess'; -import { IConfigurationService, ICurrentProcess, IPythonSettings } from '../../../client/common/types'; +import { ICurrentProcess } from '../../../client/common/types'; import { sleep } from '../../../client/common/utils/async'; import { EnvironmentVariablesService } from '../../../client/common/variables/environment'; import { EnvironmentVariablesProvider } from '../../../client/common/variables/environmentVariablesProvider'; @@ -29,26 +27,27 @@ suite('Multiroot Environment Variables Provider', () => { let envVarsService: IEnvironmentVariablesService; let platform: IPlatformService; let workspace: IWorkspaceService; - let configuration: IConfigurationService; let currentProcess: ICurrentProcess; - let settings: IPythonSettings; + let envFile: string; setup(() => { + envFile = ''; envVarsService = mock(EnvironmentVariablesService); platform = mock(PlatformService); workspace = mock(WorkspaceService); - configuration = mock(ConfigurationService); currentProcess = mock(CurrentProcess); - settings = mock(PythonSettings); - when(configuration.getSettings(anything())).thenReturn(instance(settings)); when(workspace.onDidChangeConfiguration).thenReturn(noop as any); + when(workspace.getConfiguration('python', anything())).thenReturn({ + get: (settingName: string) => { + return settingName === 'envFile' ? envFile : ''; + }, + } as any); provider = new EnvironmentVariablesProvider( instance(envVarsService), [], instance(platform), instance(workspace), - instance(configuration), instance(currentProcess), ); @@ -198,12 +197,11 @@ suite('Multiroot Environment Variables Provider', () => { }); test(`Getting environment variables (without an envfile, without PATH in current env, without PYTHONPATH in current env) & ${workspaceTitle}`, async () => { - const envFile = path.join('a', 'b', 'env.file'); + envFile = path.join('a', 'b', 'env.file'); const workspaceFolder = workspaceUri ? { name: '', index: 0, uri: workspaceUri } : undefined; const currentProcEnv = { SOMETHING: 'wow' }; when(currentProcess.env).thenReturn(currentProcEnv); - when(settings.envFile).thenReturn(envFile); when(workspace.getWorkspaceFolder(workspaceUri)).thenReturn(workspaceFolder); when(envVarsService.parseFile(envFile, currentProcEnv)).thenResolve(undefined); when(platform.pathVariableName).thenReturn('PATH'); @@ -211,20 +209,18 @@ suite('Multiroot Environment Variables Provider', () => { const vars = await provider.getEnvironmentVariables(workspaceUri); verify(currentProcess.env).atLeast(1); - verify(settings.envFile).atLeast(1); verify(envVarsService.parseFile(envFile, currentProcEnv)).atLeast(1); verify(envVarsService.mergeVariables(deepEqual(currentProcEnv), deepEqual({}))).once(); verify(platform.pathVariableName).atLeast(1); assert.deepEqual(vars, {}); }); test(`Getting environment variables (with an envfile, without PATH in current env, without PYTHONPATH in current env) & ${workspaceTitle}`, async () => { - const envFile = path.join('a', 'b', 'env.file'); + envFile = path.join('a', 'b', 'env.file'); const workspaceFolder = workspaceUri ? { name: '', index: 0, uri: workspaceUri } : undefined; const currentProcEnv = { SOMETHING: 'wow' }; const envFileVars = { MY_FILE: '1234' }; when(currentProcess.env).thenReturn(currentProcEnv); - when(settings.envFile).thenReturn(envFile); when(workspace.getWorkspaceFolder(workspaceUri)).thenReturn(workspaceFolder); when(envVarsService.parseFile(envFile, currentProcEnv)).thenCall(async () => ({ ...envFileVars })); when(platform.pathVariableName).thenReturn('PATH'); @@ -232,20 +228,18 @@ suite('Multiroot Environment Variables Provider', () => { const vars = await provider.getEnvironmentVariables(workspaceUri); verify(currentProcess.env).atLeast(1); - verify(settings.envFile).atLeast(1); verify(envVarsService.parseFile(envFile, currentProcEnv)).atLeast(1); verify(envVarsService.mergeVariables(deepEqual(currentProcEnv), deepEqual(envFileVars))).once(); verify(platform.pathVariableName).atLeast(1); assert.deepEqual(vars, envFileVars); }); test(`Getting environment variables (with an envfile, with PATH in current env, with PYTHONPATH in current env) & ${workspaceTitle}`, async () => { - const envFile = path.join('a', 'b', 'env.file'); + envFile = path.join('a', 'b', 'env.file'); const workspaceFolder = workspaceUri ? { name: '', index: 0, uri: workspaceUri } : undefined; const currentProcEnv = { SOMETHING: 'wow', PATH: 'some path value', PYTHONPATH: 'some python path value' }; const envFileVars = { MY_FILE: '1234' }; when(currentProcess.env).thenReturn(currentProcEnv); - when(settings.envFile).thenReturn(envFile); when(workspace.getWorkspaceFolder(workspaceUri)).thenReturn(workspaceFolder); when(envVarsService.parseFile(envFile, currentProcEnv)).thenCall(async () => ({ ...envFileVars })); when(platform.pathVariableName).thenReturn('PATH'); @@ -253,7 +247,6 @@ suite('Multiroot Environment Variables Provider', () => { const vars = await provider.getEnvironmentVariables(workspaceUri); verify(currentProcess.env).atLeast(1); - verify(settings.envFile).atLeast(1); verify(envVarsService.parseFile(envFile, currentProcEnv)).atLeast(1); verify(envVarsService.mergeVariables(deepEqual(currentProcEnv), deepEqual(envFileVars))).once(); verify(envVarsService.appendPath(deepEqual(envFileVars), currentProcEnv.PATH)).once(); @@ -263,12 +256,11 @@ suite('Multiroot Environment Variables Provider', () => { }); test(`Getting environment variables which are already cached does not reinvoke the method ${workspaceTitle}`, async () => { - const envFile = path.join('a', 'b', 'env.file'); + envFile = path.join('a', 'b', 'env.file'); const workspaceFolder = workspaceUri ? { name: '', index: 0, uri: workspaceUri } : undefined; const currentProcEnv = { SOMETHING: 'wow' }; when(currentProcess.env).thenReturn(currentProcEnv); - when(settings.envFile).thenReturn(envFile); when(workspace.getWorkspaceFolder(workspaceUri)).thenReturn(workspaceFolder); when(envVarsService.parseFile(envFile, currentProcEnv)).thenResolve(undefined); when(platform.pathVariableName).thenReturn('PATH'); @@ -280,7 +272,7 @@ suite('Multiroot Environment Variables Provider', () => { await provider.getEnvironmentVariables(workspaceUri); // Verify that the contents of `_getEnvironmentVariables()` method are only invoked once - verify(configuration.getSettings(anything())).once(); + verify(workspace.getConfiguration('python', anything())).once(); assert.deepEqual(vars, {}); }); @@ -290,7 +282,6 @@ suite('Multiroot Environment Variables Provider', () => { const currentProcEnv = { SOMETHING: 'wow' }; when(currentProcess.env).thenReturn(currentProcEnv); - when(settings.envFile).thenReturn(envFile); when(workspace.getWorkspaceFolder(workspaceUri)).thenReturn(workspaceFolder); when(envVarsService.parseFile(envFile, currentProcEnv)).thenResolve(undefined); when(platform.pathVariableName).thenReturn('PATH'); @@ -300,7 +291,6 @@ suite('Multiroot Environment Variables Provider', () => { [], instance(platform), instance(workspace), - instance(configuration), instance(currentProcess), 100, ); @@ -312,14 +302,14 @@ suite('Multiroot Environment Variables Provider', () => { await provider.getEnvironmentVariables(workspaceUri); // Verify that the contents of `_getEnvironmentVariables()` method are invoked twice - verify(configuration.getSettings(anything())).twice(); + verify(workspace.getConfiguration('python', anything())).twice(); assert.deepEqual(vars, {}); }); test(`Environment variables are updated when env file changes ${workspaceTitle}`, async () => { const root = workspaceUri?.fsPath ?? ''; const sourceDir = path.join(root, 'a', 'b'); - const envFile = path.join(sourceDir, 'env.file'); + envFile = path.join(sourceDir, 'env.file'); const sourceFile = path.join(sourceDir, 'main.py'); const workspaceFolder = workspaceUri ? { name: '', index: 0, uri: workspaceUri } : undefined; @@ -339,7 +329,6 @@ suite('Multiroot Environment Variables Provider', () => { when(workspace.createFileSystemWatcher(envFile)).thenReturn(fileSystemWatcher.object); when(currentProcess.env).thenReturn(currentProcEnv); - when(settings.envFile).thenReturn(envFile); when(workspace.getWorkspaceFolder(anything())).thenReturn(workspaceFolder); when(envVarsService.parseFile(envFile, currentProcEnv)).thenCall(async () => ({ ...envFileVars })); when(platform.pathVariableName).thenReturn('PATH');