Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import * as path from 'path';

export const ENVS_EXTENSION_ID = 'ms-python.vscode-python-envs';
export const PYTHON_EXTENSION_ID = 'ms-python.python';
export const CONDA_MANAGER_ID = `${PYTHON_EXTENSION_ID}:conda`;
export const INLINE_SCRIPT_MANAGER_ID = `${PYTHON_EXTENSION_ID}:inline-script`;
export const JUPYTER_EXTENSION_ID = 'ms-toolsai.jupyter';
export const EXTENSION_ROOT_DIR = path.dirname(__dirname);
export const ISSUES_URL = 'https://github.com/microsoft/vscode-python-environments/issues';
Expand Down
45 changes: 41 additions & 4 deletions src/common/inlineScriptCacheKey.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,41 @@ function normalizeExtras(inner: string): string {
return deduped.length > 0 ? `[${deduped.join(',')}]` : '';
}

/** ` >= 2 ; python_version < "3.13"` becomes `>=2 ; python_version<"3.13"`. */
function normalizeRequirementTail(value: string): string {
Comment thread
StellaHuang95 marked this conversation as resolved.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not entirely sure, but I think this could be further simplified with some regexes rather than iterating over the characters. If not, this looks good to me :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking to address the feedback while getting 5c pr ready. I considered this, but the loop is intended to be quotes aware. It normalizes whitespace outside marker literals while preserving it inside quoted values, and include quotes. It's difficult to get regex to do that. I'd keep it this way for now.

let result = '';
let unquoted = '';
let quote: "'" | '"' | undefined;
let escaped = false;

const flushUnquoted = () => {
result += unquoted.replace(/\s+/g, ' ').replace(/\s*([<>=!~]=?)\s*/g, '$1');
Comment thread
StellaHuang95 marked this conversation as resolved.
unquoted = '';
};

// Preserve PEP 508 marker literals verbatim; a backslash escapes the next character.
for (const character of value) {
if (quote) {
result += character;
if (character === quote && !escaped) {
quote = undefined;
}
escaped = character === '\\' && !escaped;
if (character !== '\\') {
escaped = false;
}
} else if (character === "'" || character === '"') {
flushUnquoted();
quote = character;
result += character;
} else {
unquoted += character;
}
}
flushUnquoted();
return result.trim();
}

/**
* Canonicalize a PEP 723 dependency entry so common variants of the same
* requirement produce identical strings. Not a full PEP 508 parser — only
Expand Down Expand Up @@ -70,10 +105,12 @@ export function normalizeDependency(dep: string): string {
rest = rest.slice(extrasMatch[0].length);
}

const compactedRest = rest
.replace(/\s+/g, ' ')
.replace(/\s*([<>=!~]=?)\s*/g, '$1')
.trim();
const directReference = rest.trim();
if (directReference.startsWith('@')) {
return `${name}${extras} ${directReference}`;
}

const compactedRest = normalizeRequirementTail(rest);

return `${name}${extras}${compactedRest}`;
}
Expand Down
152 changes: 108 additions & 44 deletions src/common/inlineScriptCacheLayout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,14 @@ import * as crypto from 'crypto';
import * as fsapi from 'fs-extra';
import * as path from 'path';
import { Uri } from 'vscode';
import type { PythonEnvironment } from '../api';
import { INLINE_SCRIPT_MANAGER_ID } from './constants';
import { traceWarn } from './logging';
import { normalizePath } from './utils/pathUtils';
import { isWindows } from './utils/platformUtils';
import { getVenvPythonPath } from './utils/virtualEnvironment';

/**
* Versioned name of the cache root under the extension's `globalStorageUri`.
*
* Bump the `-v1` suffix together with {@link META_SCHEMA_VERSION} on any
* incompatible on-disk change, so old envs sit unread and TTL out naturally
* instead of being migrated in place.
*/
/** Bump this and {@link META_SCHEMA_VERSION} together for incompatible cache formats. */
export const INLINE_SCRIPT_CACHE_DIR_NAME = 'script-envs-v1';

