From d4f58d685f3eca2b831b1522d6cc56593bb0558c Mon Sep 17 00:00:00 2001 From: namabeeru Date: Fri, 3 Jul 2026 23:27:45 +0900 Subject: [PATCH] fix: detect source checkout updates Signed-off-by: namabeeru --- app/updater.py | 173 +++++++++++++++++++++++++++++--------- scripts/updater.bat | 19 +++-- scripts/updater.sh | 54 ++++++++++++ tests/test_updater.py | 189 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 390 insertions(+), 45 deletions(-) create mode 100755 scripts/updater.sh create mode 100644 tests/test_updater.py diff --git a/app/updater.py b/app/updater.py index 3bd0f0f0..f882b155 100644 --- a/app/updater.py +++ b/app/updater.py @@ -38,50 +38,129 @@ def is_newer(remote: str, local: str) -> bool: # --------------------------------------------------------------------------- GITHUB_REPO = "CraftOS-dev/CraftBot" -GITHUB_LATEST_RELEASE_URL = f"https://api.github.com/repos/{GITHUB_REPO}/tags" +GITHUB_TAGS_URL = f"https://api.github.com/repos/{GITHUB_REPO}/tags" +GITHUB_LATEST_RELEASE_URL = GITHUB_TAGS_URL +UPDATE_BRANCH = "main" +GIT_PROBE_TIMEOUT = 15 async def check_for_update() -> Tuple[bool, str, str]: """Check whether a newer version is available on the remote repo. - Fetches the latest git tag from GitHub (e.g. ``v1.2.2``) and compares - it against the local version stored in settings.json. + Source checkouts can be ahead or behind the update branch while still + carrying the same tagged app version. Keep the release tag as the primary + version signal, then use a git comparison to catch source-checkout updates + when the release tag is unchanged. Returns: (update_available, current_version, latest_version) """ from app.config import get_app_version + current = get_app_version() + project_root = Path(__file__).resolve().parent.parent + + release_update = await _check_release_update(current) + if release_update[0]: + return release_update + + source_update = await _check_source_update(project_root, current) + if source_update is not None: + return source_update + + return release_update + + +async def _check_source_update( + project_root: Path, + current: str, + branch: str = UPDATE_BRANCH, +) -> Optional[Tuple[bool, str, str]]: + """Check whether this git checkout is behind the configured update branch. + + Returns ``None`` when the probe is inconclusive so callers can fall back to + the release-tag check instead of blocking update checks on git-specific + failures. + """ + if getattr(sys, "frozen", False): + return None + + try: + inside, _ = await _run_git( + ["git", "rev-parse", "--is-inside-work-tree"], str(project_root) + ) + if _decode_git_stdout(inside) != "true": + return None + + await _run_git( + ["git", "fetch", "origin", f"{branch}:refs/remotes/origin/{branch}"], + str(project_root), + ) + local_stdout, _ = await _run_git( + ["git", "rev-parse", "HEAD"], str(project_root) + ) + remote_stdout, _ = await _run_git( + ["git", "rev-parse", f"origin/{branch}"], str(project_root) + ) + + local_revision = _decode_git_stdout(local_stdout) + remote_revision = _decode_git_stdout(remote_stdout) + if not local_revision or not remote_revision: + return None + if local_revision == remote_revision: + return False, current, current + + count_stdout, _ = await _run_git( + [ + "git", + "rev-list", + "--left-right", + "--count", + f"HEAD...origin/{branch}", + ], + str(project_root), + ) + _ahead_text, behind_text = _decode_git_stdout(count_stdout).split() + behind = int(behind_text) + if behind > 0: + latest = f"{current}+{branch}.{remote_revision[:7]}" + return True, current, latest + + return False, current, current + except Exception: + return None + + +async def _check_release_update(current: str) -> Tuple[bool, str, str]: + """Check GitHub release tags against the local app version.""" import aiohttp - current = get_app_version() try: headers = {"Accept": "application/vnd.github.v3+json"} async with aiohttp.ClientSession() as session: async with session.get( - GITHUB_LATEST_RELEASE_URL, + GITHUB_TAGS_URL, headers=headers, timeout=aiohttp.ClientTimeout(total=15), ) as resp: tags = await resp.json(content_type=None) - - if not tags or not isinstance(tags, list): - return False, current, current - - # Find the highest semver tag (tags are not guaranteed sorted) - latest = "0.0.0" - for tag in tags: - name = tag.get("name", "") - try: - if parse_version(name) > parse_version(latest): - latest = name.strip().lstrip("vV") - except (ValueError, AttributeError): - continue - except Exception: - # Network error — treat as "no update available" + # Network error — treat as "no update available". + return False, current, current + + if not tags or not isinstance(tags, list): return False, current, current + # Find the highest semver tag. GitHub tags are not guaranteed sorted. + latest = "0.0.0" + for tag in tags: + name = tag.get("name", "") + try: + if parse_version(name) > parse_version(latest): + latest = name.strip().lstrip("vV") + except (ValueError, AttributeError): + continue + return is_newer(latest, current), current, latest @@ -95,14 +174,12 @@ async def check_for_update() -> Tuple[bool, str, str]: async def perform_update( progress_callback: Optional[Callable[[str], Awaitable[None]]] = None, ) -> None: - """Launch the external updater script in a new window, then shut down. - - The updater script (scripts/updater.bat on Windows) runs in its own - visible terminal and handles: waiting for us to exit, git pull, install, - and relaunch. This keeps the update logic out of the running Python - process — no in-process git mutation, no exit-code signalling, no - console-visibility hacks. If the updater fails, its window stays open - showing the error. + """Launch the external updater script, then shut down. + + The script waits for CraftBot to exit, pulls the update branch, installs + dependencies, and relaunches CraftBot. Running that work outside this + process avoids in-process git mutation and exit-code signalling. Failures + are written to updater.log. """ async def emit(msg: str) -> None: @@ -111,12 +188,8 @@ async def emit(msg: str) -> None: project_root = Path(__file__).resolve().parent.parent - target_branch = "main" - - if sys.platform == "win32": - updater_script = project_root / "scripts" / "updater.bat" - else: - updater_script = project_root / "scripts" / "updater.sh" + target_branch = UPDATE_BRANCH + updater_script = _updater_script_path(project_root) if not updater_script.exists(): raise RuntimeError(f"Updater script not found: {updater_script}") @@ -131,14 +204,14 @@ async def emit(msg: str) -> None: CREATE_NO_WINDOW = 0x08000000 DETACHED_PROCESS = 0x00000008 subprocess.Popen( - [str(updater_script), target_branch], + [str(updater_script), target_branch, sys.executable], cwd=str(project_root), creationflags=DETACHED_PROCESS | CREATE_NO_WINDOW, close_fds=True, ) else: subprocess.Popen( - ["sh", str(updater_script), target_branch], + ["sh", str(updater_script), target_branch, sys.executable], cwd=str(project_root), start_new_session=True, ) @@ -155,15 +228,39 @@ async def emit(msg: str) -> None: # --------------------------------------------------------------------------- -async def _run_git(cmd: list, cwd: str) -> Tuple[bytes, bytes]: +def _decode_git_stdout(stdout: bytes) -> str: + return stdout.decode("utf-8", errors="replace").strip() + + +def _updater_script_path(project_root: Path, platform: str = sys.platform) -> Path: + if platform == "win32": + return project_root / "scripts" / "updater.bat" + return project_root / "scripts" / "updater.sh" + + +async def _run_git( + cmd: list, cwd: str, timeout: int = GIT_PROBE_TIMEOUT +) -> Tuple[bytes, bytes]: """Run a git command asynchronously; raise on non-zero exit.""" + env = os.environ.copy() + env.setdefault("GIT_TERMINAL_PROMPT", "0") + proc = await asyncio.create_subprocess_exec( *cmd, cwd=cwd, + env=env, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, ) - stdout, stderr = await proc.communicate() + try: + stdout, stderr = await asyncio.wait_for(proc.communicate(), timeout=timeout) + except asyncio.TimeoutError as exc: + proc.kill() + stdout, stderr = await proc.communicate() + raise RuntimeError( + f"{' '.join(cmd)} timed out after {timeout} seconds" + ) from exc + if proc.returncode != 0: err = ( stderr.decode("utf-8", errors="replace").strip() diff --git a/scripts/updater.bat b/scripts/updater.bat index 9a75c2ff..fd60cf30 100644 --- a/scripts/updater.bat +++ b/scripts/updater.bat @@ -2,43 +2,48 @@ setlocal rem Argument 1 is the git branch to pull (default: main). -set BRANCH=%1 +set "BRANCH=%~1" if "%BRANCH%"=="" set BRANCH=main +rem Argument 2 is the Python executable that launched CraftBot. +set "PYTHON_BIN=%~2" +if "%PYTHON_BIN%"=="" set PYTHON_BIN=python + rem Project root is the parent of scripts/ cd /d "%~dp0.." rem Log everything to updater.log so failures are debuggable (we run headlessly). -set LOG=%~dp0..\updater.log +set "LOG=%~dp0..\updater.log" echo. >> "%LOG%" echo ============================================ >> "%LOG%" echo %DATE% %TIME% - Updater start (branch=%BRANCH%) >> "%LOG%" echo CWD=%CD% >> "%LOG%" +echo Python=%PYTHON_BIN% >> "%LOG%" rem Wait for current CraftBot to fully terminate. timeout /t 3 /nobreak > nul echo --- git fetch --- >> "%LOG%" -git fetch origin %BRANCH% >> "%LOG%" 2>&1 +git fetch origin "%BRANCH%" >> "%LOG%" 2>&1 if errorlevel 1 goto :fail echo --- git checkout --- >> "%LOG%" -git checkout %BRANCH% >> "%LOG%" 2>&1 +git checkout "%BRANCH%" >> "%LOG%" 2>&1 if errorlevel 1 goto :fail echo --- git pull --- >> "%LOG%" -git pull origin %BRANCH% >> "%LOG%" 2>&1 +git pull --ff-only origin "%BRANCH%" >> "%LOG%" 2>&1 if errorlevel 1 goto :fail if exist install.py ( echo --- install.py --- >> "%LOG%" - python install.py >> "%LOG%" 2>&1 + "%PYTHON_BIN%" install.py >> "%LOG%" 2>&1 if errorlevel 1 goto :fail ) echo --- relaunching CraftBot --- >> "%LOG%" rem Launch the new CraftBot. This bat process exits and the new run.py takes over. -start "CraftBot" python run.py --conda +start "CraftBot" "%PYTHON_BIN%" run.py --conda echo %DATE% %TIME% - Updater done, relaunched CraftBot >> "%LOG%" exit /b 0 diff --git a/scripts/updater.sh b/scripts/updater.sh new file mode 100755 index 00000000..be01ad00 --- /dev/null +++ b/scripts/updater.sh @@ -0,0 +1,54 @@ +#!/bin/sh +set -u + +# Argument 1 is the git branch to pull (default: main). +BRANCH="${1:-main}" + +# Argument 2 is the Python executable that launched CraftBot. Reusing it keeps +# updates on the same interpreter and avoids python/python3 mismatches. +PYTHON_BIN="${2:-${PYTHON:-python3}}" + +SCRIPT_DIR=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd) +ROOT_DIR=$(CDPATH= cd -- "$SCRIPT_DIR/.." && pwd) +LOG="$ROOT_DIR/updater.log" + +log() { + printf '%s\n' "$*" >> "$LOG" +} + +fail() { + log "$(date) - UPDATE FAILED: $*" + exit 1 +} + +{ + printf '\n' + printf '============================================\n' + printf '%s - Updater start (branch=%s)\n' "$(date)" "$BRANCH" + printf 'CWD=%s\n' "$ROOT_DIR" + printf 'Python=%s\n' "$PYTHON_BIN" +} >> "$LOG" + +cd "$ROOT_DIR" || fail "could not cd to $ROOT_DIR" + +# Wait for current CraftBot to fully terminate. +sleep 3 + +log "--- git fetch ---" +git fetch origin "$BRANCH" >> "$LOG" 2>&1 || fail "git fetch failed" + +log "--- git checkout ---" +git checkout "$BRANCH" >> "$LOG" 2>&1 || fail "git checkout failed" + +log "--- git pull ---" +git pull --ff-only origin "$BRANCH" >> "$LOG" 2>&1 || fail "git pull failed" + +if [ -f install.py ]; then + log "--- install.py ---" + "$PYTHON_BIN" install.py >> "$LOG" 2>&1 || fail "install.py failed" +fi + +log "--- relaunching CraftBot ---" +nohup "$PYTHON_BIN" run.py --conda >> "$LOG" 2>&1 & +log "$(date) - Updater done, relaunched CraftBot" +exit 0 diff --git a/tests/test_updater.py b/tests/test_updater.py new file mode 100644 index 00000000..89f1eeb0 --- /dev/null +++ b/tests/test_updater.py @@ -0,0 +1,189 @@ +# -*- coding: utf-8 -*- + +import asyncio +from pathlib import Path + +import pytest + +from app import updater + + +INSIDE_WORK_TREE = ("git", "rev-parse", "--is-inside-work-tree") +FETCH_MAIN = ("git", "fetch", "origin", "main:refs/remotes/origin/main") +HEAD = ("git", "rev-parse", "HEAD") +ORIGIN_MAIN = ("git", "rev-parse", "origin/main") +COUNT_MAIN = ("git", "rev-list", "--left-right", "--count", "HEAD...origin/main") + +LOCAL_REVISION = b"1111111111111111111111111111111111111111\n" +REMOTE_REVISION = b"2222222222222222222222222222222222222222\n" +LOCAL_AHEAD_REVISION = b"3333333333333333333333333333333333333333\n" + + +def run(coro): + return asyncio.run(coro) + + +def stub_git(monkeypatch, responses): + calls = [] + + async def fake_run_git(cmd, cwd): + key = tuple(cmd) + calls.append(key) + if key not in responses: + raise AssertionError(f"unexpected git command: {cmd}") + return responses[key], b"" + + monkeypatch.setattr(updater, "_run_git", fake_run_git) + return calls + + +def test_source_update_detects_remote_main_ahead(monkeypatch, tmp_path): + calls = stub_git( + monkeypatch, + { + INSIDE_WORK_TREE: b"true\n", + FETCH_MAIN: b"", + HEAD: LOCAL_REVISION, + ORIGIN_MAIN: REMOTE_REVISION, + COUNT_MAIN: b"0\t3\n", + }, + ) + + result = run(updater._check_source_update(tmp_path, "1.3.4", branch="main")) + + assert result == (True, "1.3.4", "1.3.4+main.2222222") + assert FETCH_MAIN in calls + + +@pytest.mark.parametrize( + ("local_revision", "remote_revision", "count_output"), + [ + (LOCAL_REVISION, LOCAL_REVISION, None), + (LOCAL_AHEAD_REVISION, REMOTE_REVISION, b"2\t0\n"), + ], +) +def test_source_update_reports_no_update_without_remote_commits( + monkeypatch, tmp_path, local_revision, remote_revision, count_output +): + responses = { + INSIDE_WORK_TREE: b"true\n", + FETCH_MAIN: b"", + HEAD: local_revision, + ORIGIN_MAIN: remote_revision, + } + if count_output is not None: + responses[COUNT_MAIN] = count_output + stub_git(monkeypatch, responses) + + assert run(updater._check_source_update(tmp_path, "1.3.4", branch="main")) == ( + False, + "1.3.4", + "1.3.4", + ) + + +def test_source_update_returns_none_outside_git_checkout(monkeypatch, tmp_path): + async def fake_run_git(cmd, cwd): + raise RuntimeError("not a git checkout") + + monkeypatch.setattr(updater, "_run_git", fake_run_git) + + assert run(updater._check_source_update(tmp_path, "1.3.4", branch="main")) is None + + +def test_check_for_update_prefers_release_tag_when_semver_is_newer(monkeypatch): + async def fail_source_update(project_root, current, branch=updater.UPDATE_BRANCH): + raise AssertionError("source check should not run when release is newer") + + async def newer_release_check(current): + return True, current, "1.3.5" + + monkeypatch.setattr("app.config.get_app_version", lambda: "1.3.4") + monkeypatch.setattr(updater, "_check_source_update", fail_source_update) + monkeypatch.setattr(updater, "_check_release_update", newer_release_check) + + assert run(updater.check_for_update()) == (True, "1.3.4", "1.3.5") + + +def test_check_for_update_uses_source_checkout_when_release_tag_is_current(monkeypatch): + async def fake_source_update(project_root, current, branch=updater.UPDATE_BRANCH): + return True, current, "1.3.4+main.2222222" + + async def current_release_check(current): + return False, current, current + + monkeypatch.setattr("app.config.get_app_version", lambda: "1.3.4") + monkeypatch.setattr(updater, "_check_source_update", fake_source_update) + monkeypatch.setattr(updater, "_check_release_update", current_release_check) + + assert run(updater.check_for_update()) == ( + True, + "1.3.4", + "1.3.4+main.2222222", + ) + + +def test_check_for_update_falls_back_to_release_tags(monkeypatch): + async def no_source_update(project_root, current, branch=updater.UPDATE_BRANCH): + return None + + async def fake_release_check(current): + return False, current, "1.3.4" + + monkeypatch.setattr("app.config.get_app_version", lambda: "1.3.4") + monkeypatch.setattr(updater, "_check_source_update", no_source_update) + monkeypatch.setattr(updater, "_check_release_update", fake_release_check) + + assert run(updater.check_for_update()) == (False, "1.3.4", "1.3.4") + + +def test_perform_update_launches_posix_script_with_current_python(monkeypatch): + class ExitCalled(Exception): + def __init__(self, code): + self.code = code + super().__init__(code) + + launched = {} + + def fake_popen(args, **kwargs): + launched["args"] = args + launched["kwargs"] = kwargs + + async def no_sleep(delay): + return None + + def fake_exit(code): + raise ExitCalled(code) + + project_root = Path(__file__).resolve().parent.parent + updater_script = project_root / "scripts" / "updater.sh" + + monkeypatch.setattr(updater.sys, "platform", "linux") + monkeypatch.setattr(updater.subprocess, "Popen", fake_popen) + monkeypatch.setattr(updater.asyncio, "sleep", no_sleep) + monkeypatch.setattr(updater.os, "_exit", fake_exit) + + with pytest.raises(ExitCalled) as exc_info: + run(updater.perform_update()) + + assert exc_info.value.code == 0 + assert launched["args"] == [ + "sh", + str(updater_script), + updater.UPDATE_BRANCH, + updater.sys.executable, + ] + assert launched["kwargs"]["cwd"] == str(project_root) + assert launched["kwargs"]["start_new_session"] is True + + +@pytest.mark.parametrize( + ("platform", "expected"), + [("win32", "updater.bat"), ("darwin", "updater.sh"), ("linux", "updater.sh")], +) +def test_updater_script_path_is_platform_specific(platform, expected): + assert updater._updater_script_path(Path("/repo"), platform).name == expected + + +def test_posix_updater_script_is_versioned(): + assert (Path(__file__).resolve().parent.parent / "scripts" / "updater.sh").is_file()