Merge branch 'solver3' into 'main'

solver3: Fix Recommends/Suggests vs Enhances confusion

See merge request apt-team/apt!461

2 files changed, 35 insertions, 20 deletions

diff --git a/apt-pkg/solver3.cc b/apt-pkg/solver3.cc
index 739309c59..cebface27 100644
--- a/apt-pkg/solver3.cc
+++ b/apt-pkg/solver3.cc
@@ -831,44 +831,54 @@ APT::Solver::Clause APT::Solver::TranslateOrGroup(pkgCache::DepIterator start, p
    if (clause.optional && start.ParentPkg()->CurrentVer)
    {
       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);
-
+      auto importantToKeep = [this](pkgCache::DepIterator d)
+      {
+	 return policy.IsImportantDep(d) || (KeepRecommends && d->Type == pkgCache::Dep::Recommends) || (KeepSuggests && d->Type == pkgCache::Dep::Suggests);
+      };
       bool satisfied = std::any_of(clause.solutions.begin(), clause.solutions.end(), [this](auto var)
 				   { return var.Pkg(cache) ? var.Pkg(cache)->CurrentVer != nullptr : Var(var.CastPkg(cache).CurrentVer()) == var; });
 
-      if (important && wasImportant && not newOptional && not satisfied)
+      // Find the existing dependency
+      pkgCache::DepIterator existing;
+      for (auto D = start.ParentPkg().CurrentVer().DependsList(); not D.end(); D++)
+	 if (not D.IsCritical() && importantToKeep(D) && D.TargetPkg() == start.TargetPkg())
+	 {
+	    existing = D;
+	    break;
+	 }
+
+      if (not existing.end() && not important && importantToKeep(start) && satisfied)
       {
 	 if (unlikely(debug >= 3))
-	    std::cerr << "Ignoring unsatisfied Recommends " << clause.toString(cache) << std::endl;
-	 clause.solutions.clear();
+	    std::cerr << "Try to keep satisfied: " << clause.toString(cache, true) << std::endl;
       }
-      else if (not important && not wasImportant && not newOptional && satisfied)
+      else if (not important)
       {
 	 if (unlikely(debug >= 3))
-	    std::cerr << "Promoting satisfied Suggests to Recommends: " << clause.toString(cache) << std::endl;
-	 important = true;
+	    std::cerr << "Ignore unimportant clause: " << clause.toString(cache, true) << std::endl;
+	 return Clause{reason, Group::Satisfy, true};
       }
-      else if (satisfied && important && wasImportant && clause.solutions.size() > 0)
+      else if (not existing.end() && policy.IsImportantDep(existing) && not satisfied)
       {
 	 if (unlikely(debug >= 3))
-	    std::cerr << "Promoting existing Recommends " << clause.toString(cache) << " to depends in upgrade" << std::endl;
-	 clause.optional = false;
+	    std::cerr << "Ignoring unsatisfied clause: " << clause.toString(cache, true) << std::endl;
+	 return Clause{reason, Group::Satisfy, true};
       }
-      else if (newOptional && important && reason.Ver() && clause.solutions.size() > 0 && reason.Ver(cache) != reason.CastPkg(cache).CurrentVer() && IsUpgrade)
+      else if (IsUpgrade && not existing.end() && satisfied)
       {
 	 if (unlikely(debug >= 3))
-	    std::cerr << "Promoting new Recommends " << clause.toString(cache) << " to depends in upgrade" << std::endl;
+	    std::cerr << "Promoting previously satisfied clause to hard dependency: " << clause.toString(cache, true) << std::endl;
 	 clause.optional = false;
       }
-      else if (not important)
+      else if (
+	 IsUpgrade && not(AllowRemove && AllowInstall)				    // promote Recommends to Depends in upgrade, not in dist-upgrade.
+	 && reason.Ver() && reason.Ver(cache) != reason.CastPkg(cache).CurrentVer() // and if this an upgrade to an installed package
+	 && (existing.end() || not policy.IsImportantDep(existing))		    // new Recommends, or upgraded from Suggests
+      )
       {
 	 if (unlikely(debug >= 3))
-	    std::cerr << "Ignoring Suggests " << clause.toString(cache) << std::endl;
-	 return Clause{reason, Group::Satisfy, true};
+	    std::cerr << "Promoting new clause to hard dependency: " << clause.toString(cache) << std::endl;
+	 clause.optional = false;
       }
    }
 
diff --git a/apt-pkg/solver3.h b/apt-pkg/solver3.h
index b7000c52e..1bdc13559 100644
--- a/apt-pkg/solver3.h
+++ b/apt-pkg/solver3.h
@@ -243,6 +243,11 @@ class Solver
    // \brief If set, we use strict pinning.
    int Timeout{_config->FindI("APT::Solver::Timeout", 10)};
 
+   // \brief Keep recommends installed
+   bool KeepRecommends{_config->FindB("APT::AutoRemove::RecommendsImportant", true)};
+   // \brief Keep suggests installed
+   bool KeepSuggests{_config->FindB("APT::AutoRemove::SuggestsImportant", true)};
+
    // \brief Discover a variable, translating the underlying dependencies to the SAT presentation
    //
    // This does a breadth-first search of the entire dependency tree of var,