Move Repository to context manager

kiwi/repository/base.py

@@ -44,6 +44,9 @@ def __init__(
 
         self.post_init(custom_args or [])
 
+    def __enter__(self):
+        return self
+
     def post_init(self, custom_args: List = []) -> None:
         """
         Post initialization method
@@ -166,3 +169,6 @@ def run_repo_customize(script_path: str, repo_file) -> None:
         Command.run(
             ['bash', '--norc', script_path, repo_file]
         )
+
+    def __exit__(self, exc_type, exc_value, traceback):
+        pass  # pragma: no cover

kiwi/repository/zypper.py

@@ -450,7 +450,9 @@ def _restore_package_cache(self) -> None:
         """
         self._move_package_cache(restore=True)
 
-    def _move_package_cache(self, backup: bool = False, restore: bool = False) -> None:
+    def _move_package_cache(
+        self, backup: bool = False, restore: bool = False
+    ) -> None:
         package_cache = self.shared_location + '/packages'
         package_cache_moved = package_cache + '.moved'
         if backup and os.path.exists(package_cache):
@@ -462,5 +464,5 @@ def _move_package_cache(self, backup: bool = False, restore: bool = False) -> No
                 ['mv', '-f', package_cache_moved, package_cache]
             )
 
-    def __del__(self) -> None:
+    def __exit__(self, exc_type, exc_value, traceback):
         self._restore_package_cache()

kiwi/system/prepare.py

@@ -17,6 +17,7 @@
 #
 import os
 import logging
+from contextlib import ExitStack
 from typing import (
     List, Any, Optional
 )
