Recognize propagated protected in pkgProblemResolver

Turns out that pkgDepCache and pkgProblemResolver maintain two (semi)
independent sets of protected flags – except that a package if marked
protected in the pkgProblemResolver is automatically also marked in the
pkgDepCache as protected. This way the pkgProblemResolver will have as
protected only the direct user requests while pkgDepCache will
(hopefully) propagate the flag to unavoidable dependencies of these
requests nowadays. The pkgProblemResolver was only checking his own
protected flag though and based on that calls our Mark* methods usually
without checking return, leading to it believing it could e.g. remove
packages it actually can't remove as pkgDepCache will not allow it as it
is marked as protected there. Teaching it to check for the flag in the
pkgDepCache instead avoids it believing in the wrong things eventually
giving up.

The scoring is keeping the behaviour of adding the large score boost
only for the direct user requests though as there is no telling which
other sideeffects this might have if too many packages get too many
points from the get-go.

Second part of fixing #960705, now with pkgProblemResolver output which
looks more like the whole class of problem is resolved rather than a
teeny tiny edgecase it was before.

5 files changed, 33 insertions, 17 deletions

diff --git a/apt-pkg/algorithms.cc b/apt-pkg/algorithms.cc
index f2977b9af..f000b7f39 100644
--- a/apt-pkg/algorithms.cc
+++ b/apt-pkg/algorithms.cc
@@ -599,7 +599,8 @@ bool pkgProblemResolver::DoUpgrade(pkgCache::PkgIterator Pkg)
    Flags[Pkg->ID] &= ~Upgradable;
    
    bool WasKept = Cache[Pkg].Keep();
-   Cache.MarkInstall(Pkg, false, 0, false);
+   if (not Cache.MarkInstall(Pkg, false, 0, false))
+     return false;
 
    // This must be a virtual package or something like that.
    if (Cache[Pkg].InstVerIter(Cache).end() == true)
@@ -633,7 +634,7 @@ bool pkgProblemResolver::DoUpgrade(pkgCache::PkgIterator Pkg)
 	 
 	 // Do not change protected packages
 	 PkgIterator P = Start.SmartTargetPkg();
