Do not save new if we have already installed providers

Companion of the previous commit that uses the same reasoning, but from
the other side of the logic (as we start from reverse deps here) so that
we are dropping the useless NEW from the solution.

Ideally this code would not live in -private, but in -pkg, but there is
not really a pre-existent good central place for this currently as a
sort-of fixup for our default resolver given this is the result of
resolver decision to not do something which had ripple effects that
aren't reverted along with it.

It is a bit unlikely that this is fixing all instances that this bug
has given we have dealt with a few instances of it already as they are
relatively obscure situations to clean up after and the cost of being
wrong (= autoremover generating breaks) is much higher than being
correct (= no pointless install – that is likely installed later anyhow
after the temporary automatic hold is lifted). And by virtue of being
the result of mostly temporary situations its also unlikely to be
debugable in hindsight months later; so if there is another situation,
its better to deal with it in a new bugreport.

Closes: #839546

2 files changed, 20 insertions, 9 deletions

diff --git a/apt-private/private-install.cc b/apt-private/private-install.cc
index 4f71f18c4..599caac6d 100644
--- a/apt-private/private-install.cc
+++ b/apt-private/private-install.cc
@@ -658,6 +658,24 @@ bool DoAutomaticRemove(CacheFile &Cache)
 		    if (not Cache->VS().CheckDep(PVerStr, R->CompareOp, R.TargetVer()))
 		       continue;
 		 }
+		 // ignore new providers if we have installed providers
+		 if (Pkg->CurrentVer == 0)
+		 {
+		    std::unique_ptr<pkgCache::Version *[]> VList(R.AllTargets());
+		    bool has_installed_alt_prov = false;
+		    for (pkgCache::Version **V = VList.get(); *V != 0; ++V)
+		    {
+		       pkgCache::VerIterator Ver(Cache, *V);
+		       auto const P = Ver.ParentPkg();
+		       if (not P.end() && P->CurrentVer != 0 && Cache[P].InstallVer == *V)
+		       {
+			  has_installed_alt_prov = true;
+			  break;
+		       }
+		    }
+		    if (has_installed_alt_prov)
+		       continue;
+		 }
 		 if (Debug == true)
 		    std::clog << "Save " << APT::PrettyPkg(Cache, Pkg) << " as another installed package depends on it: " << APT::PrettyPkg(Cache, RP) << std::endl;
 		 Cache->MarkInstall(Pkg, false, 0, false);
diff --git a/test/integration/test-bug-604222-new-and-autoremove b/test/integration/test-bug-604222-new-and-autoremove
index 92219fc07..6a251c8f5 100755
--- a/test/integration/test-bug-604222-new-and-autoremove
+++ b/test/integration/test-bug-604222-new-and-autoremove
@@ -29,19 +29,12 @@ Recommends: libgphoto2-l10n (= 2)'
 
 setupaptarchive
 
-testsuccessequal "Reading package lists...
+testsuccessequal 'Reading package lists...
 Building dependency tree...
 Calculating upgrade...
-The following package was automatically installed and is no longer required:
-  libfoto1t64
-Use 'apt autoremove' to remove it.
-The following NEW packages will be installed:
-  libfoto1t64
 The following packages have been kept back:
   photoapp2
-0 upgraded, 1 newly installed, 0 to remove and 1 not upgraded.
-Inst libfoto1t64 (1 stable [i386])
-Conf libfoto1t64 (1 stable [i386])" apt upgrade -s
+0 upgraded, 0 newly installed, 0 to remove and 1 not upgraded.' apt upgrade -s
 
 insertinstalledpackage 'libvtk5.4' 'i386' '5.4.2-7'
 testsuccess aptmark markauto 'libvtk5.4'