From 0b9d56749331c2cba235ca2ada26c5c83b6acf6e Mon Sep 17 00:00:00 2001
From: Sunil Mohan Adapa <sunil@medhas.org>
Date: Thu, 15 Dec 2022 16:46:18 -0800
Subject: [PATCH] package, email: Move conflicting package removal to framework

- Warning that exim4 will be removed is incorrectly not shown during email app
installation. Fix that.

Tests:

- Unit tests pass.

- On a fresh testing container, trying to install email app shows the warning
message that exim will be removed. Installing the email app succeeds. exim is
removed during installation.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
---
 plinth/modules/email/__init__.py | 14 ++------------
 plinth/package.py                |  7 +++++++
 plinth/tests/test_package.py     | 30 ++++++++++++++++++++++++++++++
 3 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/plinth/modules/email/__init__.py b/plinth/modules/email/__init__.py
index 33832622b..e9dd2a2a0 100644
--- a/plinth/modules/email/__init__.py
+++ b/plinth/modules/email/__init__.py
@@ -15,7 +15,7 @@ from plinth.modules.config import get_domainname
 from plinth.modules.firewall.components import (Firewall,
                                                 FirewallLocalProtection)
 from plinth.modules.letsencrypt.components import LetsEncrypt
-from plinth.package import Packages, uninstall
+from plinth.package import Packages
 from plinth.privileged import service as service_privileged
 from plinth.signals import domain_added, domain_removed
 from plinth.utils import format_lazy
@@ -97,7 +97,7 @@ class EmailApp(plinth.app.App):
                 'dovecot-lmtpd', 'dovecot-managesieved', 'dovecot-ldap',
                 'rspamd', 'redis-server', 'openssl'
             ], conflicts=['exim4-base', 'exim4-config', 'exim4-daemon-light'],
-            conflicts_action=Packages.ConflictsAction.IGNORE)
+            conflicts_action=Packages.ConflictsAction.REMOVE)
         self.add(packages)
 
         listen_ports = [(25, 'tcp4'), (25, 'tcp6'), (465, 'tcp4'),
@@ -171,17 +171,7 @@ class EmailApp(plinth.app.App):
 
     def setup(self, old_version):
         """Install and configure the app."""
-
-        def _clear_conflicts():
-            component = self.get_component('packages-email')
-            packages_to_remove = component.find_conflicts()
-            if packages_to_remove:
-                logger.info('Removing conflicting packages: %s',
-                            packages_to_remove)
-                uninstall(packages_to_remove)
-
         # Install
-        _clear_conflicts()
         super().setup(old_version)
 
         # Setup
diff --git a/plinth/package.py b/plinth/package.py
index c4e2a80dc..6ed4d2245 100644
--- a/plinth/package.py
+++ b/plinth/package.py
@@ -170,6 +170,13 @@ class Packages(app.FollowerComponent):
 
     def setup(self, old_version):
         """Install the packages."""
+        packages_to_remove = self.find_conflicts()
+        if packages_to_remove and \
+           self.conflicts_action not in (None, self.ConflictsAction.IGNORE):
+            logger.info('Removing conflicting packages: %s',
+                        packages_to_remove)
+            uninstall(packages_to_remove)
+
         install(self.get_actual_packages(),
                 skip_recommends=self.skip_recommends)
 
diff --git a/plinth/tests/test_package.py b/plinth/tests/test_package.py
index 66ddafb15..3b40ae535 100644
--- a/plinth/tests/test_package.py
+++ b/plinth/tests/test_package.py
@@ -114,6 +114,36 @@ def test_packages_setup(install):
     install.assert_has_calls([call(['python3'], skip_recommends=False)])
 
 
+@patch('plinth.package.packages_installed')
+@patch('plinth.package.uninstall')
+@patch('plinth.package.install')
+def test_packages_setup_with_conflicts(install, uninstall, packages_installed):
+    """Test setting up packages with conflicts."""
+    packages_installed.return_value = ['exim4-base']
+
+    component = Packages('test-component', ['bash'], conflicts=['exim4-base'],
+                         conflicts_action=Packages.ConflictsAction.REMOVE)
+    component.setup(old_version=0)
+    uninstall.assert_has_calls([call(['exim4-base'])])
+    install.assert_has_calls([call(['bash'], skip_recommends=False)])
+
+    uninstall.reset_mock()
+    install.reset_mock()
+    component = Packages('test-component', ['bash'], conflicts=['exim4-base'])
+    component.setup(old_version=0)
+    uninstall.assert_not_called()
+    install.assert_has_calls([call(['bash'], skip_recommends=False)])
+
+    uninstall.reset_mock()
+    install.reset_mock()
+    component = Packages('test-component', ['bash'],
+                         conflicts=['exim4-base', 'not-installed-package'],
+                         conflicts_action=Packages.ConflictsAction.IGNORE)
+    component.setup(old_version=0)
+    uninstall.assert_not_called()
+    install.assert_has_calls([call(['bash'], skip_recommends=False)])
+
+
 @patch('plinth.package.uninstall')
 def test_packages_uninstall(uninstall):
     """Test uninstalling packages component."""