Reset candidate version explicitly for internal state-keeping

For a (partially) installed package like the one MarkInstall operates on
at the moment we want to discard the candidate from, we have to first
remove the package from the internal state keeping to have proper broken
counts and such and only then reset the candidate version which is a
trivial operation in comparison.

Take a look at the testcase: Now, what is the problem? Correct,
git:i386. Didn't see that coming, right? It is M-A:foreign so apt tries
to switch the architecture of git here (which is pointless, it knows
that this won't work, but lets fix that in another commit) will
eventually realize that it can't install it and wants to discard the
candidate of git:i386 first removing the broken indication like it
should, removing the install flag and then reapplies the broken
indication: Expect it doesn't as it wants to do that over the candidate
version which the package no longer had so seemingly nothing is broken.

It is a bit of a hairball to figure out which commit it is exactly that
is wrong here as they are all influencing each other a bit, but >= 2.1
is an acceptable ballpark. Bisect says 57df273 but that is mostly a lie.

Closes: #961266

6 files changed, 152 insertions, 17 deletions

diff --git a/apt-pkg/depcache.cc b/apt-pkg/depcache.cc
index 842630ec6..02a80b2e8 100644
--- a/apt-pkg/depcache.cc
+++ b/apt-pkg/depcache.cc
@@ -1111,7 +1111,15 @@ bool pkgDepCache::MarkInstall_StateChange(pkgCache::PkgIterator const &Pkg, bool
    return true;
 }
 									/*}}}*/
-bool pkgDepCache::MarkInstall_Discard(pkgCache::PkgIterator const &Pkg)	/*{{{*/
+static bool MarkInstall_DiscardCandidate(pkgDepCache &Cache, pkgCache::PkgIterator const &Pkg) /*{{{*/
+{
+   auto &State = Cache[Pkg];
+   State.CandidateVer = State.InstallVer;
+   State.Update(Pkg, Cache);
+   return true;
+}
+									/*}}}*/
+bool pkgDepCache::MarkInstall_DiscardInstall(pkgCache::PkgIterator const &Pkg) /*{{{*/
 {
    StateCache &State = PkgState[Pkg->ID];
    if (State.Mode == ModeKeep && State.InstallVer == State.CandidateVer && State.CandidateVer == Pkg.CurrentVer())
@@ -1119,15 +1127,14 @@ bool pkgDepCache::MarkInstall_Discard(pkgCache::PkgIterator const &Pkg)	/*{{{*/
    RemoveSizes(Pkg);
    RemoveStates(Pkg);
    if (Pkg->CurrentVer != 0)
-      State.InstallVer = State.CandidateVer = Pkg.CurrentVer();
+      State.InstallVer = Pkg.CurrentVer();
    else
-      State.InstallVer = State.CandidateVer = nullptr;
+      State.InstallVer = nullptr;
    State.Mode = ModeKeep;
-   State.Update(Pkg, *this);
    AddStates(Pkg);
    Update(Pkg);
    AddSizes(Pkg);
-   return true;
+   return MarkInstall_DiscardCandidate(*this, Pkg);
 }
 									/*}}}*/
 static bool MarkInstall_CollectDependencies(pkgDepCache const &Cache, pkgCache::VerIterator const &PV, std::vector<pkgCache::DepIterator> &toInstall, std::vector<pkgCache::DepIterator> &toRemove) /*{{{*/
@@ -1202,7 +1209,7 @@ static bool MarkInstall_MarkDeleteForNotUpgradeable(pkgDepCache &Cache, bool con
    }
    if (propagateProctected)
    {
-      State.CandidateVer = nullptr;
+      MarkInstall_DiscardCandidate(Cache, Pkg);
       Cache.MarkProtected(Pkg);
    }
    return true;
@@ -1230,10 +1237,7 @@ static bool MarkInstall_RemoveConflictsIfNotUpgradeable(pkgDepCache &Cache, bool
 	    }
 	    else if (propagateProctected)
 	    {
-	       if (Pkg->CurrentVer != 0)
-		  State.CandidateVer = Pkg.CurrentVer();
-	       else
-		  State.CandidateVer = nullptr;
+	       MarkInstall_DiscardCandidate(Cache, Pkg);
 	       if (Pkg->CurrentVer == 0)
 		  Cache.MarkProtected(Pkg);
 	    }
@@ -1491,7 +1495,7 @@ bool pkgDepCache::MarkInstall(PkgIterator const &Pkg, bool AutoInst,
    {
       if (failEarly)
       {
-	 MarkInstall_Discard(Pkg);
+	 MarkInstall_DiscardInstall(Pkg);
 	 return false;
       }
       hasFailed = true;
@@ -1519,7 +1523,7 @@ bool pkgDepCache::MarkInstall(PkgIterator const &Pkg, bool AutoInst,
    {
       if (failEarly)
       {
-	 MarkInstall_Discard(Pkg);
+	 MarkInstall_DiscardInstall(Pkg);
 	 return false;
       }
       hasFailed = true;
diff --git a/apt-pkg/depcache.h b/apt-pkg/depcache.h
index fa4da36a6..ff1b37374 100644
--- a/apt-pkg/depcache.h
+++ b/apt-pkg/depcache.h
@@ -520,7 +520,7 @@ class APT_PUBLIC pkgDepCache : protected pkgCache::Namespace
 			unsigned long const Depth, bool const FromUser);
 
    APT_HIDDEN bool MarkInstall_StateChange(PkgIterator const &Pkg, bool AutoInst, bool FromUser);
-   APT_HIDDEN bool MarkInstall_Discard(PkgIterator const &Pkg);
+   APT_HIDDEN bool MarkInstall_DiscardInstall(PkgIterator const &Pkg);
 };
 
 #endif
diff --git a/test/integration/test-bug-632221-cross-dependency-satisfaction b/test/integration/test-bug-632221-cross-dependency-satisfaction
index d52652cad..752e2ad63 100755
--- a/test/integration/test-bug-632221-cross-dependency-satisfaction
+++ b/test/integration/test-bug-632221-cross-dependency-satisfaction
@@ -296,7 +296,7 @@ The following packages will be REMOVED:
 The following NEW packages will be installed:
   amdboot:amd64 arm-stuff cool doxygen libc6 libc6-dev libfwibble-dev
   libfwibble1
-0 upgraded, 8 newly installed, 1 to remove and 1 not upgraded.
+0 upgraded, 8 newly installed, 1 to remove and 2 not upgraded.
 Remv cool:amd64 [0.5]
 Inst amdboot:amd64 (1.0 unstable [amd64])
 Inst arm-stuff (1.0 unstable [armel])
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 943e968a4..689699e15 100755
--- a/test/integration/test-bug-960705-propagate-protected-to-satisfied-conflict
+++ b/test/integration/test-bug-960705-propagate-protected-to-satisfied-conflict
@@ -25,7 +25,7 @@ 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 >
+Broken init:amd64 PreDepends on sysvinit-core:amd64 < none @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
diff --git a/test/integration/test-bug-961266-hold-means-hold b/test/integration/test-bug-961266-hold-means-hold
new file mode 100755
index 000000000..a104bfc31
--- /dev/null
+++ b/test/integration/test-bug-961266-hold-means-hold
@@ -0,0 +1,130 @@
+#!/bin/sh
+set -e
+
+TESTDIR="$(readlink -f "$(dirname "$0")")"
+. "$TESTDIR/framework"
+setupenvironment
+configarchitecture 'amd64' 'i386'
+
+insertinstalledpackage 'git' 'amd64' '1:2.25.1-1' 'Multi-Arch: foreign'
+insertinstalledpackage 'git-cvs' 'amd64' '1:2.25.1-1' 'Depends: git (>> 1:2.25.1), git (<< 1:2.25.1-.)'
+
+
+insertpackage 'unstable' 'git' 'amd64,i386' '1:2.26.2-1' 'Multi-Arch: foreign'
+insertpackage 'unstable' 'git-cvs' 'amd64,i386' '1:2.26.2-1' 'Depends: git (>> 1:2.26.2), git (<< 1:2.26.2-.)'
+insertpackage 'unstable' 'git-ng' 'amd64,i386' '1:2.26.2-1' 'Depends: git (>> 1:2.26.2), git (<< 1:2.26.2-.)'
+
+setupaptarchive
+
+msgmsg 'The setup is' 'fine'
+UPGRADE='Reading package lists...
+Building dependency tree...
+Calculating upgrade...
+The following packages will be upgraded:
+  git git-cvs
+2 upgraded, 0 newly installed, 0 to remove and 0 not upgraded.
+Inst git-cvs [1:2.25.1-1] (1:2.26.2-1 unstable [amd64]) []
+Inst git [1:2.25.1-1] (1:2.26.2-1 unstable [amd64])
+Conf git-cvs (1:2.26.2-1 unstable [amd64])
+Conf git (1:2.26.2-1 unstable [amd64])'
+testsuccessequal "$UPGRADE" apt upgrade -s
+testsuccessequal "$UPGRADE" aptget upgrade -s
+testsuccessequal "$UPGRADE" apt full-upgrade -s
+testsuccessequal "$UPGRADE" aptget dist-upgrade -s
+
+testsuccessequal 'Reading package lists...
+Building dependency tree...
+The following additional packages will be installed:
+  git git-cvs
+The following NEW packages will be installed:
+  git-ng
+The following packages will be upgraded:
+  git git-cvs
+2 upgraded, 1 newly installed, 0 to remove and 0 not upgraded.
+Inst git-cvs [1:2.25.1-1] (1:2.26.2-1 unstable [amd64]) []
+Inst git [1:2.25.1-1] (1:2.26.2-1 unstable [amd64])
+Inst git-ng (1:2.26.2-1 unstable [amd64])
+Conf git-cvs (1:2.26.2-1 unstable [amd64])
+Conf git (1:2.26.2-1 unstable [amd64])
+Conf git-ng (1:2.26.2-1 unstable [amd64])' apt install git-ng -s
+
+
+msgmsg 'Now mix it up by' 'holding git'
+testsuccessequal 'git set on hold.' aptmark hold git
+testsuccessequal 'git' aptmark showholds
+
+NOUPGRADE='Reading package lists...
+Building dependency tree...
+Calculating upgrade...
+The following packages have been kept back:
+  git
+0 upgraded, 0 newly installed, 0 to remove and 2 not upgraded.'
+NOUPGRADE2='Reading package lists...
+Building dependency tree...
+Calculating upgrade...
+The following packages have been kept back:
+  git git-cvs
+0 upgraded, 0 newly installed, 0 to remove and 2 not upgraded.'
+
+testsuccessequal "$NOUPGRADE" apt upgrade -s
+testsuccessequal "$NOUPGRADE" apt full-upgrade -s
+testsuccessequal "$NOUPGRADE" aptget dist-upgrade -s
+testsuccessequal "$NOUPGRADE2" aptget upgrade -s
+
+testfailureequal 'Reading package lists...
+Building dependency tree...
+Some packages could not be installed. This may mean that you have
+requested an impossible situation or if you are using the unstable
+distribution that some required packages have not yet been created
+or been moved out of Incoming.
+The following information may help to resolve the situation:
+
+The following packages have unmet dependencies:
+ git-ng : Depends: git (> 1:2.26.2)
+E: Unable to correct problems, you have held broken packages.' apt install git-ng -s
+
+
+msgmsg 'Now mix it up by' 'holding git-cvs'
+testsuccessequal 'Canceled hold on git.' aptmark unhold git
+testsuccessequal 'git-cvs set on hold.' aptmark hold git-cvs
+testsuccessequal 'git-cvs' aptmark showholds
+
+testsuccessequal "$NOUPGRADE2" apt upgrade -s
+testsuccessequal "$NOUPGRADE2" apt full-upgrade -s
+testsuccessequal "$NOUPGRADE2" aptget upgrade -s
+testsuccessequal "$NOUPGRADE2" aptget dist-upgrade -s
+
+testfailureequal 'Reading package lists...
+Building dependency tree...
+Some packages could not be installed. This may mean that you have
+requested an impossible situation or if you are using the unstable
+distribution that some required packages have not yet been created
+or been moved out of Incoming.
+The following information may help to resolve the situation:
+
+The following packages have unmet dependencies:
+ git-cvs : Depends: git (< 1:2.25.1-.)
+E: Error, pkgProblemResolver::Resolve generated breaks, this may be caused by held packages.' apt install git-ng -s
+
+
+msgmsg 'Now mix it up by' 'holding both'
+testsuccessequal 'git set on hold.' aptmark hold git
+testsuccessequal 'git
+git-cvs' aptmark showholds
+
+testsuccessequal "$NOUPGRADE2" apt upgrade -s
+testsuccessequal "$NOUPGRADE2" apt full-upgrade -s
+testsuccessequal "$NOUPGRADE2" aptget upgrade -s
+testsuccessequal "$NOUPGRADE2" aptget dist-upgrade -s
+
+testfailureequal 'Reading package lists...
+Building dependency tree...
+Some packages could not be installed. This may mean that you have
+requested an impossible situation or if you are using the unstable
+distribution that some required packages have not yet been created
+or been moved out of Incoming.
+The following information may help to resolve the situation:
+
+The following packages have unmet dependencies:
+ git-ng : Depends: git (> 1:2.26.2)
+E: Unable to correct problems, you have held broken packages.' apt install git-ng -s
diff --git a/test/integration/test-explore-or-groups-in-markinstall b/test/integration/test-explore-or-groups-in-markinstall
index e24145fd8..ba3376cf6 100755
--- a/test/integration/test-explore-or-groups-in-markinstall
+++ b/test/integration/test-explore-or-groups-in-markinstall
@@ -70,6 +70,7 @@ The following additional packages will be installed:
 The following NEW packages will be installed:
   foo-${1}-level${level} okay
 0 upgraded, 2 newly installed, 0 to remove and 3 not upgraded." apt install foo-${1}-level${level} -s
+		if [ "$level" = '0' ]; then NOT=2; else NOT=1; fi
 		testsuccessheadequal 9 "Reading package lists...
 Building dependency tree...
 The following additional packages will be installed:
@@ -78,7 +79,7 @@ The following NEW packages will be installed:
   foo-${1}-upgrade-level${level}
 The following packages will be upgraded:
   upgrade
-1 upgraded, 1 newly installed, 0 to remove and $((2-${level})) not upgraded." apt install foo-${1}-upgrade-level${level} -s
+1 upgraded, 1 newly installed, 0 to remove and $NOT not upgraded." apt install foo-${1}-upgrade-level${level} -s
 	done
 
 	testsuccessheadequal 7 "Reading package lists...
@@ -137,7 +138,7 @@ 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.
+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-upgrade-level1 -sy
 	# aptitude does not show the kept back message in this case…