do unpacks before configures in SmartConfigure

Splits the big loop over dependencies in SmartConfigure which unpacks and
configures dependencies into two loops and reverse their order, so that all
dependencies which need to be unpacked are handled first and only after that
configures are issued for dependencies.

This is needed as otherwise the unpack of a (new) dependency will be issued
in between a configure call for two (or more) packages which form a loop,
which means the configure calls aren't part of the same dpkg call and
therefore dpkg bails out.

Such tight loops should really be avoided as they are usually wrong – and in
reality the dependencies in libreoffice were greatly simplified thanks to
Rene Engelhard so the problem is gone for the benefit of all.

Closes: 707578

3 files changed, 115 insertions, 45 deletions

diff --git a/apt-pkg/packagemanager.cc b/apt-pkg/packagemanager.cc
index e2d7dbf2a..b8932753d 100644
--- a/apt-pkg/packagemanager.cc
+++ b/apt-pkg/packagemanager.cc
@@ -340,6 +340,7 @@ bool pkgPackageManager::SmartConfigure(PkgIterator Pkg, int const Depth)
    bool Bad = false, Changed = false;
    const unsigned int max_loops = _config->FindI("APT::pkgPackageManager::MaxLoopCount", 500);
    unsigned int i=0;
+   std::list<DepIterator> needConfigure;
    do
    {
       Changed = false;
@@ -353,7 +354,7 @@ bool pkgPackageManager::SmartConfigure(PkgIterator Pkg, int const Depth)
 	    continue;
 	 Bad = true;
 
-	 // Search for dependencies which are unpacked but aren't configured yet (maybe loops)
+	 // Check for dependencies that have not been unpacked, probably due to loops.
 	 for (DepIterator Cur = Start; true; ++Cur)
 	 {
 	    SPtrArray<Version *> VList = Cur.AllTargets();
@@ -373,51 +374,63 @@ bool pkgPackageManager::SmartConfigure(PkgIterator Pkg, int const Depth)
 	       }
 
 	       // Check if the version that is going to be installed will satisfy the dependency
-	       if (Cache[DepPkg].InstallVer != *I)
+	       if (Cache[DepPkg].InstallVer != *I || List->IsNow(DepPkg) == false)
 		  continue;
 
-	       if (List->IsFlag(DepPkg,pkgOrderList::UnPacked))
+	       if (PkgLoop == true)
 	       {
-		  if (List->IsFlag(DepPkg,pkgOrderList::Loop) && PkgLoop)
-		  {
-		    // This dependency has already been dealt with by another SmartConfigure on Pkg
-		    Bad = false;
-		    break;
-		  }
-		  /* Check for a loop to prevent one forming
-		       If A depends on B and B depends on A, SmartConfigure will
-		       just hop between them if this is not checked. Dont remove the
-		       loop flag after finishing however as loop is already set.
-		       This means that there is another SmartConfigure call for this
-		       package and it will remove the loop flag */
+		  if (Debug)
+		     std::clog << OutputInDepth(Depth) << "Package " << Pkg << " loops in SmartConfigure" << std::endl;
+	          Bad = false;
+		  break;
+	       }
+	       else
+	       {
+		  if (Debug)
+		     clog << OutputInDepth(Depth) << "Unpacking " << DepPkg.FullName() << " to avoid loop " << Cur << endl;
 		  if (PkgLoop == false)
 		     List->Flag(Pkg,pkgOrderList::Loop);
-		  if (SmartConfigure(DepPkg, Depth + 1) == true)
+		  if (SmartUnPack(DepPkg, true, Depth + 1) == true)
 		  {
 		     Bad = false;
 		     if (List->IsFlag(DepPkg,pkgOrderList::Loop) == false)
-			Changed = true;
+		        Changed = true;
 		  }
 		  if (PkgLoop == false)
-		    List->RmFlag(Pkg,pkgOrderList::Loop);
-		  // If SmartConfigure was succesfull, Bad is false, so break
+		     List->RmFlag(Pkg,pkgOrderList::Loop);
 		  if (Bad == false)
 		     break;
 	       }
-	       else if (List->IsFlag(DepPkg,pkgOrderList::Configured))
-	       {
-		  Bad = false;
-		  break;
-	       }
 	    }
