prevent MarkInstall of unsynced Multi-Arch:same siblings

Multi-Arch: same packages can be co-installed, but need to have the same
version for all installed packages (aka "siblings"). Otherwise the
unsynced versions will fight against each other and the auto-install as
wel as the problem resolver will later have to decide between holding the
packages or to remove one of the siblings (usually a foreign) taking a
bunch of packages (like the entire foreign setup) with them.

The idea here is now to be more pro-active: MarkInstall will fail for
a package if the siblings aren't synced, so we don't allow a situation
in which a resolver has to decide if to hold or to remove-upgrade under
the assumption that the remove-upgrade decision is always wrong and
doesn't deserve to be explored (expect valid out-of-syncs of course).

Thats a pretty bold move to take for a library which is used by
different solvers so this check is done in IsInstallOk and can be
overridden if front-ends want to.

3 files changed, 166 insertions, 17 deletions

diff --git a/apt-pkg/depcache.cc b/apt-pkg/depcache.cc
index 9f8422fec..2c6eb43bf 100644
--- a/apt-pkg/depcache.cc
+++ b/apt-pkg/depcache.cc
@@ -865,6 +865,11 @@ bool pkgDepCache::MarkDelete(PkgIterator const &Pkg, bool rPurge,
 bool pkgDepCache::IsDeleteOk(PkgIterator const &Pkg,bool rPurge,
 			      unsigned long Depth, bool FromUser)
 {
+   return IsDeleteOkProtectInstallRequests(Pkg, rPurge, Depth, FromUser);
+}
+bool pkgDepCache::IsDeleteOkProtectInstallRequests(PkgIterator const &Pkg,
+      bool const rPurge, unsigned long const Depth, bool const FromUser)
+{
    if (FromUser == false && Pkg->CurrentVer == 0)
    {
       StateCache &P = PkgState[Pkg->ID];
@@ -1047,9 +1052,10 @@ bool pkgDepCache::MarkInstall(PkgIterator const &Pkg,bool AutoInst,
       return true;
    }
 
-   // check if we are allowed to install the package
-   if (IsInstallOk(Pkg,AutoInst,Depth,FromUser) == false)
-      return false;
+   // check if we are allowed to install the package (if we haven't already)
+   if (P.Mode != ModeInstall || P.InstallVer != P.CandidateVer)
+      if (IsInstallOk(Pkg,AutoInst,Depth,FromUser) == false)
+	 return false;
 
    ActionGroup group(*this);
    P.iFlags &= ~AutoKept;
@@ -1271,11 +1277,50 @@ bool pkgDepCache::MarkInstall(PkgIterator const &Pkg,bool AutoInst,
 									/*}}}*/
 // DepCache::IsInstallOk - check if it is ok to install this package	/*{{{*/
 // ---------------------------------------------------------------------
-/* The default implementation does nothing.
+/* The default implementation checks if the installation of an M-A:same
+   package would lead us into a version-screw and if so forbids it.
    dpkg holds are enforced by the private IsModeChangeOk */
 bool pkgDepCache::IsInstallOk(PkgIterator const &Pkg,bool AutoInst,
 			      unsigned long Depth, bool FromUser)
 {
+   return IsInstallOkMultiArchSameVersionSynced(Pkg,AutoInst, Depth, FromUser);
+}
+bool pkgDepCache::IsInstallOkMultiArchSameVersionSynced(PkgIterator const &Pkg,
+      bool const AutoInst, unsigned long const Depth, bool const FromUser)
+{
+   if (FromUser == true) // as always: user is always right
+      return true;
+
+   // ignore packages with none-M-A:same candidates
+   VerIterator const CandVer = PkgState[Pkg->ID].CandidateVerIter(*this);
+   if (unlikely(CandVer.end() == true) || CandVer == Pkg.CurrentVer() ||
+	 (CandVer->MultiArch & pkgCache::Version::Same) != pkgCache::Version::Same)
+      return true;
+
+   GrpIterator const Grp = Pkg.Group();
+   for (PkgIterator P = Grp.PackageList(); P.end() == false; P = Grp.NextPkg(P))
+   {
+      // not installed or version synced: fine by definition
+      // (simple string-compare as stuff like '1' == '0:1-0' can't happen here)
+      if (P->CurrentVer == 0 || strcmp(Pkg.CandVersion(), P.CandVersion()) == 0)
+	 continue;
+      // packages loosing M-A:same can be out-of-sync
+      VerIterator CV = PkgState[P->ID].CandidateVerIter(*this);
+      if (unlikely(CV.end() == true) ||
+	    (CV->MultiArch & pkgCache::Version::Same) != pkgCache::Version::Same)
+	 continue;
+
+      // not downloadable means the package is obsolete, so allow out-of-sync
+      if (CV.Downloadable() == false)
+	 continue;
+
+      PkgState[Pkg->ID].iFlags |= AutoKept;
+      if (unlikely(DebugMarker == true))
+	 std::clog << OutputInDepth(Depth) << "Ignore MarkInstall of " << Pkg
+	    << " as its M-A:same siblings are not version-synced" << std::endl;
+      return false;
+   }
+
    return true;
 }
 									/*}}}*/
diff --git a/apt-pkg/depcache.h b/apt-pkg/depcache.h
index 7358048ed..d9c95349b 100644
--- a/apt-pkg/depcache.h
+++ b/apt-pkg/depcache.h
@@ -442,16 +442,15 @@ class pkgDepCache : protected pkgCache::Namespace
    /** \return \b true if it's OK for MarkInstall to install
     *  the given package.
     *
-    *  See the default implementation for a simple example how this
-    *  method can be used.
-    *  Overriding implementations should use the hold-state-flag to cache
-    *  results from previous checks of this package - also it should
-    *  be used if the default resolver implementation is also used to
-    *  ensure that these packages are handled like "normal" dpkg holds.
+    *  The default implementation simply calls all IsInstallOk*
+    *  method mentioned below.
+    *
+    *  Overriding implementations should use the hold-state-flag to
+    *  cache results from previous checks of this package - if possible.
     *
     *  The parameters are the same as in the calling MarkInstall:
     *  \param Pkg       the package that MarkInstall wants to install.
-    *  \param AutoInst  needs a previous MarkInstall this package?
+    *  \param AutoInst  install this and all its dependencies
     *  \param Depth     recursive deep of this Marker call
     *  \param FromUser  was the install requested by the user?
     */
@@ -461,12 +460,8 @@ class pkgDepCache : protected pkgCache::Namespace
    /** \return \b true if it's OK for MarkDelete to remove
     *  the given package.
     *
-    *  See the default implementation for a simple example how this
-    *  method can be used.
-    *  Overriding implementations should use the hold-state-flag to cache
-    *  results from previous checks of this package - also it should
-    *  be used if the default resolver implementation is also used to
-    *  ensure that these packages are handled like "normal" dpkg holds.
+    *  The default implementation simply calls all IsDeleteOk*
+    *  method mentioned below, see also #IsInstallOk.
     *
     *  The parameters are the same as in the calling MarkDelete:
     *  \param Pkg       the package that MarkDelete wants to remove.
@@ -498,6 +493,15 @@ class pkgDepCache : protected pkgCache::Namespace
    pkgDepCache(pkgCache *Cache,Policy *Plcy = 0);
    virtual ~pkgDepCache();
 
+   protected:
+   // methods call by IsInstallOk
+   bool IsInstallOkMultiArchSameVersionSynced(PkgIterator const &Pkg,
+	 bool const AutoInst, unsigned long const Depth, bool const FromUser);
+
+   // methods call by IsDeleteOk
+   bool IsDeleteOkProtectInstallRequests(PkgIterator const &Pkg,
+	 bool const rPurge, unsigned long const Depth, bool const FromUser);
+
    private:
    bool IsModeChangeOk(ModeList const mode, PkgIterator const &Pkg,
 			unsigned long const Depth, bool const FromUser);
diff --git a/test/integration/test-prevent-markinstall-multiarch-same-versionscrew b/test/integration/test-prevent-markinstall-multiarch-same-versionscrew
new file mode 100755
index 000000000..fed12dad0
--- /dev/null
+++ b/test/integration/test-prevent-markinstall-multiarch-same-versionscrew
@@ -0,0 +1,100 @@
+#!/bin/sh
+set -e
+
+TESTDIR=$(readlink -f $(dirname $0))
+. $TESTDIR/framework
+setupenvironment
+configarchitecture 'amd64' 'i386' 'armel'
+
+insertpackage 'stable' 'allarchs' 'all' '1'
+insertpackage 'unstable' 'allarchs' 'all' '2'
+
+insertinstalledpackage 'fine' 'i386,amd64' '1' 'Multi-Arch: same'
+insertpackage 'stable' 'fine' 'i386,amd64' '1' 'Multi-Arch: same'
+insertpackage 'unstable' 'fine' 'amd64,i386' '2' 'Multi-Arch: same'
+
+insertinstalledpackage 'fine-installed' 'i386,amd64' '1' 'Multi-Arch: same'
+insertpackage 'stable' 'fine-installed' 'i386,amd64,armel' '1' 'Multi-Arch: same'
+insertpackage 'unstable' 'fine-installed' 'i386,amd64' '2' 'Multi-Arch: same'
+
+insertinstalledpackage 'out-of-sync-native' 'i386,amd64' '1' 'Multi-Arch: same'
+insertpackage 'stable' 'out-of-sync-native' 'i386,amd64' '1' 'Multi-Arch: same'
+insertpackage 'unstable' 'out-of-sync-native' 'amd64' '2' 'Multi-Arch: same'
+
+insertinstalledpackage 'out-of-sync-foreign' 'i386,amd64' '1' 'Multi-Arch: same'
+insertpackage 'stable' 'out-of-sync-foreign' 'i386,amd64' '1' 'Multi-Arch: same'
+insertpackage 'unstable' 'out-of-sync-foreign' 'i386' '2' 'Multi-Arch: same'
+
+insertinstalledpackage 'out-of-sync-gone-native' 'i386,amd64' '1' 'Multi-Arch: same'
+insertpackage 'stable' 'out-of-sync-gone-native' 'i386' '1' 'Multi-Arch: same'
+insertpackage 'unstable' 'out-of-sync-gone-native' 'i386' '2' 'Multi-Arch: same'
+
+insertinstalledpackage 'out-of-sync-gone-foreign' 'i386,amd64' '1' 'Multi-Arch: same'
+insertpackage 'stable' 'out-of-sync-gone-foreign' 'amd64' '1' 'Multi-Arch: same'
+insertpackage 'unstable' 'out-of-sync-gone-foreign' 'amd64' '2' 'Multi-Arch: same'
+
+insertpackage 'stable' 'libsame2' 'i386' '1' 'Multi-Arch: same'
+insertpackage 'unstable' 'libsame2' 'amd64' '2' 'Multi-Arch: same'
+insertpackage 'unstable' 'depender2' 'all' '2' 'Depends: libsame2 (= 2)'
+insertpackage 'stable' 'libsame3' 'i386' '1' 'Multi-Arch: same'
+insertpackage 'unstable' 'libsame3' 'i386,amd64' '3' 'Multi-Arch: same'
+insertpackage 'unstable' 'depender3' 'all' '3' 'Depends: libsame3 (= 3)'
+setupaptarchive
+
+testequal 'Reading package lists...
+Building dependency tree...
+The following packages will be REMOVED:
+  out-of-sync-gone-foreign:i386 out-of-sync-gone-native
+The following packages have been kept back:
+  out-of-sync-foreign:i386 out-of-sync-native
+The following packages will be upgraded:
+  fine fine:i386 fine-installed fine-installed:i386 out-of-sync-gone-foreign
+  out-of-sync-gone-native:i386
+6 upgraded, 0 newly installed, 2 to remove and 2 not upgraded.
+Remv out-of-sync-gone-foreign:i386 [1]
+Remv out-of-sync-gone-native [1]
+Inst fine [1] (2 unstable [amd64]) [fine:amd64 on fine:i386] [fine:i386 on fine:amd64] [fine:i386 ]
+Inst fine:i386 [1] (2 unstable [i386])
+Conf fine (2 unstable [amd64])
+Conf fine:i386 (2 unstable [i386])
+Inst fine-installed [1] (2 unstable [amd64]) [fine-installed:amd64 on fine-installed:i386] [fine-installed:i386 on fine-installed:amd64] [fine-installed:i386 ]
+Inst fine-installed:i386 [1] (2 unstable [i386])
+Conf fine-installed (2 unstable [amd64])
+Conf fine-installed:i386 (2 unstable [i386])
+Inst out-of-sync-gone-foreign [1] (2 unstable [amd64])
+Inst out-of-sync-gone-native:i386 [1] (2 unstable [i386])
+Conf out-of-sync-gone-foreign (2 unstable [amd64])
+Conf out-of-sync-gone-native:i386 (2 unstable [i386])' aptget dist-upgrade -s #-o Debug::pkgDepCache::Marker=1
+
+rm rootdir/var/lib/dpkg/status
+insertinstalledpackage 'libsame2' 'i386' '1' 'Multi-Arch: same'
+insertinstalledpackage 'libsame3' 'i386' '1' 'Multi-Arch: same'
+
+# the error message isn't great, but better than nothing, right?
+testequal '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:
+ depender2 : Depends: libsame2 (= 2) but it is not going to be installed
+E: Unable to correct problems, you have held broken packages.' aptget install depender2 -s
+
+testequal 'Reading package lists...
+Building dependency tree...
+The following extra packages will be installed:
+  libsame3:i386 libsame3
+The following NEW packages will be installed:
+  depender3 libsame3
+The following packages will be upgraded:
+  libsame3:i386
+1 upgraded, 2 newly installed, 0 to remove and 0 not upgraded.
+Inst libsame3:i386 [1] (3 unstable [i386])
+Inst libsame3 (3 unstable [amd64])
+Inst depender3 (3 unstable [all])
+Conf libsame3:i386 (3 unstable [i386])
+Conf libsame3 (3 unstable [amd64])
+Conf depender3 (3 unstable [all])' aptget install depender3 -s