Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add proposed api to get custom environment variables for a workspace #19999

Merged
merged 5 commits into from
Oct 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/client/common/variables/environment.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { pathExistsSync, readFileSync } from 'fs-extra';
import { inject, injectable } from 'inversify';
import * as path from 'path';
import { traceError } from '../../logging';
Expand Down Expand Up @@ -36,6 +37,22 @@ export class EnvironmentVariablesService implements IEnvironmentVariablesService
return parseEnvFile(contents, baseVars);
}

public parseFileSync(filePath?: string, baseVars?: EnvironmentVariables): EnvironmentVariables | undefined {
if (!filePath || !pathExistsSync(filePath)) {
return;
}
let contents: string | undefined;
try {
contents = readFileSync(filePath, { encoding: 'utf8' });
} catch (ex) {
traceError('Custom .env is likely not pointing to a valid file', ex);
}
if (!contents) {
return;
}
return parseEnvFile(contents, baseVars);
}

public mergeVariables(
source: EnvironmentVariables,
target: EnvironmentVariables,
Expand Down
59 changes: 46 additions & 13 deletions src/client/common/variables/environmentVariablesProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

import { inject, injectable, optional } from 'inversify';
import { ConfigurationChangeEvent, Disposable, Event, EventEmitter, FileSystemWatcher, Uri } from 'vscode';
import { traceVerbose } from '../../logging';
import { sendFileCreationTelemetry } from '../../telemetry/envFileTelemetry';
import { IWorkspaceService } from '../application/types';
import { PythonSettings } from '../configSettings';
Expand Down Expand Up @@ -54,29 +53,55 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid
}

public async getEnvironmentVariables(resource?: Uri): Promise<EnvironmentVariables> {
const cacheKey = this.getWorkspaceFolderUri(resource)?.fsPath ?? '';
let cache = this.envVarCaches.get(cacheKey);
const cached = this.getCachedEnvironmentVariables(resource);
if (cached) {
return cached;
}
const vars = await this._getEnvironmentVariables(resource);
this.setCachedEnvironmentVariables(resource, vars);
return vars;
}

public getEnvironmentVariablesSync(resource?: Uri): EnvironmentVariables {
const cached = this.getCachedEnvironmentVariables(resource);
if (cached) {
return cached;
}
const vars = this._getEnvironmentVariablesSync(resource);
this.setCachedEnvironmentVariables(resource, vars);
return vars;
}

private getCachedEnvironmentVariables(resource?: Uri): EnvironmentVariables | undefined {
const cacheKey = this.getWorkspaceFolderUri(resource)?.fsPath ?? '';
const cache = this.envVarCaches.get(cacheKey);
if (cache) {
const cachedData = cache.data;
if (cachedData) {
traceVerbose(
`Cached data exists getEnvironmentVariables, ${resource ? resource.fsPath : '<No Resource>'}`,
);
return { ...cachedData };
}
} else {
cache = new InMemoryCache(this.cacheDuration);
this.envVarCaches.set(cacheKey, cache);
}
return undefined;
}

const vars = await this._getEnvironmentVariables(resource);
private setCachedEnvironmentVariables(resource: Uri | undefined, vars: EnvironmentVariables): void {
const cacheKey = this.getWorkspaceFolderUri(resource)?.fsPath ?? '';
const cache = new InMemoryCache<EnvironmentVariables>(this.cacheDuration);
this.envVarCaches.set(cacheKey, cache);
cache.data = { ...vars };
return vars;
}

public async _getEnvironmentVariables(resource?: Uri): Promise<EnvironmentVariables> {
let mergedVars = await this.getCustomEnvironmentVariables(resource);
const customVars = await this.getCustomEnvironmentVariables(resource);
return this.getMergedEnvironmentVariables(customVars);
}

public _getEnvironmentVariablesSync(resource?: Uri): EnvironmentVariables {
const customVars = this.getCustomEnvironmentVariablesSync(resource);
return this.getMergedEnvironmentVariables(customVars);
}

private getMergedEnvironmentVariables(mergedVars?: EnvironmentVariables): EnvironmentVariables {
if (!mergedVars) {
mergedVars = {};
}
Expand All @@ -93,6 +118,14 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid
}

