Fix keeping back removals of obsolete packages

ResolveByKeepInternal() inadvertently considered packages that were
marked for removals as not being possible to keep back, but that's
not true, all they need is to have a currently installed version.

The test case is a bit awkward to construct because our upgrade code
doesn't get into a place where this happens that we know off. Or maybe
it does with phased upgrades, but here we simplify this by using a
helper binary that dumps state, marks for removal, and then calls
ResolveByKeep and checks the removal is undone.

LP: #2078720

5 files changed, 90 insertions, 2 deletions

diff --git a/apt-pkg/algorithms.cc b/apt-pkg/algorithms.cc
index 5869668cd..6131ecd11 100644
--- a/apt-pkg/algorithms.cc
+++ b/apt-pkg/algorithms.cc
@@ -1380,8 +1380,7 @@ bool pkgProblemResolver::ResolveByKeepInternal()
 	       pkgCache::PkgIterator Pkg = Ver.ParentPkg();
 	       
 	       // It is not keepable
-	       if (Cache[Pkg].InstallVer == 0 ||
-		   Pkg->CurrentVer == 0)
+	       if (Pkg->CurrentVer == 0)
 		  continue;
 
 	       if (not Cache[Pkg].Protect())
diff --git a/test/integration/solver3.broken b/test/integration/solver3.broken
index 75ddbe5df..dd06ed0eb 100644
--- a/test/integration/solver3.broken
+++ b/test/integration/solver3.broken
@@ -26,6 +26,7 @@ test-phased-updates-upgrade
 test-prevent-markinstall-multiarch-same-versionscrew
 test-release-candidate-switching
 test-resolve-by-keep-new-recommends
+test-resolve-by-keep-obsolete-removals
 test-ubuntu-bug-1304403-obsolete-priority-standard
 test-ubuntu-bug-1990586
 test-ubuntu-bug-2025462-phased-dist-upgrade
diff --git a/test/integration/test-resolve-by-keep-obsolete-removals b/test/integration/test-resolve-by-keep-obsolete-removals
new file mode 100755
index 000000000..ab08ea81a
--- /dev/null
+++ b/test/integration/test-resolve-by-keep-obsolete-removals
@@ -0,0 +1,29 @@
+#!/bin/sh
+set -e
+
+TESTDIR="$(readlink -f "$(dirname "$0")")"
+. "$TESTDIR/framework"
+setupenvironment
+
+configarchitecture "amd64"
+
+insertpackage 'installed' 'obsolete-dependency' 'amd64' '1.0'
+insertpackage 'installed,unstable' 'foo' 'amd64' '2.0' 'Depends: obsolete-dependency'
+
+setupaptarchive
+
+echo 'Debug::pkgProblemResolver "1";' > rootdir/etc/apt/apt.conf.d/debug-problemresolver
+
+# We mark obsolete dependency for removal in B, then in C it is restored.
+testsuccessequal "A: foo:amd64 < 2.0 @ii mK >
+A: obsolete-dependency:amd64 < 1.0 @ii mK >
+B: foo:amd64 < 2.0 @ii mK Ib >
+B: obsolete-dependency:amd64 < 1.0 @ii mR >
+Entering ResolveByKeep
+  Dependencies are not satisfied for foo:amd64 < 2.0 @ii mK Ib >
+Keeping package foo:amd64
+  Dependencies are not satisfied for foo:amd64 < 2.0 @ii mK Ib >
+Package foo:amd64 foo:amd64 Depends on obsolete-dependency:amd64 < 1.0 @ii mR >
+  Keeping Package obsolete-dependency:amd64 due to Depends
+C: foo:amd64 < 2.0 @ii mK >
+C: obsolete-dependency:amd64 < 1.0 @ii mK >" runapt ${APTTESTHELPERSBINDIR}/testkeep obsolete-dependency
diff --git a/test/interactive-helper/CMakeLists.txt b/test/interactive-helper/CMakeLists.txt
index 8e8764353..bfca6c8f6 100644
--- a/test/interactive-helper/CMakeLists.txt
+++ b/test/interactive-helper/CMakeLists.txt
@@ -14,6 +14,8 @@ add_executable(mthdcat mthdcat.cc)
 target_link_libraries(mthdcat ${APTPKG_LIB})
 add_executable(testdeb testdeb.cc)
 target_link_libraries(testdeb ${APTPKG_LIB})
+add_executable(testkeep testkeep.cc)
+target_link_libraries(testkeep ${APTPKG_LIB})
 add_executable(extract-control extract-control.cc)
 target_link_libraries(extract-control ${APTPKG_LIB})
 add_executable(aptwebserver aptwebserver.cc)
diff --git a/test/interactive-helper/testkeep.cc b/test/interactive-helper/testkeep.cc
new file mode 100644
index 000000000..f65f43203
--- /dev/null
+++ b/test/interactive-helper/testkeep.cc
@@ -0,0 +1,57 @@
+#include <config.h>
+
+#include <apt-pkg/init.h>
+#include <apt-pkg/algorithms.h>
+#include <apt-pkg/cachefile.h>
+#include <apt-pkg/error.h>
+#include <apt-pkg/pkgsystem.h>
+#include <apt-pkg/prettyprinters.h>
+
+#include <iostream>
+#include <string>
+
+static bool test(const char *name, bool hold)
+{
+   pkgCacheFile cache;
+   if (not cache.Open())
+      return false;
+
+   for (auto Pkg = cache->PkgBegin(); Pkg.end() == false; ++Pkg)
+      std::cout << "A: " << APT::PrettyPkg(cache, Pkg) << std::endl;
+
+   cache->MarkDelete(cache->FindPkg(name));
+   if (hold)
+      cache->MarkProtected(cache->FindPkg(name));
+
+   for (auto Pkg = cache->PkgBegin(); Pkg.end() == false; ++Pkg)
+      std::cout << "B: " << APT::PrettyPkg(cache, Pkg) << std::endl;
+
+   pkgProblemResolver resolve(cache);
+   bool res = resolve.ResolveByKeep();
+
+   for (auto Pkg = cache->PkgBegin(); Pkg.end() == false; ++Pkg)
+      std::cout << "C: " << APT::PrettyPkg(cache, Pkg) << std::endl;
+
+   return res;
+}
+
+int main(int argc, const char *argv[])
+{
+   if (argc != 2 && argc != 3)
+   {
+      return _error->Error("Expected one argument");
+      goto err;
+   }
+   if (not pkgInitConfig(*_config))
+      goto err;
+   if (not pkgInitSystem(*_config, _system))
+      goto err;
+   if (not test(argv[1], argc > 2 && strcmp(argv[2], "--hold") == 0))
+      goto err;
+
+   return 0;
+err:
+   _error->DumpErrors();
+   return 1;
+}
+