-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Modify environment info worker to support new type and to work with resolver #13997
Changes from 7 commits
07dac60
b3e4222
d31f823
87d84e6
148aaae
6cf5a3e
eece52a
b5ce54a
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,89 @@ | ||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT License. | ||
|
||
import { PythonExecutableInfo, PythonVersion } from '.'; | ||
import { interpreterInfo as getInterpreterInfoCommand, PythonEnvInfo } from '../../../common/process/internal/scripts'; | ||
import { Architecture } from '../../../common/utils/platform'; | ||
import { copyPythonExecInfo, PythonExecInfo } from '../../exec'; | ||
import { parseVersion } from './pythonVersion'; | ||
|
||
export type InterpreterInformation = { | ||
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. There's already an Not part of this PR, but we also have multiple Up to you. 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. We'll be getting rid of the old I'll take care of the |
||
arch: Architecture; | ||
executable: PythonExecutableInfo; | ||
version: PythonVersion; | ||
}; | ||
|
||
/** | ||
* Compose full interpreter information based on the given data. | ||
* | ||
* The data format corresponds to the output of the `interpreterInfo.py` script. | ||
* | ||
* @param python - the path to the Python executable | ||
* @param raw - the information returned by the `interpreterInfo.py` script | ||
*/ | ||
function extractInterpreterInfo(python: string, raw: PythonEnvInfo): InterpreterInformation { | ||
const rawVersion = `${raw.versionInfo.slice(0, 3).join('.')}-${raw.versionInfo[3]}`; | ||
return { | ||
arch: raw.is64Bit ? Architecture.x64 : Architecture.x86, | ||
executable: { | ||
filename: python, | ||
sysPrefix: raw.sysPrefix, | ||
mtime: -1, | ||
ctime: -1, | ||
}, | ||
version: { | ||
...parseVersion(rawVersion), | ||
sysVersion: raw.sysVersion, | ||
}, | ||
}; | ||
} | ||
|
||
type ShellExecResult = { | ||
stdout: string; | ||
stderr?: string; | ||
}; | ||
type ShellExecFunc = (command: string, timeout: number) => Promise<ShellExecResult>; | ||
|
||
type Logger = { | ||
info(msg: string): void; | ||
error(msg: string): void; | ||
}; | ||
|
||
/** | ||
* Collect full interpreter information from the given Python executable. | ||
* | ||
* @param python - the information to use when running Python | ||
* @param shellExec - the function to use to exec Python | ||
* @param logger - if provided, used to log failures or other info | ||
*/ | ||
export async function getInterpreterInfo( | ||
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. Most of it is copied from |
||
python: PythonExecInfo, | ||
shellExec: ShellExecFunc, | ||
logger?: Logger, | ||
): Promise<InterpreterInformation | undefined> { | ||
const [args, parse] = getInterpreterInfoCommand(); | ||
const info = copyPythonExecInfo(python, args); | ||
const argv = [info.command, ...info.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 shellExec(quoted, 15000); | ||
if (result.stderr) { | ||
if (logger) { | ||
logger.error(`Failed to parse interpreter information for ${argv} stderr: ${result.stderr}`); | ||
} | ||
return undefined; | ||
} | ||
const json = parse(result.stdout); | ||
if (logger) { | ||
logger.info(`Found interpreter for ${argv}`); | ||
} | ||
return extractInterpreterInfo(python.pythonExecutable, json); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT License. | ||
|
||
import { PythonReleaseLevel, PythonVersion } from '.'; | ||
import { EMPTY_VERSION, parseBasicVersionInfo } from '../../../common/utils/version'; | ||
|
||
export function parseVersion(versionStr: string): PythonVersion { | ||
const parsed = parseBasicVersionInfo<PythonVersion>(versionStr); | ||
if (!parsed) { | ||
if (versionStr === '') { | ||
return EMPTY_VERSION as PythonVersion; | ||
} | ||
throw Error(`invalid version ${versionStr}`); | ||
} | ||
const { version, after } = parsed; | ||
const match = after.match(/^(a|b|rc)(\d+)$/); | ||
if (match) { | ||
const [, levelStr, serialStr] = match; | ||
let level: PythonReleaseLevel; | ||
if (levelStr === 'a') { | ||
level = PythonReleaseLevel.Alpha; | ||
} else if (levelStr === 'b') { | ||
level = PythonReleaseLevel.Beta; | ||
} else if (levelStr === 'rc') { | ||
level = PythonReleaseLevel.Candidate; | ||
} else { | ||
throw Error('unreachable!'); | ||
} | ||
version.release = { | ||
level, | ||
serial: parseInt(serialStr, 10), | ||
}; | ||
} | ||
return version; | ||
} |
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.
The JSON expected in the code contains
sysVersion
field.vscode-python/src/client/common/process/internal/scripts/index.ts
Lines 44 to 63 in 23725ab
This bug existed since 3 years ago lol, when we first added
interpreterInfo.py
.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.
How come it hadn't been caught until now, we never needed this field?
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.
Looks like we actually don't. I'll create a separate issue to remove it.