mirror of
https://github.com/freedombox/FreedomBox.git
synced 2026-08-19 12:36:06 +00:00
package: Don't remove packages of other apps on uninstall
Fixes: #2376. Fixes: #2317. When an app is removed, its packages are purged. However, there is another installed app that needs these packages, we should keep those packages. We have already implemented checking the packages against other apps' packages. However, we are not checking if we are removing dependencies of other apps' packages. This will still result in removal apps' packages. To solve this problem, get list of packages of all the apps, then iterate over their dependencies recursively and compile a comprehensive list of packages to keep. Use this to reduce the set of packages to remove. Tests: - Without the patch, install bepasty and janus. Uninstall janus app and notice that bepasty package is removed. With the patch, the problem is not observed. - Printing the comprehensive list of packages to keep shows an extensive set computed. Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org> Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
This commit is contained in:
parent
709f58ac90
commit
1612318b60
@ -6,6 +6,7 @@ import logging
|
||||
import pathlib
|
||||
import time
|
||||
|
||||
import apt
|
||||
import apt.cache
|
||||
from django.utils.translation import gettext as _
|
||||
from django.utils.translation import gettext_lazy, gettext_noop
|
||||
@ -183,24 +184,15 @@ class Packages(app_module.FollowerComponent):
|
||||
|
||||
def uninstall(self):
|
||||
"""Uninstall and purge the packages."""
|
||||
# Ensure package list is update-to-date before looking at dependencies.
|
||||
refresh_package_lists()
|
||||
|
||||
# List of packages to purge from the system
|
||||
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
|
||||
logger.info('App\'s list of packages to remove: %s', packages)
|
||||
|
||||
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],
|
||||
purge=True)
|
||||
packages = self._filter_packages_to_keep(packages)
|
||||
uninstall(packages, purge=True)
|
||||
|
||||
def diagnose(self) -> list[DiagnosticCheck]:
|
||||
"""Run diagnostics and return results."""
|
||||
@ -271,6 +263,60 @@ class Packages(app_module.FollowerComponent):
|
||||
|
||||
return False
|
||||
|
||||
def _filter_packages_to_keep(self, packages: list[str]) -> list[str]:
|
||||
"""Filter out the list of packages to keep from given list.
|
||||
|
||||
Packages to keep are packages needed by other installed apps and their
|
||||
dependencies (PreDepends, Depends, Recommends).
|
||||
"""
|
||||
packages_set: set[str] = set(packages)
|
||||
|
||||
# Get list of packages needed by other installed apps (packages to
|
||||
# keep).
|
||||
keep_packages: set[str] = set()
|
||||
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):
|
||||
keep_packages |= set(component.get_actual_packages())
|
||||
|
||||
# Get list of all the dependencies of packages to keep.
|
||||
keep_packages_with_deps: set[str] = set()
|
||||
cache = apt.Cache()
|
||||
while keep_packages:
|
||||
package_name = keep_packages.pop()
|
||||
if package_name in keep_packages_with_deps:
|
||||
continue # Already processed
|
||||
|
||||
keep_packages_with_deps.add(package_name)
|
||||
if package_name not in cache:
|
||||
continue # Package is not available in sources
|
||||
|
||||
if not cache[package_name].is_installed:
|
||||
continue # Package is not installed
|
||||
|
||||
version = cache[package_name].installed
|
||||
if not version:
|
||||
continue
|
||||
|
||||
dependencies = version.dependencies + version.recommends
|
||||
for dependency in dependencies:
|
||||
for or_dependency in dependency.or_dependencies:
|
||||
keep_packages.add(or_dependency.name)
|
||||
|
||||
# Filter out any packages that are to be kept or their dependencies.
|
||||
packages_set -= keep_packages_with_deps
|
||||
|
||||
# Preserve order of packages for ease of testing.
|
||||
return [package for package in packages if package in packages_set]
|
||||
|
||||
|
||||
class PackageException(Exception):
|
||||
"""A package operation has failed."""
|
||||
|
||||
@ -151,8 +151,9 @@ def test_packages_setup_with_conflicts(install, uninstall, packages_installed):
|
||||
install.assert_has_calls([call(['bash'], skip_recommends=False)])
|
||||
|
||||
|
||||
@patch('plinth.package.refresh_package_lists')
|
||||
@patch('plinth.package.uninstall')
|
||||
def test_packages_uninstall(uninstall):
|
||||
def test_packages_uninstall(uninstall, _refresh_package_lists):
|
||||
"""Test uninstalling packages component."""
|
||||
|
||||
class TestApp(App):
|
||||
@ -166,15 +167,53 @@ def test_packages_uninstall(uninstall):
|
||||
uninstall.assert_has_calls([call(['python3', 'bash'], purge=True)])
|
||||
|
||||
|
||||
@patch('plinth.package.refresh_package_lists')
|
||||
@patch('plinth.package.uninstall')
|
||||
@patch('apt.Cache')
|
||||
def test_packages_uninstall_exclusion(cache, uninstall):
|
||||
def test_packages_uninstall_exclusion(cache, uninstall,
|
||||
_refresh_package_lists):
|
||||
"""Test excluding packages from other installed apps when uninstalling."""
|
||||
|
||||
def _get_mock_package(installed_version='1.0', dependencies=None,
|
||||
recommends=None):
|
||||
mock_dependencies = []
|
||||
for or_dependencies in (dependencies or []):
|
||||
mock_or_dependency = Mock(or_dependencies=[])
|
||||
mock_dependencies.append(mock_or_dependency)
|
||||
for dependency in or_dependencies:
|
||||
mock = Mock()
|
||||
mock.name = dependency
|
||||
mock_or_dependency.or_dependencies.append(mock)
|
||||
|
||||
mock_recommends = []
|
||||
for or_dependencies in (recommends or []):
|
||||
mock_or_dependency = Mock(or_dependencies=[])
|
||||
mock_recommends.append(mock_or_dependency)
|
||||
for dependency in or_dependencies:
|
||||
mock = Mock()
|
||||
mock.name = dependency
|
||||
mock_or_dependency.or_dependencies.append(mock)
|
||||
|
||||
mock = Mock(
|
||||
version=installed_version or '1.0',
|
||||
is_installed=bool(installed_version),
|
||||
installed=Mock(dependencies=mock_dependencies,
|
||||
recommends=mock_recommends))
|
||||
return mock
|
||||
|
||||
package2 = _get_mock_package('4.0', [['dep1', 'dep2'], ['dep3'], ['dep4']],
|
||||
[['dep5']])
|
||||
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)),
|
||||
'package11': _get_mock_package('2.0'),
|
||||
'package12': _get_mock_package(None),
|
||||
'package2': package2,
|
||||
'package3': _get_mock_package('5.0', ['unknown-dep1']),
|
||||
'dep1': _get_mock_package('6.0'),
|
||||
'dep2': _get_mock_package('6.1'),
|
||||
'dep3': _get_mock_package('6.2'),
|
||||
'dep4': _get_mock_package(None),
|
||||
'dep5': _get_mock_package('6.4'),
|
||||
'dep6': _get_mock_package('6.5'),
|
||||
}
|
||||
|
||||
class TestApp1(App):
|
||||
@ -183,8 +222,10 @@ def test_packages_uninstall_exclusion(cache, uninstall):
|
||||
|
||||
def __init__(self):
|
||||
super().__init__()
|
||||
component = Packages('test-component11',
|
||||
['package11', 'package2', 'package3'])
|
||||
component = Packages('test-component11', [
|
||||
'package11', 'package2', 'package3', 'dep1', 'dep2', 'dep3',
|
||||
'dep4', 'dep6'
|
||||
])
|
||||
self.add(component)
|
||||
|
||||
component = Packages('test-component12',
|
||||
@ -220,7 +261,7 @@ def test_packages_uninstall_exclusion(cache, uninstall):
|
||||
TestApp3()
|
||||
app1.uninstall()
|
||||
uninstall.assert_has_calls([
|
||||
call(['package11', 'package3'], purge=True),
|
||||
call(['package11', 'package3', 'dep6'], purge=True),
|
||||
call(['package12', 'package3'], purge=True)
|
||||
])
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user