upgrades: Use apt_hold contextmanager

Signed-off-by: James Valleroy <jvalleroy@mailbox.org>
Reviewed-by: Sunil Mohan Adapa <sunil@medhas.org>

actions/packages

@@ -11,7 +11,6 @@ import os
 import subprocess
 import sys
 from collections import defaultdict
-from contextlib import contextmanager
 from importlib import import_module
 
 import apt.cache
@@ -19,7 +18,7 @@ import apt.cache
 import apt_inst
 import apt_pkg
 from plinth import cfg
-from plinth.action_utils import run_apt_command
+from plinth.action_utils import apt_hold, run_apt_command
 
 LOCK_FILE = '/var/lib/dpkg/lock'
 
@@ -62,19 +61,6 @@ def parse_arguments():
     return parser.parse_args()
 
 
-@contextmanager
-def _apt_hold():
-    """Do not allow freedombox package to be removed during package install."""
-    current_hold = subprocess.check_output(
-        ['apt-mark', 'showhold', 'freedombox'])
-    try:
-        yield current_hold or subprocess.run(
-            ['apt-mark', 'hold', 'freedombox'], check=True)
-    finally:
-        if not current_hold:
-            subprocess.run(['apt-mark', 'unhold', 'freedombox'], check=True)
-
-
 def subcommand_update(arguments):
     """Update apt package lists."""
     sys.exit(run_apt_command(['update']))
@@ -107,7 +93,7 @@ def subcommand_install(arguments):
         extra_arguments += ['-o', 'Dpkg::Options::=--force-confmiss']
 
     subprocess.run(['dpkg', '--configure', '-a'])
-    with _apt_hold():
+    with apt_hold():
         run_apt_command(['--fix-broken', 'install'])
         returncode = run_apt_command(['install'] + extra_arguments +
                                      arguments.packages)

actions/upgrades

@@ -11,7 +11,7 @@ import re
 import subprocess
 import sys
 
-from plinth.action_utils import run_apt_command, service_restart
+from plinth.action_utils import apt_hold, run_apt_command, service_restart
 from plinth.modules.apache.components import check_url
 from plinth.modules.upgrades import (BACKPORTS_SOURCES_LIST, SOURCES_LIST,
                                      get_current_release, is_backports_current)
@@ -378,28 +378,22 @@ def _check_and_dist_upgrade(develop=False, test_upgrade=False):
 def _perform_dist_upgrade():
     """Perform upgrade to next release of Debian."""
     # Hold freedombox package during entire dist upgrade.
-    subprocess.run(['apt-mark', 'hold', 'freedombox'])
+    with apt_hold():
+        run_apt_command(['update'])
+        run_apt_command(['install', 'base-files'])
+        run_apt_command(['install', 'unattended-upgrades'])
+        subprocess.run(['unattended-upgrade', '--verbose'])
 
-    run_apt_command(['update'])
-    run_apt_command(['install', 'base-files'])
-    run_apt_command(['install', 'unattended-upgrades'])
-    subprocess.run(['unattended-upgrade', '--verbose'])
+        # Remove obsolete packages that may prevent other packages from
+        # upgrading.
+        run_apt_command(['remove', 'libgcc1'])
 
-    # Remove obsolete packages that may prevent other packages from
-    # upgrading.
-    run_apt_command(['remove', 'libgcc1'])
+        # Hold packages known to have conffile prompts. FreedomBox service
+        # will handle their upgrade later.
+        with apt_hold(['firewalld', 'radicale']):
+            run_apt_command(['full-upgrade'])
 
-    # Hold packages known to have conffile prompts. FreedomBox service
-    # will handle their upgrade later.
-    HOLD_PACKAGES = ['firewalld', 'radicale']
-    try:
-        subprocess.run(['apt-mark', 'hold'] + HOLD_PACKAGES)
-        run_apt_command(['full-upgrade'])
-    finally:
-        subprocess.run(['apt-mark', 'unhold'] + HOLD_PACKAGES)
-
-    run_apt_command(['autoremove'])
-    subprocess.run(['apt-mark', 'unhold', 'freedombox'])
+        run_apt_command(['autoremove'])
 
     print('Dist upgrade complete. Removing flag.')
     if dist_upgrade_flag.exists():

plinth/action_utils.py

@@ -8,6 +8,7 @@ import os
 import shutil
 import subprocess
 import tempfile
+from contextlib import contextmanager
 
 logger = logging.getLogger(__name__)
 
@@ -409,3 +410,18 @@ def run_apt_command(arguments):
                              stdout=subprocess.DEVNULL, close_fds=False,
                              env=env)
     return process.returncode
+
+
+@contextmanager
+def apt_hold(packages=None):
+    """Prevent packages from being removed during apt operations."""
+    if not packages:
+        packages = ['freedombox']
+
+    current_hold = subprocess.check_output(['apt-mark', 'showhold'] + packages)
+    try:
+        yield current_hold or subprocess.run(['apt-mark', 'hold'] + packages,
+                                             check=True)
+    finally:
+        if not current_hold:
+            subprocess.run(['apt-mark', 'unhold'] + packages, check=True)