From 1d4e9eacff91d025308e71b9caa19a2b868771f1 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Thu, 27 Mar 2025 20:34:58 -0700 Subject: [PATCH] packages: Don't run force upgrade hooks when freedombox perform ops - We have a hook that triggers when 'apt update' is successfully run. This hook handles the force upgrading mechanism. It's intended purpose is to handle packages with configuration file prompts that unattended-upgrades does not touch. 'apt update' is run on behalf of unattended-upgrades every day on a schedule. This is the primary time the hook is intended to run. However, the hook also run every time FreedomBox runs 'apt update' before installing an app. Also no operations are performed, there is a race to see of apt is available for the operation. - Avoid these unnecessary runs by setting an environmental variable and by checking it before running the trigger. - There is one place where we want to genuinely run the trigger. That is after a distribution upgrade. Handle this case. Tests: - When apt update is run on the command line, the hook is triggered. - When installing an app, however, the hook is not triggered. - During a dist-upgrade, the hook is triggered at the end. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/action_utils.py | 5 ++++- .../share/freedombox/etc/apt/apt.conf.d/20freedombox | 2 +- plinth/modules/upgrades/distupgrade.py | 11 ++++++----- plinth/modules/upgrades/tests/test_distupgrade.py | 6 +++++- 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/plinth/action_utils.py b/plinth/action_utils.py index d03dadef4..bbaf6f406 100644 --- a/plinth/action_utils.py +++ b/plinth/action_utils.py @@ -449,12 +449,15 @@ def is_disk_image(): return os.path.exists('/var/lib/freedombox/is-freedombox-disk-image') -def run_apt_command(arguments, stdout=subprocess.DEVNULL): +def run_apt_command(arguments, stdout=subprocess.DEVNULL, + enable_triggers: bool = False): """Run apt-get with provided arguments.""" command = ['apt-get', '--assume-yes', '--quiet=2'] + arguments env = os.environ.copy() env['DEBIAN_FRONTEND'] = 'noninteractive' + if not enable_triggers: + env['FREEDOMBOX_INVOKED'] = 'true' process = subprocess.run(command, stdin=subprocess.DEVNULL, stdout=stdout, env=env, check=False) return process.returncode diff --git a/plinth/modules/upgrades/data/usr/share/freedombox/etc/apt/apt.conf.d/20freedombox b/plinth/modules/upgrades/data/usr/share/freedombox/etc/apt/apt.conf.d/20freedombox index 574055587..19873a34b 100644 --- a/plinth/modules/upgrades/data/usr/share/freedombox/etc/apt/apt.conf.d/20freedombox +++ b/plinth/modules/upgrades/data/usr/share/freedombox/etc/apt/apt.conf.d/20freedombox @@ -4,7 +4,7 @@ // via it's D-Bus API. FreedomBox may then handle upgrade of some packages. // APT::Update::Post-Invoke-Success { - "/usr/bin/test -S /var/run/dbus/system_bus_socket && /usr/bin/gdbus call --system --dest org.freedombox.Service --object-path /org/freedombox/Service/PackageHandler --timeout 10 --method org.freedombox.Service.PackageHandler.CacheUpdated 2> /dev/null > /dev/null; /bin/echo > /dev/null"; + "/usr/bin/test x$FREEDOMBOX_INVOKED != 'xtrue' && /usr/bin/test -S /var/run/dbus/system_bus_socket && /usr/bin/gdbus call --system --dest org.freedombox.Service --object-path /org/freedombox/Service/PackageHandler --timeout 10 --method org.freedombox.Service.PackageHandler.CacheUpdated 2> /dev/null > /dev/null; /bin/echo > /dev/null"; }; // Clean apt cache every 7 days diff --git a/plinth/modules/upgrades/distupgrade.py b/plinth/modules/upgrades/distupgrade.py index 706dee034..0020fd046 100644 --- a/plinth/modules/upgrades/distupgrade.py +++ b/plinth/modules/upgrades/distupgrade.py @@ -30,9 +30,10 @@ sources_list = pathlib.Path('/etc/apt/sources.list') temp_sources_list = pathlib.Path('/etc/apt/sources.list.fbx-dist-upgrade') -def _apt_run(arguments): +def _apt_run(arguments: list[str], enable_triggers: bool = False): """Run an apt command and ensure that output is written to stdout.""" - return action_utils.run_apt_command(arguments, stdout=None) + return action_utils.run_apt_command(arguments, stdout=None, + enable_triggers=enable_triggers) def _sources_list_update(old_codename: str, new_codename: str): @@ -217,10 +218,10 @@ def _packages_remove_obsolete() -> None: _apt_run(['remove'] + OBSOLETE_PACKAGES) -def _apt_update(): +def _apt_update(enable_triggers: bool = False): """Run 'apt update'.""" logger.info('Updating Apt cache...') - _apt_run(['update']) + _apt_run(['update'], enable_triggers=enable_triggers) def _apt_fix(): @@ -314,7 +315,7 @@ def perform(): _unattended_upgrades_run() _freedombox_restart() _wait() - _apt_update() + _apt_update(enable_triggers=True) _trigger_on_complete() diff --git a/plinth/modules/upgrades/tests/test_distupgrade.py b/plinth/modules/upgrades/tests/test_distupgrade.py index 91ab44aa2..ae86908c2 100644 --- a/plinth/modules/upgrades/tests/test_distupgrade.py +++ b/plinth/modules/upgrades/tests/test_distupgrade.py @@ -280,7 +280,11 @@ def test_packages_remove_obsolete(apt_run): def test_apt_update(apt_run): """Test that apt update works.""" distupgrade._apt_update() - apt_run.assert_called_with(['update']) + apt_run.assert_called_with(['update'], enable_triggers=False) + + apt_run.reset_mock() + distupgrade._apt_update(enable_triggers=True) + apt_run.assert_called_with(['update'], enable_triggers=True) @patch('plinth.modules.upgrades.distupgrade._apt_run')