diff --git a/app/boot.js b/app/boot.js index cc4a7781..c61a1a72 100644 --- a/app/boot.js +++ b/app/boot.js @@ -21,6 +21,7 @@ import { createInspector } from "./shell/inspector.js"; import { createWheel } from "./shell/wheel.js"; import { offerAction } from "./shell/actions.js"; import { open as openWizard } from "./shell/wizard.js"; +import { open as openUpdates } from "./shell/updates.js"; const root = document.querySelector("#app"); @@ -106,6 +107,7 @@ async function start() { onGenerate: () => { ranFor = project.selectedId; session.generate(); }, onCancel: () => run.cancel(), onConstructor: () => page.constructor.open(), + onUpdates: () => openUpdates(), // What the Edit menu offers, and what the keyboard reaches. One list, so a // menu item and its shortcut cannot drift apart. edits: { diff --git a/app/composer/elements/floating.js b/app/composer/elements/floating.js index a8ce87ff..35fefc07 100644 --- a/app/composer/elements/floating.js +++ b/app/composer/elements/floating.js @@ -2,6 +2,7 @@ import { define } from "../internals/register.js"; import { el } from "../internals/el.js"; +import { setText } from "../internals/text.js"; import { uid } from "../internals/ids.js"; import { mount, unmount } from "../internals/portal.js"; import { claimFor } from "../internals/zlayer.js"; @@ -203,9 +204,10 @@ define("floating", "window", ({ /** A full-screen "please wait" that genuinely blocks. */ define("overlay", "blocking", ({ message, progress, cancel } = {}) => { + const words = el("p", { cls: "cx-t cx-t-sm", text: message }); const card = el("div", { cls: "cx-blocking-card", children: [ el("span", { cls: ["cx-spinner", "cx-spinner-md"], attrs: { "aria-hidden": "true" } }), - el("p", { cls: "cx-t cx-t-sm", text: message }), + words, progress && progress.node ? progress.node : null, cancel && cancel.node ? cancel.node : null, ].filter(Boolean) }); @@ -220,6 +222,13 @@ define("overlay", "blocking", ({ message, progress, cancel } = {}) => { let closed = false; const handle = { node, isOverlay: true, + // What it is waiting for can change while it waits -- "restarting" becomes + // "it has not come back", and a spinner that cannot say so is a spinner + // somebody watches for ten minutes. + setMessage(next) { + setText(words, next); + node.setAttribute("aria-label", next); + }, close() { if (closed) return; closed = true; release(); layer.release(); unmount(node); }, destroy() { handle.close(); }, }; diff --git a/app/shell/layout.js b/app/shell/layout.js index 0464a71a..de75b2dc 100644 --- a/app/shell/layout.js +++ b/app/shell/layout.js @@ -187,6 +187,7 @@ export function build(root, handlers = {}) { const menubar = createMenubar({ workspace, onPipeline: handlers.onPipeline, + onUpdates: handlers.onUpdates, onProject: handlers.onProject, edits: handlers.edits, projects: handlers.projects, diff --git a/app/shell/menubar.js b/app/shell/menubar.js index 772abc23..23a2b225 100644 --- a/app/shell/menubar.js +++ b/app/shell/menubar.js @@ -70,7 +70,7 @@ function menu(label, itemsOf, onPick) { } export function createMenubar({ workspace, onPipeline, onProject, projects = () => [], - current = () => null, edits = null, + current = () => null, edits = null, onUpdates, theme = services.theme } = {}) { const connection = createConnection(); @@ -117,7 +117,12 @@ export function createMenubar({ workspace, onPipeline, onProject, projects = () const settings = menu("Settings", () => [ { id: "pipeline", label: "Models and pipeline…" }, - ], (id) => { if (id === "pipeline" && onPipeline) onPipeline(); }); + { separator: true }, + { id: "updates", label: "Updates…" }, + ], (id) => { + if (id === "pipeline" && onPipeline) onPipeline(); + else if (id === "updates" && onUpdates) onUpdates(); + }); const bar = composer.toolbar.default({ label: "FunPack", diff --git a/app/shell/updates.js b/app/shell/updates.js new file mode 100644 index 00000000..7ffea647 --- /dev/null +++ b/app/shell/updates.js @@ -0,0 +1,180 @@ +// Updating FunPack, switching branch, and getting back afterwards. +// +// The most-used feature in a pack whose value is experimental work shipped +// continuously -- so it is a window in the app rather than a terminal command, +// and it is honest about the one thing that makes it frightening: every one of +// these restarts ComfyUI. The window says so before it does it, and then stays +// up, polling, until the server answers again. A restart with nothing on screen +// is indistinguishable from a crash. + +import { composer } from "../composer/composer.js"; + +const BASE = "/funpack/api/git"; + +async function ask(method, path, body) { + const res = await fetch(`${BASE}${path}`, { + method, + headers: body === undefined ? {} : { "Content-Type": "application/json" }, + body: body === undefined ? undefined : JSON.stringify(body), + }); + const payload = await res.json().catch(() => ({})); + if (!res.ok) throw new Error(payload.detail || `HTTP ${res.status}`); + return payload; +} + +export const status = ({ remote = true } = {}) => + ask("GET", `/status${remote ? "" : "?remote=0"}`); + +/** How far behind, in words. The number alone does not say whether to care. */ +function summarise(s) { + if (!s.ok) return s.detail || "FunPack cannot read its own git checkout."; + if (!s.checked_remote) return "Checking for updates…"; + if (!s.fetch_ok) return "Could not reach the remote, so this may be out of date."; + if (s.behind) return `${s.behind} update${s.behind === 1 ? "" : "s"} available.`; + if (s.ahead) return `Up to date, with ${s.ahead} local commit${s.ahead === 1 ? "" : "s"} of your own.`; + return "Up to date."; +} + +/** + * The overlay that stays up across a restart. + * + * Polls /api/health rather than guessing a duration: an update that installs + * requirements takes as long as it takes, and a fixed wait either lies or gives + * up early. Reloads once the server answers, because the code that just changed + * is the code the page is running. + */ +export function waitForRestart({ poll = 1500, give_up = 180000 } = {}) { + const overlay = composer.overlay.blocking({ + message: "Restarting ComfyUI. This page comes back on its own.", + }); + const started = Date.now(); + + const tick = async () => { + try { + const res = await fetch("/funpack/api/health", { cache: "no-store" }); + if (res.ok) { window.location.reload(); return; } + } catch { /* still down, which is the expected half of this */ } + if (Date.now() - started > give_up) { + overlay.setMessage("ComfyUI has not come back. Check the terminal it was started from."); + return; + } + setTimeout(tick, poll); + }; + setTimeout(tick, poll); + return overlay; +} + +/** + * open() -> the window. One at a time. + * + * Built from a live status every time it opens: what branch you are on and how + * far behind are exactly the facts that go stale while a window is closed. + */ +export function open({ onRestart = waitForRestart } = {}) { + let window_ = null; + const body = composer.region.stack({ gap: "sm", label: "Updates" }); + + const act = async (run) => { + body.set([composer.hint.default({ text: "Working. This can take a few minutes if " + + "requirements changed." })]); + try { + await run(); + if (window_) window_.close("restarting"); + onRestart(); + } catch (err) { + // Said in the window, where the button was pressed. Every refusal this can + // give names what to do: commit or stash, install git, pick another branch. + draw(null, err.message); + } + }; + + function draw(s, problem) { + if (problem) { + body.set([composer.banner.danger({ text: problem }), + composer.button.md({ label: "Try again", onClick: () => load() })]); + return; + } + if (!s) { body.set([composer.hint.default({ text: "Reading the checkout…" })]); return; } + + const rows = [ + composer.settingsRow.default({ label: "Version", hint: s.codename || undefined, + control: composer.text.sm({ text: s.version || "unknown" }) }), + composer.settingsRow.default({ label: "Branch", hint: summarise(s), + control: composer.text.sm({ text: s.branch || "?" }) }), + ]; + + if (s.dirty) { + // The one state that blocks both actions, said before either is pressed + // rather than as a refusal afterwards. + rows.push(composer.banner.warn({ + text: "This checkout has local changes. Commit or stash them before updating " + + "or switching branch.", + })); + } + + rows.push(composer.settingsRow.default({ + label: "Switch to", + hint: "Switching pulls that branch and restarts ComfyUI.", + control: composer.select.md({ + label: "Branch", + value: s.branch, + options: (s.branches || []).map((b) => ({ value: b, label: b })), + onChange: (branch) => { if (branch !== s.branch) act(() => ask("POST", "/checkout", { branch })); }, + disabled: Boolean(s.dirty), + }), + })); + + if (s.rollback_target) { + rows.push(composer.settingsRow.default({ + label: "Roll back", + hint: `Back to ${String(s.rollback_target.commit || "").slice(0, 8)} — the version before the last update.`, + control: composer.button.md({ label: "Roll back", tone: "danger", + onClick: () => act(() => ask("POST", "/rollback")) }), + })); + } + + body.set([composer.group.default({ rows })]); + if (window_) { + window_.setFooter({ + note: s.ok ? "" : "FunPack is not a git checkout, so it cannot update itself.", + actions: [ + composer.button.md({ label: "Close", tone: "ghost", + onClick: () => window_.close("done") }), + composer.button.md({ + label: s.behind ? `Update (${s.behind})` : "Update", tone: "primary", + disabled: !s.ok || Boolean(s.dirty), + onClick: () => act(() => ask("POST", "/update", {})), + }), + ], + }); + } + } + + async function load() { + draw(null); + // Twice: the checkout alone first, so there is something to read instantly, + // then again with the remote for ahead and behind. A `git fetch` takes + // seconds on a good day and blocks until it times out on a machine that + // cannot reach the network -- and neither is a reason to show nothing. + let local = null; + try { + local = await status({ remote: false }); + draw(local); + } catch (err) { + draw(null, err.message); + return; + } + try { draw(await status()); } + catch { /* the local answer stands, and says it has not checked */ } + } + + window_ = composer.modal.generic({ + title: "Updates", + subtitle: "FunPack updates itself from git, then restarts ComfyUI.", + size: "md", + body, + onClose: () => { window_ = null; }, + }); + load(); + return window_; +} diff --git a/app/tests-layout/updates.spec.js b/app/tests-layout/updates.spec.js new file mode 100644 index 00000000..426f6f12 --- /dev/null +++ b/app/tests-layout/updates.spec.js @@ -0,0 +1,103 @@ +// Updating, against the real routes. +// +// The dev server IS a git checkout of this repo, so the status route answers +// truthfully and the window can be read the way a person would read it. What is +// NOT exercised here is a real pull or a real restart: those change the checkout +// the tests are running from. Every path that would do that is stubbed at the +// fetch, and the stub is what proves the request was the right one. + +import { test, expect } from "@playwright/test"; + +const openUpdates = async (page) => { + await page.getByRole("button", { name: "Settings" }).click(); + await page.getByRole("menuitem", { name: /Updates/ }).click(); + await expect(page.locator(".cx-modal")).toBeVisible(); +}; + +test("the window says which branch you are on and whether there is anything new", async ({ page }) => { + await page.goto("/funpack/"); + await page.waitForFunction(() => window.FunPack !== undefined); + await openUpdates(page); + + const modal = page.locator(".cx-modal"); + // Read from the real checkout, not from a fixture: this repo is on a branch + // and the route reports it. + const branch = await page.evaluate(async () => { + const r = await fetch("/funpack/api/git/status"); + return (await r.json()).branch; + }); + await expect(modal).toContainText(branch); + await expect(modal.getByText(/Up to date|update.? available|out of date/i)).toBeVisible(); +}); + +test("a checkout with local changes says so before anything is pressed", async ({ page }) => { + // The one state that blocks both actions. A refusal after the press would be + // a surprise; this is the same fact, said first. + await page.goto("/funpack/"); + await page.waitForFunction(() => window.FunPack !== undefined); + await page.route("**/api/git/status", (route) => route.fulfill({ + json: { ok: true, version: "5.0", branch: "v5", branches: ["v5", "dev"], + dirty: true, ahead: 0, behind: 2, fetch_ok: true, repo: "/x" }, + })); + await openUpdates(page); + + const modal = page.locator(".cx-modal"); + await expect(modal).toContainText(/local changes/i); + await expect(modal.getByRole("button", { name: /Update/ })).toBeDisabled(); +}); + +test("update asks the server to update, and then waits for it to come back", async ({ page }) => { + await page.goto("/funpack/"); + await page.waitForFunction(() => window.FunPack !== undefined); + await page.route("**/api/git/status", (route) => route.fulfill({ + json: { ok: true, version: "5.0", branch: "v5", branches: ["v5", "dev"], + dirty: false, ahead: 0, behind: 3, fetch_ok: true, repo: "/x" }, + })); + + const asked = []; + await page.route("**/api/git/update", (route) => { + asked.push(route.request().method()); + return route.fulfill({ json: { restarting: true, updated: true } }); + }); + // The page would reload on a healthy answer; keep the server "down" so the + // overlay stays up and can be read. + await page.route("**/api/health", (route) => route.abort()); + + await openUpdates(page); + await page.getByRole("button", { name: /Update \(3\)/ }).click(); + + await expect.poll(() => asked).toEqual(["POST"]); + await expect(page.locator(".cx-blocking")).toBeVisible(); + await expect(page.locator(".cx-blocking")).toContainText(/Restarting/i); +}); + +test("a refusal from git is shown in the window, not swallowed", async ({ page }) => { + await page.goto("/funpack/"); + await page.waitForFunction(() => window.FunPack !== undefined); + await page.route("**/api/git/status", (route) => route.fulfill({ + json: { ok: true, version: "5.0", branch: "v5", branches: ["v5", "dev"], + dirty: false, ahead: 0, behind: 1, fetch_ok: true, repo: "/x" }, + })); + await page.route("**/api/git/update", (route) => route.fulfill({ + status: 400, json: { detail: "Working tree has local changes." }, + })); + + await openUpdates(page); + await page.getByRole("button", { name: /Update/ }).click(); + + await expect(page.locator(".cx-modal")).toContainText("Working tree has local changes."); + await expect(page.locator(".cx-blocking"), "it waited for a restart that was refused") + .toHaveCount(0); +}); + +test("an install that is not a git checkout says so instead of offering to update", async ({ page }) => { + await page.goto("/funpack/"); + await page.waitForFunction(() => window.FunPack !== undefined); + await page.route("**/api/git/status", (route) => route.fulfill({ + json: { ok: false, version: "5.0", detail: "FunPack is not a git checkout (no .git directory)." }, + })); + + await openUpdates(page); + await expect(page.locator(".cx-modal")).toContainText(/not a git checkout/i); + await expect(page.locator(".cx-modal").getByRole("button", { name: /Update/ })).toBeDisabled(); +}); diff --git a/core/restart.py b/core/restart.py new file mode 100644 index 00000000..cf63b877 --- /dev/null +++ b/core/restart.py @@ -0,0 +1,43 @@ +"""Relaunching ComfyUI, and nothing else. + +Its own file because it is the one operation here that ends the process: every +route that calls it has already finished answering, and anything that imports it +by accident should be obvious in the import list. + +Carried across from v4. The three launch shapes it handles -- comfy-cli's reboot +file, `python -m comfy`, and a plain script -- are three ways somebody's install +would otherwise fail to come back, and each was found by an install failing to +come back. +""" + +from __future__ import annotations + +def restart() -> None: + """Relaunch the ComfyUI process in place. Mirrors ComfyUI-Manager's reboot so it + works the same whether launched directly, as a module, or via comfy-cli.""" + import os + import sys + try: + sys.stdout.close_log() # type: ignore[attr-defined] # Manager's tee logger, if present + except Exception: + pass + # comfy-cli watches for a .reboot file and relaunches us itself. + if "__COMFY_CLI_SESSION__" in os.environ: + try: + open(os.environ["__COMFY_CLI_SESSION__"] + ".reboot", "w").close() + except Exception: + pass + print("\n[FunPack] Restarting ComfyUI...\n", flush=True) + os._exit(0) + sys_argv = sys.argv.copy() + if "--windows-standalone-build" in sys_argv: + sys_argv.remove("--windows-standalone-build") + if sys_argv and sys_argv[0].endswith("__main__.py"): # python -m comfy + module_name = os.path.basename(os.path.dirname(sys_argv[0])) + cmds = [sys.executable, "-m", module_name] + sys_argv[1:] + elif sys.platform.startswith("win32"): + cmds = ['"' + sys.executable + '"', '"' + sys_argv[0] + '"'] + sys_argv[1:] + else: + cmds = [sys.executable] + sys_argv + print(f"\n[FunPack] Restarting ComfyUI... {cmds}\n", flush=True) + os.execv(sys.executable, cmds) diff --git a/core/routes.py b/core/routes.py index 6bb7b8b0..7bcd19ea 100644 --- a/core/routes.py +++ b/core/routes.py @@ -7,7 +7,7 @@ thin adapters over pure functions in `serve`. import json -from . import (config, graph as graph_mod, log, projects, +from . import (config, graph as graph_mod, log, projects, update as update_mod, registry as registry_mod, serve as static, widgets) from .contract import CONTRACT_VERSION from .relations import order @@ -236,6 +236,65 @@ def register(routes, prefix=None): "prompt": prompt if not (problems or incomplete) else None, }) + # --- keeping the install alive ------------------------------------------- + # + # Updating is the most-used feature in a pack whose value is experimental + # work shipped continuously, so it is in the app rather than in a terminal. + # + # Every one of these ends by relaunching ComfyUI, which means the answer has + # to be written BEFORE the process goes. They schedule the restart and return + # `restarting: true`; the app then polls /api/health until the server is back. + # + # `restart` is looked up on the module rather than captured, so a test can + # replace it -- otherwise running the suite would relaunch the test runner. + + async def _git(action, **kwargs): + """Run one git operation off the event loop, or say why it did not.""" + import asyncio + try: + result = await asyncio.to_thread(action, **kwargs) + except update_mod.GitUpdateError as exc: + # A refusal, not a crash: no git, not a checkout, a dirty tree, a + # branch that exists on neither side. Each names what to do. + return web.json_response({"detail": str(exc)}, status=400) + except Exception as exc: # noqa: BLE001 + log.broke("git", exc, doing=getattr(action, "__name__", "git")) + return web.json_response({"detail": f"{type(exc).__name__}: {exc}"}, status=500) + + import asyncio as _asyncio + from . import restart as restart_mod + # After the response is written, not before. + _asyncio.get_event_loop().call_later(0.7, restart_mod.restart) + return web.json_response({"restarting": True, **(result or {})}) + + @routes.get(P + "/api/git/status") + async def _git_status(req): + # `?remote=0` answers from the checkout alone. The fetch is the only part + # that touches the network and so the only part that can hang; the app + # asks without it first so it has something to draw. + import asyncio + remote = req.query.get("remote", "1") not in ("0", "false", "no") + return web.json_response(await asyncio.to_thread(update_mod.status, remote=remote)) + + @routes.post(P + "/api/git/update") + async def _git_pull(req): + body = await req.json() if req.can_read_body else {} + branch = str((body or {}).get("branch") or "").strip() or None + return await _git(update_mod.pull, branch=branch, install_deps=True) + + @routes.post(P + "/api/git/checkout") + async def _git_checkout(req): + body = await req.json() if req.can_read_body else {} + branch = str((body or {}).get("branch") or "").strip() + if not branch: + return web.json_response({"detail": "Which branch? Name one."}, status=400) + return await _git(update_mod.checkout, branch=branch, + pull_after=True, install_deps=True) + + @routes.post(P + "/api/git/rollback") + async def _git_rollback(_req): + return await _git(update_mod.rollback) + @routes.get(P + "/api/nodes") async def _nodes(req): """What the nodes in a pipeline look like to someone editing them. diff --git a/core/tests/test_routes_update.py b/core/tests/test_routes_update.py new file mode 100644 index 00000000..0afa4a4b --- /dev/null +++ b/core/tests/test_routes_update.py @@ -0,0 +1,164 @@ +"""Keeping the install alive, over HTTP. + +Every one of these routes ends by relaunching ComfyUI. The point of the tests is +that the ANSWER is written first and the restart is a scheduled callback -- so +they replace it, and a suite that did not would relaunch the test runner. +""" + +import json +import socket +import threading + +import pytest + +pytest.importorskip("aiohttp") + +from core import routes, update as update_mod # noqa: E402 + + +@pytest.fixture +def server(comfyui, monkeypatch): + from aiohttp import web as aioweb + + restarts = [] + from core import restart as restart_mod + monkeypatch.setattr(restart_mod, "restart", lambda: restarts.append(1)) + + app = aioweb.Application() + table = aioweb.RouteTableDef() + routes.register(table, prefix="/funpack") + app.add_routes(table) + + holder = {"ready": threading.Event(), "restarts": restarts} + + def run(): + import asyncio + loop = asyncio.new_event_loop() + asyncio.set_event_loop(loop) + runner = aioweb.AppRunner(app) + loop.run_until_complete(runner.setup()) + site = aioweb.TCPSite(runner, "127.0.0.1", 0) + loop.run_until_complete(site.start()) + holder["port"] = site._server.sockets[0].getsockname()[1] + holder["loop"] = loop + holder["ready"].set() + loop.run_forever() + + thread = threading.Thread(target=run, daemon=True) + thread.start() + assert holder["ready"].wait(20), "the test server did not start" + yield holder + holder["loop"].call_soon_threadsafe(holder["loop"].stop) + + +def _request(port, method, path, body=None): + payload = b"" if body is None else json.dumps(body).encode() + head = (f"{method} {path} HTTP/1.1\r\nHost: localhost\r\n" + f"Content-Type: application/json\r\n" + f"Content-Length: {len(payload)}\r\nConnection: close\r\n\r\n").encode() + with socket.create_connection(("127.0.0.1", port), timeout=60) as sock: + sock.sendall(head + payload) + chunks = [] + while True: + got = sock.recv(65536) + if not got: + break + chunks.append(got) + raw = b"".join(chunks) + header, _, rest = raw.partition(b"\r\n\r\n") + status = int(header.split()[1]) + try: + return status, json.loads(rest.decode() or "{}") + except ValueError: + return status, {"raw": rest.decode(errors="replace")} + + +def test_the_app_can_see_which_branch_it_is_on(server): + """The one question every update starts from.""" + status, body = _request(server["port"], "GET", "/funpack/api/git/status") + assert status == 200 + assert body["ok"] is True, body + assert body["branch"], "no branch reported" + assert isinstance(body["branches"], list) and body["branches"] + assert body["repo"].endswith("ComfyUI-FunPack-v5") + assert server["restarts"] == [], "reading the status restarted ComfyUI" + + +def test_switching_to_a_branch_that_is_not_there_is_refused_by_name(server): + """A refusal has to say which branch and that it exists nowhere -- otherwise + the user cannot tell it from a typo in their own request.""" + status, body = _request(server["port"], "POST", "/funpack/api/git/checkout", + {"branch": "no-such-branch-here"}) + assert status == 400 + assert "no-such-branch-here" in body["detail"] + assert server["restarts"] == [], "a refused switch still restarted ComfyUI" + + +def test_switching_to_nothing_is_refused_before_git_is_touched(server): + status, body = _request(server["port"], "POST", "/funpack/api/git/checkout", {}) + assert status == 400 + assert "branch" in body["detail"].lower() + assert server["restarts"] == [] + + +def test_a_refusal_from_git_is_reported_as_a_reason_not_a_crash(server, monkeypatch): + """Dirty tree, no git on PATH, not a checkout: the user has to be told which.""" + def refuse(*_a, **_k): + raise update_mod.GitUpdateError("Working tree has local changes.") + monkeypatch.setattr(update_mod, "pull", refuse) + + status, body = _request(server["port"], "POST", "/funpack/api/git/update", {}) + assert status == 400 + assert "local changes" in body["detail"] + assert server["restarts"] == [] + + +def test_an_update_that_worked_answers_before_it_restarts(server, monkeypatch): + """The answer has to be written first: the process is about to go.""" + monkeypatch.setattr(update_mod, "pull", + lambda **_k: {"updated": True, "before": "a1", "after": "b2"}) + + status, body = _request(server["port"], "POST", "/funpack/api/git/update", + {"branch": "dev"}) + assert status == 200, body + assert body["restarting"] is True + assert body["after"] == "b2" + + +def test_an_unexpected_failure_is_a_500_with_the_reason_not_a_hang(server, monkeypatch): + def explode(*_a, **_k): + raise OSError("disk went away") + monkeypatch.setattr(update_mod, "rollback", explode) + + status, body = _request(server["port"], "POST", "/funpack/api/git/rollback") + assert status == 500 + assert "disk went away" in body["detail"] + assert server["restarts"] == [], "a failed rollback restarted ComfyUI anyway" + + +def test_the_status_can_be_had_without_touching_the_network(server): + """The fetch is the only part that can hang. On a machine that cannot reach + the remote it must still be possible to see which branch you are on.""" + status, body = _request(server["port"], "GET", "/funpack/api/git/status?remote=0") + assert status == 200 + assert body["ok"] is True + assert body["branch"] + assert body["checked_remote"] is False + assert body["behind"] == 0, "behind was reported without asking the remote" + + +def test_a_remote_that_cannot_be_reached_is_said_rather_than_guessed(server, monkeypatch): + def no_network(*args, **kwargs): + class Failed: + returncode = 1 + stdout = "" + stderr = "could not resolve host" + return Failed() if args and args[0] == "fetch" else _real(*args, **kwargs) + _real = update_mod._run_git + monkeypatch.setattr(update_mod, "_run_git", no_network) + + status, body = _request(server["port"], "GET", "/funpack/api/git/status") + assert status == 200 + assert body["fetch_ok"] is False + assert body["checked_remote"] is True + assert body["behind"] == 0, "a failed fetch was read as 'up to date'" diff --git a/core/update.py b/core/update.py new file mode 100644 index 00000000..011ada62 --- /dev/null +++ b/core/update.py @@ -0,0 +1,498 @@ +"""Git pull / branch switch for the FunPack repo (ComfyUI custom node root). + +Carried across from v4 unchanged but for REPO_ROOT, and deliberately: this is +the most-used feature in an experimental pack, it has been switching branches on +real rentals for months, and every one of its refusals -- no git on PATH, not a +checkout, a dirty tree, a branch that exists on neither side -- is a lesson that +cost somebody an afternoon. Rewriting it to look more like v5 would be trading +proven plumbing for a fresh set of edge cases. + +It is core rather than a module because it is about the INSTALL, not about a +feature: no module can announce the thing that updates every module. +""" +from __future__ import annotations + +import re +import shutil +import subprocess +import sys +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[1] +_BRANCH_PRIORITY = ("dev", "cutting_edge", "main") + + +class GitUpdateError(RuntimeError): + pass + + +def _run_git(*args: str, timeout: int = 120) -> subprocess.CompletedProcess[str]: + if not shutil.which("git"): + raise GitUpdateError("git not found on PATH.") + if not (REPO_ROOT / ".git").exists(): + raise GitUpdateError("FunPack is not a git checkout (no .git directory).") + proc = subprocess.run( + ["git", *args], + cwd=REPO_ROOT, + capture_output=True, + text=True, + timeout=timeout, + ) + return proc + + +def funpack_version() -> str: + """Custom-node version from pyproject.toml (shown in Settings ▸ About).""" + try: + text = (REPO_ROOT / "pyproject.toml").read_text(encoding="utf-8") + except OSError: + return "" + m = re.search(r'^version\s*=\s*"([^"]+)"', text, re.MULTILINE) + return m.group(1) if m else "" + + +# Adjective + vegetable sharing an initial, advancing alphabetically. Keyed by MAJOR, so +# every 3.x release ships under one name. +CODENAMES = { + "3": "Auspicious Asparagus", + "4": "Blinding Blackout", +} + + +def funpack_codename(version: str = "") -> str: + """Codename for `version` (default: the installed one); "" when its major has none.""" + major = (version or funpack_version()).split(".", 1)[0].strip() + return CODENAMES.get(major, "") + + +def _current_branch() -> str: + proc = _run_git("rev-parse", "--abbrev-ref", "HEAD") + if proc.returncode != 0: + raise GitUpdateError((proc.stderr or proc.stdout or "git rev-parse failed").strip()) + return (proc.stdout or "").strip() + + +def _current_commit() -> str: + proc = _run_git("rev-parse", "--short", "HEAD") + if proc.returncode != 0: + raise GitUpdateError((proc.stderr or proc.stdout or "git rev-parse failed").strip()) + return (proc.stdout or "").strip() + + +def _is_dirty() -> bool: + proc = _run_git("status", "--porcelain") + if proc.returncode != 0: + raise GitUpdateError((proc.stderr or "git status failed").strip()) + return bool((proc.stdout or "").strip()) + + +def _ahead_behind(branch: str) -> tuple[int, int]: + proc = _run_git("rev-list", "--left-right", "--count", f"HEAD...origin/{branch}") + if proc.returncode != 0: + return 0, 0 + parts = (proc.stdout or "").strip().split() + if len(parts) != 2: + return 0, 0 + try: + return int(parts[0]), int(parts[1]) + except ValueError: + return 0, 0 + + +def _unique_local_commits(branch: str) -> list[str]: + """Local commits with no patch-equivalent upstream — i.e. real work that a reset + would destroy. `git cherry` marks those "+" and marks "-" the ones whose CONTENT is + already upstream under a different hash, which is exactly what a rewritten history + leaves behind.""" + proc = _run_git("cherry", f"origin/{branch}", branch) + if proc.returncode != 0: + return [""] # can't prove it's safe → treat as if there were local work + return [ln[2:].strip() for ln in (proc.stdout or "").splitlines() if ln.startswith("+")] + + +def _remote_names() -> set[str]: + proc = _run_git("remote") + if proc.returncode != 0: + return set() + return {ln.strip() for ln in (proc.stdout or "").splitlines() if ln.strip()} + + +def _list_branches() -> list[str]: + proc = _run_git("branch", "-a", "--format=%(refname:short)") + if proc.returncode != 0: + raise GitUpdateError((proc.stderr or "git branch failed").strip()) + # refs/remotes/origin/HEAD shortens to a bare "origin", which is not a branch and + # fails on checkout — drop anything that is just a remote's name. + remotes = _remote_names() + names: set[str] = set() + for raw in (proc.stdout or "").splitlines(): + line = raw.strip() + if not line or line == "HEAD" or line.endswith("/HEAD") or line in remotes: + continue + if line.startswith("origin/"): + names.add(line[7:]) + elif not line.startswith("remotes/"): + names.add(line) + ordered = [b for b in _BRANCH_PRIORITY if b in names] + rest = sorted(b for b in names if b not in _BRANCH_PRIORITY) + return ordered + rest + + +def rollback_target() -> dict | None: + """The commit HEAD pointed to before its last move (an Update or Switch branch — pull() + and checkout() are the only things in this app that move HEAD), read from git's own + reflog rather than a separate state file: git already records exactly this. None if + there is no prior entry (a fresh clone) or HEAD hasn't moved since.""" + proc = _run_git("rev-parse", "--verify", "HEAD@{1}") + if proc.returncode != 0: + return None + commit = proc.stdout.strip() + if not commit or commit == _current_commit(): + return None + subject = _run_git("log", "-1", "--format=%s", commit) + return {"commit": commit, "subject": (subject.stdout or "").strip()} + + +def rollback() -> dict: + """Hard-reset to HEAD@{1} -- undoes the most recent Update or Switch branch. Refuses on + a dirty tree, same as pull()/checkout(), so nothing local is silently discarded. Does + NOT reinstall requirements for the commit being rolled back to -- an update that changed + dependencies and turned out bad may need `pip install -r requirements.txt` run by hand + after rolling back the code; automatically reversing a pip install is its own can of + worms (downgrades can break OTHER already-installed packages) and out of scope here.""" + if _is_dirty(): + raise GitUpdateError("Working tree has local changes. Commit or stash them before rolling back.") + target = rollback_target() + if target is None: + raise GitUpdateError("Nothing to roll back to.") + before = _current_commit() + reset = _run_git("reset", "--hard", target["commit"]) + if reset.returncode != 0: + raise GitUpdateError((reset.stderr or reset.stdout or "git reset failed").strip()) + return {"branch": _current_branch(), "before": before, "after": _current_commit()} + + +def status(*, remote: bool = True) -> dict: + """Current branch, switchable branches, and update availability. + + `remote=False` answers from the checkout alone. Everything except ahead and + behind is local and instant; the fetch is the only part that touches the + network, and on a machine that cannot reach the remote it is the only part + that can hang. The app asks twice -- once without, to have something to draw, + then once with. + + The fetch gets a short timeout of its own for the same reason: the default + 120s is a spinner nobody waits out, and a failed fetch already has an honest + answer (`fetch_ok: false`, "this may be out of date"). + """ + try: + branch = _current_branch() + commit = _current_commit() + dirty = _is_dirty() + branches = _list_branches() + fetch_ok = False + if remote: + fetch = _run_git("fetch", "--prune", "origin", timeout=20) + fetch_ok = fetch.returncode == 0 + ahead, behind = _ahead_behind(branch) if fetch_ok else (0, 0) + return { + "ok": True, + "version": funpack_version(), + "codename": funpack_codename(), + "branch": branch, + "commit": commit, + "dirty": dirty, + "branches": branches, + "ahead": ahead, + "behind": behind, + "fetch_ok": fetch_ok, + "checked_remote": bool(remote), + "repo": str(REPO_ROOT), + "rollback_target": rollback_target(), + } + except GitUpdateError as e: + return {"ok": False, "version": funpack_version(), + "codename": funpack_codename(), "detail": str(e)} + + +REQUIREMENTS = "requirements.txt" + + +def requirements_changed(before: str, after: str) -> bool: + """Did this update touch requirements.txt? + + The only honest trigger for installing: running pip on every pull would be slow and + surprising, and running it on none leaves an update that added a dependency looking + like a broken build instead of an unfinished install. + """ + if not before or not after or before == after: + return False + proc = _run_git("diff", "--name-only", f"{before}..{after}", "--", REQUIREMENTS) + if proc.returncode != 0: + # Cannot tell — install rather than skip. A redundant pip run costs seconds; a + # skipped one costs a broken node pack and a confusing error. + return True + 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: + """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. + + 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"} + 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", + *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 {' '.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 " + 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": (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: + """{name: version} for the environment ComfyUI is running in. {} if pip cannot be read — + an unreadable freeze must never block the install it was only meant to describe.""" + try: + out = subprocess.run([sys.executable, "-m", "pip", "freeze", + "--disable-pip-version-check"], + capture_output=True, text=True, timeout=120) + if out.returncode != 0: + return {} + found = {} + for line in (out.stdout or "").splitlines(): + if "==" in line: + name, _, ver = line.partition("==") + found[name.strip().lower()] = ver.strip() + return found + except Exception: # noqa: BLE001 + return {} + + +def _pip_diff(before: dict, after: dict) -> list[str]: + """Human-readable list of what moved. Empty when nothing did, or when either side is + unknown — reporting every package as "new" because the freeze failed would be worse + than saying nothing.""" + if not before or not after: + return [] + out = [] + for name, ver in sorted(after.items()): + was = before.get(name) + if was is None: + out.append(f"{name} {ver} (new)") + elif was != ver: + out.append(f"{name} {was} -> {ver}") + for name in sorted(set(before) - set(after)): + out.append(f"{name} {before[name]} (removed)") + return out + + +def pull(branch: str | None = None, *, install_deps: bool = False) -> dict: + """Fast-forward pull from origin on the given branch (current if omitted). + + `install_deps` is OFF by default and turned on by the update route. Running pip is a + side effect no caller should acquire by accident just for asking git to move a branch — + it has to be asked for at the point that means "the user pressed Update". + """ + branch = (branch or _current_branch()).strip() + if not branch: + raise GitUpdateError("Could not determine current branch.") + if _is_dirty(): + raise GitUpdateError("Working tree has local changes. Commit or stash them before updating.") + before = _current_commit() + fetch = _run_git("fetch", "--prune", "origin") + if fetch.returncode != 0: + raise GitUpdateError((fetch.stderr or fetch.stdout or "git fetch failed").strip()) + if branch != _current_branch(): + co = _run_git("checkout", branch) + if co.returncode != 0: + raise GitUpdateError((co.stderr or co.stdout or "git checkout failed").strip()) + pull_proc = _run_git("pull", "--ff-only", "origin", branch) + realigned = False + if pull_proc.returncode != 0: + msg = (pull_proc.stderr or pull_proc.stdout or "git pull failed").strip() + # A fast-forward is impossible once upstream history has been rewritten: every + # local commit is a different hash, so git reports the branches as diverged even + # though the content matches. Only realign when nothing would actually be lost — + # a clean tree (checked above) and no local commit whose content is missing + # upstream. Anything else keeps the original error and the user's work. + unique = _unique_local_commits(branch) + if unique: + raise GitUpdateError( + f"{msg}\n\nThis checkout has {len(unique)} local commit(s) that are not on " + f"origin/{branch}. Push or move them first — updating would discard them.") + reset = _run_git("reset", "--hard", f"origin/{branch}") + if reset.returncode != 0: + raise GitUpdateError(msg) + realigned = True + after = _current_commit() + # Dependencies are part of the update. Done BEFORE the response, so the restart the + # caller schedules cannot race an install that is still running. + deps = None + if install_deps and before != after and requirements_changed(before, after): + deps = install_requirements() + return { + "branch": branch, + "before": before, + "after": after, + "updated": before != after, + "requirements": deps, + # Surfaced so the update reads as what it was, not a silent jump to another commit. + "realigned": realigned, + "output": ((pull_proc.stdout or "").strip() if not realigned + else f"Upstream history was rewritten — realigned to origin/{branch}. " + "No local commits were lost."), + } + + +def checkout(branch: str, *, pull_after: bool = True, install_deps: bool = False) -> dict: + """Switch branch, optionally pull, return combined result.""" + branch = (branch or "").strip() + if not branch: + raise GitUpdateError("Branch name is required.") + branches = _list_branches() + if branch not in branches: + raise GitUpdateError(f'Branch "{branch}" is not available locally or on origin.') + if _is_dirty(): + raise GitUpdateError("Working tree has local changes. Commit or stash them before switching branches.") + before_branch = _current_branch() + before_commit = _current_commit() + if branch != before_branch: + co = _run_git("checkout", branch) + if co.returncode != 0: + raise GitUpdateError((co.stderr or co.stdout or "git checkout failed").strip()) + result = {"branch": branch, "before_branch": before_branch, "before": before_commit} + if pull_after: + pulled = pull(branch, install_deps=install_deps) + result.update(pulled) + else: + result["after"] = _current_commit() + result["updated"] = result["before"] != result["after"] + # A branch switch alone can cross a requirements change just as a pull can — the + # checkout above already moved the working tree onto the other branch's files. + if install_deps and result["updated"] and requirements_changed(before_commit, + result["after"]): + result["requirements"] = install_requirements() + return result