public async getCustomEnvironmentVariables(resource?: Uri): Promise<EnvironmentVariables | undefined> {
return this.envVarsService.parseFile(this.getEnvFile(resource), this.process.env);
}

private getCustomEnvironmentVariablesSync(resource?: Uri): EnvironmentVariables | undefined {
return this.envVarsService.parseFileSync(this.getEnvFile(resource), this.process.env);
}

private getEnvFile(resource?: Uri): string {
const systemVariables: SystemVariables = new SystemVariables(
undefined,
PythonSettings.getSettingsUriAndTarget(resource, this.workspaceService).uri?.fsPath,
Expand All @@ -103,7 +136,7 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid
const workspaceFolderUri = this.getWorkspaceFolderUri(resource);
this.trackedWorkspaceFolders.add(workspaceFolderUri ? workspaceFolderUri.fsPath : '');
this.createFileWatcher(envFile, workspaceFolderUri);
return this.envVarsService.parseFile(envFile, this.process.env);
return envFile;
}

public configurationChanged(e: ConfigurationChangeEvent): void {
Expand Down
3 changes: 2 additions & 1 deletion src/client/common/variables/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export const IEnvironmentVariablesService = Symbol('IEnvironmentVariablesService

export interface IEnvironmentVariablesService {
parseFile(filePath?: string, baseVars?: EnvironmentVariables): Promise<EnvironmentVariables | undefined>;
parseFileSync(filePath?: string, baseVars?: EnvironmentVariables): EnvironmentVariables | undefined;
mergeVariables(source: EnvironmentVariables, target: EnvironmentVariables, options?: { overwrite?: boolean }): void;
appendPythonPath(vars: EnvironmentVariables, ...pythonPaths: string[]): void;
appendPath(vars: EnvironmentVariables, ...paths: string[]): void;
Expand Down Expand Up @@ -38,5 +39,5 @@ export const IEnvironmentVariablesProvider = Symbol('IEnvironmentVariablesProvid
export interface IEnvironmentVariablesProvider {
onDidEnvironmentVariablesChange: Event<Uri | undefined>;
getEnvironmentVariables(resource?: Uri): Promise<EnvironmentVariables>;
getCustomEnvironmentVariables(resource?: Uri): Promise<EnvironmentVariables | undefined>;
getEnvironmentVariablesSync(resource?: Uri): EnvironmentVariables;
}
22 changes: 22 additions & 0 deletions src/client/proposedApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
EnvironmentType,
EnvironmentTools,
EnvironmentPath,
EnvironmentVariablesChangeEvent,
} from './proposedApiTypes';
import { PythonEnvInfo, PythonEnvKind, PythonEnvType } from './pythonEnvironments/base/info';
import { getEnvPath } from './pythonEnvironments/base/info/env';
Expand All @@ -33,6 +34,8 @@ import {
reportInterpretersChanged,
} from './deprecatedProposedApi';
import { DeprecatedProposedAPI } from './deprecatedProposedApiTypes';
import { IEnvironmentVariablesProvider } from './common/variables/types';
import { IWorkspaceService } from './common/application/types';

type ActiveEnvironmentChangeEvent = {
resource: WorkspaceFolder | undefined;
Expand All @@ -46,6 +49,7 @@ export function reportActiveInterpreterChanged(e: ActiveEnvironmentChangeEvent):
}

const onEnvironmentsChanged = new EventEmitter<EnvironmentsChangeEvent>();
const onEnvironmentVariablesChanged = new EventEmitter<EnvironmentVariablesChangeEvent>();
const environmentsReference = new Map<string, EnvironmentReference>();

/**
Expand Down Expand Up @@ -106,6 +110,8 @@ export function buildProposedApi(
const configService = serviceContainer.get<IConfigurationService>(IConfigurationService);
const disposables = serviceContainer.get<IDisposableRegistry>(IDisposableRegistry);
const extensions = serviceContainer.get<IExtensions>(IExtensions);
const workspaceService = serviceContainer.get<IWorkspaceService>(IWorkspaceService);
const envVarsProvider = serviceContainer.get<IEnvironmentVariablesProvider>(IEnvironmentVariablesProvider);
function sendApiTelemetry(apiName: string) {
extensions
.determineExtensionFromCallStack()
Expand Down Expand Up @@ -148,7 +154,14 @@ export function buildProposedApi(
]);
}
}),
envVarsProvider.onDidEnvironmentVariablesChange((e) => {
onEnvironmentVariablesChanged.fire({
resource: workspaceService.getWorkspaceFolder(e),
env: envVarsProvider.getEnvironmentVariablesSync(e),
});
}),
onEnvironmentsChanged,
onEnvironmentVariablesChanged,
);

/**
Expand All @@ -166,6 +179,15 @@ export function buildProposedApi(
const proposed: ProposedExtensionAPI & DeprecatedProposedAPI = {
...deprecatedProposedApi,
environments: {
getEnvironmentVariables: (resource?: Resource) => {
sendApiTelemetry('getEnvironmentVariables');
resource = resource && 'uri' in resource ? resource.uri : resource;
return envVarsProvider.getEnvironmentVariablesSync(resource);
},
get onDidEnvironmentVariablesChange() {
sendApiTelemetry('onDidEnvironmentVariablesChange');
return onEnvironmentVariablesChanged.event;
},
getActiveEnvironmentPath(resource?: Resource) {
sendApiTelemetry('getActiveEnvironmentPath');
resource = resource && 'uri' in resource ? resource.uri : resource;
Expand Down
29 changes: 29 additions & 0 deletions src/client/proposedApiTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,19 @@ export interface ProposedExtensionAPI {
resolveEnvironment(
environment: Environment | EnvironmentPath | string,
): Promise<ResolvedEnvironment | undefined>;
/**
* Returns the environment variables used by the extension for a resource, which includes the custom
* variables configured by user in `.env` files.
* @param resource : Uri of a file or workspace folder. This is used to determine the env in a multi-root
* scenario. If `undefined`, then the API returns what ever is set for the workspace.
*/
getEnvironmentVariables(resource?: Resource): EnvironmentVariables;
/**
* This event is fired when the environment variables for a resource change. Note it's currently not
* possible to detect if environment variables in the system change, so this only fires if custom
* environment variables are updated in `.env` files.
*/
readonly onDidEnvironmentVariablesChange: Event<EnvironmentVariablesChangeEvent>;
};
}

Expand Down Expand Up @@ -263,3 +276,19 @@ export type ResolvedVersionInfo = {
readonly micro: number;
readonly release: PythonVersionRelease;
};

/**
* A record containing readonly keys.
*/
export type EnvironmentVariables = { readonly [key: string]: string | undefined };

export type EnvironmentVariablesChangeEvent = {
/**
* Workspace folder the environment variables changed for.
*/
readonly resource: WorkspaceFolder | undefined;
/**
* Updated value of environment variables.
*/
readonly env: EnvironmentVariables;
};
54 changes: 54 additions & 0 deletions src/test/proposedApi.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,24 +30,32 @@ import { normCasePath } from '../client/common/platform/fs-paths';
import {
ActiveEnvironmentPathChangeEvent,
EnvironmentsChangeEvent,
EnvironmentVariablesChangeEvent,
ProposedExtensionAPI,
} from '../client/proposedApiTypes';
import { IWorkspaceService } from '../client/common/application/types';
import { IEnvironmentVariablesProvider } from '../client/common/variables/types';

suite('Proposed Extension API', () => {
let serviceContainer: typemoq.IMock<IServiceContainer>;
let discoverAPI: typemoq.IMock<IDiscoveryAPI>;
let interpreterPathService: typemoq.IMock<IInterpreterPathService>;
let configService: typemoq.IMock<IConfigurationService>;
let extensions: typemoq.IMock<IExtensions>;
let workspaceService: typemoq.IMock<IWorkspaceService>;
let envVarsProvider: typemoq.IMock<IEnvironmentVariablesProvider>;
let onDidChangeRefreshState: EventEmitter<ProgressNotificationEvent>;
let onDidChangeEnvironments: EventEmitter<PythonEnvCollectionChangedEvent>;
let onDidChangeEnvironmentVariables: EventEmitter<Uri | undefined>;

let proposed: ProposedExtensionAPI;

setup(() => {
serviceContainer = typemoq.Mock.ofType<IServiceContainer>();
discoverAPI = typemoq.Mock.ofType<IDiscoveryAPI>();
extensions = typemoq.Mock.ofType<IExtensions>();
workspaceService = typemoq.Mock.ofType<IWorkspaceService>();
envVarsProvider = typemoq.Mock.ofType<IEnvironmentVariablesProvider>();
extensions
.setup((e) => e.determineExtensionFromCallStack())
.returns(() => Promise.resolve({ extensionId: 'id', displayName: 'displayName', apiName: 'apiName' }))
Expand All @@ -56,10 +64,16 @@ suite('Proposed Extension API', () => {
configService = typemoq.Mock.ofType<IConfigurationService>();
onDidChangeRefreshState = new EventEmitter();
onDidChangeEnvironments = new EventEmitter();
onDidChangeEnvironmentVariables = new EventEmitter();

serviceContainer.setup((s) => s.get(IExtensions)).returns(() => extensions.object);
serviceContainer.setup((s) => s.get(IInterpreterPathService)).returns(() => interpreterPathService.object);
serviceContainer.setup((s) => s.get(IConfigurationService)).returns(() => configService.object);
serviceContainer.setup((s) => s.get(IWorkspaceService)).returns(() => workspaceService.object);
serviceContainer.setup((s) => s.get(IEnvironmentVariablesProvider)).returns(() => envVarsProvider.object);
envVarsProvider
.setup((e) => e.onDidEnvironmentVariablesChange)
.returns(() => onDidChangeEnvironmentVariables.event);
serviceContainer.setup((s) => s.get(IDisposableRegistry)).returns(() => []);

discoverAPI.setup((d) => d.onProgress).returns(() => onDidChangeRefreshState.event);
Expand All @@ -73,6 +87,46 @@ suite('Proposed Extension API', () => {
extensions.verifyAll();
});

test('Provide an event to track when environment variables change', async () => {
const resource = Uri.file('x');
const folder = ({ uri: resource } as unknown) as WorkspaceFolder;
const envVars = { PATH: 'path' };
envVarsProvider.setup((e) => e.getEnvironmentVariablesSync(resource)).returns(() => envVars);
workspaceService.setup((w) => w.getWorkspaceFolder(resource)).returns(() => folder);
const events: EnvironmentVariablesChangeEvent[] = [];
proposed.environments.onDidEnvironmentVariablesChange((e) => {
events.push(e);
});
onDidChangeEnvironmentVariables.fire(resource);
await sleep(1);
assert.deepEqual(events, [{ env: envVars, resource: folder }]);
});

test('getEnvironmentVariables: No resource', async () => {
const resource = undefined;
const envVars = { PATH: 'path' };
envVarsProvider.setup((e) => e.getEnvironmentVariablesSync(resource)).returns(() => envVars);
const vars = proposed.environments.getEnvironmentVariables(resource);
assert.deepEqual(vars, envVars);
});

test('getEnvironmentVariables: With Uri resource', async () => {
const resource = Uri.file('x');
const envVars = { PATH: 'path' };
envVarsProvider.setup((e) => e.getEnvironmentVariablesSync(resource)).returns(() => envVars);
const vars = proposed.environments.getEnvironmentVariables(resource);
assert.deepEqual(vars, envVars);
});

test('getEnvironmentVariables: With WorkspaceFolder resource', async () => {
const resource = Uri.file('x');
const folder = ({ uri: resource } as unknown) as WorkspaceFolder;
const envVars = { PATH: 'path' };
envVarsProvider.setup((e) => e.getEnvironmentVariablesSync(resource)).returns(() => envVars);
const vars = proposed.environments.getEnvironmentVariables(folder);
assert.deepEqual(vars, envVars);
});

test('Provide an event to track when active environment details change', async () => {
const events: ActiveEnvironmentPathChangeEvent[] = [];
proposed.environments.onDidChangeActiveEnvironmentPath((e) => {
Expand Down