correct cross & disappear progress detection

Given that we use the progress information to skip over actions dpkg has
already done like not purging a package which was already removed and
had no config files or not acting on disappeared packages and such it is
important that apt and dpkg agree on which states the package has to
pass through.

To ensure that we keep tabs on this in the future a warning is added at
the end if apt hasn't seen all the action it was supposed to see. I
can't wait for the first bugreporters to wonder about this…

diff --git a/apt-pkg/deb/dpkgpm.cc b/apt-pkg/deb/dpkgpm.cc
index 7c41e02b4059f5f3b0d31ad1e4697ec6d0eb0fd7..70e7592a78b2e0321d94c5d73acde987e780ca61 100644 (file)
--- a/apt-pkg/deb/dpkgpm.cc
+++ b/apt-pkg/deb/dpkgpm.cc
@@ -143,6 +143,8 @@ namespace
   // of each array is the key, entry 1 is the value.
   const std::pair<const char *, const char *> PackageProcessingOps[] = {
     std::make_pair("install",   N_("Installing %s")),
+    // we don't care for the difference
+    std::make_pair("upgrade",   N_("Installing %s")),
     std::make_pair("configure", N_("Configuring %s")),
     std::make_pair("remove",    N_("Removing %s")),
     std::make_pair("purge",    N_("Completely removing %s")),
@@ -615,9 +617,6 @@ void pkgDPkgPM::ProcessDpkgStatusLine(char *line)
    {
       pkgname = APT::String::Strip(list[2]);
       action = APT::String::Strip(list[1]);
-      // we don't care for the difference (as dpkg doesn't really either)
-      if (action == "upgrade")
-        action = "install";
    }
    // "status" has the form: "status: pkg: state"
    // with state in ["half-installed", "unpacked", "half-configured", 
@@ -690,10 +689,7 @@ void pkgDPkgPM::ProcessDpkgStatusLine(char *line)
    // 'processing: action: pkg'
    if(prefix == "processing")
    {
-      const std::pair<const char *, const char *> * const iter =
-       std::find_if(PackageProcessingOpsBegin,
-                    PackageProcessingOpsEnd,
-                    MatchProcessingOp(action.c_str()));
+      auto const iter = std::find_if(PackageProcessingOpsBegin, PackageProcessingOpsEnd, MatchProcessingOp(action.c_str()));
       if(iter == PackageProcessingOpsEnd)
       {
         if (Debug == true)
@@ -709,6 +705,37 @@ void pkgDPkgPM::ProcessDpkgStatusLine(char *line)
       //         short pkgnames?
       if (action == "disappear")
         handleDisappearAction(pkgname);
+      else if (action == "upgrade")
+      {
+        // in a crossgrade what looked like a remove first is really an unpack over it
+        auto const Pkg = Cache.FindPkg(pkgname);
+        if (likely(Pkg.end() == false) && Cache[Pkg].Delete())
+        {
+           auto const Grp = Pkg.Group();
+           if (likely(Grp.end() == false))
+           {
+              for (auto P = Grp.PackageList(); P.end() != true; P = Grp.NextPkg(P))
+                 if(Cache[P].Install())
+                 {
+                    auto && Ops = PackageOps[P.FullName()];
+                    auto const unpackOp = std::find_if(Ops.cbegin(), Ops.cend(), [](DpkgState const &s) { return strcmp(s.state, "unpacked") == 0; });
+                    if (unpackOp != Ops.cend())
+                    {
+                       // skip ahead in the crossgraded packages
+                       auto const skipped = std::distance(Ops.cbegin(), unpackOp);
+                       PackagesDone += skipped;
+                       PackageOpsDone[P.FullName()] += skipped;
+                       // finish the crossremoved package
+                       auto const ROps = PackageOps[Pkg.FullName()].size();
+                       auto && ROpsDone = PackageOpsDone[Pkg.FullName()];
+                       PackagesDone += ROps - ROpsDone;
+                       ROpsDone = ROps;
+                       break;
+                    }
+                 }
+           }
+        }
+      }
       return;
    }
 
@@ -752,34 +779,6 @@ void pkgDPkgPM::ProcessDpkgStatusLine(char *line)
               strprintf(msg, translation, i18n_pkgname.c_str());
               d->progress->StatusChanged(pkgname, PackagesDone, PackagesTotal, msg);
            }
-           else if (action == "unpacked" && strcmp(next_action, "config-files") == 0)
-           {
-              // in a crossgrade what looked like a remove first is really an unpack over it
-              ++PackageOpsDone[pkgname];
-              ++PackagesDone;
-
-              auto const Pkg = Cache.FindPkg(pkgname);
-              if (likely(Pkg.end() == false))
-              {
-                 auto const Grp = Pkg.Group();
-                 if (likely(Grp.end() == false))
-                 {
-                    for (auto P = Grp.PackageList(); P.end() != true; P = Grp.NextPkg(P))
-                       if(Cache[P].Install())
-                       {
-                          auto && Ops = PackageOps[P.FullName()];
-                          auto const unpackOp = std::find_if(Ops.cbegin(), Ops.cend(), [](DpkgState const &s) { return strcmp(s.state, "unpacked") == 0; });
-                          if (unpackOp != Ops.cend())
-                          {
-                             auto const skipped = std::distance(Ops.cbegin(), unpackOp);
-                             PackagesDone += skipped;
-                             PackageOpsDone[P.FullName()] += skipped;
-                             break;
-                          }
-                       }
-                 }
-              }
-           }
         }
       }
       else if (action == "triggers-pending")
