solver3: Promote satisfied Suggests to Recommends

As per the test-suggests-promoted-to-recommends test. This is still
a bit messy as we ignore alternative choices and only check the last
one, but um seems useful?

3 files changed, 48 insertions, 23 deletions

diff --git a/apt-pkg/solver3.cc b/apt-pkg/solver3.cc
index 58fb07c01..3c4ca4eb7 100644
--- a/apt-pkg/solver3.cc
+++ b/apt-pkg/solver3.cc
@@ -348,8 +348,6 @@ bool APT::Solver::Install(pkgCache::VerIterator Ver, Reason reason)
       pkgCache::DepIterator end;
       dep.GlobOr(start, end); // advances dep
 
-      if (not policy.IsImportantDep(start))
-	 continue;
       if (not EnqueueOrGroup(start, end, Reason(Ver)))
 	 return false;
    }
@@ -452,8 +450,6 @@ bool APT::Solver::EnqueueCommonDependencies(pkgCache::PkgIterator Pkg)
       }
       if (not allHaveDep)
 	 continue;
-      if (not policy.IsImportantDep(start))
-	 continue;
       if (not EnqueueOrGroup(start, end, Reason(Pkg)))
 	 return false;
    }
@@ -467,6 +463,11 @@ bool APT::Solver::EnqueueOrGroup(pkgCache::DepIterator start, pkgCache::DepItera
    auto Ver = start.ParentVer();
    auto fixPolicy = _config->FindB("APT::Get::Fix-Policy-Broken");
 
+   // Non-important dependencies can only be installed if they are currently satisfied, see the check further
+   // below once we have calculated all possible solutions.
+   if (start.ParentPkg()->CurrentVer == 0 && not policy.IsImportantDep(start))
+      return true;
+
    if (unlikely(debug >= 3))
       std::cerr << "Found dependency critical " << Ver.ParentPkg().FullName() << "=" << Ver.VerStr() << " -> " << start.TargetPkg().FullName() << "\n";
 
@@ -513,32 +514,57 @@ bool APT::Solver::EnqueueOrGroup(pkgCache::DepIterator start, pkgCache::DepItera
    if (not fixPolicy)
       std::sort(workItem.solutions.begin(), workItem.solutions.end(), CompareProviders3{cache, policy, TgtPkg});
 
-   // Figure out if the reason is installed
-   bool reasonInstalled = false;
-   if (auto p = workItem.reason.Pkg())
-      reasonInstalled = pkgCache::PkgIterator(cache, cache.PkgP + p)->CurrentVer != 0;
-   else if (auto v = workItem.reason.Ver())
-      reasonInstalled = pkgCache::VerIterator(cache, cache.VerP + v).ParentPkg()->CurrentVer != 0;
-
    // Try to perserve satisfied Recommends. FIXME: We should check if the Recommends was there in the installed version?
-   if (workItem.optional && reasonInstalled && not fixPolicy &&
-       not std::any_of(workItem.solutions.begin(), workItem.solutions.end(), [this](auto ver)
-		       { return pkgCache::VerIterator(cache, ver).ParentPkg()->CurrentVer != 0; }))
+   if (workItem.optional && start.ParentPkg()->CurrentVer)
    {
-      if (unlikely(debug >= 3))
+      bool important = policy.IsImportantDep(start);
+      bool newOptional = true;
+      bool wasImportant = false;
+      for (auto D = start.ParentPkg().CurrentVer().DependsList(); not D.end(); D++)
+	 if (not D.IsCritical() && not D.IsNegative() && D.TargetPkg() == start.TargetPkg())
+	    newOptional = false, wasImportant = policy.IsImportantDep(D);
+
+      bool satisfied = std::any_of(workItem.solutions.begin(), workItem.solutions.end(), [this](auto ver)
+				   { return pkgCache::VerIterator(cache, ver).ParentPkg()->CurrentVer != 0; });
+
+      if (important && wasImportant && not newOptional && not satisfied)
       {
-	 std::cerr << "Ignoring currently unsatisfied Recommends ";
-	 workItem.Dump(cache);
-	 std::cerr << "\n";
+	 if (unlikely(debug >= 3))
+	 {
+	    std::cerr << "Ignoring unsatisfied Recommends ";
+	    workItem.Dump(cache);
+	    std::cerr << "\n";
+	 }
+	 return true;
+      }
+      if (not important && not wasImportant && not newOptional && satisfied)
+      {
+	 if (unlikely(debug >= 3))
+	 {
+	    std::cerr << "Promoting satisfied Suggests to Recommends: ";
+	    workItem.Dump(cache);
+	    std::cerr << "\n";
+	 }
+	 important = true;
+      }
+      if (not important)
+      {
+	 if (unlikely(debug >= 3))
+	 {
+	    std::cerr << "Ignoring Suggests ";
+	    workItem.Dump(cache);
+	    std::cerr << "\n";
+	 }
+	 return true;
       }
-      return true;
    }
+
    if (not workItem.solutions.empty())
    {
       // std::sort(workItem.solutions.begin(), workItem.solutions.end(), CompareProviders3{cache, TgtPkg});
       if (unlikely(debug >= 3 && workItem.optional))
       {
-	 std::cerr << "Enqueuing currently satisfied Recommends ";
+	 std::cerr << "Enqueuing Recommends ";
 	 workItem.Dump(cache);
 	 std::cerr << "\n";
       }
diff --git a/test/integration/solver3.broken b/test/integration/solver3.broken
index 65e18a7ff..9228e46c8 100644
--- a/test/integration/solver3.broken
+++ b/test/integration/solver3.broken
@@ -8,7 +8,6 @@ test-apt-move-and-forget-manual-sections
 test-apt-patterns
 test-apt-source-and-build-dep
 test-bug-470115-new-and-tighten-recommends
-test-bug-591882-conkeror
 test-bug-602412-dequote-redirect
 test-bug-611729-mark-as-manual
 test-bug-675449-essential-are-protected
@@ -29,7 +28,6 @@ test-prevent-markinstall-multiarch-same-versionscrew
 test-release-candidate-switching
 test-resolve-by-keep-new-recommends
 test-resolver-provider-exchange
-test-suggests-promoted-to-recommends
 test-ubuntu-bug-1130419-prefer-installed-ma-same-siblings
 test-ubuntu-bug-1304403-obsolete-priority-standard
 test-ubuntu-bug-1990586
diff --git a/test/integration/test-bug-591882-conkeror b/test/integration/test-bug-591882-conkeror
index 6b903431a..21c569756 100755
--- a/test/integration/test-bug-591882-conkeror
+++ b/test/integration/test-bug-591882-conkeror
@@ -4,6 +4,7 @@ set -e
 TESTDIR="$(readlink -f "$(dirname "$0")")"
 . "$TESTDIR/framework"
 setupenvironment
+allowremovemanual
 configarchitecture "i386"
 setupaptarchive
 
@@ -73,5 +74,5 @@ After this operation, 36.0 MB of additional disk space will be used.
 E: Trivial Only specified but this is not a trivial operation."
 
 # Test that the old behavior can be restored with the option
-testfailureequal "$UPGRADEFAIL" aptget dist-upgrade --trivial-only -o pkgProblemResolver::FixByInstall=0
+testfailureequal "$UPGRADEFAIL" aptget dist-upgrade --trivial-only -o pkgProblemResolver::FixByInstall=0 --solver internal
 testfailureequal "$UPGRADESUCCESS" aptget dist-upgrade --trivial-only #-o pkgProblemResolver::FixByInstall=0