diff --git a/tests/updater/conftest.py b/tests/updater/conftest.py index b6bdab52..ad2e198d 100644 --- a/tests/updater/conftest.py +++ b/tests/updater/conftest.py @@ -58,6 +58,9 @@ def svc(): mock_svc.recover = AsyncMock() mock_svc.has_fetch_failures = MagicMock(return_value=False) mock_svc.provision_missing = AsyncMock(return_value=False) + mock_svc.run_followup = AsyncMock(return_value=(False, False)) + mock_svc.background_apt_upgrade = AsyncMock() + mock_svc.daemon_restart_pending = False mock_svc.reconcile = AsyncMock(return_value=True) mock_svc.reconcile_if_pending = AsyncMock() mock_svc.deploy_pending = MagicMock(return_value=False) @@ -80,6 +83,7 @@ def svc(): s._busy = False s._provisioning = False s._provisioned = False + s._followed_up = False s._boot_pass_done = True s._deploy_hold = None s._capped_flag = None diff --git a/tests/updater/test_dbus_service_unit.py b/tests/updater/test_dbus_service_unit.py index 11e190ef..1c4d8295 100644 --- a/tests/updater/test_dbus_service_unit.py +++ b/tests/updater/test_dbus_service_unit.py @@ -473,6 +473,24 @@ async def test_hold_end_leaves_running_provision_busy(self, svc, monkeypatch): svc._provision_busy(False) assert svc._busy is False + @pytest.mark.asyncio + @pytest.mark.parametrize("followed_up", [False, True]) + async def test_hold_end_runs_pending_followup(self, svc, monkeypatch, followed_up): + """The boot hold blocks the follow-up; it runs on release, not a poll later.""" + from updater import dbus_service + + monkeypatch.setattr(dbus_service, "_DEPLOY_POLL_S", 0.01) + svc._provisioned = True + svc._followed_up = followed_up + svc._svc.run_followup = AsyncMock(return_value=(False, False)) + svc._svc.deploy_pending.return_value = True + svc._set_busy(False) + await asyncio.sleep(0.03) + svc._svc.run_followup.assert_not_awaited() + svc._svc.deploy_pending.return_value = False + await asyncio.sleep(0.05) + assert svc._svc.run_followup.await_count == (0 if followed_up else 1) + @pytest.mark.asyncio async def test_update_rejected_during_hold(self, svc): """install-updater restarts moonraker: no new batch may start under it.""" @@ -603,6 +621,55 @@ async def test_deferred_provision_repolls_soon_then_stops(self, svc): assert svc._svc.provision_missing.await_count == 2 assert sleeps == [dbus_service._FETCH_RETRY_INTERVAL_S, 86_400.0, 86_400.0] + @pytest.mark.asyncio + async def test_followup_runs_once_after_provisioning(self, svc): + svc._svc.poll_interval = 86_400.0 + sleeps = await self._run_polls(svc, polls=3) + svc._svc.run_followup.assert_awaited_once_with(svc._set_busy) + assert sleeps == [86_400.0] * 3 + + @pytest.mark.asyncio + async def test_deferred_followup_repolls_soon(self, svc): + from updater import dbus_service + + svc._svc.poll_interval = 86_400.0 + svc._svc.run_followup = AsyncMock(side_effect=[(True, False), (False, False)]) + sleeps = await self._run_polls(svc, polls=3) + assert svc._svc.run_followup.await_count == 2 + assert sleeps == [dbus_service._FETCH_RETRY_INTERVAL_S, 86_400.0, 86_400.0] + + @pytest.mark.asyncio + async def test_followup_that_ran_sweeps_apt(self, svc): + svc._provisioned = True + svc._svc.run_followup = AsyncMock(return_value=(False, True)) + await svc._followup_pass() + await asyncio.gather(*svc._background_tasks) + svc._svc.background_apt_upgrade.assert_awaited_once() + assert svc._followed_up is True + + @pytest.mark.asyncio + async def test_followup_skips_apt_when_restart_pending(self, svc): + svc._provisioned = True + svc._svc.daemon_restart_pending = True + svc._svc.run_followup = AsyncMock(return_value=(False, True)) + await svc._followup_pass() + svc._svc.background_apt_upgrade.assert_not_called() + + @pytest.mark.asyncio + async def test_followup_waits_while_busy(self, svc): + svc._provisioned = True + svc._busy = True + await svc._followup_pass() + svc._svc.run_followup.assert_not_awaited() + assert svc._followed_up is False + + @pytest.mark.asyncio + async def test_failed_followup_is_not_retried(self, svc): + svc._provisioned = True + svc._svc.run_followup = AsyncMock(side_effect=RuntimeError("boom")) + await svc._followup_pass() + assert svc._followed_up is True + class TestProvisioningFlag: def test_provision_busy_emits_provisioning_then_busy(self, svc): diff --git a/tests/updater/test_executor_unit.py b/tests/updater/test_executor_unit.py index eb7c2c89..c5d3987b 100644 --- a/tests/updater/test_executor_unit.py +++ b/tests/updater/test_executor_unit.py @@ -39,6 +39,7 @@ git_remote_url, git_repair, git_reset_to_hash, + git_show_file, git_prune_extra_remotes, git_untracked_paths, enable_service, @@ -937,6 +938,53 @@ async def test_dead_configured_branch_reports_actionable_error(self, tmp_path): ) assert result.error == "branch origin/wip/dead not found - fix components.yaml" + @pytest.mark.asyncio + @pytest.mark.parametrize(("pin", "behind"), [(b"abc1234\n", 0), (b"def5678\n", 1)]) + async def test_pinned_version_dirty_only_off_the_pin(self, tmp_path, pin, behind): + # A moved pin must count as pending, or Update never applies it. + procs = [ + _make_proc(0, b"abc1234\n", b""), # git_get_hash + _make_proc(0, b"main\n", b""), # git_get_current_branch + _make_proc(0, pin, b""), # git_ref_hash version^{commit} + _make_proc(0, b"https://github.com/x/y\n", b""), # git_remote_url + _make_proc(0, b"", b""), # git_is_dirty (clean) + _make_proc(0, b"v1.0\n", b""), # git_describe current + _make_proc(0, b"v1.1\n", b""), # git_describe version + ] + exec_mock = AsyncMock(side_effect=procs) + with patch("asyncio.create_subprocess_exec", exec_mock): + result = await check_git_status( + "Spoolman", tmp_path, version="def5678", skip_fetch=True + ) + assert result.commits_behind == behind + assert result.remote_version == "v1.1" + describe_args = exec_mock.await_args_list[-1].args + assert describe_args[-1] == "def5678" + + +class TestGitShowFile: + @pytest.mark.asyncio + async def test_returns_content_at_ref(self, tmp_path): + exec_mock = AsyncMock(return_value=_make_proc(0, b"components: []\n", b"")) + with patch("asyncio.create_subprocess_exec", exec_mock): + out = await git_show_file(tmp_path, "a" * 40, "updater/components.yaml") + assert out == "components: []\n" + assert exec_mock.await_args.args[-1] == f"{'a' * 40}:updater/components.yaml" + + @pytest.mark.asyncio + @pytest.mark.parametrize("ref", ["main", "--output=/tmp/x", ""]) + async def test_rejects_non_sha_ref_without_running_git(self, tmp_path, ref): + exec_mock = AsyncMock() + with patch("asyncio.create_subprocess_exec", exec_mock): + assert await git_show_file(tmp_path, ref, "updater/components.yaml") == "" + exec_mock.assert_not_called() + + @pytest.mark.asyncio + async def test_git_failure_returns_empty(self, tmp_path): + exec_mock = AsyncMock(return_value=_make_proc(128, b"", b"fatal: bad object\n")) + with patch("asyncio.create_subprocess_exec", exec_mock): + assert await git_show_file(tmp_path, "a" * 40, "x") == "" + class TestCheckAptStatus: @pytest.mark.asyncio @@ -1177,6 +1225,7 @@ async def test_invalid_name_never_queries(self): assert await service_unit_missing("bad;name.service") is False mock_exec.assert_not_called() + class TestRunHook: """run_hook: resolve under hooks dir, pass timeout, reject traversal.""" diff --git a/tests/updater/test_hooks_unit.py b/tests/updater/test_hooks_unit.py index d52c553d..64c52578 100644 --- a/tests/updater/test_hooks_unit.py +++ b/tests/updater/test_hooks_unit.py @@ -11,7 +11,9 @@ HOOK = Path(__file__).resolve().parents[2] / "updater" / "hooks" / "DeviceDiscovery.sh" -def _run_hook(tmp_path: Path, repo: Path, load_state: str) -> subprocess.CompletedProcess: +def _run_hook( + tmp_path: Path, repo: Path, load_state: str +) -> subprocess.CompletedProcess: """Run the hook with a stub systemctl that reports load_state.""" stub_bin = tmp_path / "stub-bin" stub_bin.mkdir() @@ -66,3 +68,100 @@ def test_set_up_machine_rebuilds(self, tmp_path, repo): assert result.returncode == 0, result.stderr assert (repo / "built").exists() assert not (tmp_path / "run" / "sentinel").exists() + + +UI_HOOK = HOOK.with_name("BlocksScreen.sh") + + +def _git(repo: Path, *args: str) -> str: + return subprocess.run( + ["git", "-C", str(repo), *args], capture_output=True, text=True, check=True + ).stdout.strip() + + +def _commit(repo: Path, files: dict[str, str]) -> str: + for rel, text in files.items(): + (repo / rel).parent.mkdir(parents=True, exist_ok=True) + (repo / rel).write_text(text) + _git(repo, "add", "-A") + _git( + repo, + "-c", + "user.name=t", + "-c", + "user.email=t@t", + "-c", + "commit.gpgsign=false", + "commit", + "-qm", + "c", + ) + return _git(repo, "rev-parse", "HEAD") + + +class TestBlocksScreenHook: + """Updater code or manifest changes must reach the daemon within the same Update.""" + + @pytest.fixture + def ui_repo(self, tmp_path: Path) -> Path: + repo = tmp_path / "BlocksScreen" + repo.mkdir() + _git(repo, "init", "-q") + return repo + + def _run(self, tmp_path: Path, repo: Path, prev: str, new: str, **extra: str): + env = { + "PATH": os.environ["PATH"], + "HOME": str(tmp_path), + "COMPONENT_PATH": str(repo), + "PREV_HASH": prev, + "NEW_HASH": new, + "BS_UPDATER_SELF_UPDATE": "1", + "BS_UPDATER_RESTART_SENTINEL": str(tmp_path / "run" / "sentinel"), + **extra, + } + result = subprocess.run( + ["bash", str(UI_HOOK)], env=env, capture_output=True, text=True, check=False + ) + assert result.returncode == 0, result.stderr + followup = tmp_path / ".cache" / "blockscreen" / "updater_followup" + return tmp_path / "run" / "sentinel", followup + + def test_manifest_change_queues_followup_and_restart(self, tmp_path, ui_repo): + prev = _commit(ui_repo, {"updater/components.yaml": "a: 1\n"}) + new = _commit(ui_repo, {"updater/components.yaml": "a: 2\n"}) + sentinel, followup = self._run(tmp_path, ui_repo, prev, new) + assert sentinel.read_text().split() == ["code"] + assert followup.read_text().strip() == prev + + def test_code_only_change_restarts_without_followup(self, tmp_path, ui_repo): + prev = _commit(ui_repo, {"updater/service.py": "x = 1\n"}) + new = _commit(ui_repo, {"updater/service.py": "x = 2\n"}) + sentinel, followup = self._run(tmp_path, ui_repo, prev, new) + assert sentinel.read_text().split() == ["code"] + assert not followup.exists() + + def test_non_updater_change_does_nothing(self, tmp_path, ui_repo): + prev = _commit(ui_repo, {"BlocksScreen/a.py": "1\n"}) + new = _commit(ui_repo, {"BlocksScreen/a.py": "2\n"}) + sentinel, followup = self._run(tmp_path, ui_repo, prev, new) + assert not sentinel.exists() + assert not followup.exists() + + def test_chained_updates_keep_oldest_base(self, tmp_path, ui_repo): + first = _commit(ui_repo, {"updater/components.yaml": "a: 1\n"}) + mid = _commit(ui_repo, {"updater/components.yaml": "a: 2\n"}) + last = _commit(ui_repo, {"updater/components.yaml": "a: 3\n"}) + self._run(tmp_path, ui_repo, first, mid) + _, followup = self._run(tmp_path, ui_repo, mid, last) + assert followup.read_text().strip() == first + + def test_outside_updater_no_restart_sentinel(self, tmp_path, ui_repo): + # CLI/manual runs: nothing defers the restart, so none is requested. + prev = _commit(ui_repo, {"updater/components.yaml": "a: 1\n"}) + new = _commit(ui_repo, {"updater/components.yaml": "a: 2\n"}) + sentinel, followup = self._run( + tmp_path, ui_repo, prev, new, BS_UPDATER_SELF_UPDATE="" + ) + assert not sentinel.exists() + assert followup.exists() diff --git a/tests/updater/test_service_unit.py b/tests/updater/test_service_unit.py index 8b5bc204..1e9d6b36 100644 --- a/tests/updater/test_service_unit.py +++ b/tests/updater/test_service_unit.py @@ -41,6 +41,28 @@ def test_all_methods_do_not_raise(self): cb.on_recover("klipper", True) +class TestNeedsUpdate: + @pytest.mark.parametrize( + "fields", + [ + {"commits_behind": 1}, + {"packages_upgradable": 2}, + {"has_local_changes": True}, + {"needs_install": True}, + {"branch_mismatch": True}, + {"kind": "git", "error": "fetch failed"}, + ], + ) + def test_dirty(self, fields): + assert ComponentStatus(name="c", **fields).needs_update + + @pytest.mark.parametrize( + "fields", [{}, {"kind": "apt", "error": "locked"}, {"packages_upgradable": -1}] + ) + def test_clean(self, fields): + assert not ComponentStatus(name="c", **fields).needs_update + + class TestBackoff: def test_permanent_trips_long_fixed_cooldown(self): b = updater_service._Backoff(base=30, cap=1800, permanent=3600) @@ -2327,6 +2349,187 @@ async def test_runs_pending_boot_heal_before_cloning(self, tmp_path): assert order == ["heal", "clone"] +class TestRunFollowup: + """run_followup applies manifest entries the pre-update daemon never loaded.""" + + BASE = "a" * 40 + + @pytest.fixture + def svc(self, tmp_path): + s = UpdateService() + s._followup_path = tmp_path / "updater_followup" + s._components = [ + ComponentConfig(name="BlocksScreen", kind="git", path=tmp_path), + ComponentConfig( + name="Spoolman", kind="git", url="https://x/s", version="b" * 40 + ), + ComponentConfig(name="klipper", kind="git", url="https://x/k"), + ] + return s + + @pytest.fixture(autouse=True) + def env(self): + with ( + patch("updater.service.process_lock", lambda: nullcontext(True)), + patch.object( + UpdateService, "deploy_running", AsyncMock(return_value=False) + ), + patch.object( + UpdateService, "update_all", AsyncMock(return_value=True) + ) as ua, + patch("updater.service.git_remote_reachable", AsyncMock(return_value=True)), + ): + yield ua + + def _mark(self, svc, text: str | None = None) -> None: + svc._followup_path.write_text(f"{text or self.BASE}\n") + + @pytest.mark.asyncio + async def test_no_marker_is_a_noop(self, svc, env): + with patch.object(UpdateService, "_followup_targets") as targets: + assert await svc.run_followup() == (False, False) + targets.assert_not_called() + env.assert_not_called() + + @pytest.mark.asyncio + async def test_malformed_marker_is_dropped(self, svc): + self._mark(svc, "not-a-sha") + assert await svc.run_followup() == (False, False) + assert not svc._followup_path.exists() + + @pytest.mark.asyncio + @pytest.mark.parametrize("cause", ["printing", "deploy", "lock"]) + async def test_defers_and_keeps_marker(self, svc, env, printing, cause): + self._mark(svc) + printing.return_value = cause == "printing" + with ( + patch.object( + UpdateService, + "deploy_running", + AsyncMock(return_value=cause == "deploy"), + ), + patch("updater.service.process_lock", lambda: nullcontext(cause != "lock")), + ): + assert await svc.run_followup() == (True, False) + assert svc._followup_path.exists() + env.assert_not_called() + + @pytest.mark.asyncio + async def test_applies_targets_and_clears_marker(self, svc, env): + self._mark(svc) + on_busy = MagicMock() + with patch.object( + UpdateService, "_followup_targets", AsyncMock(return_value={"Spoolman"}) + ): + assert await svc.run_followup(on_busy) == (False, True) + env.assert_awaited_once_with({"Spoolman"}) + assert on_busy.call_args_list == [call(True), call(False)] + assert not svc._followup_path.exists() + + @pytest.mark.asyncio + async def test_busy_before_network_checks(self, svc, env): + # An Update pressed during the remote checks must be refused, not race the lock. + self._mark(svc) + on_busy = MagicMock() + seen = [] + + async def targets(_self, _base): + seen.append(list(on_busy.call_args_list)) + return set() + + with patch.object(UpdateService, "_followup_targets", targets): + assert await svc.run_followup(on_busy) == (False, False) + assert seen == [[call(True)]] + assert on_busy.call_args_list == [call(True), call(False)] + + @pytest.mark.asyncio + async def test_failed_update_is_not_retried(self, svc, env): + # One-shot: the marker is gone before update_all, so a crash cannot loop it. + self._mark(svc) + env.side_effect = RuntimeError("boom") + with ( + patch.object( + UpdateService, "_followup_targets", AsyncMock(return_value={"Spoolman"}) + ), + pytest.raises(RuntimeError), + ): + await svc.run_followup() + assert not svc._followup_path.exists() + + @pytest.mark.asyncio + async def test_nothing_dirty_clears_marker(self, svc, env): + self._mark(svc) + with patch.object( + UpdateService, "_followup_targets", AsyncMock(return_value=set()) + ): + assert await svc.run_followup() == (False, False) + env.assert_not_called() + assert not svc._followup_path.exists() + + @pytest.mark.asyncio + async def test_offline_defers_then_leaves_it_to_update(self, svc, env): + self._mark(svc) + tries = updater_service._OFFLINE_PROVISION_TRIES + with ( + patch.object( + UpdateService, "_followup_targets", AsyncMock(return_value={"Spoolman"}) + ), + patch( + "updater.service.git_remote_reachable", AsyncMock(return_value=False) + ), + ): + results = [await svc.run_followup() for _ in range(tries)] + assert results == [(True, False)] * (tries - 1) + [(False, False)] + env.assert_not_called() + assert not svc._followup_path.exists() + + @pytest.mark.asyncio + async def test_targets_are_changed_and_dirty_entries_except_ui(self, svc): + old = [ + ComponentConfig(name="BlocksScreen", kind="git"), # changed, but excluded + ComponentConfig(name="Spoolman", kind="git", url="https://x/s"), # new pin + ComponentConfig(name="klipper", kind="git", url="https://x/k"), # same + ] + statuses = { + n: ComponentStatus(name=n, commits_behind=1) + for n in ("BlocksScreen", "Spoolman", "klipper") + } + with ( + patch( + "updater.service.git_show_file", AsyncMock(return_value="yaml") + ) as show, + patch("updater.service.load_components", return_value=(old, 60.0)), + patch.object( + UpdateService, "check_status", AsyncMock(return_value=statuses) + ), + ): + assert await svc._followup_targets(self.BASE) == {"Spoolman"} + show.assert_awaited_once_with( + svc._components[0].path, self.BASE, "updater/components.yaml" + ) + + @pytest.mark.asyncio + async def test_clean_changed_entry_is_not_a_target(self, svc): + statuses = {"Spoolman": ComponentStatus(name="Spoolman")} + with ( + patch("updater.service.git_show_file", AsyncMock(return_value="yaml")), + patch("updater.service.load_components", return_value=([], 60.0)), + patch.object( + UpdateService, "check_status", AsyncMock(return_value=statuses) + ), + ): + assert await svc._followup_targets(self.BASE) == set() + + @pytest.mark.asyncio + async def test_missing_base_manifest_has_no_targets(self, svc): + with ( + patch("updater.service.git_show_file", AsyncMock(return_value="")), + patch.object(UpdateService, "check_status") as check, + ): + assert await svc._followup_targets(self.BASE) == set() + check.assert_not_called() + + class TestReclone: """_reclone_component: temp-clone + atomic swap as the deepest corruption rung.""" diff --git a/updater/components.py b/updater/components.py index 1b0e1b54..c024c5d6 100644 --- a/updater/components.py +++ b/updater/components.py @@ -223,12 +223,14 @@ def _merge(base: list[dict], override: list[dict]) -> list[dict]: return list(base_by_name.values()) -def _load_bundled_yaml(yaml) -> dict | None: - """Read + parse the bundled components.yaml; None on any read/parse error.""" +def _load_bundled_yaml(yaml, text: str | None = None) -> dict | None: + """Parse text, else the bundled components.yaml; None on any read/parse error.""" bundled_path = Path(__file__).parent / "components.yaml" try: - with open(bundled_path) as f: # noqa: PTH123 - bundled_data = yaml.safe_load(f) + if text is None: + with open(bundled_path) as f: # noqa: PTH123 + text = f.read() + bundled_data = yaml.safe_load(text) except OSError: logger.exception("bundled_data not found") return None @@ -300,8 +302,10 @@ def _build_configs(raw_components: list[dict]) -> list[ComponentConfig]: return configs -def load_components() -> tuple[list[ComponentConfig], float]: - """Load/validate components from bundled YAML merged with the user override.""" +def load_components( + bundled_text: str | None = None, +) -> tuple[list[ComponentConfig], float]: + """Load/validate components from bundled YAML (or bundled_text) merged with the user override.""" try: import yaml # noqa: PLC0415 @@ -311,7 +315,7 @@ def load_components() -> tuple[list[ComponentConfig], float]: logger.exception("PyYAML missing or broken; updater idle until venv repair") return [], 1440 * 60.0 - bundled_data = _load_bundled_yaml(yaml) + bundled_data = _load_bundled_yaml(yaml, bundled_text) if bundled_data is None: return [], 1440 * 60.0 raw_components: list[dict] = bundled_data.get("components", []) diff --git a/updater/dbus_service.py b/updater/dbus_service.py index b7286319..95a98016 100644 --- a/updater/dbus_service.py +++ b/updater/dbus_service.py @@ -115,6 +115,7 @@ def __init__(self) -> None: self._busy: bool = False self._provisioning: bool = False self._provisioned: bool = False + self._followed_up: bool = False # request_status waits for the boot pass, so a client never sees pre-install status. self._boot_pass_done: bool = False self._deploy_hold: asyncio.Task | None = None @@ -243,6 +244,9 @@ async def _hold_for_deploy(self) -> None: # A provisioning pass that started under the hold still owns busy. if not self._provisioning: self._emit_busy(busy=False) + # The follow-up skipped while the hold owned busy: run it now, not at the next poll. + if self._provisioned and not self._followed_up: + self._spawn(self._followup_pass(), name="followup_after_deploy") def _validate_component_name(self, name: str) -> bool: """SEC: verify component exists to prevent abuse on unknown names.""" @@ -315,15 +319,38 @@ async def _periodic_status_check(self) -> None: await self._emit_status() except Exception as exc: # noqa: BLE001 _log.error("periodic_check failed: %s", exc) + await self._followup_pass() interval = self._svc.poll_interval if self._svc.has_fetch_failures(): interval = min(_FETCH_RETRY_INTERVAL_S, interval) _log.info("fetch failures pending - re-polling in %.0fs", interval) - elif not self._provisioned: + elif not self._provisioned or not self._followed_up: interval = min(_FETCH_RETRY_INTERVAL_S, interval) - _log.info("provisioning deferred - re-polling in %.0fs", interval) + _log.info( + "provisioning/followup deferred - re-polling in %.0fs", interval + ) await asyncio.sleep(interval) + async def _followup_pass(self) -> None: + """After provisioning, apply manifest changes the previous daemon could not see.""" + if not self._provisioned or self._followed_up or self._busy: + return + try: + retry, ran = await self._svc.run_followup(self._set_busy) + if ran: + await self._emit_status() + except Exception as exc: # noqa: BLE001 + _log.error("followup pass failed: %s", exc, exc_info=True) + retry, ran = False, False + self._followed_up = not retry + if not ran: + return + # The restart that led here skipped the Update's background apt sweep. + if not self._svc.daemon_restart_pending: + self._spawn( + self._svc.background_apt_upgrade(), name="background_apt_upgrade" + ) + @sdbus.dbus_method_async(result_signature="b") async def update_all(self) -> bool: """D-Bus method: fire-and-forget; reply is sent immediately, update runs as a task.""" @@ -405,16 +432,7 @@ async def _run_update_all(self) -> None: async def _update_all_locked(self) -> None: """Update dirty components and errored git repos (update self-heals those).""" statuses = await self._svc.check_status() - dirty = { - name - for name, s in statuses.items() - if s.commits_behind - or s.packages_upgradable > 0 - or s.has_local_changes - or s.needs_install - or s.branch_mismatch - or (s.error is not None and s.kind != "apt") - } + dirty = {name for name, s in statuses.items() if s.needs_update} if dirty: await self._svc.update_all(dirty) else: diff --git a/updater/executor.py b/updater/executor.py index 50c049b0..aad41f2d 100644 --- a/updater/executor.py +++ b/updater/executor.py @@ -259,7 +259,9 @@ async def _commits_behind_or_error( ) -> tuple[int, ComponentStatus | None]: """Resolve commits-behind; return (n, None) or (-1, error ComponentStatus).""" if version: - return 0, None + # Off the pin = one pending update, else a new pin is never applied. + pin = await git_ref_hash(path, f"{version}^{{commit}}") + return (0 if pin == current_hash else 1), None commits_behind = await git_commits_behind(path, remote_ref) if commits_behind != -1: return commits_behind, None @@ -309,7 +311,7 @@ async def check_git_status( git_remote_url(path), git_is_dirty(path), git_describe(path), - git_describe(path, remote_ref), + git_describe(path, version or remote_ref), ) if remote_url == "": logger.error("Failed at git_remote_url operation") @@ -661,6 +663,14 @@ async def git_tree_has_path(path: Path | None, ref: str, repo_path: str) -> bool return ok +async def git_show_file(path: Path | None, ref: str, repo_path: str) -> str: + """Return repo_path's content at ref (git show ref:path), or empty on any error.""" + if path is None or not _GIT_SHA_RE.match(ref): + return "" + ok, output = await _run([GIT, "show", f"{ref}:{repo_path}"], cwd=path, timeout=10.0) + return output if ok else "" + + async def git_commits_behind(path: Path, remote_ref: str = "origin/HEAD") -> int: """Return how many commits the local branch is behind remote_ref.""" ok, output = await _run( diff --git a/updater/hooks/BlocksScreen.sh b/updater/hooks/BlocksScreen.sh index 73459ba2..a259168c 100755 --- a/updater/hooks/BlocksScreen.sh +++ b/updater/hooks/BlocksScreen.sh @@ -84,4 +84,22 @@ if ! git -C "$COMPONENT_PATH" diff --quiet "$PREV_HASH" "$NEW_HASH" \ _set_deploy_flag fi -# No daemon restart on updater/ changes: mid-batch it would cancel and revert the batch. +# updater/ changed: defer a daemon self-restart to post-batch (mid-batch it would revert the batch). +if ! git -C "$COMPONENT_PATH" diff --quiet "$PREV_HASH" "$NEW_HASH" -- updater/ 2>/dev/null; then + if [ -n "${BS_UPDATER_SELF_UPDATE:-}" ] && [ -n "${BS_UPDATER_RESTART_SENTINEL:-}" ]; then + mkdir -p "$(dirname "$BS_UPDATER_RESTART_SENTINEL")" 2>/dev/null || true + echo "code" >>"$BS_UPDATER_RESTART_SENTINEL" + echo "[hook:BlocksScreen] updater code changed - deferring daemon restart to post-batch" + fi +fi + +# Manifest changed: the restarted daemon applies the changed entries in the same Update. +if ! git -C "$COMPONENT_PATH" diff --quiet "$PREV_HASH" "$NEW_HASH" \ + -- updater/components.yaml 2>/dev/null; then + _followup="${HOME}/.cache/blockscreen/updater_followup" + if [ ! -e "$_followup" ]; then # keep the oldest base across chained updates + mkdir -p "$(dirname "$_followup")" + printf '%s\n' "$PREV_HASH" >"$_followup" + echo "[hook:BlocksScreen] components.yaml changed - follow-up queued" + fi +fi diff --git a/updater/models.py b/updater/models.py index edd9ea63..c1be1de9 100644 --- a/updater/models.py +++ b/updater/models.py @@ -41,3 +41,15 @@ class ComponentStatus: needs_install: bool = False branch_mismatch: bool = False current_branch: str = "" + + @property + def needs_update(self) -> bool: + """Dirty for update_all: errored git repos count (update self-heals them), apt errors don't.""" + return bool( + self.commits_behind + or self.packages_upgradable > 0 + or self.has_local_changes + or self.needs_install + or self.branch_mismatch + or (self.error is not None and self.kind != "apt") + ) diff --git a/updater/service.py b/updater/service.py index 254c7a67..2c1abba8 100644 --- a/updater/service.py +++ b/updater/service.py @@ -46,6 +46,7 @@ git_remote_reachable, git_repair, git_reset_to_hash, + git_show_file, git_tree_has_path, git_untracked_paths, is_git_repo, @@ -68,6 +69,8 @@ # Present = fast recovery saturated (golden also looped). _FAULT_MARKER_PATH = Path.home() / ".cache" / "blockscreen" / "selfheal_fault.json" _HISTORY_PATH = Path.home() / ".cache" / "blockscreen" / "update_history.jsonl" +# Written by hooks/BlocksScreen.sh: pre-update BlocksScreen hash whose manifest is stale. +_FOLLOWUP_PATH = Path.home() / ".cache" / "blockscreen" / "updater_followup" _APT_STATUS_CACHE = Path.home() / ".cache" / "blockscreen" / "apt_status_cache.json" _HISTORY_MAX_BYTES = 1_000_000 _HISTORY_KEEP_LINES = 2000 @@ -260,11 +263,13 @@ def __init__(self, callback: ProgressCallback | None = None) -> None: self._inflight_path = _INFLIGHT_PATH self._history_path = _HISTORY_PATH self._fault_marker_path = _FAULT_MARKER_PATH + self._followup_path = _FOLLOWUP_PATH self._log = logging.getLogger("updater") self._nrestarts_samples: dict[str, list[tuple[float, int]]] = {} self._restart_pending_until = 0.0 self._reconciled = False self._offline_provision_tries = 0 + self._offline_followup_tries = 0 @property def daemon_restart_pending(self) -> bool: @@ -561,6 +566,89 @@ async def provision_missing( on_busy(False) return retry + async def run_followup( + self, on_busy: Callable[[bool], None] | None = None + ) -> tuple[bool, bool]: + """Apply manifest entries changed by the last BlocksScreen update; (retry, ran).""" + base = await asyncio.to_thread(self._read_followup) + if not base: + return False, False + if await klipper_printing() or await self.deploy_running(): + self._log.info("followup: printing or deploy running, deferring") + return True, False + with process_lock() as acquired: + if not acquired: + self._log.info("followup: update in progress, deferring") + return True, False + # Busy before the network checks below, so an Update press is refused, not raced. + if on_busy: + on_busy(True) + try: + await self.reconcile_if_pending() + dirty = await self._followup_targets(base) + if dirty and not await self._followup_reachable(dirty): + self._offline_followup_tries += 1 + if self._offline_followup_tries < _OFFLINE_PROVISION_TRIES: + return True, False + # Still shown dirty, so a later Update applies it. + self._log.info("followup: still offline, leaving it to Update") + dirty = set() + # One-shot: a failing entry must not re-run on every boot. + await asyncio.to_thread(self._clear_followup) + if not dirty: + return False, False + self._log.info("followup: applying %s", sorted(dirty)) + await self.update_all(dirty) + finally: + if on_busy: + on_busy(False) + return False, True + + async def _followup_targets(self, base: str) -> set[str]: + """Dirty components whose manifest entry differs from the one at base.""" + ui = next((c for c in self._components if c.name == _UI_COMPONENT), None) + text = await git_show_file( + ui.path if ui else None, base, "updater/components.yaml" + ) + if not text: + self._log.warning("followup: no manifest at %s, dropping", base[:8]) + return set() + old = {c.name: c for c in load_components(text)[0]} + # BlocksScreen itself already updated: re-running it could loop. + changed = { + c.name + for c in self._components + if c.name != _UI_COMPONENT and old.get(c.name) != c + } + statuses = await self.check_status() + dirty = {n for n in changed if n in statuses and statuses[n].needs_update} + self._log.info("followup: changed=%s dirty=%s", sorted(changed), sorted(dirty)) + return dirty + + async def _followup_reachable(self, names: set[str]) -> bool: + """True when every target's remote answers.""" + for c in self._components: + if c.name in names and c.url and not await git_remote_reachable(c.url): + self._log.info("followup: %s unreachable, deferring", c.name) + return False + return True + + def _read_followup(self) -> str: + """The follow-up base hash, or empty when absent or malformed.""" + try: + base = self._followup_path.read_text().strip() + except OSError: + return "" + if _GIT_SHA_RE.match(base): + return base + self._clear_followup() + return "" + + def _clear_followup(self) -> None: + """Remove the follow-up marker.""" + with contextlib.suppress(OSError): + self._followup_path.unlink() + async def _preflight_fetch( self, sorted_components: list[ComponentConfig] ) -> set[str]: