Merge lp:~bjornt/landscape-client/apt-broken-dependencies into lp:~landscape/landscape-client/trunk

[Björn Tillenius]

Allow package operation even when some installed packages are in a
broken state.

It doesn't try to resolve broken packages, unless the changes introduced
some packages that weren't broken before.

I've disabled a multi-arch tests, since it failed, and it's not easy to
fix it. It turned out that the test was incorrect, it sets up a broken
system. I filed a bug about it and intend to fix it next.

We still need to figure out how to handle broken sytems in general, i.e.
but this branch makes it possible to work with them at least.

[Thomas Herve (therve)]

Looks good, +1!

review: Approve

[Alberto Donato (ack)]

+1, looks good!

review: Approve

=== modified file 'landscape/package/facade.py'
--- landscape/package/facade.py	2012-01-25 12:18:18 +0000
+++ landscape/package/facade.py	2012-01-27 16:16:42 +0000
@@ -384,6 +384,12 @@
             version for version in self.get_packages()
             if version.package.name == name]
 
+    def _get_broken_packages(self):
+        """Return the packages that are in a broken state."""
+        return set(
+            version.package for version in self.get_packages()
+            if version.package.is_inst_broken)
+
     def perform_changes(self):
         """Perform the pending package operations."""
         held_package_names = set()
@@ -397,6 +403,7 @@
         if not version_changes and not self._global_upgrade:
             return None
         fixer = apt_pkg.ProblemResolver(self._cache._depcache)
+        already_broken_packages = self._get_broken_packages()
         for version in self._version_installs:
             # Set the candidate version, so that the version we want to
             # install actually is the one getting installed.
@@ -431,7 +438,8 @@
                 "Can't perform the changes, since the following packages" +
                 " are held: %s" % ", ".join(sorted(held_package_names)))
 
-        if self._cache._depcache.broken_count > 0:
+        now_broken_packages = self._get_broken_packages()
+        if now_broken_packages != already_broken_packages:
             try:
                 fixer.resolve(True)
             except SystemError, error:

=== modified file 'landscape/package/tests/test_facade.py'
--- landscape/package/tests/test_facade.py	2012-01-25 12:17:22 +0000
+++ landscape/package/tests/test_facade.py	2012-01-27 16:16:42 +0000
@@ -1023,7 +1023,12 @@
              ("single-arch", "2.0")],
             sorted(changes))
 
