From 6f0464202db4655044aa9f4228a0b2d4e08cbcf4 Mon Sep 17 00:00:00 2001 From: digital-garbage <133010426+digital-garbage@users.noreply.github.com> Date: Sun, 23 Aug 2026 04:56:36 +0300 Subject: [PATCH] fix(update): install only missing packages, never upgrade pip install -r upgrades anything below its floor; moving numpy or transformers under ComfyUI's compiled extensions segfaults hours later. Absent packages are installed by name; outdated ones are reported with the command and left alone. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 8 ++ movie_editor/backend/git_update.py | 124 +++++++++++++++--- movie_editor/tests/test_git_update.py | 173 ++++++++++++++------------ 3 files changed, 209 insertions(+), 96 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72d18f16..dc151129 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,14 @@ ## [Unreleased] ### Fixed +- **The updater installs only what is MISSING, and never upgrades anything.** It ran + `pip install -r requirements.txt`, which upgrades any package below its version floor — + and on an install full of compiled extensions (torch, comfy-kitchen, comfy-aimdo, + onnxruntime, opencv) a numpy or transformers bump underneath them does not raise, it + segfaults hours later with nothing connecting it to the update. Absent packages are now + installed by name; a package that is present but older than FunPack asks for is reported + with the command to upgrade it yourself, and left alone. Nothing missing means pip is + never run at all. - **The stand-in tokenizer no longer downloads Gemma for a Qwen model, or reaches the network mid-run.** With no CLIP wired, Studio fetched a 12B *Gemma* tokenizer whatever the model was — wrong vocabulary for MiniMax H3, which encodes with Qwen3-VL. There is now an diff --git a/movie_editor/backend/git_update.py b/movie_editor/backend/git_update.py index b922a3db..64e5ede5 100644 --- a/movie_editor/backend/git_update.py +++ b/movie_editor/backend/git_update.py @@ -175,47 +175,141 @@ def requirements_changed(before: str, after: str) -> bool: return bool((proc.stdout or "").strip()) +def parse_requirement(line: str): + """(name, specifier) for one requirements line, or None for a blank/comment. + + Deliberately small: FunPack's requirements are plain names with at most a version floor. + Anything with an environment marker or a URL is handed back name-only, which makes it + "install if absent" and never a reason to touch what is already there. + """ + text = (line or "").split("#", 1)[0].strip() + if not text or text.startswith("-"): + return None + text = text.split(";", 1)[0].strip() # drop environment markers + if not text: + return None + m = re.match(r"^([A-Za-z0-9._-]+)\s*(\[[^\]]*\])?\s*(.*)$", text) + if not m: + return None + return m.group(1), (m.group(3) or "").strip() + + +def _installed_version(name: str): + """Installed version of `name` in THIS interpreter, or None. FunPack runs inside ComfyUI, + so importlib.metadata already sees the right environment — no pip subprocess needed.""" + try: + from importlib import metadata + return metadata.version(name) + except Exception: # noqa: BLE001 + return None + + +def requirement_status() -> dict: + """{"missing": [name], "below_floor": [(name, have, spec)], "present": [name]}. + + `below_floor` is REPORTED, never acted on. Upgrading a package the rest of ComfyUI is + already built against is how a working install stops working: torch, numpy, transformers + and the compiled extensions on top of them do not survive being moved underneath. + Deciding to upgrade is the user's call, with the command in hand. + """ + req = REPO_ROOT / REQUIREMENTS + out = {"missing": [], "below_floor": [], "present": []} + if not req.is_file(): + return out + try: + lines = req.read_text(encoding="utf-8", errors="replace").splitlines() + except OSError: + return out + for line in lines: + parsed = parse_requirement(line) + if parsed is None: + continue + name, spec = parsed + have = _installed_version(name) + if have is None: + out["missing"].append(name) + continue + out["present"].append(name) + if spec and not _satisfies(have, spec): + out["below_floor"].append((name, have, spec)) + return out + + +def _satisfies(have: str, spec: str) -> bool: + """Whether `have` meets `spec`. Unknown/unparseable -> True, because the fallback of a + false "outdated" is a warning about a package that is fine.""" + try: + from packaging.specifiers import SpecifierSet + from packaging.version import Version + return Version(have) in SpecifierSet(spec) + except Exception: # noqa: BLE001 + return True + + def install_requirements(timeout: int = 900) -> dict: - """`pip install -r requirements.txt` into the interpreter ComfyUI is running. + """Install ONLY the requirements that are absent. Never upgrades anything. `sys.executable`, never a bare `pip`: ComfyUI is usually in a venv, and the pip on PATH - belongs to whatever else is on it. Installing into the wrong environment succeeds - loudly and changes nothing. + belongs to whatever else is on it. Installing into the wrong environment succeeds loudly + and changes nothing. - Never raises. A failed install must not turn a completed update into an error — the - code IS updated by this point, and the remedy is a command the user can run. + A package that is present but below a version floor is reported, not moved. `pip install + -r requirements.txt` would upgrade it — and on a rental full of compiled extensions + (torch, comfy-kitchen, comfy-aimdo, onnxruntime, opencv) a numpy or transformers bump + underneath them does not raise. It segfaults, or corrupts memory, hours later, with + nothing in the log connecting it to the update that caused it. + + Never raises. A failed install must not turn a completed update into an error — the code + IS updated by this point, and the remedy is a command the user can run. """ req = REPO_ROOT / REQUIREMENTS if not req.is_file(): return {"ran": False, "ok": True, "detail": "no requirements.txt in this checkout"} - # What is installed BEFORE, so the update can say what it changed. An update that - # silently moves a shared dependency is the worst kind: ComfyUI is full of compiled - # extensions (torch, comfy-kitchen, comfy-aimdo, onnxruntime, opencv) and a numpy or - # transformers bump under them does not raise — it segfaults, or corrupts memory, hours - # later, with nothing in the log connecting it to the update that caused it. + status = requirement_status() + floor_note = "" + if status["below_floor"]: + floor_note = ("Present but older than FunPack asks for, and LEFT ALONE — upgrading a " + "package the rest of ComfyUI is built against is not something an " + "update should do behind you:\n" + + "\n".join(f" {n}: have {h}, wants {sp}" + for n, h, sp in status["below_floor"]) + + f"\n\nTo do it yourself:\n {sys.executable} -m pip install -r {req}") + print(f"[FunPack update] {floor_note}") + if not status["missing"]: + return {"ran": False, "ok": True, "changed": [], + "detail": ("Nothing to install — every requirement is already present." + + (f"\n\n{floor_note}" if floor_note else ""))} + before = _pip_freeze() + # Only the absent ones, by name — never `-r requirements.txt`, which would upgrade + # anything below its floor. Dependencies are resolved normally: a new package with + # nothing under it is not installed, it is broken. The freeze diff below is what says + # whether anything already present moved as a result. cmd = [sys.executable, "-m", "pip", "install", "--disable-pip-version-check", - "-r", str(req)] + *status["missing"]] try: proc = subprocess.run(cmd, cwd=REPO_ROOT, capture_output=True, text=True, timeout=timeout) except subprocess.TimeoutExpired: return {"ran": True, "ok": False, "detail": f"pip did not finish within {timeout}s — run it yourself: " - f"pip install -r {req}"} + f"pip install {' '.join(status['missing'])}"} except OSError as e: return {"ran": True, "ok": False, "detail": f"could not run pip: {e}"} if proc.returncode != 0: tail = ((proc.stderr or proc.stdout or "").strip() or "pip failed")[-800:] return {"ran": True, "ok": False, "detail": f"pip install failed — run it yourself:\n" - f" {sys.executable} -m pip install -r {req}\n\n{tail}"} + f" {sys.executable} -m pip install " + f"{' '.join(status['missing'])}\n\n{tail}"} changed = _pip_diff(before, _pip_freeze()) if changed: print("[FunPack update] pip changed these packages: " + ", ".join(changed)) return {"ran": True, "ok": True, "changed": changed, - "detail": (("Changed: " + ", ".join(changed) + "\n\n") if changed else "") - + (proc.stdout or "").strip()[-800:]} + "detail": (f"Installed missing: {', '.join(status['missing'])}\n" + + (("Changed: " + ", ".join(changed) + "\n") if changed else "") + + (f"\n{floor_note}\n" if floor_note else "") + + (proc.stdout or "").strip()[-800:])} def _pip_freeze() -> dict: diff --git a/movie_editor/tests/test_git_update.py b/movie_editor/tests/test_git_update.py index 29388c42..60545b2c 100644 --- a/movie_editor/tests/test_git_update.py +++ b/movie_editor/tests/test_git_update.py @@ -214,122 +214,133 @@ def test_an_undecidable_diff_installs_rather_than_skips(monkeypatch): assert gu.requirements_changed("aaa", "bbb") is True -def test_install_uses_the_running_interpreter(monkeypatch, tmp_path): +MISSING = {"missing": ["gguf"], "below_floor": [], "present": ["numpy"]} + + +def _fake_pip(monkeypatch, gu, status=None, freezes=None, rc=0, stderr=""): + """Drive install_requirements without touching the real environment.""" + monkeypatch.setattr(gu, "requirement_status", lambda: dict(status or MISSING)) + calls = [] + seq = iter(freezes) if freezes else None + + def fake_run(cmd, **kw): + calls.append(cmd) + if cmd[3] == "freeze": + return types.SimpleNamespace( + returncode=0, stdout=(next(seq) if seq else ""), stderr="") + return types.SimpleNamespace(returncode=rc, stdout="ok", stderr=stderr) + + monkeypatch.setattr(gu.subprocess, "run", fake_run) + return calls + + +# ── only what is missing ────────────────────────────────────────────────────── +# Upgrading a package the rest of ComfyUI is built against is how a working install stops +# working: torch, numpy, transformers and the compiled extensions on top of them do not +# survive being moved underneath, and the failure is a segfault hours later, not an error. + + +def test_only_the_absent_packages_are_installed(monkeypatch): + from movie_editor.backend import git_update as gu + calls = _fake_pip(monkeypatch, gu) + out = gu.install_requirements() + install = [c for c in calls if c[3] == "install"][0] + assert install[-1:] == ["gguf"] # only the missing one, by name + assert "-r" not in install # never the whole requirements file + assert out["ok"] and out["ran"] + + +def test_nothing_missing_means_pip_is_never_run(monkeypatch): + from movie_editor.backend import git_update as gu + calls = _fake_pip(monkeypatch, gu, {"missing": [], "below_floor": [], "present": ["numpy"]}) + out = gu.install_requirements() + assert not calls and out["ran"] is False and out["ok"] + assert "already present" in out["detail"] + + +def test_an_outdated_package_is_reported_and_left_alone(monkeypatch): + """Present but below the floor: say so, hand over the command, change nothing.""" + from movie_editor.backend import git_update as gu + calls = _fake_pip(monkeypatch, gu, { + "missing": [], "below_floor": [("transformers", "4.44.0", ">=5.0.0")], + "present": ["transformers"]}) + out = gu.install_requirements() + assert not calls + assert "transformers: have 4.44.0, wants >=5.0.0" in out["detail"] + assert "LEFT ALONE" in out["detail"] + + +def test_install_uses_the_running_interpreter(monkeypatch): """Never a bare `pip`: ComfyUI is usually in a venv, and the pip on PATH belongs to whatever else is on it — installing into the wrong environment succeeds and changes nothing.""" from movie_editor.backend import git_update as gu - calls = [] - - def fake_run(cmd, **kw): - calls.append(cmd) - return types.SimpleNamespace(returncode=0, stdout="ok", stderr="") - - monkeypatch.setattr(gu.subprocess, "run", fake_run) - out = gu.install_requirements() - assert out["ran"] and out["ok"] - # freeze / install / freeze — every one of them through THIS interpreter. + calls = _fake_pip(monkeypatch, gu) + gu.install_requirements() assert all(c[0] == sys.executable and c[1:3] == ["-m", "pip"] for c in calls) - install = [c for c in calls if c[3] == "install"] - assert len(install) == 1 def test_the_install_reports_which_packages_moved(monkeypatch): - """An update that silently bumps a shared dependency is the worst kind: ComfyUI is full - of compiled extensions, and a numpy or transformers change under them does not raise — - it segfaults hours later with nothing connecting it to the update.""" + """Installing a missing package still resolves ITS dependencies, so the freeze diff is + what says whether anything else moved.""" from movie_editor.backend import git_update as gu - freezes = iter(["numpy==1.26.4\ntransformers==4.44.0\n", - "numpy==2.1.0\ntransformers==5.0.1\ngguf==0.10.0\n"]) - - def fake_run(cmd, **kw): - if cmd[3] == "freeze": - return types.SimpleNamespace(returncode=0, stdout=next(freezes), stderr="") - return types.SimpleNamespace(returncode=0, stdout="ok", stderr="") - - monkeypatch.setattr(gu.subprocess, "run", fake_run) + _fake_pip(monkeypatch, gu, freezes=["numpy==1.26.4\n", + "numpy==2.1.0\ngguf==0.10.0\n"]) changed = gu.install_requirements()["changed"] assert "numpy 1.26.4 -> 2.1.0" in changed - assert "transformers 4.44.0 -> 5.0.1" in changed assert "gguf 0.10.0 (new)" in changed def test_an_unreadable_freeze_reports_nothing_rather_than_everything(monkeypatch): """Reporting every package as new because pip freeze failed is worse than silence.""" from movie_editor.backend import git_update as gu - - def fake_run(cmd, **kw): - if cmd[3] == "freeze": - return types.SimpleNamespace(returncode=1, stdout="", stderr="boom") - return types.SimpleNamespace(returncode=0, stdout="ok", stderr="") - - monkeypatch.setattr(gu.subprocess, "run", fake_run) + monkeypatch.setattr(gu, "requirement_status", lambda: dict(MISSING)) + monkeypatch.setattr(gu, "_pip_freeze", lambda: {}) + monkeypatch.setattr(gu.subprocess, "run", lambda cmd, **kw: types.SimpleNamespace( + returncode=0, stdout="ok", stderr="")) out = gu.install_requirements() assert out["ok"] and out["changed"] == [] -def test_nothing_changed_reports_nothing(monkeypatch): - from movie_editor.backend import git_update as gu - same = "numpy==2.1.0\ngguf==0.10.0\n" - - def fake_run(cmd, **kw): - return types.SimpleNamespace( - returncode=0, stdout=(same if cmd[3] == "freeze" else "ok"), stderr="") - - monkeypatch.setattr(gu.subprocess, "run", fake_run) - assert gu.install_requirements()["changed"] == [] - - def test_a_failed_install_reports_the_command_and_never_raises(monkeypatch): """The code IS updated by this point — turning that into an exception would leave the user with a half-finished update and no instructions.""" from movie_editor.backend import git_update as gu - monkeypatch.setattr(gu.subprocess, "run", lambda cmd, **kw: types.SimpleNamespace( - returncode=1, stdout="", stderr="No matching distribution found for gguf")) + _fake_pip(monkeypatch, gu, rc=1, stderr="No matching distribution found for gguf") out = gu.install_requirements() assert out["ran"] is True and out["ok"] is False - assert "pip install -r" in out["detail"] + assert "pip install gguf" in out["detail"] assert "No matching distribution" in out["detail"] +# ── parsing ─────────────────────────────────────────────────────────────────── + + +@pytest.mark.parametrize("line,expected", [ + ("numpy", ("numpy", "")), + ("transformers>=5.0.0", ("transformers", ">=5.0.0")), + ("opencv-python-headless", ("opencv-python-headless", "")), + ("pillow == 12.0 ; python_version > '3.8'", ("pillow", "== 12.0")), + ("gguf # a comment", ("gguf", "")), + ("", None), + (" ", None), + ("# just a comment", None), + ("--extra-index-url https://x", None), +]) +def test_requirement_lines(line, expected): + from movie_editor.backend import git_update as gu + assert gu.parse_requirement(line) == expected + + def test_a_pip_timeout_is_reported_not_raised(monkeypatch): from movie_editor.backend import git_update as gu + monkeypatch.setattr(gu, "requirement_status", lambda: dict(MISSING)) def boom(cmd, **kw): - raise gu.subprocess.TimeoutExpired(cmd, 900) + if cmd[3] == "freeze": + return types.SimpleNamespace(returncode=0, stdout="", stderr="") + raise gu.subprocess.TimeoutExpired(cmd, 1) monkeypatch.setattr(gu.subprocess, "run", boom) out = gu.install_requirements() assert out["ok"] is False and "did not finish" in out["detail"] - - -def test_pull_does_not_install_unless_asked(monkeypatch): - """Running pip is a side effect no caller should acquire just for moving a branch. - Turning this on by default made the test suite itself shell out to a real pip install - and hang — the default has to be off, with the update route opting in.""" - from movie_editor.backend import git_update as gu - monkeypatch.setattr(gu, "install_requirements", - lambda *a, **k: pytest.fail("pip must not run by default")) - monkeypatch.setattr(gu, "requirements_changed", lambda *a, **k: True) - monkeypatch.setattr(gu, "_is_dirty", lambda: False) - monkeypatch.setattr(gu, "_current_branch", lambda: "v4") - commits = iter(["aaa", "bbb"]) - monkeypatch.setattr(gu, "_current_commit", lambda: next(commits)) - monkeypatch.setattr(gu, "_run_git", lambda *a, **k: _proc(0, "")) - out = gu.pull("v4") - assert out["updated"] is True - assert out["requirements"] is None - - -def test_pull_installs_when_the_route_asks(monkeypatch): - from movie_editor.backend import git_update as gu - monkeypatch.setattr(gu, "install_requirements", - lambda *a, **k: {"ran": True, "ok": True, "detail": "done"}) - monkeypatch.setattr(gu, "requirements_changed", lambda *a, **k: True) - monkeypatch.setattr(gu, "_is_dirty", lambda: False) - monkeypatch.setattr(gu, "_current_branch", lambda: "v4") - commits = iter(["aaa", "bbb"]) - monkeypatch.setattr(gu, "_current_commit", lambda: next(commits)) - monkeypatch.setattr(gu, "_run_git", lambda *a, **k: _proc(0, "")) - out = gu.pull("v4", install_deps=True) - assert out["requirements"] == {"ran": True, "ok": True, "detail": "done"}