Point release with fix for Issue 10250 (microsoft#10254)

* Fix to return env variables of interpreter that is not current interpreter (microsoft#10251) For #10250 * Bug fix * Update change log * Fix date * Oops * Fix linter

Author: DonJayamanne

CHANGELOG.md

@@ -1,5 +1,65 @@
 # Changelog
 
+## 2020.2.3 (21 February 2020)
+
+### Fixes
+
+1. Ensure to correctly return env variables of the activated interpreter, when dealing with non-workspace interpreters.
+ ([#10250](https://github.com/Microsoft/vscode-python/issues/10250))
+
+### Thanks
+
+Thanks to the following projects which we fully rely on to provide some of
+our features:
+
+- [isort](https://pypi.org/project/isort/)
+- [jedi](https://pypi.org/project/jedi/)
+ and [parso](https://pypi.org/project/parso/)
+- [Microsoft Python Language Server](https://github.com/microsoft/python-language-server)
+- [ptvsd](https://pypi.org/project/ptvsd/)
+- [exuberant ctags](http://ctags.sourceforge.net/) (user-installed)
+- [rope](https://pypi.org/project/rope/) (user-installed)
+
+Also thanks to the various projects we provide integrations with which help
+make this extension useful:
+
+- Debugging support:
+ [Django](https://pypi.org/project/Django/),
+ [Flask](https://pypi.org/project/Flask/),
+ [gevent](https://pypi.org/project/gevent/),
+ [Jinja](https://pypi.org/project/Jinja/),
+ [Pyramid](https://pypi.org/project/pyramid/),
+ [PySpark](https://pypi.org/project/pyspark/),
+ [Scrapy](https://pypi.org/project/Scrapy/),
+ [Watson](https://pypi.org/project/Watson/)
+- Formatting:
+ [autopep8](https://pypi.org/project/autopep8/),
+ [black](https://pypi.org/project/black/),
+ [yapf](https://pypi.org/project/yapf/)
+- Interpreter support:
+ [conda](https://conda.io/),
+ [direnv](https://direnv.net/),
+ [pipenv](https://pypi.org/project/pipenv/),
+ [pyenv](https://github.com/pyenv/pyenv),
+ [venv](https://docs.python.org/3/library/venv.html#module-venv),
+ [virtualenv](https://pypi.org/project/virtualenv/)
+- Linting:
+ [bandit](https://pypi.org/project/bandit/),
+ [flake8](https://pypi.org/project/flake8/),
+ [mypy](https://pypi.org/project/mypy/),
+ [prospector](https://pypi.org/project/prospector/),
+ [pylint](https://pypi.org/project/pylint/),
+ [pydocstyle](https://pypi.org/project/pydocstyle/),
+ [pylama](https://pypi.org/project/pylama/)
+- Testing:
+ [nose](https://pypi.org/project/nose/),
+ [pytest](https://pypi.org/project/pytest/),
+ [unittest](https://docs.python.org/3/library/unittest.html#module-unittest)
+
+And finally thanks to the [Python](https://www.python.org/) development team and
+community for creating a fantastic programming language and community to be a
+part of!
+
 ## 2020.2.2 (19 February 2020)
 
 ### Fixes

package.json

@@ -2,7 +2,7 @@
  "name": "python",
  "displayName": "Python",
  "description": "Linting, Debugging (multi-threaded, remote), Intellisense, Jupyter Notebooks, code formatting, refactoring, unit tests, snippets, and more.",
- "version": "2020.2.2",
+ "version": "2020.2.3",
  "languageServerVersion": "0.5.30",
  "publisher": "ms-python",
  "author": {

src/client/common/logger.ts

@@ -285,7 +285,7 @@ function trace(message: string, options: LogOptions = LogOptions.None, logLevel?
  // tslint:disable-next-line:no-any
  function writeToLog(elapsedTime: number, returnValue?: any, ex?: Error) {
  const messagesToLog = [message];
- messagesToLog.push(`Class name = ${className}, completed in ${elapsedTime}ms`);
+ messagesToLog.push(`Class name = ${className}, completed in ${elapsedTime}ms, has a ${returnValue ? 'truthy' : 'falsy'} return value`);
  if ((options && LogOptions.Arguments) === LogOptions.Arguments) {
  messagesToLog.push(argsToLogString(args));
  }

src/client/interpreter/activation/service.ts

@@ -6,19 +6,20 @@ import '../../common/extensions';
 import { inject, injectable } from 'inversify';
 import * as path from 'path';
 
+import { IWorkspaceService } from '../../common/application/types';
 import { PYTHON_WARNINGS } from '../../common/constants';
 import { LogOptions, traceDecorators, traceError, traceVerbose } from '../../common/logger';
 import { IPlatformService } from '../../common/platform/types';
 import { IProcessServiceFactory } from '../../common/process/types';
 import { ITerminalHelper, TerminalShellType } from '../../common/terminal/types';
 import { ICurrentProcess, IDisposable, Resource } from '../../common/types';
-import { cacheResourceSpecificInterpreterData, clearCachedResourceSpecificIngterpreterData } from '../../common/utils/decorators';
+import { InMemoryCache } from '../../common/utils/cacheUtils';
 import { OSType } from '../../common/utils/platform';
 import { IEnvironmentVariablesProvider } from '../../common/variables/types';
 import { EXTENSION_ROOT_DIR } from '../../constants';
 import { captureTelemetry, sendTelemetryEvent } from '../../telemetry';
 import { EventName } from '../../telemetry/constants';
-import { PythonInterpreter } from '../contracts';
+import { IInterpreterService, PythonInterpreter } from '../contracts';
 import { IEnvironmentActivationService } from './types';
 
 const getEnvironmentPrefix = 'e8b39361-0157-4923-80e1-22d70d46dee6';
@@ -36,23 +37,46 @@ const defaultShells = {
 @injectable()
 export class EnvironmentActivationService implements IEnvironmentActivationService, IDisposable {
  private readonly disposables: IDisposable[] = [];
+ private readonly activatedEnvVariablesCache = new Map<string, InMemoryCache<NodeJS.ProcessEnv | undefined>>();
  constructor(
  @inject(ITerminalHelper) private readonly helper: ITerminalHelper,
  @inject(IPlatformService) private readonly platform: IPlatformService,
  @inject(IProcessServiceFactory) private processServiceFactory: IProcessServiceFactory,
  @inject(ICurrentProcess) private currentProcess: ICurrentProcess,
+ @inject(IWorkspaceService) private workspace: IWorkspaceService,
+ @inject(IInterpreterService) private interpreterService: IInterpreterService,
  @inject(IEnvironmentVariablesProvider) private readonly envVarsService: IEnvironmentVariablesProvider
  ) {
- this.envVarsService.onDidEnvironmentVariablesChange(this.onDidEnvironmentVariablesChange, this, this.disposables);
+ this.envVarsService.onDidEnvironmentVariablesChange(() => this.activatedEnvVariablesCache.clear(), this, this.disposables);
+
+ this.interpreterService.onDidChangeInterpreter(() => this.activatedEnvVariablesCache.clear(), this, this.disposables);
  }
 
  public dispose(): void {
  this.disposables.forEach(d => d.dispose());
  }
  @traceDecorators.verbose('getActivatedEnvironmentVariables', LogOptions.Arguments)
  @captureTelemetry(EventName.PYTHON_INTERPRETER_ACTIVATION_ENVIRONMENT_VARIABLES, { failed: false }, true)
- @cacheResourceSpecificInterpreterData('ActivatedEnvironmentVariables', cacheDuration)
  public async getActivatedEnvironmentVariables(resource: Resource, interpreter?: PythonInterpreter, allowExceptions?: boolean): Promise<NodeJS.ProcessEnv | undefined> {
+ // Cache key = resource + interpreter.
+ const workspaceKey = this.workspace.getWorkspaceFolderIdentifier(resource);
+ const interpreterPath = this.platform.isWindows ? interpreter?.path.toLowerCase() : interpreter?.path;
+ const cacheKey = `${workspaceKey}_${interpreterPath}`;
+
+ if (this.activatedEnvVariablesCache.get(cacheKey)?.hasData) {
+ return this.activatedEnvVariablesCache.get(cacheKey)!.data;
+ }
+
+ // Cache only if successful, else keep trying & failing if necessary.
+ const cache = new InMemoryCache<NodeJS.ProcessEnv | undefined>(cacheDuration, '');
+ return this.getActivatedEnvironmentVariablesImpl(resource, interpreter, allowExceptions).then(vars => {
+ cache.data = vars;
+ this.activatedEnvVariablesCache.set(cacheKey, cache);
+ return vars;
+ });
+ }
+
+ public async getActivatedEnvironmentVariablesImpl(resource: Resource, interpreter?: PythonInterpreter, allowExceptions?: boolean): Promise<NodeJS.ProcessEnv | undefined> {
  const shellInfo = defaultShells[this.platform.osType];
  if (!shellInfo) {
  return;
@@ -115,9 +139,7 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi
  }
  }
  }
- protected onDidEnvironmentVariablesChange(affectedResource: Resource) {
- clearCachedResourceSpecificIngterpreterData('ActivatedEnvironmentVariables', affectedResource);
- }
+
  protected fixActivationCommands(commands: string[]): string[] {
  // Replace 'source ' with '. ' as that works in shell exec
  return commands.map(cmd => cmd.replace(/^source\s+/, '. '));

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

@@ -7,8 +7,9 @@ import { EOL } from 'os';
 import * as path from 'path';
 import { SemVer } from 'semver';
 import { anything, capture, instance, mock, verify, when } from 'ts-mockito';
-import * as typemoq from 'typemoq';
-import { EventEmitter, Uri, workspace as workspaceType, WorkspaceConfiguration } from 'vscode';
+import { EventEmitter, Uri } from 'vscode';
+import { IWorkspaceService } from '../../../client/common/application/types';
+import { WorkspaceService } from '../../../client/common/application/workspace';
 import { PlatformService } from '../../../client/common/platform/platformService';
 import { IPlatformService } from '../../../client/common/platform/types';
 import { CurrentProcess } from '../../../client/common/process/currentProcess';
@@ -18,15 +19,14 @@ import { IProcessService, IProcessServiceFactory } from '../../../client/common/
 import { TerminalHelper } from '../../../client/common/terminal/helper';
 import { ITerminalHelper } from '../../../client/common/terminal/types';
 import { ICurrentProcess } from '../../../client/common/types';
-import { clearCache } from '../../../client/common/utils/cacheUtils';
 import { getNamesAndValues } from '../../../client/common/utils/enum';
 import { Architecture, OSType } from '../../../client/common/utils/platform';
 import { EnvironmentVariablesProvider } from '../../../client/common/variables/environmentVariablesProvider';
 import { IEnvironmentVariablesProvider } from '../../../client/common/variables/types';
 import { EXTENSION_ROOT_DIR } from '../../../client/constants';
 import { EnvironmentActivationService } from '../../../client/interpreter/activation/service';
-import { InterpreterType, PythonInterpreter } from '../../../client/interpreter/contracts';
-import { mockedVSCodeNamespaces } from '../../vscode-mock';
+import { IInterpreterService, InterpreterType, PythonInterpreter } from '../../../client/interpreter/contracts';
+import { InterpreterService } from '../../../client/interpreter/interpreterService';
 
 const getEnvironmentPrefix = 'e8b39361-0157-4923-80e1-22d70d46dee6';
 const defaultShells = {
@@ -45,8 +45,10 @@ suite('Interpreters Activation - Python Environment Variables', () => {
  let processService: IProcessService;
  let currentProcess: ICurrentProcess;
  let envVarsService: IEnvironmentVariablesProvider;
- let workspace: typemoq.IMock<typeof workspaceType>;
-
+ let workspace: IWorkspaceService;
+ let interpreterService: IInterpreterService;
+ let onDidChangeEnvVariables: EventEmitter<Uri | undefined>;
+ let onDidChangeInterpreter: EventEmitter<void>;
  const pythonInterpreter: PythonInterpreter = {
  path: '/foo/bar/python.exe',
  version: new SemVer('3.6.6-final'),
@@ -63,21 +65,22 @@ suite('Interpreters Activation - Python Environment Variables', () => {
  processService = mock(ProcessService);
  currentProcess = mock(CurrentProcess);
  envVarsService = mock(EnvironmentVariablesProvider);
- workspace = mockedVSCodeNamespaces.workspace!;
- when(envVarsService.onDidEnvironmentVariablesChange).thenReturn(new EventEmitter<Uri | undefined>().event);
- service = new EnvironmentActivationService(instance(helper), instance(platform), instance(processServiceFactory), instance(currentProcess), instance(envVarsService));
-
- const cfg = typemoq.Mock.ofType<WorkspaceConfiguration>();
- workspace.setup(w => w.getConfiguration(typemoq.It.isValue('python'), typemoq.It.isAny())).returns(() => cfg.object);
- workspace.setup(w => w.workspaceFolders).returns(() => []);
- cfg.setup(c => c.inspect(typemoq.It.isValue('pythonPath'))).returns(() => {
- return { globalValue: 'GlobalValuepython' } as any;
- });
- clearCache();
+ interpreterService = mock(InterpreterService);
+ workspace = mock(WorkspaceService);
+ onDidChangeEnvVariables = new EventEmitter<Uri | undefined>();
+ onDidChangeInterpreter = new EventEmitter<void>();
+ when(envVarsService.onDidEnvironmentVariablesChange).thenReturn(onDidChangeEnvVariables.event);
+ when(interpreterService.onDidChangeInterpreter).thenReturn(onDidChangeInterpreter.event);
+ service = new EnvironmentActivationService(
+ instance(helper),
+ instance(platform),
+ instance(processServiceFactory),
+ instance(currentProcess),
+ instance(workspace),
+ instance(interpreterService),
+ instance(envVarsService)
+ );
  }
- teardown(() => {
- mockedVSCodeNamespaces.workspace!.reset();
- });
 
  function title(resource?: Uri, interpreter?: PythonInterpreter) {
  return `${resource ? 'With a resource' : 'Without a resource'}${interpreter ? ' and an interpreter' : ''}`;
@@ -218,6 +221,64 @@ suite('Interpreters Activation - Python Environment Variables', () => {
  verify(envVarsService.getEnvironmentVariables(resource)).once();
  verify(processService.shellExec(anything(), anything())).once();
  });
+ test('Cache Variables', async () => {
+ const cmd = ['1', '2'];
+ const varsFromEnv = { one: '11', two: '22', HELLO: 'xxx' };
+ const stdout = `${getEnvironmentPrefix}${EOL}${JSON.stringify(varsFromEnv)}`;
+ when(platform.osType).thenReturn(osType.value);
+ when(helper.getEnvironmentActivationShellCommands(resource, anything(), interpreter)).thenResolve(cmd);
+ when(processServiceFactory.create(resource)).thenResolve(instance(processService));
+ when(envVarsService.getEnvironmentVariables(resource)).thenResolve({});
+ when(processService.shellExec(anything(), anything())).thenResolve({ stdout: stdout });
+
+ const env = await service.getActivatedEnvironmentVariables(resource, interpreter);
+ const env2 = await service.getActivatedEnvironmentVariables(resource, interpreter);
+ const env3 = await service.getActivatedEnvironmentVariables(resource, interpreter);
+
+ expect(env).to.deep.equal(varsFromEnv);
+ // All same objects.
+ expect(env)
+ .to.equal(env2)
+ .to.equal(env3);
+
+ // All methods invoked only once.
+ verify(helper.getEnvironmentActivationShellCommands(resource, anything(), interpreter)).once();
+ verify(processServiceFactory.create(resource)).once();
+ verify(envVarsService.getEnvironmentVariables(resource)).once();
+ verify(processService.shellExec(anything(), anything())).once();
+ });
+ async function testClearingCache(bustCache: Function) {
+ const cmd = ['1', '2'];
+ const varsFromEnv = { one: '11', two: '22', HELLO: 'xxx' };
+ const stdout = `${getEnvironmentPrefix}${EOL}${JSON.stringify(varsFromEnv)}`;
+ when(platform.osType).thenReturn(osType.value);
+ when(helper.getEnvironmentActivationShellCommands(resource, anything(), interpreter)).thenResolve(cmd);
+ when(processServiceFactory.create(resource)).thenResolve(instance(processService));
+ when(envVarsService.getEnvironmentVariables(resource)).thenResolve({});
+ when(processService.shellExec(anything(), anything())).thenResolve({ stdout: stdout });
+
+ const env = await service.getActivatedEnvironmentVariables(resource, interpreter);
+ bustCache();
+ const env2 = await service.getActivatedEnvironmentVariables(resource, interpreter);
+
+ expect(env).to.deep.equal(varsFromEnv);
+ // Objects are different (not same reference).
+ expect(env).to.not.equal(env2);
+ // However variables are the same.
+ expect(env).to.deep.equal(env2);
+
+ // All methods invoked twice as cache was blown.
+ verify(helper.getEnvironmentActivationShellCommands(resource, anything(), interpreter)).twice();
+ verify(processServiceFactory.create(resource)).twice();
+ verify(envVarsService.getEnvironmentVariables(resource)).twice();
+ verify(processService.shellExec(anything(), anything())).twice();
+ }
+ test('Cache Variables get cleared when changing interpreter', async () => {
+ await testClearingCache(onDidChangeInterpreter.fire.bind(onDidChangeInterpreter));
+ });
+ test('Cache Variables get cleared when changing env variables file', async () => {
+ await testClearingCache(onDidChangeEnvVariables.fire.bind(onDidChangeEnvVariables));
+ });
  });
  });
  });