export const META_JSON_FILENAME = '.meta.json';
Expand All @@ -33,14 +31,21 @@ const MAX_META_JSON_BYTES = 1024 * 1024;
export interface InlineScriptEnvMeta {
/** Version of the serialized metadata schema. */
readonly schemaVersion: typeof META_SCHEMA_VERSION;
/** Filesystem path of the script recorded for lifecycle bookkeeping. */
readonly scriptFsPath: string;
/** Canonical base-interpreter path. */
readonly baseInterpreterPath: string;
/** Base-interpreter version. */
readonly baseInterpreterVersion: string;
/** Last successful use as a canonical UTC string produced by `Date.toISOString()`. */
readonly lastUsedAt: string;
/** The script's `requires-python` declaration, when present. */
readonly requiresPython?: string;
}

export type InlineScriptMetaReadResult =
| { readonly kind: 'valid'; readonly metadata: InlineScriptEnvMeta }
| { readonly kind: 'missing' | 'invalid' | 'unavailable' };

export type BaseInterpreterStatus = 'available' | 'missing' | 'unavailable';
export type CacheEnvironmentInspection = 'expected' | 'stale' | 'uncertain';
Comment thread
StellaHuang95 marked this conversation as resolved.

/**
* In-memory summary of one cached entry, populated by the separate disk walk.
*/
Expand All @@ -63,36 +68,77 @@ export function getMetaJsonPath(envDir: Uri): Uri {
return Uri.joinPath(envDir, META_JSON_FILENAME);
}

/**
* Read and validate the extension-owned `.meta.json` sidecar in a cached
* environment directory.
*
* This function is intentionally non-destructive. Missing, malformed, or
* unreadable sidecars return `undefined` so callers can treat the entry as a
* cache miss. Deleting invalid entries belongs to the dedicated, guarded cache
* cleanup path because read and permission failures may be transient.
*/
/** Resolve a cache entry only when it is the requested direct child of the physical cache root. */
export async function resolveCacheEntryPath(cacheRoot: Uri, envDir: Uri): Promise<string | undefined> {
const [resolvedRoot, resolvedEntry] = await Promise.all([
fsapi.realpath(cacheRoot.fsPath),
fsapi.realpath(envDir.fsPath),
]);
const expectedEntry = path.join(resolvedRoot, path.basename(envDir.fsPath));
return isDescendantPath(resolvedRoot, resolvedEntry) &&
normalizePath(path.resolve(resolvedEntry)) === normalizePath(path.resolve(expectedEntry))
? resolvedEntry
: undefined;
}

/** Verify that a resolved environment is owned by the expected physical cache entry. */
export async function inspectOwnedCacheEntry(
environment: PythonEnvironment,
cacheRoot: Uri,
envDir: Uri,
): Promise<CacheEnvironmentInspection> {
if (environment.envId.managerId !== INLINE_SCRIPT_MANAGER_ID) {
return 'uncertain';
}
try {
const [expectedDir, resolvedPrefix, expectedPython, resolvedPython] = await Promise.all([
resolveCacheEntryPath(cacheRoot, envDir),
fsapi.realpath(environment.sysPrefix),
fsapi.realpath(getVenvPythonPath(envDir.fsPath)),
fsapi.realpath(environment.environmentPath.fsPath),
]);
if (!expectedDir) {
return 'uncertain';
}
return normalizePath(expectedDir) === normalizePath(resolvedPrefix) &&
normalizePath(expectedPython) === normalizePath(resolvedPython)
? 'expected'
: 'stale';
} catch (error) {
traceWarn('inline-script env: failed to inspect cache-entry ownership:', error);
return 'uncertain';
}
}

/** Read validated sidecar metadata, returning `undefined` for non-valid state. */
export async function readMetaJson(envDir: Uri): Promise<InlineScriptEnvMeta | undefined> {
const result = await inspectMetaJson(envDir);
return result.kind === 'valid' ? result.metadata : undefined;
}

