edsp: warn if unexpected stanzas appear in the solution

Unexpected are for examples removal requests for versions which aren't
installed, installations of already installed versions & requests to
install and remove a package at the same time.

3 files changed, 84 insertions, 17 deletions

diff --git a/apt-pkg/edsp.cc b/apt-pkg/edsp.cc
index e54f0d1df..8414c6a23 100644
--- a/apt-pkg/edsp.cc
+++ b/apt-pkg/edsp.cc
@@ -320,6 +320,7 @@ bool EDSP::ReadResponse(int const input, pkgDepCache &Cache, OpProgress *Progres
 	pkgTagFile response(&in, 100);
 	pkgTagSection section;
 
+	std::set<decltype(Cache.PkgBegin()->ID)> seenOnce;
 	while (response.Step(section) == true) {
 		std::string type;
 		if (section.Exists("Install") == true)
@@ -362,18 +363,29 @@ bool EDSP::ReadResponse(int const input, pkgDepCache &Cache, OpProgress *Progres
 		}
 
 		pkgCache::VerIterator Ver(Cache.GetCache(), Cache.GetCache().VerP + VerIdx[id]);
-		Cache.SetCandidateVersion(Ver);
-		if (type == "Install")
-		{
-			pkgCache::PkgIterator const P = Ver.ParentPkg();
-			if (Cache[P].Mode != pkgDepCache::ModeInstall)
-				Cache.MarkInstall(P, false, 0, false);
-		}
-		else if (type == "Remove")
-			Cache.MarkDelete(Ver.ParentPkg(), false);
-		else if (type == "Autoremove") {
-			Cache[Ver.ParentPkg()].Marked = false;
-			Cache[Ver.ParentPkg()].Garbage = true;
+		auto const Pkg = Ver.ParentPkg();
+		if (type == "Autoremove") {
+			Cache[Pkg].Marked = false;
+			Cache[Pkg].Garbage = true;
+		} else if (seenOnce.emplace(Pkg->ID).second == false) {
+			_error->Warning("Ignoring %s stanza received for package %s which already had a previous stanza effecting it!", type.c_str(), Pkg.FullName(false).c_str());
+		} else if (type == "Install") {
+			if (Pkg.CurrentVer() == Ver) {
+				_error->Warning("Ignoring Install stanza received for version %s of package %s which is already installed!",
+				      Ver.VerStr(), Pkg.FullName(false).c_str());
+			} else {
+				Cache.SetCandidateVersion(Ver);
+				Cache.MarkInstall(Pkg, false, 0, false);
+			}
+		} else if (type == "Remove") {
+			if (Pkg->CurrentVer == 0)
+				_error->Warning("Ignoring Remove stanza received for version %s of package %s which isn't installed!",
+				      Ver.VerStr(), Pkg.FullName(false).c_str());
+			else if (Pkg.CurrentVer() != Ver)
+				_error->Warning("Ignoring Remove stanza received for version %s of package %s which isn't the installed version %s!",
+				      Ver.VerStr(), Pkg.FullName(false).c_str(), Pkg.CurrentVer().VerStr());
+			else
+				Cache.MarkDelete(Ver.ParentPkg(), false);
 		}
 	}
 	return true;
diff --git a/doc/external-dependency-solver-protocol.txt b/doc/external-dependency-solver-protocol.txt
index d914db309..9b9073346 100644
--- a/doc/external-dependency-solver-protocol.txt
+++ b/doc/external-dependency-solver-protocol.txt
@@ -289,10 +289,15 @@ this protocol makes no assumption on the fact that a subsequent
 invocation of an Autoremove action will actually remove the very same
 packages indicated by Autoremove stanzas in the former solution.
 
-The package identifiers of install, remove and autoremove stanzas from a single
-solution are unique. That is, a package identifier does not occur more than
-once in the solution. Every package identifier is only associated with a single
-action (either install, remove or autoremove).
+A package can't be installed in multiple versions at the same time, so
+for each package there can at most one version be selected either for
+installation or removal. This especially means that a solver is neither
+allowed to represent package upgrades as a remove of the installed
+version and the installation of another (the remove is implicit and must
+be omitted from the solution) nor is it supported to revert previous
+actions in the solution with later actions. APT is allowed to show
+warnings and might even misbehave in earlier versions if a solver is
+violating this assumption.
 
 In terms of expressivity, install and remove stanzas can carry one
 single field each, as APT-IDs are enough to pinpoint packages to be
diff --git a/test/integration/test-external-dependency-solver-protocol b/test/integration/test-external-dependency-solver-protocol
index 32c5fc354..e22684ec5 100755
--- a/test/integration/test-external-dependency-solver-protocol
+++ b/test/integration/test-external-dependency-solver-protocol
@@ -113,7 +113,7 @@ The following package was automatically installed and is no longer required:
 Use '$AUTOREMOVE' to remove it.
 The following packages will be REMOVED:
   cool* somestuff*
-0 upgraded, 0 newly installed, 2 to remove and 0 not upgraded.
+0 upgraded, 0 newly installed, 2 to remove and 1 not upgraded.
 Purg somestuff [1]
 Purg cool [1]" aptget purge --solver apt cool -s
 
@@ -194,3 +194,53 @@ Priority: optional
 Section: other
 APT-Pin: 100
 ' aptinternalsolver scenario stuff
+
+cat > rootdir/usr/lib/apt/solvers/explicitremove << EOF
+#!/bin/sh
+set -e
+while read line; do
+	if [ "APT-ID" = "\${line%:*}" ]; then
+		cat << APT
+Install: \${line#*:}
+
+Remove: \${line#*:}
+
+APT
+	fi
+done
+EOF
+chmod +x rootdir/usr/lib/apt/solvers/explicitremove
+testfailure apt full-upgrade -s --solver explicitremove
+testsuccess grep 'had a previous stanza' rootdir/tmp/testfailure.output
+
+cat > rootdir/usr/lib/apt/solvers/removeall << EOF
+#!/bin/sh
+set -e
+while read line; do
+	if [ "APT-ID" = "\${line%:*}" ]; then
+		cat << APT
+Remove: \${line#*:}
+
+APT
+	fi
+done
+EOF
+chmod +x rootdir/usr/lib/apt/solvers/removeall
+testwarning apt full-upgrade -s --solver removeall
+testsuccess grep "which isn't installed!" rootdir/tmp/testwarning.output
+
+cat > rootdir/usr/lib/apt/solvers/installall << EOF
+#!/bin/sh
+set -e
+while read line; do
+	if [ "APT-ID" = "\${line%:*}" ]; then
+		cat << APT
+Install: \${line#*:}
+
+APT
+	fi
+done
+EOF
+chmod +x rootdir/usr/lib/apt/solvers/installall
+testfailure apt full-upgrade -s --solver installall
+testsuccess grep "is already installed!" rootdir/tmp/testfailure.output