-	 if ((Flags[P->ID] & Protected) == Protected)
+	 if (Cache[P].Protect())
 	 {
 	    if (Debug == true)
 	       clog << "    Reinst Failed because of protected " << P.FullName(false) << endl;
@@ -800,7 +801,7 @@ bool pkgProblemResolver::ResolveInternal(bool const BrokenFix)
 	 if (Cache[I].CandidateVer != Cache[I].InstallVer &&
 	     I->CurrentVer != 0 && Cache[I].InstallVer != 0 &&
 	     (Flags[I->ID] & PreInstalled) != 0 &&
-	     (Flags[I->ID] & Protected) == 0 &&
+	     not Cache[I].Protect() &&
 	     (Flags[I->ID] & ReInstateTried) == 0)
 	 {
 	    if (Debug == true)
@@ -849,7 +850,7 @@ bool pkgProblemResolver::ResolveInternal(bool const BrokenFix)
 	       {
 		  if (OrOp == OrRemove)
 		  {
-		     if ((Flags[I->ID] & Protected) != Protected)
+		     if (not Cache[I].Protect())
 		     {
 			if (Debug == true)
 			   clog << "  Or group remove for " << I.FullName(false) << endl;
@@ -903,7 +904,7 @@ bool pkgProblemResolver::ResolveInternal(bool const BrokenFix)
 	       targets then we keep the package and bail. This is necessary
 	       if a package has a dep on another package that can't be found */
 	    std::unique_ptr<pkgCache::Version *[]> VList(Start.AllTargets());
-	    if (VList[0] == 0 && (Flags[I->ID] & Protected) != Protected &&
+	    if (VList[0] == 0 && not Cache[I].Protect() &&
 		Start.IsNegative() == false &&
 		Cache[I].NowBroken() == false)
 	    {	       
@@ -950,7 +951,7 @@ bool pkgProblemResolver::ResolveInternal(bool const BrokenFix)
 		    End.IsNegative() == false))
 	       {
 		  // Try a little harder to fix protected packages..
-		  if ((Flags[I->ID] & Protected) == Protected)
+		  if (Cache[I].Protect())
 		  {
 		     if (DoUpgrade(Pkg) == true)
 		     {
@@ -1037,7 +1038,7 @@ bool pkgProblemResolver::ResolveInternal(bool const BrokenFix)
 		  }
 
 		  // Skip adding to the kill list if it is protected
-		  if ((Flags[Pkg->ID] & Protected) != 0)
+		  if (Cache[Pkg].Protect())
 		     continue;
 		
 		  if (Debug == true)
@@ -1053,7 +1054,7 @@ bool pkgProblemResolver::ResolveInternal(bool const BrokenFix)
 	    // Hm, nothing can possibly satisfy this dep. Nuke it.
 	    if (VList[0] == 0 &&
 		Start.IsNegative() == false &&
-		(Flags[I->ID] & Protected) != Protected)
+		not Cache[I].Protect())
 	    {
 	       bool Installed = Cache[I].Install();
 	       Cache.MarkKeep(I);
@@ -1133,7 +1134,7 @@ bool pkgProblemResolver::ResolveInternal(bool const BrokenFix)
       {
 	 if (Cache[I].InstBroken() == false)
 	    continue;
-	 if ((Flags[I->ID] & Protected) != Protected)
+	 if (not Cache[I].Protect())
 	    return _error->Error(_("Error, pkgProblemResolver::Resolve generated breaks, this may be caused by held packages."));
       }
       return _error->Error(_("Unable to correct problems, you have held broken packages."));
@@ -1253,7 +1254,7 @@ bool pkgProblemResolver::ResolveByKeepInternal()
 
       /* Keep the package. If this works then great, otherwise we have
 	 to be significantly more aggressive and manipulate its dependencies */
-      if ((Flags[I->ID] & Protected) == 0)
+      if (not Cache[I].Protect())
       {
 	 if (Debug == true)
 	    clog << "Keeping package " << I.FullName(false) << endl;
@@ -1301,7 +1302,7 @@ bool pkgProblemResolver::ResolveByKeepInternal()
 		   Pkg->CurrentVer == 0)
 		  continue;
 	       
-	       if ((Flags[I->ID] & Protected) == 0)
+	       if (not Cache[I].Protect())
 	       {
 		  if (Debug == true)
 		     clog << "  Keeping Package " << Pkg.FullName(false) << " due to " << Start.DepType() << endl;
diff --git a/test/integration/test-bug-735967-lib32-to-i386-unavailable b/test/integration/test-bug-735967-lib32-to-i386-unavailable
index 9dbd17bfd..b836451fd 100755
--- a/test/integration/test-bug-735967-lib32-to-i386-unavailable
+++ b/test/integration/test-bug-735967-lib32-to-i386-unavailable
@@ -53,7 +53,7 @@ The following information may help to resolve the situation:
 
 The following packages have unmet dependencies:
  libfoo : Depends: libfoo-bin but it is not installable
-E: Error, pkgProblemResolver::Resolve generated breaks, this may be caused by held packages.' aptget install foo -s
+E: Unable to correct problems, you have held broken packages.' aptget install foo -s
 
 # activate multiarch
 configarchitecture 'amd64' 'i386'
diff --git a/test/integration/test-bug-960705-propagate-protected-to-satisfied-conflict b/test/integration/test-bug-960705-propagate-protected-to-satisfied-conflict
index 4def47400..943e968a4 100755
--- a/test/integration/test-bug-960705-propagate-protected-to-satisfied-conflict
+++ b/test/integration/test-bug-960705-propagate-protected-to-satisfied-conflict
@@ -17,6 +17,19 @@ setupaptarchive
 
 testsuccessequal "Reading package lists...
 Building dependency tree...
+   Removing: systemd-sysv as upgrade is not an option for runit-init:amd64 (1)
+    MarkDelete systemd-sysv:amd64 < 1 @ii mK > FU=0
+  MarkInstall runit-init:amd64 < none -> 1 @un puN > FU=1
+Starting pkgProblemResolver with broken count: 1
+Starting 2 pkgProblemResolver with broken count: 1
+Investigating (0) init:amd64 < 1 @ii mK Ib >
+Broken init:amd64 PreDepends on systemd-sysv:amd64 < 1 @ii pmR >
+  Considering systemd-sysv:amd64 0 as a solution to init:amd64 5100
+Broken init:amd64 PreDepends on sysvinit-core:amd64 < none | 1 @un pH >
+  Considering sysvinit-core:amd64 0 as a solution to init:amd64 5100
+  Or group remove for init:amd64
+  MarkDelete init:amd64 < 1 @ii mK Ib > FU=0
+Done
 The following packages will be REMOVED:
   init systemd-sysv
 The following NEW packages will be installed:
@@ -24,8 +37,10 @@ The following NEW packages will be installed:
 WARNING: The following essential packages will be removed.
 This should NOT be done unless you know exactly what you are doing!
   init systemd-sysv (due to init)
+  MarkDelete init:amd64 < 1 @ii K > FU=1
 0 upgraded, 1 newly installed, 2 to remove and 0 not upgraded.
 Remv init [1]
+  MarkDelete systemd-sysv:amd64 < 1 | none @ii H > FU=1
 Remv systemd-sysv [1]
 Inst runit-init (1 unstable [all])
-Conf runit-init (1 unstable [all])" apt install runit-init -s
+Conf runit-init (1 unstable [all])" apt install runit-init -so Debug::pkgProblemResolver=1 -o Debug::pkgDepCache::Marker=1 -o Debug::pkgDepCache::AutoInstall=1
diff --git a/test/integration/test-explore-or-groups-in-markinstall b/test/integration/test-explore-or-groups-in-markinstall
index 7f49b23cb..e24145fd8 100755
--- a/test/integration/test-explore-or-groups-in-markinstall
+++ b/test/integration/test-explore-or-groups-in-markinstall
@@ -126,12 +126,12 @@ testfailureequal "$BADSOLVETEXT
 The following packages have unmet dependencies:
  bad-level0 : Depends: unknown but it is not installable
  bad-upgrade-level0 : Depends: unknown but it is not installable
-E: Error, pkgProblemResolver::Resolve generated breaks, this may be caused by held packages." apt install bad-upgrade-level1 -s
+E: Unable to correct problems, you have held broken packages." apt install bad-upgrade-level1 -s
 testfailureequal "$BADSOLVETEXT
 The following packages have unmet dependencies:
  bad-conflict-level0 : Conflicts: bad-conflict-level2 but 1 is to be installed
  bad-level0 : Depends: unknown but it is not installable
-E: Error, pkgProblemResolver::Resolve generated breaks, this may be caused by held packages." apt install bad-conflict-level2 -s
+E: Unable to correct problems, you have held broken packages." apt install bad-conflict-level2 -s
 
 if $TEST_WITH_APTITUDE; then
 	testsuccesstailequal 6 'The following packages have been kept back:
diff --git a/test/integration/test-multiarch-allowed b/test/integration/test-multiarch-allowed
index 167f0c584..a091635f0 100755
--- a/test/integration/test-multiarch-allowed
+++ b/test/integration/test-multiarch-allowed
@@ -66,12 +66,12 @@ testfailureequal "$BADPREFIX
 The following packages have unmet dependencies:
  foo : Conflicts: foo:i386 but 1 is to be installed
  foo:i386 : Conflicts: foo but 1 is to be installed
-E: Error, pkgProblemResolver::Resolve generated breaks, this may be caused by held packages." aptget install needsfoo:i386 foo:amd64 -s
+E: Unable to correct problems, you have held broken packages." aptget install needsfoo:i386 foo:amd64 -s
 testfailureequal "$BADPREFIX
 The following packages have unmet dependencies:
  foo : Conflicts: foo:i386 but 1 is to be installed
  foo:i386 : Conflicts: foo but 1 is to be installed
-E: Error, pkgProblemResolver::Resolve generated breaks, this may be caused by held packages." aptget install needsfoo foo:i386 -s
+E: Unable to correct problems, you have held broken packages." aptget install needsfoo foo:i386 -s
 
 solveableinsinglearch1() {
 	testsuccessequal "Reading package lists...