Merge branch 'solver3-refactor-from' into 'main'

Rewrite the FromDepCache method to be smaller and more correct

See merge request apt-team/apt!360

5 files changed, 60 insertions, 41 deletions

diff --git a/apt-pkg/solver3.cc b/apt-pkg/solver3.cc
index f94fa6736..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;
@@ -992,6 +979,8 @@ bool APT::Solver::ToDepCache(pkgDepCache &depcache)
    pkgDepCache::ActionGroup group(depcache);
    for (auto P = cache.PkgBegin(); not P.end(); P++)
    {
+      depcache[P].Marked = 0;
+      depcache[P].Garbage = 0;
       if ((*this)[P].decision == Decision::MUST)
       {
 	 for (auto V = P.VersionList(); not V.end(); V++)
diff --git a/test/integration/test-apt-get-mark-auto b/test/integration/test-apt-get-mark-auto
new file mode 100755
index 000000000..844412e67
--- /dev/null
+++ b/test/integration/test-apt-get-mark-auto
@@ -0,0 +1,30 @@
+#!/bin/sh
+set -e
+
+TESTDIR="$(readlink -f "$(dirname "$0")")"
+. "$TESTDIR/framework"
+setupenvironment
+configarchitecture 'amd64'
+
+insertpackage 'unstable' 'package' 'all' '3'
+
+setupaptarchive
+
+testsuccessequal "Reading package lists...
+Building dependency tree...
+The following NEW packages will be installed:
+  package
+0 upgraded, 1 newly installed, 0 to remove and 0 not upgraded.
+Inst package (3 unstable [all])
+Conf package (3 unstable [all])" aptget install package --mark-auto -s
+
+# Specifically if we mark the package as automatically installed but also have auto-remove on,
+# the package should still be installed...
+
+testsuccessequal "Reading package lists...
+Building dependency tree...
+The following NEW packages will be installed:
+  package
+0 upgraded, 1 newly installed, 0 to remove and 0 not upgraded.
+Inst package (3 unstable [all])
+Conf package (3 unstable [all])" aptget install package --mark-auto --auto-remove -s
diff --git a/test/integration/test-bug-549968-install-depends-of-not-installed b/test/integration/test-bug-549968-install-depends-of-not-installed
index 39c86cc8d..f575f9940 100755
--- a/test/integration/test-bug-549968-install-depends-of-not-installed
+++ b/test/integration/test-bug-549968-install-depends-of-not-installed
@@ -32,7 +32,7 @@ Conf coolstuff (1.0 unstable [all])" aptget install coolstuff extracoolstuff- -o
 testsuccessequal "Reading package lists...
 Building dependency tree...
 Package 'extracoolstuff' is not installed, so not removed
-Solving dependencies...MANUAL coolstuff:i386
+Solving dependencies...Install coolstuff:i386 ()
 [0] Install:coolstuff:i386=1.0 ()
 Delete extracoolstuff:i386
 [0] Reject:extracoolstuff:i386 ()
diff --git a/test/integration/test-bug-960705-propagate-protected-to-satisfied-depends b/test/integration/test-bug-960705-propagate-protected-to-satisfied-depends
index da11fc881..6ffb69bf5 100755
--- a/test/integration/test-bug-960705-propagate-protected-to-satisfied-depends
+++ b/test/integration/test-bug-960705-propagate-protected-to-satisfied-depends
@@ -20,7 +20,7 @@ setupaptarchive
 
 testsuccessequal "Reading package lists...
 Building dependency tree...
-Solving dependencies...MANUAL foobar:amd64
+Solving dependencies...Install foobar:amd64 ()
 [0] Install:foobar:amd64=1 ()
 [0] Install:requires-foo:amd64=1 (foobar:amd64=1)
 [0] Install:foo:amd64=1 (foobar:amd64=1 -> requires-foo:amd64=1)
diff --git a/test/integration/test-dpkg-i-apt-install-fix-broken b/test/integration/test-dpkg-i-apt-install-fix-broken
index 5c517aaef..a4c3c66d9 100755
--- a/test/integration/test-dpkg-i-apt-install-fix-broken
+++ b/test/integration/test-dpkg-i-apt-install-fix-broken
@@ -14,13 +14,13 @@ setupaptarchive
 testfailure dpkg -i incoming/autopkgtest-*.deb
 testsuccessequal 'Reading package lists...
 Building dependency tree...
-Correcting dependencies...MANUAL autopkgtest-satdep:amd64
+Correcting dependencies...Install autopkgtest-satdep:amd64 ()
 [0] Install:autopkgtest-satdep:amd64 ()
 [0] Install:autopkgtest-satdep:amd64=1 (autopkgtest-satdep:amd64)
 [0] Install:debhelper:amd64=1 (autopkgtest-satdep:amd64 -> autopkgtest-satdep:amd64=1)
  Done
-Solving dependencies...AUTOMATIC debhelper:amd64 - upgrade
-MANUAL autopkgtest-satdep:amd64
+Solving dependencies...Install debhelper:amd64 (M)
+Install autopkgtest-satdep:amd64 ()
 [0] Install:autopkgtest-satdep:amd64 ()
 [0] Install:autopkgtest-satdep:amd64=1 (autopkgtest-satdep:amd64)
 [0] Install:debhelper:amd64=1 (autopkgtest-satdep:amd64 -> autopkgtest-satdep:amd64=1)