From 33cbdd734b78bc94f74d183c835e714be7bd7de9 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Thu, 6 Mar 2025 13:23:47 -0800 Subject: [PATCH] upgrades: Minor refactor to pre-dist upgrade checks - Don't perform sources.list changes in the check() method. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/modules/upgrades/distupgrade.py | 20 +++++++++++------- plinth/modules/upgrades/privileged.py | 3 +-- .../upgrades/tests/test_distupgrade.py | 21 +++++++++++-------- 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/plinth/modules/upgrades/distupgrade.py b/plinth/modules/upgrades/distupgrade.py index 1a59b63ca..ea5f65c5a 100644 --- a/plinth/modules/upgrades/distupgrade.py +++ b/plinth/modules/upgrades/distupgrade.py @@ -82,7 +82,7 @@ def _get_new_codename(test_upgrade: bool) -> str | None: return None -def check(test_upgrade=False): +def _check(test_upgrade: bool = False) -> tuple[str, str]: """Check if a distribution upgrade be performed. Check for new stable release, if updates are enabled, and if there is @@ -103,18 +103,18 @@ def check(test_upgrade=False): raise RuntimeError('found-previous') from plinth.modules.upgrades import get_current_release - release, dist = get_current_release() + release, old_codename = get_current_release() if release in ['unstable', 'testing', 'n/a']: raise RuntimeError(f'already-{release}') - codename = _get_new_codename(test_upgrade) - if not codename: + new_codename = _get_new_codename(test_upgrade) + if not new_codename: raise RuntimeError('codename-not-found') - if codename == dist: - raise RuntimeError(f'already-{dist}') + if new_codename == old_codename: + raise RuntimeError(f'already-{old_codename}') - _sources_list_update(dist, codename) + return old_codename, new_codename @contextlib.contextmanager @@ -289,8 +289,12 @@ def perform(): _apt_update() -def start_service(): +def start_service(test_upgrade: bool): """Create dist upgrade service and start it.""" + old_codename, new_codename = _check(test_upgrade) + + _sources_list_update(old_codename, new_codename) + old_service_path = pathlib.Path( '/run/systemd/system/freedombox-dist-upgrade.service') if old_service_path.exists(): diff --git a/plinth/modules/upgrades/privileged.py b/plinth/modules/upgrades/privileged.py index 64d82ff00..1b9de0016 100644 --- a/plinth/modules/upgrades/privileged.py +++ b/plinth/modules/upgrades/privileged.py @@ -244,8 +244,7 @@ def start_dist_upgrade(test: bool = False): """ _release_held_freedombox() - distupgrade.check(test) - distupgrade.start_service() + distupgrade.start_service(test) @privileged diff --git a/plinth/modules/upgrades/tests/test_distupgrade.py b/plinth/modules/upgrades/tests/test_distupgrade.py index 01b7434e2..ca4c9b9aa 100644 --- a/plinth/modules/upgrades/tests/test_distupgrade.py +++ b/plinth/modules/upgrades/tests/test_distupgrade.py @@ -99,46 +99,49 @@ Description: Debian Testing distribution assert not distupgrade._get_new_codename(True) -@patch('plinth.modules.upgrades.distupgrade._sources_list_update') @patch('plinth.modules.upgrades.distupgrade._get_new_codename') @patch('plinth.modules.upgrades.get_current_release') @patch('plinth.action_utils.service_is_running') @patch('plinth.modules.upgrades.utils.is_sufficient_free_space') @patch('plinth.modules.upgrades.utils.check_auto') def test_check(check_auto, is_sufficient_free_space, service_is_running, - get_current_release, get_new_codename, sources_list_update): + get_current_release, get_new_codename): """Test checking for available dist upgrade.""" check_auto.return_value = False with pytest.raises(RuntimeError, match='upgrades-not-enabled'): - distupgrade.check() + distupgrade._check() check_auto.return_value = True is_sufficient_free_space.return_value = False with pytest.raises(RuntimeError, match='not-enough-free-space'): - distupgrade.check() + distupgrade._check() is_sufficient_free_space.return_value = True service_is_running.return_value = True with pytest.raises(RuntimeError, match='found-previous'): - distupgrade.check() + distupgrade._check() service_is_running.return_value = False for release in ['unstable', 'testing', 'n/a']: get_current_release.return_value = (release, release) with pytest.raises(RuntimeError, match=f'already-{release}'): - distupgrade.check() + distupgrade._check() get_current_release.return_value = ('12', 'bookworm') get_new_codename.return_value = None with pytest.raises(RuntimeError, match='codename-not-found'): - distupgrade.check() + distupgrade._check() + get_new_codename.assert_called_with(False) + + distupgrade._check(True) + get_new_codename.assert_called_with(True) get_new_codename.return_value = 'bookworm' with pytest.raises(RuntimeError, match='already-bookworm'): - distupgrade.check() + distupgrade._check() get_new_codename.return_value = 'trixie' - sources_list_update.call_args_list = [call('bookworm', 'trixie')] + assert distupgrade._check() == ('bookworm', 'trixie') @patch('subprocess.run')