@@ -803,6 +802,12 @@ void pkgDPkgPM::handleDisappearAction(string const &pkgname)
    if (unlikely(Pkg.end() == true))
       return;
 
+   // a disappeared package has no further actions
+   auto const ROps = PackageOps[Pkg.FullName()].size();
+   auto && ROpsDone = PackageOpsDone[Pkg.FullName()];
+   PackagesDone += ROps - ROpsDone;
+   ROpsDone = ROps;
+
    // record the package name for display and stuff later
    disappearedPkgs.insert(Pkg.FullName(true));
 
@@ -1994,6 +1999,24 @@ bool pkgDPkgPM::Go(APT::Progress::PackageManager *progress)
 
    if (noopDPkgInvocation == false)
    {
+      if (d->dpkg_error.empty() && (PackagesDone + 1) != PackagesTotal)
+      {
+        std::string pkglist;
+        for (auto const &PO: PackageOps)
+           if (PO.second.size() != PackageOpsDone[PO.first])
+           {
+              if (pkglist.empty() == false)
+                 pkglist.append(" ");
+              pkglist.append(PO.first);
+           }
+        /* who cares about correct progress? As we depend on it for skipping actions
+           our parsing should be correct. People will no doubt be confused if they see
+           this message, but the dpkg warning about unknown packages isn't much better
+           from a user POV and combined we might have a chance to figure out what is wrong */
+        _error->Warning("APT had planned for dpkg to do more than it reported back (%u vs %u).\n"
+              "Affected packages: %s", PackagesDone, PackagesTotal, pkglist.c_str());
+      }
+
       std::string const oldpkgcache = _config->FindFile("Dir::cache::pkgcache");
       if (oldpkgcache.empty() == false && RealFileExists(oldpkgcache) == true &&
          RemoveFile("pkgDPkgPM::Go", oldpkgcache))

diff --git a/test/integration/test-bug-712116-dpkg-pre-install-pkgs-hook-multiarch b/test/integration/test-bug-712116-dpkg-pre-install-pkgs-hook-multiarch
index bf93367c99b9c1d32e3d6083fd7d76b0647d1e4f..9c96bbe529d7aa8317041b25a7906d81e309d54e 100755 (executable)
--- a/test/integration/test-bug-712116-dpkg-pre-install-pkgs-hook-multiarch
+++ b/test/integration/test-bug-712116-dpkg-pre-install-pkgs-hook-multiarch
@@ -48,7 +48,7 @@ DPkg::Tools::options::\"./${hook}-v${1}.sh\"::Version \"$1\";" > rootdir/etc/apt
 observehook() {
        rm -f ${hook}-v2.list ${hook}-v3.list
        msgtest 'Observe hooks while' "$*"
-       testsuccess --nomsg aptget "$@" -y --allow-downgrades --planner $planner
+       testsuccess --nomsg aptget "$@" -y --allow-downgrades --planner $planner -o Debug::pkgDPkgProgressReporting=1
        # different planners have different orders – we don't care in this test here
        if [ -e ${hook}-v2.list ]; then
                sort < ${hook}-v2.list > ${hook}-v2.list.new

diff --git a/test/integration/test-disappearing-packages b/test/integration/test-disappearing-packages
index 177491d81e0bef51e19b9d5650887854e276ed70..e8748e6bcb31b3449e779db3991ddc4e154920a0 100755 (executable)
--- a/test/integration/test-disappearing-packages
+++ b/test/integration/test-disappearing-packages
@@ -4,21 +4,20 @@ set -e
 TESTDIR="$(readlink -f "$(dirname "$0")")"
 . "$TESTDIR/framework"
 setupenvironment
-configarchitecture 'native'
+configarchitecture 'amd64' 'i386'
 
-buildsimplenativepackage 'old-pkg' 'native' '1.0' 'stable'
-buildsimplenativepackage 'unrelated' 'all' '0.5' 'unstable'
+buildsimplenativepackage 'old-pkg' 'amd64' '1' 'stable'
 
-setupsimplenativepackage 'new-pkg' 'native' '2.0' 'unstable' 'Provides: old-pkg
+setupsimplenativepackage 'new-pkg' 'amd64' '2' 'unstable' 'Provides: old-pkg
 Replaces: old-pkg
-Conflicts: old-pkg (<< 2.0)'
-BUILDDIR='incoming/new-pkg-2.0'
+Conflicts: old-pkg (<< 2)'
+BUILDDIR='incoming/new-pkg-2'
 echo '/usr/share/doc/new-pkg /usr/share/doc/old-pkg' > "${BUILDDIR}/debian/new-pkg.links"
 buildpackage "$BUILDDIR" 'unstable' 'main'
 rm -rf "$BUILDDIR"
 
-setupsimplenativepackage 'old-pkg' 'all' '2.0' 'unstable' 'Depends: new-pkg'
-BUILDDIR='incoming/old-pkg-2.0'
+setupsimplenativepackage 'old-pkg' 'all' '2' 'unstable' 'Depends: new-pkg'
+BUILDDIR='incoming/old-pkg-2'
 echo '/usr/share/doc/new-pkg /usr/share/doc/old-pkg' > "${BUILDDIR}/debian/old-pkg.links"
 echo "
 override_dh_link:
@@ -27,39 +26,67 @@ override_dh_link:
 buildpackage "$BUILDDIR" 'unstable' 'main'
 rm -rf "$BUILDDIR"
 
-setupaptarchive
+setupsimplenativepackage 'super-new-pkg' 'i386' '3' 'experimental' 'Provides: new-pkg
+Replaces: new-pkg
+Conflicts: new-pkg (<< 3)'
+BUILDDIR='incoming/super-new-pkg-3'
+echo '/usr/share/doc/super-new-pkg /usr/share/doc/old-pkg' > "${BUILDDIR}/debian/super-new-pkg.links"
+echo '/usr/share/doc/super-new-pkg /usr/share/doc/new-pkg' >> "${BUILDDIR}/debian/super-new-pkg.links"
+buildpackage "$BUILDDIR" 'experimental' 'main'
+rm -rf "$BUILDDIR"
+
+setupsimplenativepackage 'new-pkg' 'all' '3' 'experimental' 'Depends: super-new-pkg'
+BUILDDIR='incoming/new-pkg-3'
+echo '/usr/share/doc/super-new-pkg /usr/share/doc/old-pkg' > "${BUILDDIR}/debian/new-pkg.links"
+echo '/usr/share/doc/super-new-pkg /usr/share/doc/new-pkg' >> "${BUILDDIR}/debian/new-pkg.links"
+echo "
+override_dh_link:
+       rm -rf debian/new-pkg/usr/share/doc/new-pkg/
+       dh_link" >> "${BUILDDIR}/debian/rules"
+buildpackage "$BUILDDIR" 'experimental' 'main'
+rm -rf "$BUILDDIR"
 
-testsuccess aptget install old-pkg=1.0 --trivial-only
+setupaptarchive
 
+msgmsg 'Let a package disappear' 'old-pkg'
+testsuccess aptget install old-pkg=1 --trivial-only
 testmarkedauto # old-pkg is manual installed
-
-CMD='aptget dist-upgrade -y'
-msgtest 'Test for equality of' "$CMD"
-COMPAREFILE="$(mktemp)"
-echo 'The following package disappeared from your system as
+testsuccess aptget dist-upgrade -y
+testdpkgnotinstalled old-pkg
+cp rootdir/tmp/testsuccess.output disappear.output
+testsuccessequal 'The following package disappeared from your system as
 all files have been overwritten by other packages:
   old-pkg
-Note: This is done automatically and on purpose by dpkg.' > "$COMPAREFILE"
-$CMD 2>&1 | tail -n 4 | diff -u "$COMPAREFILE" - && msgpass || msgfail
-rm "$COMPAREFILE"
+Note: This is done automatically and on purpose by dpkg.' tail -n 4 disappear.output
 
 sed -i rootdir/var/log/apt/history.log -e '/^Commandline: / d' -e '/^Start-Date: / d' -e '/^End-Date: / d' -e "s#:$(getarchitecture 'native') #:native #"
 if [ -n "$SUDO_USER" ]; then
        testfileequal 'rootdir/var/log/apt/history.log' "
 Requested-By: $SUDO_USER ($(id -u "$SUDO_USER"))
-Install: old-pkg:native (1.0)
+Install: old-pkg:native (1)
 
 Requested-By: $SUDO_USER ($(id -u "$SUDO_USER"))
-Install: new-pkg:native (2.0, automatic)
-Upgrade: old-pkg:native (1.0, 2.0)
-Disappeared: old-pkg (1.0)"
+Install: new-pkg:native (2, automatic)
+Upgrade: old-pkg:native (1, 2)
+Disappeared: old-pkg (1)"
 else
        testfileequal 'rootdir/var/log/apt/history.log' '
-Install: old-pkg:native (1.0)
+Install: old-pkg:native (1)
 
-Install: new-pkg:native (2.0, automatic)
-Upgrade: old-pkg:native (1.0, 2.0)
-Disappeared: old-pkg (1.0)'
+Install: new-pkg:native (2, automatic)
+Upgrade: old-pkg:native (1, 2)
+Disappeared: old-pkg (1)'
 fi
 
 testmarkedauto  # new-pkg should have get the manual flag from old-pkg
+
+msgmsg 'Let a package disappear which let the previous disappear' 'new-pkg'
+testsuccess aptget dist-upgrade -y -t experimental
+testdpkgnotinstalled new-pkg
+cp rootdir/tmp/testsuccess.output disappear.output
+testsuccessequal 'The following package disappeared from your system as
+all files have been overwritten by other packages:
+  new-pkg
+Note: This is done automatically and on purpose by dpkg.' tail -n 4 disappear.output
+
+testmarkedauto