Skip to content

Implements a venv linker#274

Merged
arcanis merged 1 commit intomainfrom
mael/venv-linker
Apr 8, 2026
Merged

Implements a venv linker#274
arcanis merged 1 commit intomainfrom
mael/venv-linker

Conversation

@arcanis
Copy link
Copy Markdown
Member

@arcanis arcanis commented Apr 7, 2026

Make islands able to link Python dependencies into their own venv. Still experimental (for example it doesn't yet supports package conditions during resolution).


Note

Medium Risk
Adds a new island linker that mutates workspace filesystem state under .venv and changes local-path fetching to accept absolute paths, which could affect install/link behavior across platforms. While scoped to islands, it touches core linking/fetcher code paths and introduces new resolution constraints (one version per ident) with Unsupported errors on conflicts.

Overview
Adds a new per-island venv linker that installs an island’s resolved (non-workspace) dependency set into each workspace’s .venv/lib/site-packages by symlinking local packages or extracting cached zips, and wires it into the main linking pipeline.

Introduces a new yarn python command that mirrors yarn node but, when run from a workspace in a venv island, sets VIRTUAL_ENV, prepends .venv bin/Scripts to PATH, and prepends the workspace site-packages to PYTHONPATH (with python3 fallback).

Updates config (schema.json + IslandLinker) to accept linker: "venv" for islands, and adjusts folder:, tarball:, and link: fetchers to treat absolute paths as already-resolved (instead of always resolving relative to the parent package). Adds acceptance tests and fixtures validating venv installs, mixed PnP+venv workspaces, and link/tarball/folder sources for Python imports.

Reviewed by Cursor Bugbot for commit 4f7e244. Bugbot is set up for automated code reviews on this repo. Configure here.

@arcanis
Copy link
Copy Markdown
Member Author

arcanis commented Apr 7, 2026

@cursor review

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 7, 2026

⏱️ Benchmark Results

gatsby install-full-cold

Metric Base Head Difference
Mean 2.621s 2.636s +0.56% ⚠️
Median 2.613s 2.622s +0.37% ⚠️
Min 2.536s 2.556s
Max 2.705s 2.793s
Std Dev 0.038s 0.058s
📊 Raw benchmark data (gatsby install-full-cold)

Base times: 2.579s, 2.626s, 2.609s, 2.601s, 2.595s, 2.634s, 2.613s, 2.676s, 2.603s, 2.621s, 2.585s, 2.615s, 2.596s, 2.630s, 2.612s, 2.603s, 2.599s, 2.584s, 2.604s, 2.613s, 2.607s, 2.536s, 2.657s, 2.686s, 2.626s, 2.614s, 2.625s, 2.675s, 2.705s, 2.703s

Head times: 2.793s, 2.767s, 2.640s, 2.602s, 2.630s, 2.582s, 2.619s, 2.604s, 2.606s, 2.624s, 2.629s, 2.597s, 2.667s, 2.635s, 2.660s, 2.620s, 2.774s, 2.582s, 2.633s, 2.621s, 2.639s, 2.609s, 2.582s, 2.556s, 2.603s, 2.628s, 2.585s, 2.594s, 2.679s, 2.709s


gatsby install-cache-and-lock (warm, with lockfile)

Metric Base Head Difference
Mean 0.374s 0.380s +1.55% ⚠️
Median 0.369s 0.377s +2.11% ⚠️
Min 0.361s 0.367s
Max 0.458s 0.426s
Std Dev 0.019s 0.011s
📊 Raw benchmark data (gatsby install-cache-and-lock (warm, with lockfile))

Base times: 0.418s, 0.377s, 0.370s, 0.370s, 0.368s, 0.361s, 0.364s, 0.367s, 0.367s, 0.370s, 0.363s, 0.371s, 0.369s, 0.363s, 0.458s, 0.374s, 0.368s, 0.377s, 0.372s, 0.373s, 0.367s, 0.374s, 0.366s, 0.369s, 0.364s, 0.364s, 0.373s, 0.376s, 0.367s, 0.372s

Head times: 0.371s, 0.371s, 0.367s, 0.378s, 0.370s, 0.402s, 0.377s, 0.377s, 0.369s, 0.426s, 0.371s, 0.387s, 0.392s, 0.381s, 0.375s, 0.379s, 0.379s, 0.376s, 0.377s, 0.383s, 0.385s, 0.379s, 0.381s, 0.378s, 0.381s, 0.377s, 0.374s, 0.372s, 0.374s, 0.378s

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 4f7e244. Configure here.

},

None => match &locator.reference {
Reference::Link(params) if params.path.starts_with('/') => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Inconsistent absolute path check breaks Windows Link references

Low Severity

The fallback handler for Link references when package_data is None uses params.path.starts_with('/') to detect absolute paths, which is Unix-specific. The fetchers updated in this same PR (link.rs, folder.rs, tarball.rs) all use Path::is_absolute() to correctly handle both Unix and Windows absolute paths. If this fallback path is reached on Windows (e.g., a Link reference with a C:\... path whose package data is missing), the check would fail and the code would hit unreachable!(), crashing the process.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 4f7e244. Configure here.

@arcanis arcanis merged commit 93452b3 into main Apr 8, 2026
15 checks passed
@arcanis arcanis deleted the mael/venv-linker branch April 8, 2026 06:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant