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

diff --git a/apt-pkg/depcache.cc b/apt-pkg/depcache.cc
index 508e45962af7eb63d7ce14612b706214a17a3601..e9fa097aaf11e8aa32fdfc274059daf46c9a9eea 100644 (file)
--- a/apt-pkg/depcache.cc
+++ b/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();

diff --git a/test/integration/test-bug-470115-new-and-tighten-recommends b/test/integration/test-bug-470115-new-and-tighten-recommends
new file mode 100755 (executable)
index 0000000..6bc22ea
--- /dev/null
+++ b/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