Fall back to an installed kernel when the bundled one cannot run - #526
Draft
samclark2015 wants to merge 1 commit into
Draft
Fall back to an installed kernel when the bundled one cannot run#526samclark2015 wants to merge 1 commit into
samclark2015 wants to merge 1 commit into
Conversation
A bundled kernel is built for a platform, not for every system it can be installed on. One linked against newer shared libraries than the host provides is exec'd successfully and then rejected by the dynamic linker, so it passes every filesystem check discovery makes and still cannot serve a session. On the Linux builds this is the common case rather than an edge one: the kernels are built on Ubuntu 24.04 and need GLIBC_2.39, while Positron supports back to Ubuntu 20.04 and RHEL 9. Run the bundled kernel before offering it, and put the host locations behind it as a fallback tier. selectKernelCandidates() now returns a KernelSelection carrying that tier as a callback, so the common case -- a bundled kernel that runs -- never pays for the PATH lookup. Only the bundled kernel is probed; a kernel the user installed is taken at its word. A success is cached against the extension version, keeping it to one spawn per update; a failure is not, so a host that gains the missing libraries starts working without waiting for an update. The Jupyter kernel spec is written only for a kernel that passed, because it outlives the window, is what Quarto resolves, and has no fallback. A fallback that succeeds stays silent: the runtime's name in the picker already discloses where it came from. Only the dead end interrupts -- nothing runnable anywhere, whether the bundled kernel failed or the build carries none -- with one non-modal notice per extension version. ggsql-jupyter gains --version, which the probe uses and which had no way to be asked before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #524, which is stacked on #523.
Problem
The bundled Linux kernels are built on
ubuntu-latest(24.04) andubuntu-24.04-arm, which puts a glibc 2.39 floor on them. Measured on the shippedggsql_0.4.1_amd64.debwithobjdump -p(real.gnu.version_rentries, both arches identical):Positron supports Ubuntu 20+ and RHEL 9+, and Remote SSH / Posit Workbench on RHEL 8/9/10 + Ubuntu 22/24. So the bundled kernel does not run on Ubuntu 20.04, 22.04, RHEL 8, RHEL 9, Debian 12, Amazon Linux 2023 or SLES 15.
Nothing catches it.
execvesucceeds — the ELF is valid and the interpreter exists — so this is notENOENT; the dynamic linker then fails and the process exits non-zero.isKernelAccessible()isstat+X_OK, which cannot see that. So the extension registers a runtime, and becausebundledis the default strategy and wins outright, that dead runtime shadows a working kernel onPATH— a regression for anyone on those distros who has one today.Rebuilding against an older glibc is the real fix and is not in this PR. This makes the extension degrade correctly regardless.
Change
probeKernel()execsggsql-jupyter --versionwith a 15s timeout and requires exit 0. Only theBundledcandidate is probed — a kernel the user installed keeps today's stat-only treatment.ggsql-jupyternow accepts--version. The clapArgsstruct had no#[command(version)], so--versionexited non-zero on a healthy binary — a--versionprobe would have rejected every kernel on every platform. Useful on its own.selectKernelCandidates()returns aKernelSelection { strategy, candidates, fallback }. Underbundledthe host lookup is afallback()callback rather than a flat list, so the common case never pays for thewhich/whereshell-out.globalStateagainst the extension version: one spawn per update, not per window. A failure is not cached — it is cheap to repeat, and a host that gains the missing libraries starts working without waiting for an update.writeKernelJson()ran forBundledbefore any session started, so a broken kernel poisoned the user kernelspec directory for Quarto and plain Jupyter. It now runs only for a candidate that passed.Prompting
No prompt on fallback, by agreement. Discovery runs on every window open, before the user has shown any interest in ggsql; and if the bundled kernel cannot exec and a host one can, there is no trade-off for the user to weigh. The runtime picker already discloses the source (plain
ggsqlfor bundled vsggsql (Path)/ggsql (System)), and the log records the handover.The one case that interrupts is the dead end — nothing runnable anywhere.
reportNoUsableKernel()shows a non-modal warning once per extension version withInstall ggsqlandShow Log. Its wording branches so it also covers a build that carries no kernel at all, which is thewin32-arm64VSIX added in #524. It is skipped under thepathstrategy, where the user named a binary and the log already reports it.Verification
cargo build --release --package ggsql-jupyter./target/release/ggsql-jupyter --versionggsql-jupyter 0.4.1, exit 0cargo fmt --all --checkcargo clippy --package ggsql-jupyter --all-targetsnpx tsc --noEmit -p .npm run lintnpm testNew tests cover: bundled fails the probe and a host kernel takes over; bundled fails with nothing else present (zero runtimes, notification path); bundled passes and the host lookup is never performed; probe memoization; and
probeKerneldirectly against real binaries.Two honest caveats:
npm testneedslaunchArgs: ['--user-data-dir', '/tmp/vsct-ud']in.vscode-test.mjsto run from this worktree — the path makes the VS Code IPC socket exceed macOS's 103-char limit. Environmental, not a code failure; the change was reverted before committing and CI runs from a short path.RuntimeManagerOptions.probewas added as a test seam beyond the original plan. The existing test stubs write#!/bin/shscripts namedggsql-jupyter.exe, which cannot be spawned on Windows, so a real probe by default would have broken thewindows-latestleg oftest-extension.yaml. It follows the existingkernelSpecDirseam.Minor note:
--versionemits a tracing line on stderr before clap parses args, since tracing is initialised first inmain. Harmless — the probe reads only the exit code — but relevant if anyone later parses the output.🤖 Generated with Claude Code