-    def test_wb_mark_install_upgrade_non_main_arch_dependency_error(self):
+    # XXX: The following test has been disabled, since the test setup
+    # results in the "multi-arch" package being broken, so it's not
+    # testing a valid scenario. This results in the test failing with
+    # the fix for bug 921664, even though it shouldn't.
+    # Bug 922511 has been filed to fix this test.
+    def disabled_test_wb_mark_install_upgrade_non_main_arch_dependency_error(self):
         """
         If a non-main architecture is automatically upgraded, and the
         main architecture versions hasn't been marked for installation,
@@ -1182,6 +1187,99 @@
         # objects, which are the ones that build the output string.
         self.assertEqual("", self.facade.perform_changes())
 
+    def test_perform_changes_with_broken_packages_install_simple(self):
+        """
+        Even if some installed packages are broken in the system, it's
+        still possible to install packages with no dependencies that
+        don't touch the broken ones.
+        """
+        deb_dir = self.makeDir()
+        self._add_system_package(
+            "broken", control_fields={"Depends": "missing"})
+        self._add_package_to_deb_dir(deb_dir, "foo")
+        self._add_package_to_deb_dir(deb_dir, "missing")
+        self.facade.add_channel_apt_deb("file://%s" % deb_dir, "./")
+        self.facade.reload_channels()
+        [foo] = self.facade.get_packages_by_name("foo")
+        self.facade.mark_install(foo)
+        self.facade._cache.commit = lambda fetch_progress: None
+        self.assertEqual("", self.facade.perform_changes())
+        self.assertEqual(
+            [foo.package], self.facade._cache.get_changes())
+
+    def test_perform_changes_with_broken_packages_install_deps(self):
+        """
+        Even if some installed packages are broken in the system, it's
+        still possible to install packages where the dependencies need
+        to be calculated.
+        """
+        deb_dir = self.makeDir()
+        self._add_system_package(
+            "broken", control_fields={"Depends": "missing"})
+        self._add_package_to_deb_dir(
+            deb_dir, "foo", control_fields={"Depends": "bar"})
+        self._add_package_to_deb_dir(deb_dir, "bar")
+        self._add_package_to_deb_dir(deb_dir, "missing")
+        self.facade.add_channel_apt_deb("file://%s" % deb_dir, "./")
+        self.facade.reload_channels()
+        [foo] = self.facade.get_packages_by_name("foo")
+        [bar] = self.facade.get_packages_by_name("bar")
+        self.facade.mark_install(foo)
+        self.facade._cache.commit = lambda fetch_progress: None
+        error = self.assertRaises(DependencyError, self.facade.perform_changes)
+        self.assertEqual([bar], error.packages)
+
+    def test_perform_changes_with_broken_packages_remove_simple(self):
+        """
+        Even if some installed packages are broken in the system, it's
+        still possible to remove packages that don't affect the broken ones.
+        """
+        deb_dir = self.makeDir()
+        self._add_system_package(
+            "broken", control_fields={"Depends": "missing"})
+        self._add_system_package("foo")
+        self._add_package_to_deb_dir(deb_dir, "missing")
+        self.facade.add_channel_apt_deb("file://%s" % deb_dir, "./")
+        self.facade.reload_channels()
+        [foo] = self.facade.get_packages_by_name("foo")
+        self.facade.mark_remove(foo)
+        self.facade._cache.commit = lambda fetch_progress: None
+        self.assertEqual("", self.facade.perform_changes())
+        self.assertEqual(
+            [foo.package], self.facade._cache.get_changes())
+
+    def test_perform_changes_with_broken_packages_install_broken(self):
+        """
+        If some installed package is in a broken state and you install a
+        package that fixes the broken package, as well as a new broken
+        package, C{perform_changes()} will raise a C{TransactionError}.
+
+        This test specifically tests the case where you replace the
+        broken packages, but have the same number of broken packages
+        before and after the changes.
+        """
+        deb_dir = self.makeDir()
+        self._add_system_package(
+            "broken", control_fields={"Depends": "missing"})
+        self._add_package_to_deb_dir(
+            deb_dir, "foo", control_fields={"Depends": "really-missing"})
+        self._add_package_to_deb_dir(deb_dir, "missing")
+        self.facade.add_channel_apt_deb("file://%s" % deb_dir, "./")
+        self.facade.reload_channels()
+        [broken] = self.facade.get_packages_by_name("broken")
+        [foo] = self.facade.get_packages_by_name("foo")
+        [missing] = self.facade.get_packages_by_name("missing")
+        self.assertEqual(
+            set([broken.package]), self.facade._get_broken_packages())
+        self.facade.mark_install(foo)
+        self.facade.mark_install(missing)
+        self.facade._cache.commit = lambda fetch_progress: None
+        error = self.assertRaises(
+            TransactionError, self.facade.perform_changes)
+        self.assertIn("you have held broken packages", error.args[0])
+        self.assertEqual(
+            set([foo.package]), self.facade._get_broken_packages())
+
     def test_wb_perform_changes_commit_error(self):
         """
         If an error happens when committing the changes to the cache, a
@@ -1509,7 +1607,7 @@
         # was added to python-apt. So if it's not there, it means that
         # multi-arch support isn't available.
         skip_message = "multi-arch not supported"
-        test_wb_mark_install_upgrade_non_main_arch_dependency_error.skip = (
+        disabled_test_wb_mark_install_upgrade_non_main_arch_dependency_error.skip = (
             skip_message)
         test_wb_mark_install_upgrade_non_main_arch.skip = skip_message