/** Classify sidecar state; only `unavailable` denotes transient I/O. */
export async function inspectMetaJson(envDir: Uri): Promise<InlineScriptMetaReadResult> {
const metaPath = getMetaJsonPath(envDir).fsPath;

try {
const stat = await fsapi.stat(metaPath);
const stat = await fsapi.lstat(metaPath);
if (!stat.isFile()) {
traceWarn(`inline-script meta: not a regular file at ${metaPath}`);
return undefined;
return { kind: 'invalid' };
}
if (stat.size > MAX_META_JSON_BYTES) {
traceWarn(`inline-script meta: refusing to read ${metaPath} (${stat.size} bytes > cap)`);
return undefined;
return { kind: 'invalid' };
}
} catch (err) {
if (isFileNotFoundError(err)) {
traceWarn(`inline-script meta: not found at ${metaPath}`);
return { kind: 'missing' };
} else {
const code = (err as NodeJS.ErrnoException | undefined)?.code ?? 'unknown';
traceWarn(`inline-script meta: failed to stat ${metaPath} (code=${code}):`, err);
return { kind: 'unavailable' };
}
return undefined;
}

let raw: string;
Expand All @@ -101,23 +147,23 @@ export async function readMetaJson(envDir: Uri): Promise<InlineScriptEnvMeta | u
} catch (err) {
const code = (err as NodeJS.ErrnoException | undefined)?.code ?? 'unknown';
traceWarn(`inline-script meta: failed to read ${metaPath} (code=${code}):`, err);
return undefined;
return { kind: isFileNotFoundError(err) ? 'missing' : 'unavailable' };
}

let parsed: unknown;
try {
parsed = JSON.parse(raw);
} catch (err) {
traceWarn(`inline-script meta: malformed JSON in ${metaPath}:`, err);
return undefined;
return { kind: 'invalid' };
}

const validated = validateMeta(parsed);
if (!validated) {
traceWarn(`inline-script meta: invalid shape in ${metaPath}`);
return undefined;
return { kind: 'invalid' };
}
return validated;
return { kind: 'valid', metadata: validated };
}

/**
Expand Down Expand Up @@ -160,53 +206,60 @@ export function selectStaleEntries(entries: ReadonlyArray<CacheEntrySummary>, no
* Verify that a cached env's base interpreter still exists on disk.
*/
export async function verifyBaseInterpreterExists(envDir: Uri): Promise<boolean> {
return isWindows() ? verifyWindowsBaseInterpreter(envDir) : verifyPosixBaseInterpreter(envDir);
return (await getBaseInterpreterStatus(envDir)) === 'available';
}

