Skip to content

Commit

Permalink
Separate Python env from exec. (#10883)
Browse files Browse the repository at this point in the history
For #10681

The key change here is the separation (under src/client/common/process) of Python environment helpers from running Python processes. This helps simplify later changes needed for #10681, as well as other code health benefits.

A small related change: adding PythonExecutionInfo.python (which helps simplify composition of exec args in some situations).

There should be zero change in behavior.
  • Loading branch information
ericsnowcurrently authored Apr 7, 2020
1 parent c48b306 commit 0b14333
Show file tree
Hide file tree
Showing 32 changed files with 741 additions and 613 deletions.
27 changes: 0 additions & 27 deletions src/client/common/process/condaExecutionService.ts

This file was deleted.

4 changes: 2 additions & 2 deletions src/client/common/process/pythonDaemon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ export class PythonDaemonExecutionService implements IPythonDaemonExecutionServi
return this.pythonExecutionService.getExecutablePath();
}
}
public getExecutionInfo(args: string[]): PythonExecutionInfo {
return this.pythonExecutionService.getExecutionInfo(args);
public getExecutionInfo(pythonArgs?: string[]): PythonExecutionInfo {
return this.pythonExecutionService.getExecutionInfo(pythonArgs);
}
public async isModuleInstalled(moduleName: string): Promise<boolean> {
try {
Expand Down
4 changes: 2 additions & 2 deletions src/client/common/process/pythonDaemonPool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ export class PythonDaemonExecutionServicePool implements IPythonDaemonExecutionS
const msg = { args: ['getExecutablePath'] };
return this.wrapCall((daemon) => daemon.getExecutablePath(), msg);
}
public getExecutionInfo(args: string[]): PythonExecutionInfo {
return this.pythonExecutionService.getExecutionInfo(args);
public getExecutionInfo(pythonArgs?: string[]): PythonExecutionInfo {
return this.pythonExecutionService.getExecutionInfo(pythonArgs);
}
public async isModuleInstalled(moduleName: string): Promise<boolean> {
const msg = { args: ['-m', moduleName] };
Expand Down
214 changes: 214 additions & 0 deletions src/client/common/process/pythonEnvironment.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,214 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import * as path from 'path';
import { CondaEnvironmentInfo } from '../../interpreter/contracts';
import { EXTENSION_ROOT_DIR } from '../constants';
import { traceError, traceInfo } from '../logger';
import { IFileSystem } from '../platform/types';
import { Architecture } from '../utils/platform';
import { parsePythonVersion } from '../utils/version';
import {
ExecutionResult,
InterpreterInfomation,
IProcessService,
PythonExecutionInfo,
PythonVersionInfo,
ShellOptions,
SpawnOptions
} from './types';

function getExecutionInfo(python: string[], pythonArgs: string[]): PythonExecutionInfo {
const args = python.slice(1);
args.push(...pythonArgs);
return { command: python[0], args, python };
}

class PythonEnvironment {
private cachedInterpreterInformation: InterpreterInfomation | undefined | null = null;

constructor(
protected readonly pythonPath: string,
// "deps" is the externally defined functionality used by the class.
protected readonly deps: {
getPythonArgv(python: string): string[];
getObservablePythonArgv(python: string): string[];
isValidExecutable(python: string): Promise<boolean>;
// from ProcessService:
exec(file: string, args: string[]): Promise<ExecutionResult<string>>;
shellExec(command: string, timeout: number): Promise<ExecutionResult<string>>;
}
) {}

public getExecutionInfo(pythonArgs: string[] = []): PythonExecutionInfo {
const python = this.deps.getPythonArgv(this.pythonPath);
return getExecutionInfo(python, pythonArgs);
}
public getExecutionObservableInfo(pythonArgs: string[] = []): PythonExecutionInfo {
const python = this.deps.getObservablePythonArgv(this.pythonPath);
return getExecutionInfo(python, pythonArgs);
}

public async getInterpreterInformation(): Promise<InterpreterInfomation | undefined> {
if (this.cachedInterpreterInformation === null) {
this.cachedInterpreterInformation = await this.getInterpreterInformationImpl();
}
return this.cachedInterpreterInformation;
}

public async getExecutablePath(): Promise<string> {
// If we've passed the python file, then return the file.
// This is because on mac if using the interpreter /usr/bin/python2.7 we can get a different value for the path
if (await this.deps.isValidExecutable(this.pythonPath)) {
return this.pythonPath;
}

const { command, args } = this.getExecutionInfo(['-c', 'import sys;print(sys.executable)']);
const proc = await this.deps.exec(command, args);
return proc.stdout.trim();
}

public async isModuleInstalled(moduleName: string): Promise<boolean> {
const { command, args } = this.getExecutionInfo(['-c', `import ${moduleName}`]);
try {
await this.deps.exec(command, args);
} catch {
return false;
}
return true;
}

private async getInterpreterInformationImpl(): Promise<InterpreterInfomation | undefined> {
try {
const file = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'interpreterInfo.py');
const { command, args } = this.getExecutionInfo([file]);
const argv = [command, ...args];

// Concat these together to make a set of quoted strings
const quoted = argv.reduce((p, c) => (p ? `${p} "${c}"` : `"${c.replace('\\', '\\\\')}"`), '');

// Try shell execing the command, followed by the arguments. This will make node kill the process if it
// takes too long.
// Sometimes the python path isn't valid, timeout if that's the case.
// See these two bugs:
// https://github.com/microsoft/vscode-python/issues/7569
// https://github.com/microsoft/vscode-python/issues/7760
const result = await this.deps.shellExec(quoted, 15000);
if (result.stderr) {
traceError(`Failed to parse interpreter information for ${argv} stderr: ${result.stderr}`);
return;
}
let json: {
versionInfo: PythonVersionInfo;
sysPrefix: string;
sysVersion: string;
is64Bit: boolean;
};
try {
json = JSON.parse(result.stdout);
} catch (ex) {
throw Error(`${argv} returned bad JSON (${result.stdout}) (${ex})`);
}
traceInfo(`Found interpreter for ${argv}`);
const versionValue =
json.versionInfo.length === 4
? `${json.versionInfo.slice(0, 3).join('.')}-${json.versionInfo[3]}`
: json.versionInfo.join('.');
return {
architecture: json.is64Bit ? Architecture.x64 : Architecture.x86,
path: this.pythonPath,
version: parsePythonVersion(versionValue),
sysVersion: json.sysVersion,
sysPrefix: json.sysPrefix
};
} catch (ex) {
traceError(`Failed to get interpreter information for '${this.pythonPath}'`, ex);
}
}
}

function createDeps(
isValidExecutable: (filename: string) => Promise<boolean>,
pythonArgv: string[] | undefined,
observablePythonArgv: string[] | undefined,
// from ProcessService:
exec: (file: string, args: string[], options?: SpawnOptions) => Promise<ExecutionResult<string>>,
shellExec: (command: string, options?: ShellOptions) => Promise<ExecutionResult<string>>
) {
return {
getPythonArgv: (python: string) => pythonArgv || [python],
getObservablePythonArgv: (python: string) => observablePythonArgv || [python],
isValidExecutable,
exec: async (cmd: string, args: string[]) => exec(cmd, args, { throwOnStdErr: true }),
shellExec: async (text: string, timeout: number) => shellExec(text, { timeout })
};
}

export function createPythonEnv(
pythonPath: string,
// These are used to generate the deps.
procs: IProcessService,
fs: IFileSystem
): PythonEnvironment {
const deps = createDeps(
async (filename) => fs.fileExists(filename),
// We use the default: [pythonPath].
undefined,
undefined,
(file, args, opts) => procs.exec(file, args, opts),
(command, opts) => procs.shellExec(command, opts)
);
return new PythonEnvironment(pythonPath, deps);
}

export function createCondaEnv(
condaFile: string,
condaInfo: CondaEnvironmentInfo,
pythonPath: string,
// These are used to generate the deps.
procs: IProcessService,
fs: IFileSystem
): PythonEnvironment {
const runArgs = ['run'];
if (condaInfo.name === '') {
runArgs.push('-p', condaInfo.path);
} else {
runArgs.push('-n', condaInfo.name);
}
const pythonArgv = [condaFile, ...runArgs, 'python'];
const deps = createDeps(
async (filename) => fs.fileExists(filename),
pythonArgv,
// tslint:disable-next-line:no-suspicious-comment
// TODO(gh-8473): Use pythonArgv here once 'conda run' can be
// run without buffering output.
undefined,
(file, args, opts) => procs.exec(file, args, opts),
(command, opts) => procs.shellExec(command, opts)
);
return new PythonEnvironment(pythonPath, deps);
}

export function createWindowsStoreEnv(
pythonPath: string,
// These are used to generate the deps.
procs: IProcessService
): PythonEnvironment {
const deps = createDeps(
/**
* With windows store python apps, we have generally use the
* symlinked python executable. The actual file is not accessible
* by the user due to permission issues (& rest of exension fails
* when using that executable). Hence lets not resolve the
* executable using sys.executable for windows store python
* interpreters.
*/
async (_f: string) => true,
// We use the default: [pythonPath].
undefined,
undefined,
(file, args, opts) => procs.exec(file, args, opts),
(command, opts) => procs.shellExec(command, opts)
);
return new PythonEnvironment(pythonPath, deps);
}
66 changes: 46 additions & 20 deletions src/client/common/process/pythonExecutionFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,19 @@ import { gte } from 'semver';

import { Uri } from 'vscode';
import { IEnvironmentActivationService } from '../../interpreter/activation/types';
import { ICondaService, IInterpreterService } from '../../interpreter/contracts';
import { CondaEnvironmentInfo, ICondaService, IInterpreterService } from '../../interpreter/contracts';
import { WindowsStoreInterpreter } from '../../interpreter/locators/services/windowsStoreInterpreter';
import { IWindowsStoreInterpreter } from '../../interpreter/locators/types';
import { IServiceContainer } from '../../ioc/types';
import { sendTelemetryEvent } from '../../telemetry';
import { EventName } from '../../telemetry/constants';
import { traceError } from '../logger';
import { IFileSystem } from '../platform/types';
import { IConfigurationService, IDisposableRegistry } from '../types';
import { CondaExecutionService } from './condaExecutionService';
import { ProcessService } from './proc';
import { PythonDaemonExecutionServicePool } from './pythonDaemonPool';
import { PythonExecutionService } from './pythonProcess';
import { createCondaEnv, createPythonEnv, createWindowsStoreEnv } from './pythonEnvironment';
import { createPythonProcessService } from './pythonProcess';
import {
DaemonExecutionFactoryCreationOptions,
ExecutionFactoryCreateWithEnvironmentOptions,
Expand All @@ -29,7 +30,6 @@ import {
IPythonExecutionFactory,
IPythonExecutionService
} from './types';
import { WindowsStorePythonProcess } from './windowsStorePythonProcess';

// Minimum version number of conda required to be able to use 'conda run'
export const CONDA_RUN_VERSION = '4.6.0';
Expand All @@ -54,16 +54,15 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
const processLogger = this.serviceContainer.get<IProcessLogger>(IProcessLogger);
processService.on('exec', processLogger.logProcess.bind(processLogger));

if (this.windowsStoreInterpreter.isWindowsStoreInterpreter(pythonPath)) {
return new WindowsStorePythonProcess(
this.serviceContainer,
processService,
pythonPath,
this.windowsStoreInterpreter
);
}
return new PythonExecutionService(this.serviceContainer, processService, pythonPath);
return createPythonService(
pythonPath,
processService,
this.serviceContainer.get<IFileSystem>(IFileSystem),
undefined,
this.windowsStoreInterpreter.isWindowsStoreInterpreter(pythonPath)
);
}

public async createDaemon(options: DaemonExecutionFactoryCreationOptions): Promise<IPythonExecutionService> {
const pythonPath = options.pythonPath
? options.pythonPath
Expand Down Expand Up @@ -143,15 +142,15 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
processService.on('exec', processLogger.logProcess.bind(processLogger));
this.serviceContainer.get<IDisposableRegistry>(IDisposableRegistry).push(processService);

return new PythonExecutionService(this.serviceContainer, processService, pythonPath);
return createPythonService(pythonPath, processService, this.serviceContainer.get<IFileSystem>(IFileSystem));
}
// Not using this function for now because there are breaking issues with conda run (conda 4.8, PVSC 2020.1).
// See https://github.com/microsoft/vscode-python/issues/9490
public async createCondaExecutionService(
pythonPath: string,
processService?: IProcessService,
resource?: Uri
): Promise<CondaExecutionService | undefined> {
): Promise<IPythonExecutionService | undefined> {
const processServicePromise = processService
? Promise.resolve(processService)
: this.processServiceFactory.create(resource);
Expand All @@ -169,15 +168,42 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
procService.on('exec', processLogger.logProcess.bind(processLogger));
this.serviceContainer.get<IDisposableRegistry>(IDisposableRegistry).push(procService);
}
return new CondaExecutionService(
this.serviceContainer,
procService,
return createPythonService(
pythonPath,
condaFile,
condaEnvironment
procService,
this.serviceContainer.get<IFileSystem>(IFileSystem),
// This is what causes a CondaEnvironment to be returned:
[condaFile, condaEnvironment]
);
}

return Promise.resolve(undefined);
}
}

function createPythonService(
pythonPath: string,
procService: IProcessService,
fs: IFileSystem,
conda?: [string, CondaEnvironmentInfo],
isWindowsStore?: boolean
): IPythonExecutionService {
let env = createPythonEnv(pythonPath, procService, fs);
if (conda) {
const [condaPath, condaInfo] = conda;
env = createCondaEnv(condaPath, condaInfo, pythonPath, procService, fs);
} else if (isWindowsStore) {
env = createWindowsStoreEnv(pythonPath, procService);
}
const procs = createPythonProcessService(procService, env);
return {
getInterpreterInformation: () => env.getInterpreterInformation(),
getExecutablePath: () => env.getExecutablePath(),
isModuleInstalled: (m) => env.isModuleInstalled(m),
getExecutionInfo: (a) => env.getExecutionInfo(a),
execObservable: (a, o) => procs.execObservable(a, o),
execModuleObservable: (m, a, o) => procs.execModuleObservable(m, a, o),
exec: (a, o) => procs.exec(a, o),
execModule: (m, a, o) => procs.execModule(m, a, o)
};
}
Loading

0 comments on commit 0b14333

Please sign in to comment.