From 45439aa8fbcdc93bc3cc13fd693285adb81d44d8 Mon Sep 17 00:00:00 2001 From: Pete <1178561+petetronic@users.noreply.github.com> Date: Tue, 18 Feb 2025 23:27:40 -0500 Subject: [PATCH] Exclude ipykernel native libs for macOS universal release (#6377) Fixing macOS releases. Skipping comparing files from the bundled ipykernel while preparing macOS universal release. To expedite fixing desktop macOS builds, it was decided to not try to bundle ipykernel for both arm64 AND x86 macOS. Wasim updated the PR so that we only bundle ipykernel for arm64 macOS for now. Intel x86 environments will need to continue install ipykernel on first use. ### Release Notes #### New Features - N/A #### Bug Fixes - N/A ### QA Notes Windows, Linux and MacOS arm64 should have ipykernel available in Positron python sessions, where as macOS x86 will still need ipykernel to be installed on first use. --------- Signed-off-by: Wasim Lorgat Co-authored-by: seem --- build/darwin/create-universal-app.js | 2 + build/darwin/create-universal-app.ts | 2 + extensions/positron-python/gulpfile.js | 32 ++++---- .../scripts/pip-compile-ipykernel.py | 2 +- .../src/client/positron/ipykernel.ts | 53 +++++++++---- .../src/client/positron/runtime.ts | 20 +++-- .../src/client/positron/session.ts | 77 ++++++------------- .../src/test/positron/session.unit.test.ts | 8 +- 8 files changed, 100 insertions(+), 96 deletions(-) diff --git a/build/darwin/create-universal-app.js b/build/darwin/create-universal-app.js index 5888cfa7e38..c1741cafa89 100644 --- a/build/darwin/create-universal-app.js +++ b/build/darwin/create-universal-app.js @@ -43,6 +43,8 @@ const stashPatterns = [ '**/html.icns', // Exclusions from Python language pack (positron-python) '**/pydevd/**', // Cython pre-built binaries for Python debugging + '**/lib/ipykernel/**', // Bundled IPyKernel dependencies + '**/lib/ipykernel/**/.dylibs/**', // Bundled IPyKernel dependency dylibs // Exclusions from R language pack (positron-r) '**/ark', // Compiled R kernel and LSP // Exclusions from Kallichore Jupyter supervisor diff --git a/build/darwin/create-universal-app.ts b/build/darwin/create-universal-app.ts index 0c44b70b423..c3213bb3e26 100644 --- a/build/darwin/create-universal-app.ts +++ b/build/darwin/create-universal-app.ts @@ -46,6 +46,8 @@ const stashPatterns = [ '**/html.icns', // Exclusions from Python language pack (positron-python) '**/pydevd/**', // Cython pre-built binaries for Python debugging + '**/lib/ipykernel/**', // Bundled IPyKernel dependencies + '**/lib/ipykernel/**/.dylibs/**', // Bundled IPyKernel dependency dylibs // Exclusions from R language pack (positron-r) '**/ark', // Compiled R kernel and LSP // Exclusions from Kallichore Jupyter supervisor diff --git a/extensions/positron-python/gulpfile.js b/extensions/positron-python/gulpfile.js index e8875b82f9e..b8b06c35fea 100644 --- a/extensions/positron-python/gulpfile.js +++ b/extensions/positron-python/gulpfile.js @@ -32,6 +32,10 @@ const isCI = process.env.TRAVIS === 'true' || process.env.TF_BUILD !== undefined // --- Start Positron --- const pythonCommand = locatePython(); +const arch = os.arch(); +if (arch !== 'x64' && arch !== 'arm64') { + throw new Error(`Unsupported architecture: ${arch}`); +} // --- End Positron --- gulp.task('compileCore', (done) => { @@ -279,25 +283,28 @@ async function vendorPythonKernelRequirements() { await spawnAsync(pythonCommand, ['scripts/vendor.py']); } -async function installIPyKernelPurePythonRequirements() { +async function bundleIPykernel() { + const pythonVersions = ['3.8', '3.9', '3.10', '3.11', '3.12', '3.13']; + const minimumPythonVersion = '3.8'; + + // Pure Python 3 requirements. await pipInstall([ '--target', './python_files/lib/ipykernel/py3', '--implementation', 'py', '--python-version', - '3.8', + minimumPythonVersion, '--abi', 'none', '-r', './python_files/ipykernel_requirements/py3-requirements.txt', ]); -} -async function installIPyKernelCPythonVersionAgnosticRequirements() { + // CPython 3 requirements (specific to platform and architecture). await pipInstall([ '--target', - './python_files/lib/ipykernel/cp3', + `./python_files/lib/ipykernel/${arch}/cp3`, '--implementation', 'cp', '--python-version', @@ -305,17 +312,16 @@ async function installIPyKernelCPythonVersionAgnosticRequirements() { '--abi', 'abi3', '-r', - './python_files/ipykernel_requirements/cp3-requirements.txt', + `./python_files/ipykernel_requirements/cp3-requirements.txt`, ]); -} -async function installIPyKernelCPythonVersionSpecificRequirements() { - for (const pythonVersion of ['3.8', '3.9', '3.10', '3.11', '3.12', '3.13']) { + // CPython 3.x requirements (specific to platform, architecture, and Python version). + for (const pythonVersion of pythonVersions) { const shortVersion = pythonVersion.replace('.', ''); const abi = `cp${shortVersion}`; await pipInstall([ '--target', - `./python_files/lib/ipykernel/${abi}`, + `./python_files/lib/ipykernel/${arch}/${abi}`, '--implementation', 'cp', '--python-version', @@ -328,12 +334,6 @@ async function installIPyKernelCPythonVersionSpecificRequirements() { } } -async function bundleIPykernel() { - await installIPyKernelPurePythonRequirements(); - await installIPyKernelCPythonVersionAgnosticRequirements(); - await installIPyKernelCPythonVersionSpecificRequirements(); -} - gulp.task( 'installPythonLibs', // Run in parallel since vendoring rewrites imports which is somewhat CPU-bound. diff --git a/extensions/positron-python/scripts/pip-compile-ipykernel.py b/extensions/positron-python/scripts/pip-compile-ipykernel.py index 0640b03f22e..1c5f7185c00 100644 --- a/extensions/positron-python/scripts/pip-compile-ipykernel.py +++ b/extensions/positron-python/scripts/pip-compile-ipykernel.py @@ -311,7 +311,7 @@ def write_output( main( requirement="ipykernel", python_versions=["3.8", "3.9", "3.10", "3.11", "3.12", "3.13"], - output_dir=Path("./python_files/lib/ipykernel/"), + output_dir=Path("./python_files/ipykernel_requirements/"), max_rounds=10, cache_dir=CACHE_DIR, ) diff --git a/extensions/positron-python/src/client/positron/ipykernel.ts b/extensions/positron-python/src/client/positron/ipykernel.ts index 1204925e5fd..346dd517089 100644 --- a/extensions/positron-python/src/client/positron/ipykernel.ts +++ b/extensions/positron-python/src/client/positron/ipykernel.ts @@ -3,25 +3,38 @@ * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. *--------------------------------------------------------------------------------------------*/ +import * as os from 'os'; +import * as path from 'path'; import * as vscode from 'vscode'; +import * as fs from '../common/platform/fs-paths'; import { IServiceContainer } from '../ioc/types'; import { PythonEnvironment } from '../pythonEnvironments/info'; import { IWorkspaceService } from '../common/application/types'; import { IPythonExecutionFactory } from '../common/process/types'; -import { traceVerbose } from '../logging'; +import { traceWarn } from '../logging'; +import { EXTENSION_ROOT_DIR } from '../constants'; + +/** IPyKernel bundle information. */ +export interface IPykernelBundle { + /** If bundling is disabled, the reason for it. */ + disabledReason?: string; + + /** Paths to be appended to the PYTHONPATH environment variable in this order, if bundling is enabled. */ + paths?: string[]; +} /** - * Check if an interpreter should use the bundled ipykernel. + * Get the IPyKernel bundle for a given interpreter. * * @param interpreter The interpreter to check. * @param serviceContainer The service container to use for dependency injection. * @param resource The resource to scope setting to. */ -export async function shouldUseBundledIpykernel( +export async function getIpykernelBundle( interpreter: PythonEnvironment, serviceContainer: IServiceContainer, resource?: vscode.Uri, -): Promise { +): Promise { // Get the required services. const workspaceService = serviceContainer.get(IWorkspaceService); const pythonExecutionFactory = serviceContainer.get(IPythonExecutionFactory); @@ -31,16 +44,13 @@ export async function shouldUseBundledIpykernel( .getConfiguration('python', resource) .get('useBundledIpykernel', true); if (!useBundledIpykernel) { - traceVerbose('createPythonRuntime: ipykernel bundling is disabled'); - return false; + return { disabledReason: 'useBundledIpykernel setting is disabled' }; } - traceVerbose('createPythonRuntime: ipykernel bundling is enabled, checking if interpreter is supported'); // Check if ipykernel is bundled for the interpreter version. // (defined in scripts/pip-compile-ipykernel.py). if (interpreter.version?.major !== 3 || ![8, 9, 10, 11, 12, 13].includes(interpreter.version?.minor)) { - traceVerbose(`createPythonRuntime: ipykernel not bundled for interpreter version: ${interpreter.version?.raw}`); - return false; + return { disabledReason: `unsupported interpreter version: ${interpreter.version?.raw}` }; } // Get the interpreter implementation if it's not already available. @@ -53,12 +63,25 @@ export async function shouldUseBundledIpykernel( // Check if ipykernel is bundled for the interpreter implementation. // (defined in scripts/pip-compile-ipykernel.py). if (implementation !== 'cpython') { - traceVerbose( - `createPythonRuntime: ipykernel not bundled for interpreter implementation: ${interpreter.implementation}`, - ); - return false; + return { disabledReason: `unsupported interpreter implementation: ${implementation}` }; + } + + // Append the bundle paths (defined in gulpfile.js) to the PYTHONPATH environment variable. + const arch = os.arch(); + const cpxSpecifier = `cp${interpreter.version.major}${interpreter.version.minor}`; + const paths = [ + path.join(EXTENSION_ROOT_DIR, 'python_files', 'lib', 'ipykernel', arch, cpxSpecifier), + path.join(EXTENSION_ROOT_DIR, 'python_files', 'lib', 'ipykernel', arch, 'cp3'), + path.join(EXTENSION_ROOT_DIR, 'python_files', 'lib', 'ipykernel', 'py3'), + ]; + + for (const path of paths) { + if (!(await fs.pathExists(path))) { + // This shouldn't happen. Did something go wrong during `npm install`? + traceWarn(`ipykernel bundle path does not exist: ${path}`); + return { disabledReason: `bundle path does not exist: ${path}` }; + } } - traceVerbose(`createPythonRuntime: ipykernel bundling is supported by interpreter: ${interpreter.path}`); - return true; + return { paths }; } diff --git a/extensions/positron-python/src/client/positron/runtime.ts b/extensions/positron-python/src/client/positron/runtime.ts index ff003e935aa..ba6a9cbb5ff 100644 --- a/extensions/positron-python/src/client/positron/runtime.ts +++ b/extensions/positron-python/src/client/positron/runtime.ts @@ -18,11 +18,11 @@ import { IInstaller, Product, ProductInstallStatus } from '../common/types'; import { IApplicationEnvironment, IWorkspaceService } from '../common/application/types'; import { EXTENSION_ROOT_DIR, IPYKERNEL_VERSION, PYTHON_LANGUAGE } from '../common/constants'; import { EnvLocationHeuristic, getEnvLocationHeuristic } from '../interpreter/configuration/environmentTypeComparer'; -import { shouldUseBundledIpykernel } from './ipykernel'; +import { getIpykernelBundle, IPykernelBundle } from './ipykernel'; export interface PythonRuntimeExtraData { pythonPath: string; - useBundledIpykernel?: boolean; + ipykernelBundle?: IPykernelBundle; } export async function createPythonRuntimeMetadata( @@ -43,20 +43,24 @@ export async function createPythonRuntimeMetadata( traceInfo('createPythonRuntime: getting extension runtime settings'); // Check if we should use the bundled ipykernel. - const useBundledIpykernel = await shouldUseBundledIpykernel(interpreter, serviceContainer, workspaceUri); + const ipykernelBundle = await getIpykernelBundle(interpreter, serviceContainer, workspaceUri); // Determine if a compatible version of ipykernel is available (either bundled or already installed). let hasCompatibleKernel: boolean; - if (useBundledIpykernel) { - hasCompatibleKernel = true; - } else { - traceInfo('createPythonRuntime: checking if ipykernel is installed'); + if (ipykernelBundle.disabledReason) { + traceInfo( + `createPythonRuntime: ipykernel bundling is disabled, ` + + `reason: ${ipykernelBundle.disabledReason}. ` + + `Checking if ipykernel is installed`, + ); const productInstallStatus = await installer.isProductVersionCompatible( Product.ipykernel, IPYKERNEL_VERSION, interpreter, ); hasCompatibleKernel = productInstallStatus === ProductInstallStatus.Installed; + } else { + hasCompatibleKernel = true; } // Define the startup behavior; request immediate startup if this is the @@ -116,7 +120,7 @@ export async function createPythonRuntimeMetadata( // runtime session. const extraRuntimeData: PythonRuntimeExtraData = { pythonPath: interpreter.path, - useBundledIpykernel, + ipykernelBundle, }; // Check the kernel supervisor's configuration; if it's configured to diff --git a/extensions/positron-python/src/client/positron/session.ts b/extensions/positron-python/src/client/positron/session.ts index 9ae7b81b8fe..88ac7733d57 100644 --- a/extensions/positron-python/src/client/positron/session.ts +++ b/extensions/positron-python/src/client/positron/session.ts @@ -8,7 +8,6 @@ // eslint-disable-next-line import/no-unresolved import * as positron from 'positron'; -import * as path from 'path'; import * as vscode from 'vscode'; import PQueue from 'p-queue'; import * as fs from '../common/platform/fs-paths'; @@ -34,13 +33,11 @@ import { IEnvironmentVariablesProvider, IEnvironmentVariablesService } from '../ import { PythonRuntimeExtraData } from './runtime'; import { JediLanguageServerAnalysisOptions } from '../activation/jedi/analysisOptions'; import { ILanguageServerOutputChannel } from '../activation/types'; -import { IApplicationShell, IWorkspaceService } from '../common/application/types'; +import { IWorkspaceService } from '../common/application/types'; import { IInterpreterService } from '../interpreter/contracts'; import { showErrorMessage } from '../common/vscodeApis/windowApis'; import { Console } from '../common/utils/localize'; -import { EXTENSION_ROOT_DIR } from '../constants'; -import { IPythonExecutionFactory } from '../common/process/types'; -import { shouldUseBundledIpykernel } from './ipykernel'; +import { getIpykernelBundle, IPykernelBundle } from './ipykernel'; /** * A Positron language runtime that wraps a Jupyter kernel and a Language Server @@ -74,9 +71,6 @@ export class PythonRuntimeSession implements positron.LanguageRuntimeSession, vs /** The current state of the runtime */ private _state: positron.RuntimeState = positron.RuntimeState.Uninitialized; - /** The service for managing application UI interactions */ - private _applicationShell: IApplicationShell; - /** The service for installing Python packages */ private _installer: IInstaller; @@ -89,9 +83,6 @@ export class PythonRuntimeSession implements positron.LanguageRuntimeSession, vs /** The service for managing environment variables */ private _envVarsService: IEnvironmentVariablesService; - /** The service for executing Python code */ - private _pythonExecutionFactory: IPythonExecutionFactory; - /** * Map of parent message IDs currently handled by IPyWidgets output widget comms, * keyed by comm ID. @@ -105,8 +96,8 @@ export class PythonRuntimeSession implements positron.LanguageRuntimeSession, vs /** The Python interpreter executable path */ private _pythonPath: string; - /** Whether to use the bundled ipykernel */ - private _useBundledIpykernel: boolean | undefined; + /** The IPykernel bundle paths */ + private _ipykernelBundle: IPykernelBundle | undefined; dynState: positron.LanguageRuntimeDynState; @@ -127,7 +118,7 @@ export class PythonRuntimeSession implements positron.LanguageRuntimeSession, vs throw new Error(`Runtime metadata missing Python path: ${JSON.stringify(runtimeMetadata)}`); } this._pythonPath = extraData.pythonPath; - this._useBundledIpykernel = extraData.useBundledIpykernel; + this._ipykernelBundle = extraData.ipykernelBundle; this._queue = new PQueue({ concurrency: 1 }); @@ -140,12 +131,10 @@ export class PythonRuntimeSession implements positron.LanguageRuntimeSession, vs this.onStateChange(state); }); - this._applicationShell = serviceContainer.get(IApplicationShell); this._installer = this.serviceContainer.get(IInstaller); this._interpreterService = serviceContainer.get(IInterpreterService); this._interpreterPathService = serviceContainer.get(IInterpreterPathService); this._envVarsService = serviceContainer.get(IEnvironmentVariablesService); - this._pythonExecutionFactory = serviceContainer.get(IPythonExecutionFactory); } onDidReceiveRuntimeMessage: vscode.Event; @@ -255,56 +244,38 @@ export class PythonRuntimeSession implements positron.LanguageRuntimeSession, vs } private async _setupIpykernel(interpreter: PythonEnvironment, kernelSpec: JupyterKernelSpec): Promise { - // _useBundledIpykernel may be undefined for a reticulate session, set it if needed. - if (this._useBundledIpykernel === undefined) { - this._useBundledIpykernel = await shouldUseBundledIpykernel(interpreter, this.serviceContainer); - } - // Use the bundled ipykernel if requested. - if (this._useBundledIpykernel) { - try { - await this._addIpykernelToPythonPath(interpreter, kernelSpec); - return; - } catch (err) { - this._applicationShell.showErrorMessage( - vscode.l10n.t( - 'Failed to use the bundled ipykernel, falling back to a manual installation. Reason: {0}', - (err as Error).message, - ), - ); - } - } + const didUseBundledIpykernel = await this._addBundledIpykernelToPythonPath(interpreter, kernelSpec); - // Install ipykernel for the interpreter. - await this._installIpykernel(interpreter); + // If the bundled ipykernel was not used, proceed to install ipykernel for the interpreter. + if (!didUseBundledIpykernel) { + await this._installIpykernel(interpreter); + } } - private async _addIpykernelToPythonPath( + private async _addBundledIpykernelToPythonPath( interpreter: PythonEnvironment, kernelSpec: JupyterKernelSpec, - ): Promise { - const pythonExecutionService = await this._pythonExecutionFactory.create({ pythonPath: interpreter.path }); - const info = await pythonExecutionService.getInterpreterInformation(); - if (!info || !info.implementation || !info.version) { - // This shouldn't happen. - throw new Error(`Unable to get interpreter information`); + ): Promise { + // _ipykernelBundlePaths may be undefined for a reticulate session, set it if needed. + if (!this._ipykernelBundle) { + this._ipykernelBundle = await getIpykernelBundle(interpreter, this.serviceContainer); + } + + if (!this._ipykernelBundle.paths) { + traceInfo(`Not using bundled ipykernel. Reason: ${this._ipykernelBundle.disabledReason}`); + return false; } - // Append the bundle paths (defined in gulpfile.js) to the PYTHONPATH environment variable. + traceInfo(`Using bundled ipykernel for interpreter: ${interpreter.path}`); if (!kernelSpec?.env) { kernelSpec.env = {}; } - const cpxSpecifier = `cp${info.version.major}${info.version.minor}`; - for (const specifier of [cpxSpecifier, 'cp3', 'py3']) { - const bundlePath = path.join(EXTENSION_ROOT_DIR, 'python_files', 'lib', 'ipykernel', specifier); - if (!(await fs.pathExists(bundlePath))) { - // This shouldn't happen. Did something go wrong during `npm install`? - throw new Error(`Bundled ipykernel dependencies not found at: ${bundlePath}`); - } - this._envVarsService.appendPythonPath(kernelSpec.env, bundlePath); + for (const path of this._ipykernelBundle.paths) { + this._envVarsService.appendPythonPath(kernelSpec.env, path); } - traceInfo(`Using bundled ipykernel for interpreter: ${interpreter.path}`); + return true; } private async _installIpykernel(interpreter: PythonEnvironment): Promise { diff --git a/extensions/positron-python/src/test/positron/session.unit.test.ts b/extensions/positron-python/src/test/positron/session.unit.test.ts index 92130c40c53..bb3604d8543 100644 --- a/extensions/positron-python/src/test/positron/session.unit.test.ts +++ b/extensions/positron-python/src/test/positron/session.unit.test.ts @@ -7,6 +7,7 @@ import * as assert from 'assert'; import { interfaces } from 'inversify'; +import * as os from 'os'; import * as path from 'path'; // eslint-disable-next-line import/no-unresolved import * as positron from 'positron'; @@ -231,13 +232,14 @@ suite('Python Runtime Session', () => { // Ipykernel bundles should be added to the PYTHONPATH. sinon.assert.callCount(envVarsServiceSpy.appendPythonPath, 3); + const arch = os.arch(); assert.deepStrictEqual(envVarsServiceSpy.appendPythonPath.args[0], [ kernelSpec.env, - path.join(EXTENSION_ROOT_DIR, 'python_files', 'lib', 'ipykernel', 'cp38'), + path.join(EXTENSION_ROOT_DIR, 'python_files', 'lib', 'ipykernel', arch, 'cp38'), ]); assert.deepStrictEqual(envVarsServiceSpy.appendPythonPath.args[1], [ kernelSpec.env, - path.join(EXTENSION_ROOT_DIR, 'python_files', 'lib', 'ipykernel', 'cp3'), + path.join(EXTENSION_ROOT_DIR, 'python_files', 'lib', 'ipykernel', arch, 'cp3'), ]); assert.deepStrictEqual(envVarsServiceSpy.appendPythonPath.args[2], [ kernelSpec.env, @@ -289,7 +291,7 @@ suite('Python Runtime Session', () => { sinon.assert.called(installerSpy.isProductVersionCompatible); }); - test('Start: dont bundle ipykernel on error', async () => { + test('Start: dont bundle if bundle path does not exist', async () => { // Simulate the bundle paths not existing. sinon.stub(fs, 'pathExists').resolves(false);