-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add support for detection and selection of conda environments lacking a python interpreter #18427
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
Changes from all commits
9ce2893
d311863
5fb3f7d
aaaaa46
05e50b1
56d7bb4
6c98d94
0dc5a68
4856da1
44ae604
e0d8744
ebb5206
ef0008e
1aac36e
2afd514
e058b7a
50ed39d
da25d60
0ca6dd0
6c35c5b
80fda87
b1fcc8e
468ee06
07802fb
fc33a43
d5c0086
0fae72b
26bff7c
54aaccf
38c5be8
629ef20
7dfa521
9b09188
b0cf7be
21189bd
0ec2461
657b8fc
fbe1d39
104be45
1f508dc
1220841
592d46d
6f10dcb
bc27abd
51d6eb0
7763572
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Add support for detection and selection of conda environments lacking a python interpreter. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -75,7 +75,7 @@ export class CondaInstaller extends ModuleInstaller { | |
|
||
const pythonPath = isResource(resource) | ||
? this.serviceContainer.get<IConfigurationService>(IConfigurationService).getSettings(resource).pythonPath | ||
: resource.path; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
: resource.id ?? ''; | ||
const condaLocatorService = this.serviceContainer.get<IComponentAdapter>(IComponentAdapter); | ||
const info = await condaLocatorService.getCondaEnvironment(pythonPath); | ||
const args = [flags & ModuleInstallFlags.upgrade ? 'update' : 'install']; | ||
|
@@ -127,7 +127,7 @@ export class CondaInstaller extends ModuleInstaller { | |
const condaService = this.serviceContainer.get<IComponentAdapter>(IComponentAdapter); | ||
const pythonPath = isResource(resource) | ||
? this.serviceContainer.get<IConfigurationService>(IConfigurationService).getSettings(resource).pythonPath | ||
: resource.path; | ||
: resource.id ?? ''; | ||
return condaService.isCondaEnvironment(pythonPath); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,18 +15,22 @@ import { wrapCancellationTokens } from '../cancellation'; | |
import { STANDARD_OUTPUT_CHANNEL } from '../constants'; | ||
import { IFileSystem } from '../platform/types'; | ||
import * as internalPython from '../process/internal/python'; | ||
import { IProcessServiceFactory } from '../process/types'; | ||
import { ITerminalServiceFactory, TerminalCreationOptions } from '../terminal/types'; | ||
import { ExecutionInfo, IConfigurationService, IOutputChannel, Product } from '../types'; | ||
import { Products } from '../utils/localize'; | ||
import { isResource } from '../utils/misc'; | ||
import { ProductNames } from './productNames'; | ||
import { IModuleInstaller, InterpreterUri, ModuleInstallFlags } from './types'; | ||
import { IModuleInstaller, InstallOptions, InterpreterUri, ModuleInstallFlags } from './types'; | ||
|
||
@injectable() | ||
export abstract class ModuleInstaller implements IModuleInstaller { | ||
public abstract get priority(): number; | ||
|
||
public abstract get name(): string; | ||
|
||
public abstract get displayName(): string; | ||
|
||
public abstract get type(): ModuleInstallerType; | ||
|
||
constructor(protected serviceContainer: IServiceContainer) {} | ||
|
@@ -36,24 +40,18 @@ export abstract class ModuleInstaller implements IModuleInstaller { | |
resource?: InterpreterUri, | ||
cancel?: CancellationToken, | ||
flags?: ModuleInstallFlags, | ||
options?: InstallOptions, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add support for installing modules as a process. Earlier we just sent the command in terminal. |
||
): Promise<void> { | ||
const shouldExecuteInTerminal = !options?.installAsProcess; | ||
const name = | ||
typeof productOrModuleName == 'string' | ||
typeof productOrModuleName === 'string' | ||
? productOrModuleName | ||
: translateProductToModule(productOrModuleName); | ||
const productName = typeof productOrModuleName === 'string' ? name : ProductNames.get(productOrModuleName); | ||
sendTelemetryEvent(EventName.PYTHON_INSTALL_PACKAGE, undefined, { installer: this.displayName, productName }); | ||
const uri = isResource(resource) ? resource : undefined; | ||
const options: TerminalCreationOptions = {}; | ||
if (isResource(resource)) { | ||
options.resource = uri; | ||
} else { | ||
options.interpreter = resource; | ||
} | ||
const executionInfo = await this.getExecutionInfo(name, resource, flags); | ||
const terminalService = this.serviceContainer | ||
.get<ITerminalServiceFactory>(ITerminalServiceFactory) | ||
.getTerminalService(options); | ||
|
||
const install = async (token?: CancellationToken) => { | ||
const executionInfoArgs = await this.processInstallArgs(executionInfo.args, resource); | ||
if (executionInfo.moduleName) { | ||
|
@@ -64,25 +62,38 @@ export abstract class ModuleInstaller implements IModuleInstaller { | |
const interpreter = isResource(resource) | ||
? await interpreterService.getActiveInterpreter(resource) | ||
: resource; | ||
const pythonPath = isResource(resource) ? settings.pythonPath : resource.path; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
const interpreterPath = interpreter?.path ?? settings.pythonPath; | ||
const pythonPath = isResource(resource) ? interpreterPath : resource.path; | ||
const args = internalPython.execModule(executionInfo.moduleName, executionInfoArgs); | ||
if (!interpreter || interpreter.envType !== EnvironmentType.Unknown) { | ||
await terminalService.sendCommand(pythonPath, args, token); | ||
await this.executeCommand(shouldExecuteInTerminal, resource, pythonPath, args, token); | ||
} else if (settings.globalModuleInstallation) { | ||
const fs = this.serviceContainer.get<IFileSystem>(IFileSystem); | ||
if (await fs.isDirReadonly(path.dirname(pythonPath)).catch((_err) => true)) { | ||
this.elevatedInstall(pythonPath, args); | ||
} else { | ||
await terminalService.sendCommand(pythonPath, args, token); | ||
await this.executeCommand(shouldExecuteInTerminal, resource, pythonPath, args, token); | ||
} | ||
} else if (name === translateProductToModule(Product.pip)) { | ||
// Pip should always be installed into the specified environment. | ||
await terminalService.sendCommand(pythonPath, args, token); | ||
await this.executeCommand(shouldExecuteInTerminal, resource, pythonPath, args, token); | ||
} else { | ||
await terminalService.sendCommand(pythonPath, args.concat(['--user']), token); | ||
await this.executeCommand( | ||
shouldExecuteInTerminal, | ||
resource, | ||
pythonPath, | ||
args.concat(['--user']), | ||
token, | ||
); | ||
} | ||
} else { | ||
await terminalService.sendCommand(executionInfo.execPath!, executionInfoArgs, token); | ||
await this.executeCommand( | ||
shouldExecuteInTerminal, | ||
resource, | ||
executionInfo.execPath!, | ||
executionInfoArgs, | ||
token, | ||
); | ||
} | ||
}; | ||
|
||
|
@@ -103,6 +114,7 @@ export abstract class ModuleInstaller implements IModuleInstaller { | |
await install(cancel); | ||
} | ||
} | ||
|
||
public abstract isSupported(resource?: InterpreterUri): Promise<boolean>; | ||
|
||
protected elevatedInstall(execPath: string, args: string[]) { | ||
|
@@ -131,11 +143,13 @@ export abstract class ModuleInstaller implements IModuleInstaller { | |
} | ||
}); | ||
} | ||
|
||
protected abstract getExecutionInfo( | ||
moduleName: string, | ||
resource?: InterpreterUri, | ||
flags?: ModuleInstallFlags, | ||
): Promise<ExecutionInfo>; | ||
|
||
private async processInstallArgs(args: string[], resource?: InterpreterUri): Promise<string[]> { | ||
const indexOfPylint = args.findIndex((arg) => arg.toUpperCase() === 'PYLINT'); | ||
if (indexOfPylint === -1) { | ||
|
@@ -152,6 +166,32 @@ export abstract class ModuleInstaller implements IModuleInstaller { | |
} | ||
return args; | ||
} | ||
|
||
private async executeCommand( | ||
executeInTerminal: boolean, | ||
resource: InterpreterUri | undefined, | ||
command: string, | ||
args: string[], | ||
token?: CancellationToken, | ||
) { | ||
const options: TerminalCreationOptions = {}; | ||
if (isResource(resource)) { | ||
options.resource = resource; | ||
} else { | ||
options.interpreter = resource; | ||
} | ||
if (executeInTerminal) { | ||
const terminalService = this.serviceContainer | ||
.get<ITerminalServiceFactory>(ITerminalServiceFactory) | ||
.getTerminalService(options); | ||
|
||
terminalService.sendCommand(command, args, token); | ||
} else { | ||
const processServiceFactory = this.serviceContainer.get<IProcessServiceFactory>(IProcessServiceFactory); | ||
const processService = await processServiceFactory.create(options.resource); | ||
await processService.exec(command, args); | ||
} | ||
} | ||
} | ||
|
||
export function translateProductToModule(product: Product): string { | ||
|
@@ -204,6 +244,8 @@ export function translateProductToModule(product: Product): string { | |
return 'pip'; | ||
case Product.ensurepip: | ||
return 'ensurepip'; | ||
case Product.python: | ||
return 'python'; | ||
default: { | ||
throw new Error(`Product ${product} cannot be installed as a Python Module.`); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,7 @@ | |
|
||
import { inject, injectable } from 'inversify'; | ||
import { IServiceContainer } from '../../ioc/types'; | ||
import { ModuleInstallerType } from '../../pythonEnvironments/info'; | ||
import { EnvironmentType, ModuleInstallerType } from '../../pythonEnvironments/info'; | ||
import { IWorkspaceService } from '../application/types'; | ||
import { IPythonExecutionFactory } from '../process/types'; | ||
import { ExecutionInfo, IInstaller, Product } from '../types'; | ||
|
@@ -16,6 +16,26 @@ import { ProductNames } from './productNames'; | |
import { sendTelemetryEvent } from '../../telemetry'; | ||
import { EventName } from '../../telemetry/constants'; | ||
import { IInterpreterService } from '../../interpreter/contracts'; | ||
import { isParentPath } from '../platform/fs-paths'; | ||
|
||
async function doesEnvironmentContainPython(serviceContainer: IServiceContainer, resource: InterpreterUri) { | ||
const interpreterService = serviceContainer.get<IInterpreterService>(IInterpreterService); | ||
const environment = isResource(resource) ? await interpreterService.getActiveInterpreter(resource) : resource; | ||
if (!environment) { | ||
return undefined; | ||
} | ||
if ( | ||
environment.envPath?.length && | ||
environment.envType === EnvironmentType.Conda && | ||
!isParentPath(environment?.path, environment.envPath) | ||
) { | ||
// For conda environments not containing a python interpreter, do not use pip installer due to bugs in `conda run`: | ||
// https://github.com/microsoft/vscode-python/issues/18479#issuecomment-1044427511 | ||
// https://github.com/conda/conda/issues/11211 | ||
Comment on lines
+32
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is, |
||
return false; | ||
} | ||
return true; | ||
} | ||
|
||
@injectable() | ||
export class PipInstaller extends ModuleInstaller { | ||
|
@@ -36,7 +56,10 @@ export class PipInstaller extends ModuleInstaller { | |
constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) { | ||
super(serviceContainer); | ||
} | ||
public isSupported(resource?: InterpreterUri): Promise<boolean> { | ||
public async isSupported(resource?: InterpreterUri): Promise<boolean> { | ||
if ((await doesEnvironmentContainPython(this.serviceContainer, resource)) === false) { | ||
return false; | ||
} | ||
return this.isPipAvailable(resource); | ||
} | ||
protected async getExecutionInfo( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.path
is no longer unique.