async function verifyPosixBaseInterpreter(envDir: Uri): Promise<boolean> {
/** Classify the base interpreter; `unavailable` denotes transient I/O. */
export async function getBaseInterpreterStatus(envDir: Uri): Promise<BaseInterpreterStatus> {
return isWindows() ? getWindowsBaseInterpreterStatus(envDir) : getPosixBaseInterpreterStatus(envDir);
}
Comment thread
StellaHuang95 marked this conversation as resolved.

async function getPosixBaseInterpreterStatus(envDir: Uri): Promise<BaseInterpreterStatus> {
const launcherPath = Uri.joinPath(envDir, 'bin', 'python').fsPath;
return isRegularFile(launcherPath, 'base interpreter');
return getRegularFileStatus(launcherPath, 'base interpreter');
}

async function verifyWindowsBaseInterpreter(envDir: Uri): Promise<boolean> {
async function getWindowsBaseInterpreterStatus(envDir: Uri): Promise<BaseInterpreterStatus> {
const pyvenvPath = Uri.joinPath(envDir, 'pyvenv.cfg').fsPath;
let raw: string;
try {
raw = await fsapi.readFile(pyvenvPath, 'utf8');
} catch (err) {
if (isFileNotFoundError(err)) {
traceWarn(`inline-script env: missing pyvenv.cfg at ${pyvenvPath}`);
return 'missing';
} else {
const code = (err as NodeJS.ErrnoException | undefined)?.code ?? 'unknown';
traceWarn(`inline-script env: failed to read ${pyvenvPath} (code=${code}):`, err);
return 'unavailable';
}
return false;
}
const home = parsePyvenvHome(raw);
if (home === undefined) {
traceWarn(`inline-script env: no 'home =' line in ${pyvenvPath}`);
return false;
return 'missing';
}
const launcherPath = path.join(home, 'python.exe');
return isRegularFile(launcherPath, 'base interpreter');
return getRegularFileStatus(launcherPath, 'base interpreter');
}

async function isRegularFile(filePath: string, label: string): Promise<boolean> {
async function getRegularFileStatus(filePath: string, label: string): Promise<BaseInterpreterStatus> {
try {
const stat = await fsapi.stat(filePath);
if (!stat.isFile()) {
traceWarn(`inline-script env: ${label} is not a regular file at ${filePath}`);
return false;
return 'missing';
}
return true;
return 'available';
} catch (err) {
if (isFileNotFoundError(err)) {
traceWarn(`inline-script env: ${label} missing at ${filePath}`);
return 'missing';
} else {
const code = (err as NodeJS.ErrnoException | undefined)?.code ?? 'unknown';
traceWarn(`inline-script env: failed to stat ${filePath} (code=${code}):`, err);
return 'unavailable';
}
return false;
}
}

Expand All @@ -224,6 +277,17 @@ function isFileNotFoundError(err: unknown): boolean {
return typeof err === 'object' && err !== null && 'code' in err && (err as NodeJS.ErrnoException).code === 'ENOENT';
}

function isDescendantPath(rootPath: string, candidatePath: string): boolean {
const relative = path.relative(rootPath, candidatePath);
return (
relative.length > 0 && relative !== '..' && !relative.startsWith(`..${path.sep}`) && !path.isAbsolute(relative)
);
}
Comment on lines +280 to +285

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we could see if there are other places we could use this in, and put it on an utils file or something similar?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

venv and pip utils have something similar, but for different purposes and different behavior. This one rejects the root itself on purpose because we don't want to accidentally touch the root of the cached envs and modify every cached envs.


function isNonEmptyTrimmedString(value: unknown): value is string {
return typeof value === 'string' && value.length > 0 && value.trim() === value;
}

function validateMeta(value: unknown): InlineScriptEnvMeta | undefined {
if (typeof value !== 'object' || value === null || Array.isArray(value)) {
return undefined;
Expand All @@ -232,21 +296,21 @@ function validateMeta(value: unknown): InlineScriptEnvMeta | undefined {
if (obj.schemaVersion !== META_SCHEMA_VERSION) {
return undefined;
}
if (typeof obj.scriptFsPath !== 'string' || obj.scriptFsPath.length === 0) {
if (!isNonEmptyTrimmedString(obj.baseInterpreterPath) || !path.isAbsolute(obj.baseInterpreterPath)) {
return undefined;
}
if (!isCanonicalIsoTimestamp(obj.lastUsedAt)) {
if (!isNonEmptyTrimmedString(obj.baseInterpreterVersion)) {
return undefined;
}
if (obj.requiresPython !== undefined && typeof obj.requiresPython !== 'string') {
if (!isCanonicalIsoTimestamp(obj.lastUsedAt)) {
return undefined;
}

return {
schemaVersion: META_SCHEMA_VERSION,
scriptFsPath: obj.scriptFsPath,
baseInterpreterPath: obj.baseInterpreterPath,
baseInterpreterVersion: obj.baseInterpreterVersion,
lastUsedAt: obj.lastUsedAt,
requiresPython: obj.requiresPython,
};
}

Expand Down
3 changes: 2 additions & 1 deletion src/common/inlineScriptInterpreter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ export function pickCompatibleInterpreter(
installed: ReadonlyArray<PythonEnvironment>,
requiresPython: string | undefined,
): PythonEnvironment | undefined {
const constraint = requiresPython && requiresPython.length > 0 ? requiresPython : undefined;
const trimmedConstraint = requiresPython?.trim();
const constraint = trimmedConstraint ? trimmedConstraint : undefined;
Comment thread
StellaHuang95 marked this conversation as resolved.
const candidates = installed.filter((env) => isUsableBaseInterpreter(env, constraint));
if (candidates.length === 0) {
return undefined;
Expand Down
Loading
Loading