@@ -145,78 +146,80 @@ def setup_repositories(
             repository_options.append(
                 f'_target_arch%{target_arch}'
             )
-        repo = Repository.new(
+        with Repository.new(
             self.root_bind, package_manager, repository_options
-        )
-        repo.setup_package_database_configuration()
-        if signing_keys:
-            repo.import_trusted_keys(signing_keys)
-        for xml_repo in repository_sections:
-            repo_type = xml_repo.get_type()
-            repo_source = xml_repo.get_source().get_path()
-            repo_user = xml_repo.get_username()
-            repo_secret = xml_repo.get_password()
-            repo_alias = xml_repo.get_alias()
-            repo_priority = xml_repo.get_priority()
-            repo_dist = xml_repo.get_distribution()
-            repo_components = xml_repo.get_components()
-            repo_repository_gpgcheck = xml_repo.get_repository_gpgcheck()
-            repo_package_gpgcheck = xml_repo.get_package_gpgcheck()
-            repo_customization_script = self._get_repo_customization_script(
-                xml_repo
-            )
-            repo_sourcetype = xml_repo.get_sourcetype()
-            repo_use_for_bootstrap = \
-                True if xml_repo.get_use_for_bootstrap() else False
-            log.info(
-                'Setting up repository %s', Uri.print_sensitive(repo_source)
-            )
-            log.info('--> Type: {0}'.format(repo_type))
-            if repo_sourcetype:
-                log.info('--> SourceType: {0}'.format(repo_sourcetype))
-            if repo_priority:
-                log.info('--> Priority: {0}'.format(repo_priority))
-
-            uri = Uri(repo_source, repo_type)
-            repo_source_translated = uri.translate()
-            log.info(
-                '--> Translated: {0}'.format(
-                    Uri.print_sensitive(repo_source_translated)
+        ) as repo:
+            repo.setup_package_database_configuration()
+            if signing_keys:
+                repo.import_trusted_keys(signing_keys)
+            for xml_repo in repository_sections:
+                repo_type = xml_repo.get_type()
+                repo_source = xml_repo.get_source().get_path()
+                repo_user = xml_repo.get_username()
+                repo_secret = xml_repo.get_password()
+                repo_alias = xml_repo.get_alias()
+                repo_priority = xml_repo.get_priority()
+                repo_dist = xml_repo.get_distribution()
+                repo_components = xml_repo.get_components()
+                repo_repository_gpgcheck = xml_repo.get_repository_gpgcheck()
+                repo_package_gpgcheck = xml_repo.get_package_gpgcheck()
+                repo_customization_script = self._get_repo_customization_script(
+                    xml_repo
                 )
-            )
-            if not repo_alias:
-                repo_alias = uri.alias()
-            log.info('--> Alias: {0}'.format(repo_alias))
-
-            if not uri.is_remote() and not os.path.exists(
-                repo_source_translated
-            ):
-                log.warning(
-                    'repository %s does not exist and will be skipped',
-                    repo_source
+                repo_sourcetype = xml_repo.get_sourcetype()
+                repo_use_for_bootstrap = \
+                    True if xml_repo.get_use_for_bootstrap() else False
+                log.info(
+                    'Setting up repository %s', Uri.print_sensitive(repo_source)
+                )
+                log.info('--> Type: {0}'.format(repo_type))
+                if repo_sourcetype:
+                    log.info('--> SourceType: {0}'.format(repo_sourcetype))
+                if repo_priority:
+                    log.info('--> Priority: {0}'.format(repo_priority))
+
+                uri = Uri(repo_source, repo_type)
+                repo_source_translated = uri.translate()
+                log.info(
+                    '--> Translated: {0}'.format(
+                        Uri.print_sensitive(repo_source_translated)
+                    )
+                )
+                if not repo_alias:
+                    repo_alias = uri.alias()
+                log.info('--> Alias: {0}'.format(repo_alias))
+
+                if not uri.is_remote() and not os.path.exists(
+                    repo_source_translated
+                ):
+                    log.warning(
+                        'repository %s does not exist and will be skipped',
+                        repo_source
+                    )
+                    continue
+
+                if not uri.is_remote():
+                    self.root_bind.mount_shared_directory(
+                        repo_source_translated
+                    )
+
+                repo.add_repo(
+                    repo_alias, repo_source_translated,
+                    repo_type, repo_priority, repo_dist, repo_components,
+                    repo_user, repo_secret, uri.credentials_file_name(),
+                    repo_repository_gpgcheck, repo_package_gpgcheck,
+                    repo_sourcetype, repo_use_for_bootstrap,
+                    repo_customization_script
                 )
-                continue
-
-            if not uri.is_remote():
-                self.root_bind.mount_shared_directory(repo_source_translated)
-
-            repo.add_repo(
-                repo_alias, repo_source_translated,
-                repo_type, repo_priority, repo_dist, repo_components,
-                repo_user, repo_secret, uri.credentials_file_name(),
-                repo_repository_gpgcheck, repo_package_gpgcheck,
-                repo_sourcetype, repo_use_for_bootstrap,
-                repo_customization_script
+                if clear_cache:
+                    repo.delete_repo_cache(repo_alias)
+                self.uri_list.append(uri)
+            repo.cleanup_unused_repos()
+            return PackageManager.new(
+                repository=repo,
+                package_manager_name=package_manager,
+                release_version=release_version
             )
-            if clear_cache:
-                repo.delete_repo_cache(repo_alias)
-            self.uri_list.append(uri)
-        repo.cleanup_unused_repos()
-        return PackageManager.new(
-            repository=repo,
-            package_manager_name=package_manager,
-            release_version=release_version
-        )
 
     def install_bootstrap(
         self, manager: PackageManagerBase, plus_packages: List = None
@@ -404,19 +407,20 @@ def pinch_system(
                 )
             )
             try:
-                if manager is None:
-                    package_manager = self.xml_state.get_package_manager()
-                    release_version = self.xml_state.get_release_version()
-                    manager = PackageManager.new(
-                        repository=Repository.new(
-                            self.root_bind, package_manager
-                        ),
-                        package_manager_name=package_manager,
-                        release_version=release_version
+                with ExitStack() as stack:
+                    if manager is None:
+                        package_manager = self.xml_state.get_package_manager()
+                        release_version = self.xml_state.get_release_version()
+                        repo = Repository.new(self.root_bind, package_manager)
+                        stack.push(repo)
+                        manager = PackageManager.new(
+                            repository=repo,
+                            package_manager_name=package_manager,
+                            release_version=release_version
+                        )
+                    self.delete_packages(
+                        manager, to_become_deleted_packages, force
                     )
-                self.delete_packages(
-                    manager, to_become_deleted_packages, force
-                )
             except Exception as issue:
                 raise KiwiPackagesDeletePhaseFailed(
                     self.issue_message.format(
@@ -535,12 +539,13 @@ def clean_package_manager_leftovers(self) -> None:
         """
         package_manager = self.xml_state.get_package_manager()
         release_version = self.xml_state.get_release_version()
-        manager = PackageManager.new(
-            repository=Repository.new(self.root_bind, package_manager),
-            package_manager_name=package_manager,
-            release_version=release_version
-        )
-        manager.clean_leftovers()
+        with Repository.new(self.root_bind, package_manager) as repo:
+            manager = PackageManager.new(
+                repository=repo,
+                package_manager_name=package_manager,
+                release_version=release_version
+            )
+            manager.clean_leftovers()
 
     def _install_archives(self, archive_list, archive_target_dir_dict):
         log.info("Installing archives")

kiwi/system/setup.py

@@ -139,52 +139,52 @@ def import_repositories_marked_as_imageinclude(self) -> None:
         root = RootInit(
             root_dir=self.root_dir, allow_existing=True
         )
-        repo = Repository.new(
+        with Repository.new(
             RootBind(root), self.xml_state.get_package_manager()
-        )
-        repo.use_default_location()
-        for xml_repo in repository_sections:
-            repo_type = xml_repo.get_type()
-            repo_source = xml_repo.get_source().get_path()
-            repo_user = xml_repo.get_username()
-            repo_secret = xml_repo.get_password()
-            repo_alias = xml_repo.get_alias()
-            repo_priority = xml_repo.get_priority()
-            repo_dist = xml_repo.get_distribution()
-            repo_components = xml_repo.get_components()
-            repo_repository_gpgcheck = xml_repo.get_repository_gpgcheck()
-            repo_package_gpgcheck = xml_repo.get_package_gpgcheck()
-            repo_customization_script = self._get_repo_customization_script(
-                xml_repo
-            )
-            repo_sourcetype = xml_repo.get_sourcetype()
-            repo_use_for_bootstrap = False
-            uri = Uri(repo_source, repo_type)
-            repo_source_translated = uri.translate(
-                check_build_environment=False
-            )
-            if not repo_alias:
-                repo_alias = uri.alias()
-            log.info(
-                'Setting up image repository {0}'.format(
-                    Uri.print_sensitive(repo_source)
+        ) as repo:
+            repo.use_default_location()
+            for xml_repo in repository_sections:
+                repo_type = xml_repo.get_type()
+                repo_source = xml_repo.get_source().get_path()
+                repo_user = xml_repo.get_username()
+                repo_secret = xml_repo.get_password()
+                repo_alias = xml_repo.get_alias()
+                repo_priority = xml_repo.get_priority()
+                repo_dist = xml_repo.get_distribution()
+                repo_components = xml_repo.get_components()
+                repo_repository_gpgcheck = xml_repo.get_repository_gpgcheck()
+                repo_package_gpgcheck = xml_repo.get_package_gpgcheck()
+                repo_customization_script = self._get_repo_customization_script(
+                    xml_repo
                 )
-            )
-            log.info('--> Type: {0}'.format(repo_type))
-            log.info(
-                '--> Translated: {0}'.format(
-                    Uri.print_sensitive(repo_source_translated)
+                repo_sourcetype = xml_repo.get_sourcetype()
+                repo_use_for_bootstrap = False
+                uri = Uri(repo_source, repo_type)
+                repo_source_translated = uri.translate(
+                    check_build_environment=False
+                )
+                if not repo_alias:
+                    repo_alias = uri.alias()
+                log.info(
+                    'Setting up image repository {0}'.format(
+                        Uri.print_sensitive(repo_source)
+                    )
+                )
+                log.info('--> Type: {0}'.format(repo_type))
+                log.info(
+                    '--> Translated: {0}'.format(
+                        Uri.print_sensitive(repo_source_translated)
+                    )
+                )
+                log.info('--> Alias: {0}'.format(repo_alias))
+                repo.add_repo(
+                    repo_alias, repo_source_translated,
+                    repo_type, repo_priority, repo_dist, repo_components,
+                    repo_user, repo_secret, uri.credentials_file_name(),
+                    repo_repository_gpgcheck, repo_package_gpgcheck,
+                    repo_sourcetype, repo_use_for_bootstrap,
+                    repo_customization_script
                 )
-            )
-            log.info('--> Alias: {0}'.format(repo_alias))
-            repo.add_repo(
-                repo_alias, repo_source_translated,
-                repo_type, repo_priority, repo_dist, repo_components,
-                repo_user, repo_secret, uri.credentials_file_name(),
-                repo_repository_gpgcheck, repo_package_gpgcheck,
-                repo_sourcetype, repo_use_for_bootstrap,
-                repo_customization_script
-            )
 
     def import_cdroot_files(self, target_dir: str) -> None:
         """

test/unit/repository/zypper_test.py

@@ -426,9 +426,10 @@ def test_delete_repo_cache(self, mock_wipe):
 
     @patch('kiwi.command.Command.run')
     @patch('os.path.exists')
-    def test_destructor(self, mock_exists, mock_command):
-        mock_exists.return_value = True
-        self.repo.__del__()
-        mock_command.assert_called_once_with(
+    def test_context_manager_exit(self, mock_os_path_exists, mock_command_run):
+        mock_os_path_exists.return_value = True
+        with RepositoryZypper(self.root_bind):
+            pass
+        mock_command_run.assert_called_once_with(
             ['mv', '-f', '/shared-dir/packages.moved', '/shared-dir/packages']
         )

test/unit/system/prepare_test.py

@@ -254,7 +254,7 @@ def test_setup_repositories(
             return_value='credentials-file'
         )
         repo = Mock()
-        mock_repo.return_value = repo
+        mock_repo.return_value.__enter__.return_value = repo
 
         self.system.setup_repositories(
             clear_cache=True,
@@ -324,7 +324,7 @@ def test_setup_repositories_local_not_existing(
             return_value='uri-alias'
         )
         repo = Mock()
-        mock_repo.return_value = repo
+        mock_repo.return_value.__enter__.return_value = repo
         with self._caplog.at_level(logging.WARNING):
             self.system.setup_repositories()
 

test/unit/system/setup_test.py

@@ -1591,7 +1591,7 @@ def test_import_repositories_marked_as_imageinclude(
         )
         mock_uri.return_value = uri
         repo = Mock()
-        mock_repo.return_value = repo
+        mock_repo.return_value.__enter__.return_value = repo
         self.setup_with_real_xml.import_repositories_marked_as_imageinclude()
         assert repo.add_repo.call_args_list[0] == call(
             'uri-alias', 'uri', 'rpm-md', None, None, None, None, None,