solver3: Refactor (rewrite) FromDepCache()

Refactor the various Install requests into a single one and move
some stuff around, this should be a bit easier to maintain and
we don't repeat ourselves all the time.

This accidentally uncovered some bugs in the code, that get fixed
by this:

- The UpgradeManual and InstallManual groups did not work correctly,
  Essential packages were Install and others were Upgrade.

- Automatically installed packages falsely got promoted to the
  manual groups as they got promoted into the manual code path,
  whenever we ran without allowing their removal.

- The combination of --mark-auto --auto-remove now works correctly

  We previously skipped the installation of automatically installed
  packages when we did autoremoval, but we did not consider if the
  action was protected, that is specified on the command-line.

  Now we do not autoremove protected actions, and have added a test
  case for it.

- The AllowRemoveManual option no longer is applied if removals are
  generally denied.

Closes: #1071519

1 files changed, 23 insertions, 36 deletions

diff --git a/apt-pkg/solver3.cc b/apt-pkg/solver3.cc
index f7f72b1a4..7580d6e18 100644
--- a/apt-pkg/solver3.cc
+++ b/apt-pkg/solver3.cc
@@ -906,10 +906,7 @@ bool APT::Solver::Solve()
 // \brief Apply the selections from the dep cache to the solver
 bool APT::Solver::FromDepCache(pkgDepCache &depcache)
 {
-   bool KeepAuto = not _config->FindB("APT::Get::AutomaticRemove");
-   bool AllowRemove = _config->FindB("APT::Solver::Remove", true);
-   bool AllowInstall = _config->FindB("APT::Solver::Install", true);
-   bool AllowRemoveManual = _config->FindB("APT::Solver::RemoveManual", false);
+   bool AllowRemoveManual = AllowRemove && _config->FindB("APT::Solver::RemoveManual", false);
    DefaultRootSetFunc2 rootSet(&cache);
 
    for (auto P = cache.PkgBegin(); not P.end(); P++)
@@ -918,10 +915,6 @@ bool APT::Solver::FromDepCache(pkgDepCache &depcache)
 	 continue;
 
       auto state = depcache[P];
-      auto maybeInstall = state.Install() || (state.Keep() && P->CurrentVer);
-      auto reject = state.Delete() || (depcache[P].Keep() && not P->CurrentVer && depcache[P].Protect());
-      auto isAuto = (depcache[P].Flags & pkgCache::Flag::Auto);
-      auto isOptional = isAuto || (AllowRemoveManual && not depcache[P].Protect());
       if (P->SelectedState == pkgCache::State::Hold && not state.Protect())
       {
 	 if (unlikely(debug >= 1))
@@ -929,45 +922,46 @@ bool APT::Solver::FromDepCache(pkgDepCache &depcache)
 	 if (P->CurrentVer ? not Install(P.CurrentVer(), {}, Group::HoldOrDelete) : not Reject(P, {}, Group::HoldOrDelete))
 	    return false;
       }
-      else if (reject)
+      else if (state.Delete()						  // Normal delete request.
+	       || (not P->CurrentVer && state.Keep() && state.Protect())  // Delete request of not installed package.
+	       || (not P->CurrentVer && state.Keep() && not AllowInstall) // New package installs not allowed.
+      )
       {
 	 if (unlikely(debug >= 1))
 	    std::cerr << "Delete " << P.FullName() << "\n";
 	 if (!Reject(P, {}, Group::HoldOrDelete))
 	    return false;
       }
-      else if (maybeInstall && P->Flags & (pkgCache::Flag::Essential | pkgCache::Flag::Important))
-      {
-	 if (unlikely(debug >= 1))
-	    std::cerr << "ESSENTIAL " << P.FullName() << "\n";
-	 if (depcache[P].Keep() ? not Install(P, {}, Group::InstallManual) : not Install(depcache.GetCandidateVersion(P), {}, Group::InstallManual))
-	    return false;
-      }
-      else if (maybeInstall && not isOptional)
-      {
-	 auto Upgrade = depcache.GetCandidateVersion(P) != P.CurrentVer();
-	 auto Group = (Upgrade ? Group::UpgradeManual : Group::InstallManual);
-	 if (unlikely(debug >= 1))
-	    std::cerr << "MANUAL " << P.FullName() << "\n";
-	 if (depcache[P].Keep() ? not Install(P, {}, Group) : not Install(depcache.GetCandidateVersion(P), {}, Group))
-	    return false;
-      }
-      else if (maybeInstall && isOptional && (KeepAuto || rootSet.InRootSet(P) || not isAuto))
+      else if (state.Install() || (state.Keep() && P->CurrentVer))
       {
+	 auto isEssential = P->Flags & (pkgCache::Flag::Essential | pkgCache::Flag::Important);
+	 auto isAuto = (depcache[P].Flags & pkgCache::Flag::Auto);
+	 auto isOptional = ((isAuto && AllowRemove) || AllowRemoveManual) && not isEssential && not depcache[P].Protect();
+	 auto Root = rootSet.InRootSet(P);
 	 auto Upgrade = depcache.GetCandidateVersion(P) != P.CurrentVer();
 	 auto Group = isAuto ? (Upgrade ? Group::UpgradeAuto : Group::KeepAuto)
 			     : (Upgrade ? Group::UpgradeManual : Group::InstallManual);
+
+	 if (isAuto && not depcache[P].Protect() && not isEssential && not KeepAuto && not rootSet.InRootSet(P))
+	 {
+	    if (unlikely(debug >= 1))
+	       std::cerr << "Ignore automatic install " << P.FullName() << " (" << (isEssential ? "E" : "") << (isAuto ? "M" : "") << (Root ? "R" : "") << ")"
+			 << "\n";
+	    continue;
+	 }
 	 if (unlikely(debug >= 1))
-	    std::cerr << "AUTOMATIC " << P.FullName() << (Upgrade ? " - upgrade" : "") << "\n";
+	    std::cerr << "Install " << P.FullName() << " (" << (isEssential ? "E" : "") << (isAuto ? "M" : "") << (Root ? "R" : "") << ")"
+		      << "\n";
 
-	 if (not AllowRemove)
+	 if (not isOptional)
 	 {
+	    // Pre-empt the non-optional requests, as we don't want to queue them, we can just "unit propagate" here.
 	    if (depcache[P].Keep() ? not Install(P, {}, Group) : not Install(depcache.GetCandidateVersion(P), {}, Group))
 	       return false;
 	 }
 	 else
 	 {
-	    Work w{Reason(), depth(), Group, true, Upgrade};
+	    Work w{Reason(), depth(), Group, isOptional, Upgrade};
 	    for (auto V = P.VersionList(); not V.end(); ++V)
 	       if (IsAllowedVersion(V))
 		  w.solutions.push_back(V);
@@ -975,13 +969,6 @@ bool APT::Solver::FromDepCache(pkgDepCache &depcache)
 	    AddWork(std::move(w));
 	 }
       }
-      else if (P->CurrentVer == 0 && not AllowInstall)
-      {
-	 if (unlikely(debug >= 1))
-	    std::cerr << "NOT ALLOWING INSTALL OF " << P.FullName() << "\n";
-	 if (not Reject(P, {}, Group::HoldOrDelete))
-	    return false;
-      }
    }
 
    return true;