-	    if (Cur == End)
+
+	    if (Cur == End || Bad == false)
 	       break;
-         }
+	 }
 
 	 if (Bad == false)
 	    continue;
 
-	 // Check for dependencies that have not been unpacked, probably due to loops.
+	 needConfigure.push_back(Start);
+      }
+      if (i++ > max_loops)
+         return _error->Error("Internal error: MaxLoopCount reached in SmartUnPack (1) for %s, aborting", Pkg.FullName().c_str());
+   } while (Changed == true);
+
+   Bad = false, Changed = false, i = 0;
+   do
+   {
+      Changed = false;
+      for (std::list<DepIterator>::iterator D = needConfigure.begin(); D != needConfigure.end(); ++D)
+      {
+	 // Compute a single dependency element (glob or)
+	 pkgCache::DepIterator Start, End;
+	 D->GlobOr(Start,End);
+
+	 if (End->Type != pkgCache::Dep::Depends)
+	    continue;
+	 Bad = true;
+
+	 // Search for dependencies which are unpacked but aren't configured yet (maybe loops)
 	 for (DepIterator Cur = Start; true; ++Cur)
 	 {
 	    SPtrArray<Version *> VList = Cur.AllTargets();
@@ -428,44 +441,54 @@ bool pkgPackageManager::SmartConfigure(PkgIterator Pkg, int const Depth)
 	       PkgIterator DepPkg = Ver.ParentPkg();
 
 	       // Check if the version that is going to be installed will satisfy the dependency
-	       if (Cache[DepPkg].InstallVer != *I || List->IsNow(DepPkg) == false)
+	       if (Cache[DepPkg].InstallVer != *I)
 		  continue;
 
-	       if (PkgLoop == true)
-	       {
-		  if (Debug)
-		     std::clog << OutputInDepth(Depth) << "Package " << Pkg << " loops in SmartConfigure" << std::endl;
-	          Bad = false;
-		  break;
-	       }
-	       else
+	       if (List->IsFlag(DepPkg,pkgOrderList::UnPacked))
 	       {
-		  if (Debug)
-		     clog << OutputInDepth(Depth) << "Unpacking " << DepPkg.FullName() << " to avoid loop " << Cur << endl;
+		  if (List->IsFlag(DepPkg,pkgOrderList::Loop) && PkgLoop)
+		  {
+		    // This dependency has already been dealt with by another SmartConfigure on Pkg
+		    Bad = false;
+		    break;
+		  }
+		  /* Check for a loop to prevent one forming
+		       If A depends on B and B depends on A, SmartConfigure will
+		       just hop between them if this is not checked. Dont remove the
+		       loop flag after finishing however as loop is already set.
+		       This means that there is another SmartConfigure call for this
+		       package and it will remove the loop flag */
 		  if (PkgLoop == false)
 		     List->Flag(Pkg,pkgOrderList::Loop);
-		  if (SmartUnPack(DepPkg, true, Depth + 1) == true)
+		  if (SmartConfigure(DepPkg, Depth + 1) == true)
 		  {
 		     Bad = false;
 		     if (List->IsFlag(DepPkg,pkgOrderList::Loop) == false)
-		        Changed = true;
+			Changed = true;
 		  }
 		  if (PkgLoop == false)
-		     List->RmFlag(Pkg,pkgOrderList::Loop);
+		    List->RmFlag(Pkg,pkgOrderList::Loop);
+		  // If SmartConfigure was succesfull, Bad is false, so break
 		  if (Bad == false)
 		     break;
 	       }
+	       else if (List->IsFlag(DepPkg,pkgOrderList::Configured))
+	       {
+		  Bad = false;
+		  break;
+	       }
 	    }
-
-	    if (Cur == End)
+	    if (Cur == End || Bad == false)
 	       break;
-	 }
+         }
+
+
 
 	 if (Bad == true && Changed == false && Debug == true)
 	    std::clog << OutputInDepth(Depth) << "Could not satisfy " << Start << std::endl;
       }
       if (i++ > max_loops)
