Skip to content
Draft
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
20 changes: 14 additions & 6 deletions marimo/_cli/sandbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,15 @@ def _is_versioned(dependency: str) -> bool:
return any(c in dependency for c in ("==", ">=", "<=", ">", "<", "~"))


def _requirement_name(requirement: str) -> str:
"""Bare package name, lowercased. Enough to detect duplicates."""
token = requirement.strip().split(";", 1)[0].split("#", 1)[0]
token = token.split("[", 1)[0]
for spec in ("===", "==", ">=", "<=", "~=", "!=", ">", "<"):
token = token.split(spec, 1)[0]
return token.split("@", 1)[0].strip().lower()


def _normalize_sandbox_dependencies(
dependencies: list[str],
marimo_version: str,
Expand Down Expand Up @@ -570,15 +579,14 @@ def get_sandbox_requirements(
dependencies, __version__, additional_features=[]
)

# Add additional deps if not already present
if additional_deps:
existing_lower = {
d.lower().split("[")[0].split(">=")[0].split("==")[0]
for d in normalized
}
existing = {_requirement_name(dep) for dep in normalized}

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.

P1: When a notebook’s existing pyzmq line has a false environment marker, this name-only check suppresses the unconditional IPC dependency, leaving the kernel venv without pyzmq. Evaluate PEP 508 markers before treating an existing line as satisfying get_ipc_kernel_deps().

Prompt for AI agents
Check if this issue is valid β€” if so, understand the root cause and fix it. At marimo/_cli/sandbox.py, line 583:

<comment>When a notebook’s existing `pyzmq` line has a false environment marker, this name-only check suppresses the unconditional IPC dependency, leaving the kernel venv without `pyzmq`. Evaluate PEP 508 markers before treating an existing line as satisfying `get_ipc_kernel_deps()`.</comment>

<file context>
@@ -570,15 +579,14 @@ def get_sandbox_requirements(
-            d.lower().split("[")[0].split(">=")[0].split("==")[0]
-            for d in normalized
-        }
+        existing = {_requirement_name(dep) for dep in normalized}
+        existing.discard("")
         for dep in additional_deps:
</file context>

existing.discard("")
for dep in additional_deps:
if dep.lower() not in existing_lower:
name = _requirement_name(dep)
if name and name not in existing:
normalized.append(dep)
existing.add(name)

return normalized

Expand Down
14 changes: 4 additions & 10 deletions marimo/_session/_venv.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import os
import subprocess
import sys
from importlib.metadata import version
from pathlib import Path
from typing import TYPE_CHECKING

Expand All @@ -30,17 +29,12 @@


def get_ipc_kernel_deps() -> list[str]:
"""Get dependencies required for IPC kernel communication.
"""Dependencies the sandbox kernel needs for host IPC.

Returns pyzmq pinned to the currently installed version to ensure
compatibility between host and sandbox environments.
A lower bound, not a host pin. The notebook lock may already have
pyzmq at another version, and ZMQ does not require an exact match.
"""
try:
pyzmq_version = version("pyzmq")
return [f"pyzmq=={pyzmq_version}"]
except Exception:
# Fallback if pyzmq not installed
return ["pyzmq>=27.1.0"]
return ["pyzmq>=27.1.0"]


def _find_python_in_venv(venv_path: str) -> str | None:
Expand Down
47 changes: 47 additions & 0 deletions tests/_cli/test_sandbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
_ensure_marimo_in_script_metadata,
_ensure_python_version_in_script_metadata,
_normalize_sandbox_dependencies,
_requirement_name,
build_sandbox_venv,
cleanup_sandbox_dir,
construct_uv_command,
Expand Down Expand Up @@ -784,6 +785,52 @@ def test_get_sandbox_requirements_none_filename() -> None:
assert "pyzmq" in reqs


def test_requirement_name_strips_specifiers_and_markers() -> None:
assert _requirement_name("pyzmq==27.1.0") == "pyzmq"
assert (
_requirement_name(
"pyzmq==27.2.0 ; python_full_version < '3.15' "
"and sys_platform != 'emscripten'"
)
== "pyzmq"
)
assert _requirement_name("marimo[lsp]>=0.24.0") == "marimo"


def test_get_sandbox_requirements_skips_additional_dep_already_present(
tmp_path: Path,
) -> None:
"""Do not add a second pyzmq pin next to the uv-export lock line."""
from marimo._cli.sandbox import get_sandbox_requirements

script_path = tmp_path / "test.py"
script_path.write_text(
"""# /// script
# dependencies = ["marimo>=0.24.0"]
# ///
import marimo
"""
)
exported_pyzmq = (
"pyzmq==27.2.0 ; python_full_version < '3.15' "
"and sys_platform != 'emscripten'"
)

with (
patch("marimo._cli.sandbox.is_editable", return_value=False),
patch(
"marimo._cli.sandbox._resolve_requirements_txt_lines",
return_value=["marimo==0.24.0", exported_pyzmq],
),
):
reqs = get_sandbox_requirements(
str(script_path),
additional_deps=["pyzmq>=27.1.0"],
)

assert reqs == [exported_pyzmq, "marimo==0.24.0"]


def test_cleanup_sandbox_dir_removes_directory(tmp_path: Path) -> None:
"""Test that cleanup_sandbox_dir removes the directory."""
from marimo._cli.sandbox import cleanup_sandbox_dir
Expand Down
5 changes: 5 additions & 0 deletions tests/_session/test_venv.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,16 @@
from marimo._session._venv import (
check_python_version_compatibility,
get_configured_venv_python,
get_ipc_kernel_deps,
get_kernel_pythonpath,
has_marimo_installed,
)


def test_get_ipc_kernel_deps_is_a_lower_bound() -> None:
assert get_ipc_kernel_deps() == ["pyzmq>=27.1.0"]


def test_get_configured_venv_python_returns_none_when_not_configured() -> None:
"""Test returns None when venv not in config."""
config: dict[str, Any] = {} # No venv configured
Expand Down
Loading