From 16cb8ee02154dacd3c6872ed32586e47956e41ae Mon Sep 17 00:00:00 2001
From: Sunil Mohan Adapa <sunil@medhas.org>
Date: Mon, 2 Jan 2023 15:36:19 -0800
Subject: [PATCH] package: Don't uninstall packages that are in use by other
 apps

Closes: #2262.

Tests:

- Unit tests work

- Uninstall of email app succeeds without uninstalling openssl package.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
---
 plinth/package.py            | 22 +++++++++++--
 plinth/tests/test_package.py | 64 ++++++++++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+), 3 deletions(-)

diff --git a/plinth/package.py b/plinth/package.py
index 6ed4d2245..3c4ea5b33 100644
--- a/plinth/package.py
+++ b/plinth/package.py
@@ -12,7 +12,7 @@ from django.utils.translation import gettext as _
 from django.utils.translation import gettext_lazy
 
 import plinth.privileged.packages as privileged
-from plinth import app
+from plinth import app as app_module
 from plinth.errors import MissingPackageError
 from plinth.utils import format_lazy
 
@@ -94,7 +94,7 @@ class PackageOr(PackageExpression):
             return self.package2.actual()
 
 
-class Packages(app.FollowerComponent):
+class Packages(app_module.FollowerComponent):
     """Component to manage the packages of an app.
 
     This component is responsible for installation, upgrades and uninstallation
@@ -182,7 +182,23 @@ class Packages(app.FollowerComponent):
 
     def uninstall(self):
         """Uninstall and purge the packages."""
-        uninstall(self.get_actual_packages())
+        packages = self.get_actual_packages()
+        packages_set = set(packages)
+        for app in app_module.App.list():
+            # uninstall() will be called on Packages of this app separately
+            # for uninstalling this app.
+            if app == self.app:
+                continue
+
+            if app.get_setup_state() == app_module.App.SetupState.NEEDS_SETUP:
+                continue
+
+            # Remove packages used by other installed apps
+            for component in app.get_components_of_type(Packages):
+                packages_set -= set(component.get_actual_packages())
+
+        # Preserve order of packages for ease of testing
+        uninstall([package for package in packages if package in packages_set])
 
     def diagnose(self):
         """Run diagnostics and return results."""
diff --git a/plinth/tests/test_package.py b/plinth/tests/test_package.py
index 3b40ae535..31c0a3e04 100644
--- a/plinth/tests/test_package.py
+++ b/plinth/tests/test_package.py
@@ -13,6 +13,12 @@ from plinth.errors import MissingPackageError
 from plinth.package import Package, Packages, packages_installed
 
 
+@pytest.fixture(autouse=True)
+def fixture_clean_apps():
+    """Fixture to ensure clean set of global apps."""
+    App._all_apps = {}
+
+
 class TestPackageExpressions(unittest.TestCase):
 
     def test_package(self):
@@ -159,6 +165,64 @@ def test_packages_uninstall(uninstall):
     uninstall.assert_has_calls([call(['python3', 'bash'])])
 
 
+@patch('plinth.package.uninstall')
+@patch('apt.Cache')
+def test_packages_uninstall_exclusion(cache, uninstall):
+    """Test excluding packages from other installed apps when uninstalling."""
+    cache.return_value = {
+        'package11': Mock(candidate=Mock(version='2.0', is_installed=True)),
+        'package12': Mock(candidate=Mock(version='3.0', is_installed=False)),
+        'package2': Mock(candidate=Mock(version='4.0', is_installed=True)),
+        'package3': Mock(candidate=Mock(version='5.0', is_installed=True)),
+    }
+
+    class TestApp1(App):
+        """Test app."""
+        app_id = 'test-app1'
+
+        def __init__(self):
+            super().__init__()
+            component = Packages('test-component11',
+                                 ['package11', 'package2', 'package3'])
+            self.add(component)
+
+            component = Packages('test-component12',
+                                 ['package12', 'package2', 'package3'])
+            self.add(component)
+
+    class TestApp2(App):
+        """Test app."""
+        app_id = 'test-app2'
+
+        def __init__(self):
+            super().__init__()
+            component = Packages('test-component2', ['package2'])
+            self.add(component)
+
+        def get_setup_state(self):
+            return App.SetupState.UP_TO_DATE
+
+    class TestApp3(App):
+        """Test app."""
+        app_id = 'test-app3'
+
+        def __init__(self):
+            super().__init__()
+            component = Packages('test-component3', ['package3'])
+            self.add(component)
+
+        def get_setup_state(self):
+            return App.SetupState.NEEDS_SETUP
+
+    app1 = TestApp1()
+    TestApp2()
+    TestApp3()
+    app1.uninstall()
+    uninstall.assert_has_calls(
+        [call(['package11', 'package3']),
+         call(['package12', 'package3'])])
+
+
 @patch('apt.Cache')
 def test_diagnose(cache):
     """Test checking for latest version of the package."""