Allow aptitude to MarkInstall broken packages via FromUser

apt marks packages coming from the commandline among others
as protected to ensure the various resolver parts do not fiddle
with the state of these packages. aptitude (and potentially others)
do not so the state is modified (to a Keep which for uninstalled means
it is not going to be installed) due to being uninstallable before
the call fails – basically reverting at least some state changes the
call made before it realized it has to fail, which is usually a good
idea, except if users expect you to not do it.

They do set the FromUser option though which has beside controlling
autobit also gained the notion of "the user is always right" over time
and can be used for this one here as well preventing the state revert.

References: 0de399391372450d0162b5a09bfca554b2d27c3d
Reported-By: Jessica Clarke <jrtc27@debian.org> on IRC

3 files changed, 59 insertions, 10 deletions

diff --git a/apt-pkg/depcache.cc b/apt-pkg/depcache.cc
index 7730aaf5b..f86ae7d24 100644
--- a/apt-pkg/depcache.cc
+++ b/apt-pkg/depcache.cc
@@ -1196,7 +1196,7 @@ bool pkgDepCache::MarkInstall_UpgradeOrRemoveConflicts(bool const propagateProct
    return true;
 }
 									/*}}}*/
-bool pkgDepCache::MarkInstall_InstallDependencies(PkgIterator const &Pkg, unsigned long Depth, bool const ForceImportantDeps, std::vector<pkgCache::DepIterator> &toInstall, APT::PackageVector *const toMoveAuto, bool const propagateProctected) /*{{{*/
+bool pkgDepCache::MarkInstall_InstallDependencies(PkgIterator const &Pkg, unsigned long Depth, bool const ForceImportantDeps, std::vector<pkgCache::DepIterator> &toInstall, APT::PackageVector *const toMoveAuto, bool const propagateProctected, bool const FromUser) /*{{{*/
 {
    auto const IsSatisfiedByInstalled = [&](auto const D) { return (DepState[D.ID] & DepInstall) == DepInstall; };
    for (auto &&Dep : toInstall)
@@ -1303,7 +1303,7 @@ bool pkgDepCache::MarkInstall_InstallDependencies(PkgIterator const &Pkg, unsign
       }
       if (not foundSolution && IsCriticalDep)
       {
-	 if (not propagateProctected)
+	 if (not propagateProctected && not FromUser)
 	 {
 	    StateCache &State = PkgState[Pkg->ID];
 	    RemoveSizes(Pkg);
@@ -1354,6 +1354,10 @@ bool pkgDepCache::MarkInstall(PkgIterator const &Pkg, bool AutoInst,
    if (not IsInstallOk(Pkg, AutoInst, Depth, FromUser))
       return false;
 
+   ActionGroup group(*this);
+   if (FromUser && not MarkInstall_StateChange(Pkg, AutoInst, FromUser))
+      return false;
+
    bool const AutoSolve = AutoInst && _config->Find("APT::Solver", "internal") == "internal";
 
    std::vector<pkgCache::DepIterator> toInstall, toRemove;
@@ -1370,8 +1374,7 @@ bool pkgDepCache::MarkInstall(PkgIterator const &Pkg, bool AutoInst,
 	 return false;
    }
 
-   ActionGroup group(*this);
-   if (not MarkInstall_StateChange(Pkg, AutoInst, FromUser))
+   if (not FromUser && not MarkInstall_StateChange(Pkg, AutoInst, FromUser))
       return false;
 
    if (not AutoSolve)
@@ -1419,7 +1422,7 @@ bool pkgDepCache::MarkInstall(PkgIterator const &Pkg, bool AutoInst,
    }();
 
    APT::PackageVector toMoveAuto;
-   if (not MarkInstall_InstallDependencies(Pkg, Depth, ForceImportantDeps, toInstall, MoveAutoBitToDependencies ? &toMoveAuto : nullptr, propagateProctected))
+   if (not MarkInstall_InstallDependencies(Pkg, Depth, ForceImportantDeps, toInstall, MoveAutoBitToDependencies ? &toMoveAuto : nullptr, propagateProctected, FromUser))
       return false;
 
    if (MoveAutoBitToDependencies)
diff --git a/apt-pkg/depcache.h b/apt-pkg/depcache.h
index 1579fedbe..858de5505 100644
--- a/apt-pkg/depcache.h
+++ b/apt-pkg/depcache.h
@@ -522,7 +522,7 @@ class APT_PUBLIC pkgDepCache : protected pkgCache::Namespace
    APT_HIDDEN bool MarkInstall_CollectDependencies(pkgCache::VerIterator const &PV, std::vector<pkgCache::DepIterator> &toInstall, std::vector<pkgCache::DepIterator> &toRemove);
    APT_HIDDEN bool MarkInstall_RemoveConflictsIfNotUpgradeable(pkgCache::VerIterator const &PV, unsigned long Depth, std::vector<pkgCache::DepIterator> &toRemove, APT::PackageVector &toUpgrade);
    APT_HIDDEN bool MarkInstall_UpgradeOrRemoveConflicts(bool const propagateProtected, unsigned long Depth, bool const ForceImportantDeps, APT::PackageVector &toUpgrade);
-   APT_HIDDEN bool MarkInstall_InstallDependencies(PkgIterator const &Pkg, unsigned long Depth, bool const ForceImportantDeps, std::vector<pkgCache::DepIterator> &toInstall, APT::PackageVector *const toMoveAuto, bool const propagateProtected);
+   APT_HIDDEN bool MarkInstall_InstallDependencies(PkgIterator const &Pkg, unsigned long Depth, bool const ForceImportantDeps, std::vector<pkgCache::DepIterator> &toInstall, APT::PackageVector *const toMoveAuto, bool const propagateProtected, bool const FromUser);
 };
 
 #endif
diff --git a/test/integration/test-explore-or-groups-in-markinstall b/test/integration/test-explore-or-groups-in-markinstall
index 5669e47b3..763852fa5 100755
--- a/test/integration/test-explore-or-groups-in-markinstall
+++ b/test/integration/test-explore-or-groups-in-markinstall
@@ -41,16 +41,24 @@ insertfoos 'r' 'Recommends'
 
 setupaptarchive
 
-testsuccessheadequal() {
-	msggroup 'testsuccessheadequal'
+_testsuccessheadtailequal() {
+	local TYPE="$1"
+	shift
+	msggroup "testsuccess${TYPE}equal"
 	local HEADLINES="$1"
 	local CMP="$2"
 	shift 2
 	testsuccesswithglobalerror 'testsuccess' 'EW' "$@"
-	cp "${TMPWORKINGDIRECTORY}/rootdir/tmp/testsuccess.output" "${TMPWORKINGDIRECTORY}/rootdir/tmp/testsuccessheadequal.output"
-	testsuccessequal "$CMP" head -n "$HEADLINES" "${TMPWORKINGDIRECTORY}/rootdir/tmp/testsuccessheadequal.output"
+	cp "${TMPWORKINGDIRECTORY}/rootdir/tmp/testsuccess.output" "${TMPWORKINGDIRECTORY}/rootdir/tmp/testsuccess${TYPE}equal.output"
+	testsuccessequal "$CMP" "${TYPE}" -n "$HEADLINES" "${TMPWORKINGDIRECTORY}/rootdir/tmp/testsuccess${TYPE}equal.output"
 	msggroup
 }
+testsuccessheadequal() {
+	_testsuccessheadtailequal 'head' "$@"
+}
+testsuccesstailequal() {
+	_testsuccessheadtailequal 'tail' "$@"
+}
 checkfoos() {
 	msgmsg 'Install checks with foos dependency type' "$2"
 	for level in 0 1 2; do
@@ -82,3 +90,41 @@ The following NEW packages will be installed:
 }
 checkfoos 'd' 'Depends'
 checkfoos 'r' 'Recommends'
+
+TEST_WITH_APTITUDE=false
+msgtest 'Check if aptitude is available for additional tests'
+if dpkg-checkbuilddeps -d 'aptitude' /dev/null >/dev/null 2>&1; then
+	TEST_WITH_APTITUDE=true
+	# we don't document aptitude config options
+	sed -i -e '/^#x-apt-configure-index/ d' "$(getaptconfig)"
+	msgpass
+else
+	msgskip 'not installed'
+fi
+
+if $TEST_WITH_APTITUDE; then
+	OKAYAPTITUDE='0 packages upgraded, 2 newly installed, 0 to remove and 3 not upgraded.
+Need to get 0 B of archives. After unpacking 86.0 kB will be used.
+Would download/install/remove packages.'
+	testsuccesstailequal 3 "$OKAYAPTITUDE" aptitude install foo-d-level2 -sy
+	testsuccesstailequal 3 "$OKAYAPTITUDE" aptitude install foo-r-level2 -sy
+	testsuccesstailequal 3 "$OKAYAPTITUDE" aptitude install foo-d-conflict -sy
+	testsuccesstailequal 3 "$OKAYAPTITUDE" aptitude install foo-r-conflict -sy
+fi
+
+testfailure apt install bad-upgrade-level1 -s
+testfailure apt install bad-conflict-level2 -s
+
+if $TEST_WITH_APTITUDE; then
+	testsuccesstailequal 6 'The following packages have been kept back:
+  bad-upgrade-level1 
+No packages will be installed, upgraded, or removed.
+0 packages upgraded, 0 newly installed, 0 to remove and 2 not upgraded.
+Need to get 0 B of archives. After unpacking 0 B will be used.
+Would download/install/remove packages.' aptitude install bad-upgrade-level1 -sy
+	# aptitude does not show the kept back message in this case…
+	testsuccesstailequal 4 'No packages will be installed, upgraded, or removed.
+0 packages upgraded, 0 newly installed, 0 to remove and 3 not upgraded.
+Need to get 0 B of archives. After unpacking 0 B will be used.
+Would download/install/remove packages.' aptitude install bad-conflict-level2 -sy
+fi