rewrite new and previously satisfied important dependency detection in

MarkInstall to be a bit more easy to read and possibily a bit faster -
and add a small testcase for these situations to ensure the same behaviour

apt-pkg/depcache.cc

@@ -1101,53 +1101,50 @@ void pkgDepCache::MarkInstall(PkgIterator const &Pkg,bool AutoInst,
        * package should follow that Recommends rather than causing the
        * dependency to be removed. (bug #470115)
        */
-      bool isNewImportantDep = false;
-      bool isPreviouslySatisfiedImportantDep = false;
-      if(!ForceImportantDeps && !Start.IsCritical())
+      if (Pkg->CurrentVer != 0 && ForceImportantDeps == false && Start.IsCritical() == false)
       {
-	 bool found=false;
-	 VerIterator instVer = Pkg.CurrentVer();
-	 if(!instVer.end())
+	 bool isNewImportantDep = true;
+	 bool isPreviouslySatisfiedImportantDep = false;
+	 for (DepIterator D = Pkg.CurrentVer().DependsList(); D.end() != true; ++D)
 	 {
-	   for (DepIterator D = instVer.DependsList(); D.end() != true; D++)
-	     {
-	       //FIXME: deal better with or-groups(?)
-	       if(IsImportantDep(D) && !D.IsCritical() &&
-		  Start.TargetPkg() == D.TargetPkg())
-		 {
-		   if(!isPreviouslySatisfiedImportantDep)
-		     {
-		       DepIterator D2 = D;
-		       while((D2->CompareOp & Dep::Or) != 0)
-			 ++D2;
-
-		       isPreviouslySatisfiedImportantDep =
-			 (((*this)[D2] & DepGNow) != 0);
-		     }
-
-		   found=true;
-		 }
-	     }
-	    // this is a new dep if it was not found to be already
-	    // a important dep of the installed pacakge
-	    isNewImportantDep = !found;
+	    //FIXME: Should we handle or-group better here?
+	    // We do not check if the package we look for is part of the same or-group
+	    // we might find while searching, but could that really be a problem?
+	    if (D.IsCritical() == true || IsImportantDep(D) == false ||
+		Start.TargetPkg() != D.TargetPkg())
+	       continue;
+
+	    isNewImportantDep = false;
+
+	    while ((D->CompareOp & Dep::Or) != 0)
+	       ++D;
+
+	    isPreviouslySatisfiedImportantDep = (((*this)[D] & DepGNow) != 0);
+	    if (isPreviouslySatisfiedImportantDep == true)
+	       break;
+	 }
+
+	 if(isNewImportantDep == true)
+	 {
+	    if (DebugAutoInstall == true)
+	       std::clog << OutputInDepth(Depth) << "new important dependency: "
+			 << Start.TargetPkg().FullName() << std::endl;
+	 }
+	 else if(isPreviouslySatisfiedImportantDep == true)
+	 {
+	    if (DebugAutoInstall == true)
+	       std::clog << OutputInDepth(Depth) << "previously satisfied important dependency on "
+			 << Start.TargetPkg().FullName() << std::endl;
+	 }
+	 else
+	 {
+	    if (DebugAutoInstall == true)
+	       std::clog << OutputInDepth(Depth) << "ignore old unsatisfied important dependency on "
+			 << Start.TargetPkg().FullName() << std::endl;
+	    continue;
 	 }
       }
-      if(isNewImportantDep)
-	 if(DebugAutoInstall == true)
-	    std::clog << OutputInDepth(Depth) << "new important dependency: "
-		      << Start.TargetPkg().Name() << std::endl;
-      if(isPreviouslySatisfiedImportantDep)
-	if(DebugAutoInstall == true)
-	  std::clog << OutputInDepth(Depth) << "previously satisfied important dependency on "
-		    << Start.TargetPkg().Name() << std::endl;
-
-      // skip important deps if the package is already installed
-      if (Pkg->CurrentVer != 0 && Start.IsCritical() == false 
-	  && !isNewImportantDep && !isPreviouslySatisfiedImportantDep
-	  && !ForceImportantDeps)
-	 continue;
-      
+
       /* This bit is for processing the possibilty of an install/upgrade
          fixing the problem */
       SPtrArray<Version *> List = Start.AllTargets();

test/integration/test-bug-470115-new-and-tighten-recommends

@@ -0,0 +1,172 @@
+#!/bin/sh
+set -e
+
+TESTDIR=$(readlink -f $(dirname $0))
+. $TESTDIR/framework
+setupenvironment
+configarchitecture 'i386'
+
+insertinstalledpackage 'cool' 'all' '1'
+insertinstalledpackage 'stuff' 'all' '1'
+insertinstalledpackage 'coolstuff' 'all' '1'
+
+insertpackage 'unstable' 'cool' 'all' '2'
+insertpackage 'unstable' 'stuff' 'all' '2'
+insertpackage 'unstable' 'coolstuff' 'all' '2'
+insertpackage 'unstable' 'super' 'all' '2'
+
+insertinstalledpackage 'tighten-cool' 'all' '1' 'Recommends: cool (>= 1)'
+insertpackage 'unstable' 'tighten-cool' 'all' '2' 'Recommends: cool (>= 2)'
+
+insertinstalledpackage 'tighten-coolorstuff' 'all' '1' 'Recommends: cool (>= 1) | stuff (>= 1)'
+insertpackage 'unstable' 'tighten-coolorstuff' 'all' '2' 'Recommends: stuff (>= 2) | cool (>= 2)'
+
+insertinstalledpackage 'tighten-coolorstuff2' 'all' '1' 'Recommends: cool (>= 1) | stuff (>= 1)'
+insertpackage 'unstable' 'tighten-coolorstuff2' 'all' '2' 'Recommends: stuff2 (>= 2) | cool (>= 2)'
+
+insertinstalledpackage 'newrec-cool' 'all' '1'
+insertpackage 'unstable' 'newrec-cool' 'all' '2' 'Recommends: cool (>= 2)'
+
+insertinstalledpackage 'newrec-super' 'all' '1'
+insertpackage 'unstable' 'newrec-super' 'all' '2' 'Recommends: super'
+
+insertinstalledpackage 'newrec-coolorstuff' 'all' '1'
+insertpackage 'unstable' 'newrec-coolorstuff' 'all' '2' 'Recommends: cool (>= 2) | stuff (>= 2)'
+
+insertinstalledpackage 'cool-gone' 'all' '1' 'Recommends: cool (>= 1) | stuff (>= 2)'
+insertpackage 'unstable' 'cool-gone' 'all' '2' 'Recommends: stuff (>= 2)'
+
+insertinstalledpackage 'super-overtake' 'all' '1' 'Recommends: cool | super, stuff | super'
+insertpackage 'unstable' 'super-overtake' 'all' '2' 'Recommends: stuff (>= 3) | super, super | cool (>= 2)'
+
+insertinstalledpackage 'upgrade-over-new' 'all' '1' 'Recommends: cool'
+insertpackage 'unstable' 'upgrade-over-new' 'all' '2' 'Recommends: cool (>= 2) | super'
+
+insertinstalledpackage 'now-satisfiable' 'all' '1' 'Recommends: cool (>= 3)'
+insertpackage 'unstable' 'now-satisfiable' 'all' '2' 'Recommends: cool (>= 2)'
+
+setupaptarchive
+
+testequal 'Reading package lists...
+Building dependency tree...
+The following extra packages will be installed:
+  cool
+The following packages will be upgraded:
+  cool tighten-cool
+2 upgraded, 0 newly installed, 0 to remove and 11 not upgraded.
+Inst cool [1] (2 unstable [all])
+Inst tighten-cool [1] (2 unstable [all])
+Conf cool (2 unstable [all])
+Conf tighten-cool (2 unstable [all])' aptget install tighten-cool -s
+
+testequal 'Reading package lists...
+Building dependency tree...
+The following extra packages will be installed:
+  stuff
+The following packages will be upgraded:
+  stuff tighten-coolorstuff
+2 upgraded, 0 newly installed, 0 to remove and 11 not upgraded.
+Inst stuff [1] (2 unstable [all])
+Inst tighten-coolorstuff [1] (2 unstable [all])
+Conf stuff (2 unstable [all])
+Conf tighten-coolorstuff (2 unstable [all])' aptget install tighten-coolorstuff -s
+
+testequal 'Reading package lists...
+Building dependency tree...
+The following extra packages will be installed:
+  cool
+The following packages will be upgraded:
+  cool tighten-coolorstuff2
+2 upgraded, 0 newly installed, 0 to remove and 11 not upgraded.
+Inst cool [1] (2 unstable [all])
+Inst tighten-coolorstuff2 [1] (2 unstable [all])
+Conf cool (2 unstable [all])
+Conf tighten-coolorstuff2 (2 unstable [all])' aptget install tighten-coolorstuff2 -s
+
+testequal 'Reading package lists...
+Building dependency tree...
+The following extra packages will be installed:
+  cool
+The following packages will be upgraded:
+  cool newrec-cool
+2 upgraded, 0 newly installed, 0 to remove and 11 not upgraded.
+Inst cool [1] (2 unstable [all])
+Inst newrec-cool [1] (2 unstable [all])
+Conf cool (2 unstable [all])
+Conf newrec-cool (2 unstable [all])' aptget install newrec-cool -s
+
+testequal 'Reading package lists...
+Building dependency tree...
+The following extra packages will be installed:
+  super
+The following NEW packages will be installed:
+  super
+The following packages will be upgraded:
+  newrec-super
+1 upgraded, 1 newly installed, 0 to remove and 12 not upgraded.
+Inst newrec-super [1] (2 unstable [all])
+Inst super (2 unstable [all])
+Conf newrec-super (2 unstable [all])
+Conf super (2 unstable [all])' aptget install newrec-super -s
+
+testequal 'Reading package lists...
+Building dependency tree...
+The following extra packages will be installed:
+  cool
+The following packages will be upgraded:
+  cool newrec-coolorstuff
+2 upgraded, 0 newly installed, 0 to remove and 11 not upgraded.
+Inst cool [1] (2 unstable [all])
+Inst newrec-coolorstuff [1] (2 unstable [all])
+Conf cool (2 unstable [all])
+Conf newrec-coolorstuff (2 unstable [all])' aptget install newrec-coolorstuff -s
+
+testequal 'Reading package lists...
+Building dependency tree...
+The following extra packages will be installed:
+  stuff
+The following packages will be upgraded:
+  cool-gone stuff
+2 upgraded, 0 newly installed, 0 to remove and 11 not upgraded.
+Inst cool-gone [1] (2 unstable [all])
+Inst stuff [1] (2 unstable [all])
+Conf cool-gone (2 unstable [all])
+Conf stuff (2 unstable [all])' aptget install cool-gone -s
+
+testequal 'Reading package lists...
+Building dependency tree...
+The following extra packages will be installed:
+  super
+The following NEW packages will be installed:
+  super
+The following packages will be upgraded:
+  super-overtake
+1 upgraded, 1 newly installed, 0 to remove and 12 not upgraded.
+Inst super (2 unstable [all])
+Inst super-overtake [1] (2 unstable [all])
+Conf super (2 unstable [all])
+Conf super-overtake (2 unstable [all])' aptget install super-overtake -s
+
+# if super would be in front, we would get a new here as it is new and
+# the first option in an or-group should be the preferred one…
+testequal 'Reading package lists...
+Building dependency tree...
+The following extra packages will be installed:
+  cool
+The following packages will be upgraded:
+  cool upgrade-over-new
+2 upgraded, 0 newly installed, 0 to remove and 11 not upgraded.
+Inst cool [1] (2 unstable [all])
+Inst upgrade-over-new [1] (2 unstable [all])
+Conf cool (2 unstable [all])
+Conf upgrade-over-new (2 unstable [all])' aptget install upgrade-over-new -s
+
+# the recommends wasn't used before so while we could do it now,
+# the user doesn't seem to need it so avoid upgrading it
+testequal 'Reading package lists...
+Building dependency tree...
+The following packages will be upgraded:
+  now-satisfiable
+1 upgraded, 0 newly installed, 0 to remove and 12 not upgraded.
+Inst now-satisfiable [1] (2 unstable [all])
+Conf now-satisfiable (2 unstable [all])' aptget install now-satisfiable -s