Use conda run to validate conda interpreters - #18128
Conversation
621d778 to
ee27f7f
Compare
ee27f7f to
06ffbf8
Compare
2f09a8c to
6f33e1d
Compare
| // Note: Single quotes maybe converted to double quotes for printing purposes. | ||
| let commandList: string[]; | ||
| if (!args) { | ||
| // It's a quoted command. |
There was a problem hiding this comment.
Command args are no longer guaranteed to be quoted, for eg. conda, -p etc. are not quoted below:
conda run -p "~\OneDrive - Microsoft\Desktop\vscode-python\.venv" --no-capture-output python "~\OneDrive - Microsoft\Desktop\vscode-python\pythonFiles\interpreterInfo.py"
So no longer rely on this logic.
|
|
||
| private getDisplayCommands(command: string): string { | ||
| if (this.workspaceService.workspaceFolders && this.workspaceService.workspaceFolders.length === 1) { | ||
| command = replaceMatchesWithCharacter(command, this.workspaceService.workspaceFolders[0].uri.fsPath, '.'); |
There was a problem hiding this comment.
Replace workspace folder path with .
| } | ||
| const home = getUserHomeDir(); | ||
| if (home) { | ||
| command = replaceMatchesWithCharacter(command, home, '~'); |
There was a problem hiding this comment.
Find traces of path to home in command and replace it with ~.
|
|
||
| // Concat these together to make a set of quoted strings | ||
| const quoted = argv.reduce((p, c) => (p ? `${p} "${c}"` : `"${c}"`), ''); | ||
| const quoted = argv.reduce((p, c) => (p ? `${p} ${c.toCommandArgument()}` : `${c.toCommandArgument()}`), ''); |
There was a problem hiding this comment.
This ensures flags like -n/-p are not quoted.
karrtikr
changed the title
| } | ||
| // Use a bogus version, at least to indicate the fact that a version was returned. | ||
| traceWarn(`Unable to parse Version of Conda, ${versionString}`); | ||
| return new SemVer('0.0.1'); |
There was a problem hiding this comment.
Why did you choose to return a valid object with an invalid value, instead of undefined, an error or throwing an exception?
There was a problem hiding this comment.
I just moved this method here from condaService.ts. As to why it was previously done, it is stated in the comment above that we want to distinguish it from the case where no version string is found.
I guess it makes sense to assume it's a really old version when the version string isn't parsable, hence 0.0.1
There was a problem hiding this comment.
we want to distinguish it from the case where no version string is found.
What do we need this for?
There was a problem hiding this comment.
This is existing logic from condaService.ts so I don't really know what it was needed for originally 🤷♂️
But my guess is, we still want to use conda related commands for activation, even though conda --version returns incorrect/unparsable version string (atleast it returned something). Returning undefined would mean we're not using conda at all.
kimadeline
left a comment
kimadeline
left a comment
There was a problem hiding this comment.
Most of it looks fine to me, the only part I don't understand is the need to return a 0.0.1 invalid version, but this can be addressed separately since it's not blocking and there's already a 2nd PR planned anyway.
…#18128) * Use conda run to validate conda interpreters * Add tests * Add news entry * Fix tests * Add comment to clarify * Use traceError instead of traceWarn
2 participants
Closes #17941