fix: resolve relative path sources in --sandbox uv export#9052
fix: resolve relative path sources in --sandbox uv export#9052VishakBaddur wants to merge 4 commits intomarimo-team:mainfrom
--sandbox uv export#9052Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Bundle ReportChanges will increase total bundle size by 4.44kB (0.02%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: marimo-esmAssets Changed:
|
There was a problem hiding this comment.
Pull request overview
Fixes sandbox failures when uv export --script emits relative local path requirements by rewriting those paths to be absolute based on the script’s directory before writing the temp requirements file used by uv run --with-requirements.
Changes:
- Resolve
.-prefixed path requirement lines fromuv export --scriptinto absolute paths in the sandbox CLI. - Add a regression test covering editable and non-editable relative path lines as well as unchanged “normal” and already-absolute deps.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
marimo/_cli/sandbox.py |
Converts relative path lines from uv export --script output to absolute paths rooted at the script directory. |
tests/_cli/test_sandbox.py |
Adds a unit test to ensure relative paths in exported requirements are resolved correctly. |
| editable = line.startswith("-e ") | ||
| path = line[3:].strip() if editable else line.strip() | ||
| if path.startswith("."): | ||
| path = str((script_dir / path).resolve()) | ||
| prefix = "-e " if editable else "" |
There was a problem hiding this comment.
Current parsing treats the entire line (minus optional "-e ") as a filesystem path. If a requirements line contains an environment marker ("; ...") or an inline comment, e.g. "../pkg ; python_version<'3.12'" or "../pkg # comment", this code will resolve the marker/comment as part of the path and emit an invalid requirement. Consider splitting the line into a path token + remainder (markers/comments), resolving only the path token, then re-attaching the untouched remainder (and similarly for editable lines).
| "-e ../../\n../other_pkg\nnumpy==1.26.0\n/absolute/path\n\n" | ||
| ) | ||
|
|
||
| with patch("subprocess.run", return_value=mock_result): |
There was a problem hiding this comment.
This test patches the global "subprocess.run" symbol. To avoid affecting other code that might call subprocess during the patch context (and to be consistent with other tests in this file), patch the call site instead ("marimo._cli.sandbox.subprocess.run").
| with patch("subprocess.run", return_value=mock_result): | |
| with patch("marimo._cli.sandbox.subprocess.run", return_value=mock_result): |
| # Split off any environment markers ("; ...") or inline comments ("# ...") | ||
| # so we only resolve the path token itself. | ||
| for sep in (" ;", " #"): | ||
| if sep in rest: | ||
| path_token, remainder = rest.split(sep, 1) | ||
| remainder = sep.lstrip() + remainder | ||
| break |
There was a problem hiding this comment.
The separator handling for environment markers / inline comments strips the leading whitespace (sep.lstrip()), so a line like ../pkg # comment would be reconstructed as /abs/path#comment (no whitespace), which is no longer an inline comment in requirements parsing and can change semantics (e.g., treated as a URL fragment). Consider preserving the original separator (keep the space before #/;) and also handling markers that may appear without a preceding space (e.g., ;python_version<...).
| # Split off any environment markers ("; ...") or inline comments ("# ...") | |
| # so we only resolve the path token itself. | |
| for sep in (" ;", " #"): | |
| if sep in rest: | |
| path_token, remainder = rest.split(sep, 1) | |
| remainder = sep.lstrip() + remainder | |
| break | |
| # Split off any environment markers ("; ...") or inline comments | |
| # ("# ...") so we only resolve the path token itself. | |
| # | |
| # Preserve the original separator exactly as written. This avoids | |
| # turning `../pkg # comment` into `/abs/path# comment`, which changes | |
| # requirements parsing semantics. Environment markers may also appear | |
| # without a leading space, e.g. `../pkg;python_version<"3.12"`. | |
| marker_idx = rest.find(";") | |
| comment_idx = -1 | |
| for i, char in enumerate(rest): | |
| if char == "#" and i > 0 and rest[i - 1].isspace(): | |
| comment_idx = i | |
| break | |
| split_points = [idx for idx in (marker_idx, comment_idx) if idx != -1] | |
| if split_points: | |
| split_idx = min(split_points) | |
| path_token = rest[:split_idx] | |
| remainder = rest[split_idx:] |
Fixes #8980
Problem
marimo edit --sandboxfails when a script's inline metadata uses relativepathsources in[tool.uv.sources].uv export --scriptreturns paths relative to the script file, but those lines get written to a temp requirements file. Whenuv run --with-requirementsprocesses that file, it resolves paths relative to CWD - not the script's directory - causing path resolution failures.Fix
In
_uv_export_script_requirements_txt(marimo/_cli/sandbox.py), convert any relative paths (.-prefixed) to absolute paths using the script's parent directory as the base. Handles both editable (-e ../../) and non-editable (../../) path sources. Non-path deps and already-absolute paths are left unchanged.Test
Added
test_uv_export_script_requirements_txt_resolves_relative_pathscovering:-e ../../) → resolved to absolute../other_pkg) → resolved to absolutenumpy==1.26.0) → unchanged