Add support for detection and selection of conda environments lacking a python interpreter#18427
Conversation
71c441c to
37f0910
Compare
b1e8a5a to
39d7ecd
Compare
40fbc61 to
20a90f1
Compare
ccbf154 to
dd00f2c
Compare
| const interpreter = isResource(resource) | ||
| ? await interpreterService.getActiveInterpreter(resource) | ||
| : resource; | ||
| const pythonPath = isResource(resource) ? settings.pythonPath : resource.path; |
There was a problem hiding this comment.
.pythonPath can also be path to environment, so use interpreter.path.
|
|
||
| const pythonPath = isResource(resource) | ||
| ? this.serviceContainer.get<IConfigurationService>(IConfigurationService).getSettings(resource).pythonPath | ||
| : resource.path; |
|
|
||
| const pythonPath = isResource(resource) | ||
| ? this.serviceContainer.get<IConfigurationService>(IConfigurationService).getSettings(resource).pythonPath | ||
| : resource.path; |
| resource?: InterpreterUri, | ||
| cancel?: CancellationToken, | ||
| flags?: ModuleInstallFlags, | ||
| options?: InstallOptions, |
There was a problem hiding this comment.
Add support for installing modules as a process. Earlier we just sent the command in terminal.
…o condawithoutpython
| // 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 |
There was a problem hiding this comment.
That is, conda run -n <envname> python -m pip install <module> doesn't work.
| return true; | ||
| } | ||
|
|
||
| public async install( |
There was a problem hiding this comment.
Runs conda install -n <envname> python for environments lacking an interpreter, so they become normal conda environments.
| (file, args, opts) => procs.exec(file, args, opts), | ||
| (command, opts) => procs.shellExec(command, opts), | ||
| ); | ||
| return new PythonEnvironment(pythonPath, deps); |
There was a problem hiding this comment.
pythonPath can also be path to env folder, whereas the first parameter should refer to interpreter path.
| itemsWithFullName, | ||
| this.workspaceService.getWorkspaceFolder(resource)?.uri, | ||
| ); | ||
| if (recommended && arePathsSame(items[0].interpreter.path, recommended.interpreter.path)) { |
There was a problem hiding this comment.
.path is no longer unique so use .id.
| OtherVirtual = 'virt-other', | ||
| } | ||
|
|
||
| export interface EnvPathType { |
There was a problem hiding this comment.
Uniquely identifies an environment.
| return resolvedEnv; | ||
| } | ||
|
|
||
| async function getExecutablePathAndEnvPath(path: string) { |
There was a problem hiding this comment.
path can either be a python executable or env path.
kimadeline
left a comment
There was a problem hiding this comment.
I understand the broad context of these changes (and renaming many __interpreter__ functions to __environment__ to reflect the new support), but the intricacies of this PR flew above my head 😅
You already got Karthik's approval so I assume this PR is good to go, are there any specific changes I should focus on?
src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts
Outdated
Show resolved
Hide resolved
src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts
Outdated
Show resolved
Hide resolved
|
@kimadeline I see 😅 I think it's better to validate all the intricacies anyways. So I would like to merge this now so it can be tested properly for testing week. |
… a python interpreter (microsoft/vscode-python#18427) * Support conda environments without a python executable * Fix interpreter display * Fix areSameEnv * Add support to select env folders as interpreterPaths * Allow selecting such environments * Handle resolving environment path * Document resolver works for both env path and executable * Fxi bug * Simplify conda.ts * Define identifiers for an environment * Fix getCondaEnvironment * Introduce an id property to environments * Update proposed discovery API to handle environments * Fix bug with interpreter display * Normalize path passed in resolveEnv * Update environment details API * Dont use pythonPath for getting active interpreter for activation commands * Support conda activation * Support ${command:python.interpreterPath} with this envs * Fix getActiveItem * Add comment justifying using `.pythonPath` for middleware * Fix shebang codelens * Fix startup telemetry * Do not support pip installer for such environments * Trigger discovery once installation is finished for such environments * Automatically install python into environment once it is selected * Add telemetry for new interpreters discovered * Add telemtry if such an env is selected * Fix bugs and cache installer * Fix compile errors, tests, and add tests * Fix some tests * Ignore id when comparing envs * Phew, fixed terrible tests in module installer * Fix more tests * MOre * Skip resolver tests on linux * Fix tests * If environment doesn't contain python do not support pip installer * Skip module install tests for python as it is not a module * News entry * Remove unnecssary commnet * Fix bug introduced by merges * Code reviews
Closes #18357
Key changes:
ConfigService.pythonPathcan now also carry the path to the environment folder. So we remove uses of.pythonPathin favor of usingInterpreterService.getActiveInterpreterto get interpreter path instead.