Run all extension-internal uses of Python "isolated".

news/3 Code Health/10681.md

@@ -0,0 +1,3 @@
+Run internal modules and scripts in isolated manner.
+
+This helps avoid problems like shadowing stdlib modules.

pythonFiles/pyvsc-run-isolated.py

@@ -13,7 +13,11 @@
 del sys.path[0]
 del sys.argv[0]
 module = sys.argv[0]
-if module.startswith("-"):
+if module == "-c":
+    ns = {}
+    for code in sys.argv[1:]:
+        exec(code, ns, ns)
+elif module.startswith("-"):
     raise NotImplementedError(sys.argv)
 elif module.endswith(".py"):
     runpy.run_path(module, run_name="__main__")

src/client/common/installer/moduleInstaller.ts

@@ -12,6 +12,7 @@ import { IApplicationShell } from '../application/types';
 import { wrapCancellationTokens } from '../cancellation';
 import { STANDARD_OUTPUT_CHANNEL } from '../constants';
 import { IFileSystem } from '../platform/types';
+import * as internalPython from '../process/internal/python';
 import { ITerminalServiceFactory } from '../terminal/types';
 import { ExecutionInfo, IConfigurationService, IOutputChannel } from '../types';
 import { Products } from '../utils/localize';
@@ -38,13 +39,13 @@ export abstract class ModuleInstaller implements IModuleInstaller {
             if (executionInfo.moduleName) {
                 const configService = this.serviceContainer.get<IConfigurationService>(IConfigurationService);
                 const settings = configService.getSettings(uri);
-                const args = ['-m', executionInfo.moduleName].concat(executionInfoArgs);
 
                 const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
                 const interpreter = isResource(resource)
                     ? await interpreterService.getActiveInterpreter(resource)
                     : resource;
                 const pythonPath = isResource(resource) ? settings.pythonPath : resource.path;
+                const args = internalPython.execModule(executionInfo.moduleName, executionInfoArgs);
                 if (!interpreter || interpreter.type !== InterpreterType.Unknown) {
                     await terminalService.sendCommand(pythonPath, args, token);
                 } else if (settings.globalModuleInstallation) {

src/client/common/process/internal/python.ts

@@ -0,0 +1,126 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+import { _ISOLATED as ISOLATED } from './scripts';
+
+// "python" contains functions corresponding to the various ways that
+// the extension invokes a Python executable internally.  Each function
+// takes arguments relevant to the specific use case.  However, each
+// always *returns* a list of strings for the commandline arguments that
+// should be used when invoking the Python executable for the specific
+// use case, whether through spawn/exec or a terminal.
+//
+// Where relevant (nearly always), the function also returns a "parse"
+// function that may be used to deserialize the stdout of the command
+// into the corresponding object or objects.  "parse()" takes a single
+// string as the stdout text and returns the relevant data.
+
+export function execCode(code: string, isolated = true): string[] {
+    const args = ['-c', code];
+    if (isolated) {
+        args.splice(0, 0, ISOLATED);
+    }
+    // "code" isn't specific enough to know how to parse it,
+    // so we only return the args.
+    return args;
+}
+
+export function execModule(name: string, moduleArgs: string[], isolated = true): string[] {
+    const args = ['-m', name, ...moduleArgs];
+    if (isolated) {
+        args[0] = ISOLATED; // replace
+    }
+    // "code" isn't specific enough to know how to parse it,
+    // so we only return the args.
+    return args;
+}
+
+export function getVersion(): [string[], (out: string) => string] {
+    // There is no need to isolate this.
+    const args = ['--version'];
+
+    function parse(out: string): string {
+        return out.trim();
+    }
+
+    return [args, parse];
+}
+
+export function getSysPrefix(): [string[], (out: string) => string] {
+    const args = [ISOLATED, '-c', 'import sys;print(sys.prefix)'];
+
+    function parse(out: string): string {
+        return out.trim();
+    }
+
+    return [args, parse];
+}
+
+export function getExecutable(): [string[], (out: string) => string] {
+    const args = [ISOLATED, '-c', 'import sys;print(sys.executable)'];
+
+    function parse(out: string): string {
+        return out.trim();
+    }
+
+    return [args, parse];
+}
+
+export function getSitePackages(): [string[], (out: string) => string] {
+    const args = [
+        ISOLATED,
+        '-c',
+        // On windows we also need the libs path (second item will
+        // return c:\xxx\lib\site-packages).  This is returned by
+        // the following:
+        'from distutils.sysconfig import get_python_lib; print(get_python_lib())'
+    ];
+
+    function parse(out: string): string {
+        return out.trim();
+    }
+
+    return [args, parse];
+}
+
+export function getUserSitePackages(): [string[], (out: string) => string] {
+    const args = [ISOLATED, 'site', '--user-site'];
+
+    function parse(out: string): string {
+        return out.trim();
+    }
+
+    return [args, parse];
+}
+
+export function isValid(): [string[], (out: string) => boolean] {
+    // There is no need to isolate this.
+    const args = ['-c', 'print(1234)'];
+
+    function parse(out: string): boolean {
+        return out.startsWith('1234');
+    }
+
+    return [args, parse];
+}
+
+export function isModuleInstalled(name: string): [string[], (out: string) => boolean] {
+    const args = [ISOLATED, '-c', `import ${name}`];
+
+    function parse(_out: string): boolean {
+        // If the command did not fail then the module is installed.
+        return true;
+    }
+
+    return [args, parse];
+}
+
+export function getModuleVersion(name: string): [string[], (out: string) => string] {
+    const args = [ISOLATED, name, '--version'];
+
+    function parse(out: string): string {
+        return out.trim();
+    }
+
+    return [args, parse];
+}

src/client/common/process/pythonDaemonPool.ts

@@ -17,6 +17,7 @@ import { IDisposableRegistry } from '../types';
 import { createDeferred, sleep } from '../utils/async';
 import { noop } from '../utils/misc';
 import { StopWatch } from '../utils/stopWatch';
+import * as internalPython from './internal/python';
 import { ProcessService } from './proc';
 import { PythonDaemonExecutionService } from './pythonDaemon';
 import {
@@ -101,7 +102,8 @@ export class PythonDaemonExecutionServicePool implements IPythonDaemonExecutionS
         return this.pythonExecutionService.getExecutionInfo(pythonArgs);
     }
     public async isModuleInstalled(moduleName: string): Promise<boolean> {
-        const msg = { args: ['-m', moduleName] };
+        const args = internalPython.execModule(moduleName, []);
+        const msg = { args };
         return this.wrapCall((daemon) => daemon.isModuleInstalled(moduleName), msg);
     }
     public async exec(args: string[], options: SpawnOptions): Promise<ExecutionResult<string>> {
@@ -110,10 +112,11 @@ export class PythonDaemonExecutionServicePool implements IPythonDaemonExecutionS
     }
     public async execModule(
         moduleName: string,
-        args: string[],
+        moduleArgs: string[],
         options: SpawnOptions
     ): Promise<ExecutionResult<string>> {
-        const msg = { args: ['-m', moduleName].concat(args), options };
+        const args = internalPython.execModule(moduleName, moduleArgs);
+        const msg = { args, options };
         return this.wrapCall((daemon) => daemon.execModule(moduleName, args, options), msg);
     }
     public execObservable(args: string[], options: SpawnOptions): ObservableExecutionResult<string> {
@@ -122,10 +125,11 @@ export class PythonDaemonExecutionServicePool implements IPythonDaemonExecutionS
     }
     public execModuleObservable(
         moduleName: string,
-        args: string[],
+        moduleArgs: string[],
         options: SpawnOptions
     ): ObservableExecutionResult<string> {
-        const msg = { args: ['-m', moduleName].concat(args), options };
+        const args = internalPython.execModule(moduleName, moduleArgs);
+        const msg = { args, options };
         return this.wrapObservableCall((daemon) => daemon.execModuleObservable(moduleName, args, options), msg);
     }
     /**

src/client/common/process/pythonEnvironment.ts

@@ -6,6 +6,7 @@ import { traceError, traceInfo } from '../logger';
 import { IFileSystem } from '../platform/types';
 import { Architecture } from '../utils/platform';
 import { parsePythonVersion } from '../utils/version';
+import * as internalPython from './internal/python';
 import * as internalScripts from './internal/scripts';
 import {
     ExecutionResult,
@@ -61,15 +62,18 @@ class PythonEnvironment {
             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();
+        const [args, parse] = internalPython.getExecutable();
+        const info = this.getExecutionInfo(args);
+        const proc = await this.deps.exec(info.command, info.args);
+        return parse(proc.stdout);
     }
 
     public async isModuleInstalled(moduleName: string): Promise<boolean> {
-        const { command, args } = this.getExecutionInfo(['-c', `import ${moduleName}`]);
+        // prettier-ignore
+        const [args,] = internalPython.isModuleInstalled(moduleName);
+        const info = this.getExecutionInfo(args);
         try {
-            await this.deps.exec(command, args);
+            await this.deps.exec(info.command, info.args);
         } catch {
             return false;
         }

src/client/common/process/pythonProcess.ts

@@ -3,6 +3,7 @@
 
 import { ErrorUtils } from '../errors/errorUtils';
 import { ModuleNotInstalledError } from '../errors/moduleNotInstalledError';
+import * as internalPython from './internal/python';
 import {
     ExecutionResult,
     IProcessService,
@@ -33,11 +34,12 @@ class PythonProcessService {
 
     public execModuleObservable(
         moduleName: string,
-        args: string[],
+        moduleArgs: string[],
         options: SpawnOptions
     ): ObservableExecutionResult<string> {
+        const args = internalPython.execModule(moduleName, moduleArgs);
         const opts: SpawnOptions = { ...options };
-        const executable = this.deps.getExecutionObservableInfo(['-m', moduleName, ...args]);
+        const executable = this.deps.getExecutionObservableInfo(args);
         return this.deps.execObservable(executable.command, executable.args, opts);
     }
 
@@ -49,11 +51,12 @@ class PythonProcessService {
 
     public async execModule(
         moduleName: string,
-        args: string[],
+        moduleArgs: string[],
         options: SpawnOptions
     ): Promise<ExecutionResult<string>> {
+        const args = internalPython.execModule(moduleName, moduleArgs);
         const opts: SpawnOptions = { ...options };
-        const executable = this.deps.getExecutionInfo(['-m', moduleName, ...args]);
+        const executable = this.deps.getExecutionInfo(args);
         const result = await this.deps.exec(executable.command, executable.args, opts);
 
         // If a module is not installed we'll have something in stderr.

src/client/interpreter/display/shebangCodeLensProvider.ts

@@ -2,6 +2,7 @@ import { inject, injectable } from 'inversify';
 import { CancellationToken, CodeLens, Command, Event, Position, Range, TextDocument, Uri } from 'vscode';
 import { IWorkspaceService } from '../../common/application/types';
 import { IPlatformService } from '../../common/platform/types';
+import * as internalPython from '../../common/process/internal/python';
 import { IProcessServiceFactory } from '../../common/process/types';
 import { IConfigurationService } from '../../common/types';
 import { IShebangCodeLensProvider } from '../contracts';
@@ -37,20 +38,20 @@ export class ShebangCodeLensProvider implements IShebangCodeLensProvider {
     }
     private async getFullyQualifiedPathToInterpreter(pythonPath: string, resource: Uri) {
         let cmdFile = pythonPath;
-        let args = ['-c', 'import sys;print(sys.executable)'];
+        const [args, parse] = internalPython.getExecutable();
         if (pythonPath.indexOf('bin/env ') >= 0 && !this.platformService.isWindows) {
             // In case we have pythonPath as '/usr/bin/env python'.
             const parts = pythonPath
                 .split(' ')
                 .map((part) => part.trim())
                 .filter((part) => part.length > 0);
             cmdFile = parts.shift()!;
-            args = parts.concat(args);
+            args.splice(0, 0, ...parts);
         }
         const processService = await this.processServiceFactory.create(resource);
         return processService
             .exec(cmdFile, args)
-            .then((output) => output.stdout.trim())
+            .then((output) => parse(output.stdout))
             .catch(() => '');
     }
     private async createShebangCodeLens(document: TextDocument) {

src/client/interpreter/interpreterVersion.ts

@@ -1,5 +1,6 @@
 import { inject, injectable } from 'inversify';
 import '../common/extensions';
+import * as internalPython from '../common/process/internal/python';
 import { IProcessServiceFactory } from '../common/process/types';
 import { IInterpreterVersionService } from './contracts';
 
@@ -9,21 +10,24 @@ export const PIP_VERSION_REGEX = '\\d+\\.\\d+(\\.\\d+)?';
 export class InterpreterVersionService implements IInterpreterVersionService {
     constructor(@inject(IProcessServiceFactory) private readonly processServiceFactory: IProcessServiceFactory) {}
     public async getVersion(pythonPath: string, defaultValue: string): Promise<string> {
+        const [args, parse] = internalPython.getVersion();
         const processService = await this.processServiceFactory.create();
         return processService
-            .exec(pythonPath, ['--version'], { mergeStdOutErr: true })
-            .then((output) => output.stdout.splitLines()[0])
+            .exec(pythonPath, args, { mergeStdOutErr: true })
+            .then((output) => parse(output.stdout).splitLines()[0])
             .then((version) => (version.length === 0 ? defaultValue : version))
             .catch(() => defaultValue);
     }
     public async getPipVersion(pythonPath: string): Promise<string> {
+        const [args, parse] = internalPython.getModuleVersion('pip');
         const processService = await this.processServiceFactory.create();
-        const output = await processService.exec(pythonPath, ['-m', 'pip', '--version'], { mergeStdOutErr: true });
-        if (output.stdout.length > 0) {
+        const output = await processService.exec(pythonPath, args, { mergeStdOutErr: true });
+        const version = parse(output.stdout);
+        if (version.length > 0) {
             // Here's a sample output:
             // pip 9.0.1 from /Users/donjayamanne/anaconda3/lib/python3.6/site-packages (python 3.6).
             const re = new RegExp(PIP_VERSION_REGEX, 'g');
-            const matches = re.exec(output.stdout);
+            const matches = re.exec(version);
             if (matches && matches.length > 0) {
                 return matches[0].trim();
             }

src/client/interpreter/locators/services/currentPathService.ts

@@ -3,6 +3,7 @@ import { inject, injectable } from 'inversify';
 import { Uri } from 'vscode';
 import { traceError, traceInfo } from '../../../common/logger';
 import { IFileSystem, IPlatformService } from '../../../common/platform/types';
+import * as internalPython from '../../../common/process/internal/python';
 import { IProcessServiceFactory } from '../../../common/process/types';
 import { IConfigurationService } from '../../../common/types';
 import { OSType } from '../../../common/utils/platform';
@@ -92,24 +93,25 @@ export class CurrentPathService extends CacheableLocatorService {
     private async getInterpreter(options: { command: string; args?: string[] }) {
         try {
             const processService = await this.processServiceFactory.create();
-            const args = Array.isArray(options.args) ? options.args : [];
+            const pyArgs = Array.isArray(options.args) ? options.args : [];
+            const [args, parse] = internalPython.getExecutable();
             return processService
-                .exec(options.command, args.concat(['-c', 'import sys;print(sys.executable)']), {})
-                .then((output) => output.stdout.trim())
+                .exec(options.command, pyArgs.concat(args), {})
+                .then((output) => parse(output.stdout))
                 .then(async (value) => {
                     if (value.length > 0 && (await this.fs.fileExists(value))) {
                         return value;
                     }
                     traceError(
-                        `Detection of Python Interpreter for Command ${options.command} and args ${args.join(
+                        `Detection of Python Interpreter for Command ${options.command} and args ${pyArgs.join(
                             ' '
                         )} failed as file ${value} does not exist`
                     );
                     return '';
                 })
                 .catch((_ex) => {
                     traceInfo(
-                        `Detection of Python Interpreter for Command ${options.command} and args ${args.join(
+                        `Detection of Python Interpreter for Command ${options.command} and args ${pyArgs.join(
                             ' '
                         )} failed`
                     );