-         return _error->Error("Internal error: MaxLoopCount reached in SmartUnPack for %s, aborting", Pkg.FullName().c_str());
+         return _error->Error("Internal error: MaxLoopCount reached in SmartUnPack (2) for %s, aborting", Pkg.FullName().c_str());
    } while (Changed == true);
    
    if (Bad) {
diff --git a/debian/changelog b/debian/changelog
index b64a57bef..8a1194b1b 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -8,6 +8,7 @@ apt (0.9.8.3) UNRELEASED; urgency=low
   * prefer Essentials over Removals in ordering score
   * fix priority sorting by prefering higher in MarkInstall
   * try all providers in order if uninstallable in MarkInstall
+  * do unpacks before configures in SmartConfigure (Closes: #707578)
 
  -- David Kalnischkies <kalnischkies@gmail.com>  Sun, 09 Jun 2013 15:06:24 +0200
 
diff --git a/test/integration/test-very-tight-loop-configure-with-unpacking-new-packages b/test/integration/test-very-tight-loop-configure-with-unpacking-new-packages
new file mode 100755
index 000000000..7f3b05e59
--- /dev/null
+++ b/test/integration/test-very-tight-loop-configure-with-unpacking-new-packages
@@ -0,0 +1,46 @@
+#!/bin/sh
+set -e
+
+TESTDIR=$(readlink -f $(dirname $0))
+. $TESTDIR/framework
+setupenvironment
+configarchitecture 'amd64'
+
+# the difference between version 3 and 4 is the new package 'ure' which
+# we have to unpack before we start configuring parts of the loop
+insertinstalledpackage 'libreoffice' 'amd64' '3' 'Depends: libreoffice-core (= 3)'
+insertinstalledpackage 'libreoffice-core' 'amd64' '3' 'Depends: libreoffice-common (>= 3)'
+insertinstalledpackage 'libreoffice-common' 'all' '3' 'Depends: libreoffice-style
+Breaks: libreoffice-core (>= 3+), libreoffice-core (<= 3~), libreoffice-style-galaxy (>= 3+), libreoffice-style-galaxy (<= 3~)'
+insertinstalledpackage 'libreoffice-style-galaxy' 'amd64' '3' 'Depends: libreoffice-core
+Provides: libreoffice-style'
+
+buildsimplenativepackage 'libreoffice' 'amd64' '4' 'sid' 'Depends: libreoffice-core (= 4)'
+buildsimplenativepackage 'libreoffice-core' 'amd64' '4' 'sid' 'Depends: libreoffice-common (>= 4)
+Breaks: libreoffice-common (<< 4), libreoffice-style-galaxy (<< 4)'
+buildsimplenativepackage 'libreoffice-common' 'all' '4' 'sid' 'Depends: libreoffice-style, ure
+Breaks: libreoffice-core (>= 4+), libreoffice-core (<= 4~), libreoffice-style-galaxy (>= 4+), libreoffice-style-galaxy (<= 4~)'
+buildsimplenativepackage 'libreoffice-style-galaxy' 'amd64' '4' 'sid' 'Depends: libreoffice-core
+Provides: libreoffice-style'
+
+buildsimplenativepackage 'ure' 'amd64' '4' 'sid'
+
+setupaptarchive
+
+testequal 'Reading package lists...
+Building dependency tree...
+The following NEW packages will be installed:
+  ure
+The following packages will be upgraded:
+  libreoffice libreoffice-common libreoffice-core libreoffice-style-galaxy
+4 upgraded, 1 newly installed, 0 to remove and 0 not upgraded.
+Inst libreoffice [3] (4 sid [amd64]) []
+Inst libreoffice-style-galaxy [3] (4 sid [amd64]) [libreoffice-common:amd64 on libreoffice-style-galaxy:amd64] [libreoffice-common:amd64 ]
+Inst libreoffice-core [3] (4 sid [amd64]) [libreoffice-core:amd64 on libreoffice-common:amd64] [libreoffice-common:amd64 on libreoffice-core:amd64] [libreoffice-common:amd64 on libreoffice-style-galaxy:amd64] [libreoffice-common:amd64 ]
+Inst libreoffice-common [3] (4 sid [all]) []
+Inst ure (4 sid [amd64])
+Conf ure (4 sid [amd64])
+Conf libreoffice-style-galaxy (4 sid [amd64])
+Conf libreoffice-common (4 sid [all])
+Conf libreoffice-core (4 sid [amd64])
+Conf libreoffice (4 sid [amd64])' aptget dist-upgrade -s