From fecd6a3577684ec48afa97fca627577b5d63895e Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Wed, 3 Sep 2025 16:46:20 -0700 Subject: [PATCH] upgrades: Overhaul detection of distribution - Move some utilities to utils.py from distupgrade.py and __init__.py. - This fixes issues with apt preferences being set on unstable distribution (despite code that tries to prevent it). - There is no way to distinguish between 'testing' and 'unstable' distributions in Debian using commands like lsb_release (powered by /etc/os-release). See: https://lwn.net/Articles/984635/ . So, use the value set in /etc/apt/sources.list. Tests: (tested entire patchset) - Deluge can be installed in trixie. - Auto-distribution upgrade button is checked during setup on stable and oldstable but not on testing and unstable. - Auto-distribution upgrade button is enabled in the form on stable and oldstable but not on testing and unstable. - Backports wizard step is skipped on unstable (non-develop mode), but not on oldstable, stable, testing, and unstable (develop mode). - If backports are not activated during first wizard, then backports can be activated on upgrades app page if distribution is oldstable, stable, testing, or unstable (non-develop mode) but not unstable (develop mode). - During re-run of setup, setting up backport sources is skipped if already setup. - Backports sources files are not added in testing (non-develop) and unstable (non-develop) distributions. Backports sources are added to oldstable, stable, testing (develop) and unstable (develop). Unstable sources sources are not added to unstable but added to oldstable, stable, and testing. - Backports sources file is added with correct code name bookworm/trixie for oldstable, stable, and testing distributions. - When backports sources is set to 'bookworm-backports' on Trixie distribution, re-running setup updates them to 'trixie-backports'. - Preferences files are added in oldstable, stable, and testing distributions but not unstable. - If unstable and another distro is present in apt sources, then it is treated as unstable as shown in the distribution upgrade page. - Current codename is shown properly from sources.list in oldstable, stable, testing, and unstable in distribution upgrade page. - NOT TESTED: If distribution upgrade is interrupted, then continue page is shown. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/modules/deluge/__init__.py | 2 +- plinth/modules/upgrades/__init__.py | 20 +--- plinth/modules/upgrades/distupgrade.py | 28 +----- plinth/modules/upgrades/privileged.py | 17 ++-- .../upgrades/tests/test_distupgrade.py | 38 +------- plinth/modules/upgrades/tests/test_utils.py | 95 +++++++++++++++++++ plinth/modules/upgrades/utils.py | 64 +++++++++++++ 7 files changed, 176 insertions(+), 88 deletions(-) create mode 100644 plinth/modules/upgrades/tests/test_utils.py diff --git a/plinth/modules/deluge/__init__.py b/plinth/modules/deluge/__init__.py index e9215fc8c..ff30b39bd 100644 --- a/plinth/modules/deluge/__init__.py +++ b/plinth/modules/deluge/__init__.py @@ -11,7 +11,7 @@ from plinth.modules.apache.components import Webserver from plinth.modules.backups.components import BackupRestore from plinth.modules.firewall.components import (Firewall, FirewallLocalProtection) -from plinth.modules.upgrades import get_current_release +from plinth.modules.upgrades.utils import get_current_release from plinth.modules.users import add_user_to_share_group from plinth.modules.users.components import UsersAndGroups from plinth.package import Packages diff --git a/plinth/modules/upgrades/__init__.py b/plinth/modules/upgrades/__init__.py index e4797d7a3..be4bf2d80 100644 --- a/plinth/modules/upgrades/__init__.py +++ b/plinth/modules/upgrades/__init__.py @@ -20,7 +20,7 @@ from plinth.diagnostic_check import DiagnosticCheck, Result from plinth.modules.backups.components import BackupRestore from plinth.package import Packages -from . import distupgrade, manifest, privileged +from . import distupgrade, manifest, privileged, utils first_boot_steps = [ { @@ -350,21 +350,12 @@ def is_backports_enabled(): return os.path.exists(privileged.BACKPORTS_SOURCES_LIST) -def get_current_release(): - """Return current release and codename as a tuple.""" - output = subprocess.check_output( - ['lsb_release', '--release', '--codename', - '--short']).decode().strip() - lines = output.split('\n') - return lines[0], lines[1] - - def is_backports_current(): """Return whether backports are enabled for the current release.""" if not is_backports_enabled(): return False - _, dist = get_current_release() + _, dist = utils.get_current_release() dist += '-backports' sources = sourceslist.SourcesList() for source in sources: @@ -379,15 +370,12 @@ def can_activate_backports(): if cfg.develop: return True - # Release will be 'n/a' in latest unstable and testing distributions. - release, _ = get_current_release() - return release not in ['unstable', 'testing', 'n/a'] + return not utils.is_distribution_rolling() def can_enable_dist_upgrade(): """Return whether dist upgrade can be enabled.""" - release, _ = get_current_release() - return release not in ['unstable', 'testing', 'n/a'] + return not utils.is_distribution_rolling() def _diagnose_held_packages(): diff --git a/plinth/modules/upgrades/distupgrade.py b/plinth/modules/upgrades/distupgrade.py index 7bdb97d52..ce95eff6f 100644 --- a/plinth/modules/upgrades/distupgrade.py +++ b/plinth/modules/upgrades/distupgrade.py @@ -105,30 +105,6 @@ def _sources_list_update(old_codename: str, new_codename: str): aug_path.rename(temp_sources_list) -def _get_sources_list_codename() -> str | None: - """Return the codename set in the /etc/apt/sources.list file.""" - aug = augeas.Augeas(flags=augeas.Augeas.NO_LOAD + - augeas.Augeas.NO_MODL_AUTOLOAD) - aug.transform('aptsources', str(sources_list)) - aug.set('/augeas/context', '/files' + str(sources_list)) - aug.load() - - dists = set() - for match_ in aug.match('*'): - dist = aug.get(match_ + '/distribution') - if not dist: - continue - - dist = dist.removesuffix('-updates') - dist = dist.removesuffix('-security') - dists.add(dist) - - if len(dists) != 1: - return None - - return dists.pop() - - def get_status() -> dict[str, bool | str | None]: """Check if a distribution upgrade be performed. @@ -158,7 +134,7 @@ def get_status() -> dict[str, bool | str | None]: has_free_space = utils.is_sufficient_free_space() running = action_utils.service_is_running('freedombox-dist-upgrade') - current_codename = _get_sources_list_codename() + current_codename = utils.get_sources_list_codename() status = { 'updates_enabled': updates_enabled, 'dist_upgrade_enabled': dist_upgrade_enabled, @@ -177,7 +153,7 @@ def get_status() -> dict[str, bool | str | None]: if current_codename in (None, 'testing', 'unstable'): return status - _, base_files_codename = upgrades.get_current_release() + _, base_files_codename = utils.get_current_release() if current_codename == 'stable': current_codename = base_files_codename diff --git a/plinth/modules/upgrades/privileged.py b/plinth/modules/upgrades/privileged.py index 3be661812..e94ae9e21 100644 --- a/plinth/modules/upgrades/privileged.py +++ b/plinth/modules/upgrades/privileged.py @@ -158,8 +158,7 @@ def _check_and_backports_sources(develop=False): if os.path.exists(old_sources_list): os.remove(old_sources_list) - from plinth.modules.upgrades import (get_current_release, - is_backports_current) + from plinth.modules.upgrades import is_backports_current if is_backports_current(): logging.info('Repositories list up-to-date. Skipping update.') return @@ -181,15 +180,16 @@ def _check_and_backports_sources(develop=False): 'backports.') return - release, dist = get_current_release() - if release in ['unstable', 'testing', 'n/a'] and not develop: - logging.info(f'System release is {release}. Skip enabling backports.') + if utils.is_distribution_rolling() and not develop: + logging.info( + 'System release is unstable/testing. Skip enabling backports.') return protocol = utils.get_http_protocol() if protocol == 'tor+http': logging.info('Package download over Tor is enabled.') + _, dist = utils.get_current_release() if not utils.is_release_file_available(protocol, dist, backports=True): logging.info( f'Release file for {dist}-backports is not available yet.') @@ -212,13 +212,12 @@ def _add_apt_preferences(): # Don't try to remove 50freedombox3.pref as this file is shipped with the # Debian package and is removed using maintainer scripts. - from plinth.modules.upgrades import get_current_release - _, dist = get_current_release() - if dist == 'sid': + if utils.is_distribution_unstable(): logging.info( - f'System distribution is {dist}. Skip setting apt preferences ' + 'System distribution is "unstable". Skip setting apt preferences ' 'for backports.') else: + _, dist = utils.get_current_release() logging.info(f'Setting apt preferences for {dist}-backports.') with open(base_path / '50freedombox4.pref', 'w', encoding='utf-8') as file_handle: diff --git a/plinth/modules/upgrades/tests/test_distupgrade.py b/plinth/modules/upgrades/tests/test_distupgrade.py index adca37591..8852de9f8 100644 --- a/plinth/modules/upgrades/tests/test_distupgrade.py +++ b/plinth/modules/upgrades/tests/test_distupgrade.py @@ -80,43 +80,9 @@ deb https://deb.debian.org/debian bookwormish main assert temp_sources_list.read_text() == modified -def test_get_sources_list_codename(tmp_path): - """Test retrieving codename from sources.list file.""" - list1 = ''' -deb http://deb.debian.org/debian bookworm main non-free-firmware -deb-src http://deb.debian.org/debian bookworm main non-free-firmware - -deb http://deb.debian.org/debian bookworm-updates main non-free-firmware -deb-src http://deb.debian.org/debian bookworm-updates main non-free-firmware - -deb http://security.debian.org/debian-security/ bookworm-security main non-free-firmware -deb-src http://security.debian.org/debian-security/ bookworm-security main non-free-firmware -''' # noqa: E501 - - list2 = ''' -deb http://deb.debian.org/debian stable main non-free-firmware -deb-src http://deb.debian.org/debian stable main non-free-firmware - -deb http://deb.debian.org/debian bookworm-updates main non-free-firmware -deb-src http://deb.debian.org/debian bookworm-updates main non-free-firmware - -deb http://security.debian.org/debian-security/ bookworm-security main non-free-firmware -deb-src http://security.debian.org/debian-security/ bookworm-security main non-free-firmware -''' # noqa: E501 - - sources_list = tmp_path / 'sources.list' - module = 'plinth.modules.upgrades.distupgrade' - with patch(f'{module}.sources_list', sources_list): - sources_list.write_text(list1) - assert distupgrade._get_sources_list_codename() == 'bookworm' - - sources_list.write_text(list2) - assert distupgrade._get_sources_list_codename() is None - - @patch('datetime.datetime') -@patch('plinth.modules.upgrades.get_current_release') -@patch('plinth.modules.upgrades.distupgrade._get_sources_list_codename') +@patch('plinth.modules.upgrades.utils.get_current_release') +@patch('plinth.modules.upgrades.distupgrade.utils.get_sources_list_codename') @patch('plinth.action_utils.service_is_running') @patch('plinth.modules.upgrades.utils.is_sufficient_free_space') @patch('plinth.modules.upgrades.is_dist_upgrade_enabled') diff --git a/plinth/modules/upgrades/tests/test_utils.py b/plinth/modules/upgrades/tests/test_utils.py new file mode 100644 index 000000000..8336c9731 --- /dev/null +++ b/plinth/modules/upgrades/tests/test_utils.py @@ -0,0 +1,95 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +""" +Test various upgrade related utilities. +""" + +from unittest.mock import patch + +from .. import utils + + +def test_get_sources_list_codename(tmp_path): + """Test retrieving codename from sources.list file.""" + list1 = ''' +deb http://deb.debian.org/debian bookworm main non-free-firmware +deb-src http://deb.debian.org/debian bookworm main non-free-firmware + +deb http://deb.debian.org/debian bookworm-updates main non-free-firmware +deb-src http://deb.debian.org/debian bookworm-updates main non-free-firmware + +deb http://security.debian.org/debian-security/ bookworm-security main non-free-firmware +deb-src http://security.debian.org/debian-security/ bookworm-security main non-free-firmware +''' # noqa: E501 + + list2 = ''' +deb http://deb.debian.org/debian stable main non-free-firmware +deb-src http://deb.debian.org/debian stable main non-free-firmware + +deb http://deb.debian.org/debian bookworm-updates main non-free-firmware +deb-src http://deb.debian.org/debian bookworm-updates main non-free-firmware + +deb http://security.debian.org/debian-security/ bookworm-security main non-free-firmware +deb-src http://security.debian.org/debian-security/ bookworm-security main non-free-firmware +''' # noqa: E501 + + list3 = ''' +deb http://deb.debian.org/debian unstable main +deb http://deb.debian.org/debian trixie main +''' + list4 = ''' +deb http://deb.debian.org/debian sid main +deb http://deb.debian.org/debian trixie main +''' + list5 = ''' +deb http://deb.debian.org/debian testing main +deb http://deb.debian.org/debian trixie main +''' + + sources_list = tmp_path / 'sources.list' + module = 'plinth.modules.upgrades.utils' + with patch(f'{module}.sources_list', sources_list): + sources_list.write_text(list1) + assert utils.get_sources_list_codename() == 'bookworm' + + sources_list.write_text(list2) + assert utils.get_sources_list_codename() is None + + sources_list.write_text(list3) + assert utils.get_sources_list_codename() == 'unstable' + + sources_list.write_text(list4) + assert utils.get_sources_list_codename() == 'unstable' + + sources_list.write_text(list5) + assert utils.get_sources_list_codename() == 'testing' + + +@patch('subprocess.check_output') +def test_get_current_release(check_output): + """Test that getting current release works.""" + check_output.return_value = b'test-release\ntest-codename\n\n' + assert utils.get_current_release() == ('test-release', 'test-codename') + + +@patch('plinth.modules.upgrades.utils.get_sources_list_codename') +def test_is_distribution_unstable(get_sources_list_codename): + """Test that checking for unstable distribution works.""" + get_sources_list_codename.return_value = 'unstable' + assert utils.is_distribution_unstable() + + get_sources_list_codename.return_value = 'sid' + assert utils.is_distribution_unstable() + + get_sources_list_codename.return_value = 'testing' + assert not utils.is_distribution_unstable() + + +@patch('plinth.modules.upgrades.utils.get_current_release') +def test_is_distribution_rolling(get_current_release): + """Test that checking for testing/unstable distribution works.""" + for value in ['unstable', 'testing', 'n/a']: + get_current_release.return_value = (value, None) + assert utils.is_distribution_rolling() + + get_current_release.return_value = ('forky', None) + assert not utils.is_distribution_rolling() diff --git a/plinth/modules/upgrades/utils.py b/plinth/modules/upgrades/utils.py index 0c1780749..5904554fc 100644 --- a/plinth/modules/upgrades/utils.py +++ b/plinth/modules/upgrades/utils.py @@ -1,9 +1,12 @@ # SPDX-License-Identifier: AGPL-3.0-or-later """Utilities for regular updates and dist-upgrades.""" +import pathlib import re import subprocess +import augeas + from plinth.modules.apache.components import check_url RELEASE_FILE_URL = \ @@ -11,6 +14,8 @@ RELEASE_FILE_URL = \ DIST_UPGRADE_REQUIRED_FREE_SPACE = 5000000 +sources_list = pathlib.Path('/etc/apt/sources.list') + def check_auto() -> bool: """Return whether automatic updates are enabled.""" @@ -60,3 +65,62 @@ def is_sufficient_free_space() -> bool: output = subprocess.check_output(['df', '--output=avail', '/']) free_space = int(output.decode().split('\n')[1]) return free_space >= DIST_UPGRADE_REQUIRED_FREE_SPACE + + +def get_sources_list_codename() -> str | None: + """Return the codename set in the /etc/apt/sources.list file.""" + aug = augeas.Augeas(flags=augeas.Augeas.NO_LOAD + + augeas.Augeas.NO_MODL_AUTOLOAD) + aug.transform('aptsources', str(sources_list)) + aug.set('/augeas/context', '/files' + str(sources_list)) + aug.load() + + dists = set() + for match_ in aug.match('*'): + dist = aug.get(match_ + '/distribution') + if not dist: + continue + + dist = dist.removesuffix('-updates') + dist = dist.removesuffix('-security') + dists.add(dist) + + if 'unstable' in dists or 'sid' in dists: + return 'unstable' + + if 'testing' in dists: + return 'testing' + + # Multiple distributions are not understood. + if len(dists) != 1: + return None + + return dists.pop() + + +def get_current_release(): + """Return current release and codename as a tuple.""" + output = subprocess.check_output( + ['lsb_release', '--release', '--codename', + '--short']).decode().strip() + lines = output.split('\n') + return lines[0], lines[1] + + +def is_distribution_unstable() -> bool: + """Return whether the current distribution is unstable. + + There is no way to distinguish between 'testing' and 'unstable' + distributions in Debian using commands like lsb_release (powered by + /etc/os-release). See: https://lwn.net/Articles/984635/ . So, use the value + set in /etc/apt/sources.list. + """ + codename = get_sources_list_codename() + return codename in ['unstable', 'sid'] + + +def is_distribution_rolling() -> bool: + """Return whether the current distribution is testing or unstable.""" + # Release will be 'n/a' in latest unstable and testing distributions. + release, _ = get_current_release() + return release in ['unstable', 'testing', 'n/a']