improve arch-unqualified dpkg-progress parsing

Our old idea was to look for the first package which would be "touched"
and take this as the package dpkg is talking about, but that is
incorrect in complicated situations like a package upgraded to/from
multiple M-A:same siblings installed.

As we us the progress report to decide what is still needed we have to
be reasonabily right about the package dpkg is talking about, so we jump
to quite a few loops to get it.

diff --git a/apt-pkg/deb/dpkgpm.cc b/apt-pkg/deb/dpkgpm.cc
index 70e7592a78b2e0321d94c5d73acde987e780ca61..abf91fe2a88bb1f2ebd8b9368d9e5aa6b15bf8b5 100644 (file)
--- a/apt-pkg/deb/dpkgpm.cc
+++ b/apt-pkg/deb/dpkgpm.cc
@@ -619,63 +619,152 @@ void pkgDPkgPM::ProcessDpkgStatusLine(char *line)
       action = APT::String::Strip(list[1]);
    }
    // "status" has the form: "status: pkg: state"
       action = APT::String::Strip(list[1]);
    }
    // "status" has the form: "status: pkg: state"

-   // with state in ["half-installed", "unpacked", "half-configured", 
+   // with state in ["half-installed", "unpacked", "half-configured",

    //                "installed", "config-files", "not-installed"]
    else if (prefix == "status")
    {
       pkgname = APT::String::Strip(list[1]);
       action = APT::String::Strip(list[2]);
    //                "installed", "config-files", "not-installed"]
    else if (prefix == "status")
    {
       pkgname = APT::String::Strip(list[1]);
       action = APT::String::Strip(list[2]);

-   } else {
-      if (Debug == true)
-        std::clog << "unknown prefix '" << prefix << "'" << std::endl;
-      return;
-   }

 
 

+      /* handle the special cases first:

 
 

-   /* handle the special cases first:
-
-      errors look like this:
-      'status: /var/cache/apt/archives/krecipes_0.8.1-0ubuntu1_i386.deb : error : trying to overwrite `/usr/share/doc/kde/HTML/en/krecipes/krectip.png', which is also in package krecipes-data 
-      and conffile-prompt like this
-      'status:/etc/compiz.conf/compiz.conf :  conffile-prompt: 'current-conffile' 'new-conffile' useredited distedited
-   */
-   if (prefix == "status")
-   {
+        errors look like this:
+        'status: /var/cache/apt/archives/krecipes_0.8.1-0ubuntu1_i386.deb : error : trying to overwrite `/usr/share/doc/kde/HTML/en/krecipes/krectip.png', which is also in package krecipes-data
+        and conffile-prompt like this
+        'status:/etc/compiz.conf/compiz.conf :  conffile-prompt: 'current-conffile' 'new-conffile' useredited distedited
+        */

       if(action == "error")
       {
       if(action == "error")
       {

-         d->progress->Error(pkgname, PackagesDone, PackagesTotal,
-                            list[3]);
-         pkgFailures++;
+         d->progress->Error(pkgname, PackagesDone, PackagesTotal, list[3]);
+         ++pkgFailures;

          WriteApportReport(pkgname.c_str(), list[3].c_str());
          return;
       }
       else if(action == "conffile-prompt")
       {
          WriteApportReport(pkgname.c_str(), list[3].c_str());
          return;
       }
       else if(action == "conffile-prompt")
       {

-         d->progress->ConffilePrompt(pkgname, PackagesDone, PackagesTotal,
-                                     list[3]);
+         d->progress->ConffilePrompt(pkgname, PackagesDone, PackagesTotal, list[3]);

          return;
       }
          return;
       }

+   } else {
+      if (Debug == true)
+        std::clog << "unknown prefix '" << prefix << "'" << std::endl;
+      return;

    }
 
    }
 

-   // at this point we know that we should have a valid pkgname, so build all 
-   // the info from it
-
-   // dpkg does not always send "pkgname:arch" so we add it here if needed
+   // At this point we have a pkgname, but it might not be arch-qualified !

    if (pkgname.find(":") == std::string::npos)
    {
    if (pkgname.find(":") == std::string::npos)
    {

-      // find the package in the group that is touched by dpkg
-      // if there are multiple pkgs dpkg would send us a full pkgname:arch

       pkgCache::GrpIterator Grp = Cache.FindGrp(pkgname);
       pkgCache::GrpIterator Grp = Cache.FindGrp(pkgname);

-      if (Grp.end() == false)
-        for (auto P = Grp.PackageList(); P.end() != true; P = Grp.NextPkg(P))
-           if(Cache[P].Keep() == false || Cache[P].ReInstall() == true)
+      /* No arch means that dpkg believes there can only be one package
+         this can refer to so lets see what could be candidates here: */
+      std::vector<pkgCache::PkgIterator> candset;
+      for (auto P = Grp.PackageList(); P.end() != true; P = Grp.NextPkg(P))
+      {
+        if (PackageOps.find(P.FullName()) != PackageOps.end())
+           candset.push_back(P);
+        // packages can disappear without them having any interaction itself
+        // so we have to consider these as candidates, too
+        else if (P->CurrentVer != 0 && action == "disappear")
+           candset.push_back(P);
+      }
+      if (unlikely(candset.empty()))
+      {
+        if (Debug == true)
+           std::clog << "unable to figure out which package is dpkg refering to with '" << pkgname << "'! (1)" << std::endl;
+        return;
+      }
+      else if (candset.size() == 1) // we are lucky
+        pkgname = candset.cbegin()->FullName();
+      else
+      {
+        /* here be dragons^Wassumptions about dpkg:
+           - an M-A:same version is always arch-qualified
+           - a package from a foreign arch is (in newer versions) */
+        size_t installedInstances = 0, wannabeInstances = 0;
+        for (auto const &P: candset)
+        {
+           if (P->CurrentVer != 0)
+           {
+              ++installedInstances;
+              if (Cache[P].Delete() == false)
+                 ++wannabeInstances;
+           }
+           else if (Cache[P].Install())
+              ++wannabeInstances;
+        }
+        // the package becomes M-A:same, so we are still talking about current
+        if (installedInstances == 1 && wannabeInstances >= 2)
+        {
+           for (auto const &P: candset)

            {
            {

-              auto fullname = P.FullName();
-              if (Cache[P].Delete() && PackageOps[fullname].size() <= PackageOpsDone[fullname])
+              if (P->CurrentVer == 0)

                  continue;
                  continue;

-              pkgname = std::move(fullname);
+              pkgname = P.FullName();

               break;
            }
               break;
            }

+        }
+        // the package was M-A:same, it isn't now, so we can only talk about that
+        else if (installedInstances >= 2 && wannabeInstances == 1)
+        {
+           for (auto const &P: candset)
+           {
+              auto const IV = Cache[P].InstVerIter(Cache);
+              if (IV.end())
+                 continue;
+              pkgname = P.FullName();
+              break;
+           }
+        }
+        // that is a crossgrade
+        else if (installedInstances == 1 && wannabeInstances == 1 && candset.size() == 2)
+        {
+           auto const PkgHasCurrentVersion = [](pkgCache::PkgIterator const &P) { return P->CurrentVer != 0; };
+           auto const P = std::find_if(candset.begin(), candset.end(), PkgHasCurrentVersion);
+           if (unlikely(P == candset.end()))
+           {
+              if (Debug == true)
+                 std::clog << "situation for '" << pkgname << "' looked like a crossgrade, but no current version?!" << std::endl;
+              return;
+           }
+           auto fullname = P->FullName();
+           if (PackageOps[fullname].size() != PackageOpsDone[fullname])
+              pkgname = std::move(fullname);
+           else
+              pkgname = std::find_if_not(candset.begin(), candset.end(), PkgHasCurrentVersion)->FullName();
+        }
+        // we are desperate: so "just" take the native one, but that might change mid-air,
+        // so we have to ask dpkg what it believes native is at the moment… all the time
+        else
+        {
+           std::vector<std::string> sArgs = debSystem::GetDpkgBaseCommand();
+           sArgs.push_back("--print-architecture");
+           int outputFd = -1;
+           pid_t const dpkgNativeArch = debSystem::ExecDpkg(sArgs, nullptr, &outputFd, true);
+           if (unlikely(dpkgNativeArch == -1))
+           {
+              if (Debug == true)
+                 std::clog << "calling dpkg failed to ask it for its current native architecture to expand '" << pkgname << "'!" << std::endl;
+              return;
+           }
+           FILE *dpkg = fdopen(outputFd, "r");
+           if(dpkg != NULL)
+           {
+              char* buf = NULL;
+              size_t bufsize = 0;
+              if (getline(&buf, &bufsize, dpkg) != -1)
+                 pkgname += ':' + bufsize;
+              free(buf);
+              fclose(dpkg);
+           }
+           ExecWait(dpkgNativeArch, "dpkg --print-architecture", true);
+           if (pkgname.find(':') != std::string::npos)
+           {
+              if (Debug == true)
+                 std::clog << "unable to figure out which package is dpkg refering to with '" << pkgname << "'! (2)" << std::endl;
+              return;
+           }
+        }
+      }

    }
 
    std::string arch = "";
    }
 
    std::string arch = "";


@@ -706,36 +795,7 @@ void pkgDPkgPM::ProcessDpkgStatusLine(char *line)
       if (action == "disappear")
         handleDisappearAction(pkgname);
       else if (action == "upgrade")
       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;
-                    }
-                 }
-           }
-        }
-      }
+        handleCrossUpgradeAction(pkgname);

       return;
    }
 
       return;
    }
 


@@ -849,6 +909,38 @@ void pkgDPkgPM::handleDisappearAction(string const &pkgname)
    }
 }
                                                                        /*}}}*/
    }
 }
                                                                        /*}}}*/

+void pkgDPkgPM::handleCrossUpgradeAction(string const &pkgname)                /*{{{*/
+{
+   // 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;
+              }
+           }
+      }
+   }
+}
+                                                                       /*}}}*/

 // DPkgPM::DoDpkgStatusFd                                              /*{{{*/
 void pkgDPkgPM::DoDpkgStatusFd(int statusfd)
 {
 // DPkgPM::DoDpkgStatusFd                                              /*{{{*/
 void pkgDPkgPM::DoDpkgStatusFd(int statusfd)
 {

diff --git a/apt-pkg/deb/dpkgpm.h b/apt-pkg/deb/dpkgpm.h
index 193754644d44ad19b527beba48385334300cdb1e..d1c2bcf41767ca82eb6480bca20b5b32a39c27b1 100644 (file)
--- a/apt-pkg/deb/dpkgpm.h
+++ b/apt-pkg/deb/dpkgpm.h
@@ -53,6 +53,7 @@ class pkgDPkgPM : public pkgPackageManager
       \param pkgname Name of the package that disappeared
    */
    APT_HIDDEN void handleDisappearAction(std::string const &pkgname);
       \param pkgname Name of the package that disappeared
    */
    APT_HIDDEN void handleDisappearAction(std::string const &pkgname);

+   APT_HIDDEN void handleCrossUpgradeAction(std::string const &pkgname);

 
    protected:
    int pkgFailures;
 
    protected:
    int pkgFailures;

diff --git a/test/integration/test-bug-618288-multiarch-same-lockstep b/test/integration/test-bug-618288-multiarch-same-lockstep
index d9582f6b6b5a26409a0657c6dcc163da73da0994..eeecb40eea42c93e1c106ea3baf06efe9f82c2e0 100755 (executable)
--- a/test/integration/test-bug-618288-multiarch-same-lockstep
+++ b/test/integration/test-bug-618288-multiarch-same-lockstep
@@ -36,3 +36,6 @@ test "$LS_U_AMD" -lt "$LS_C_INT" && msgpass || msgfail
 
 msgtest 'Test if' 'libsame:i386 unpack is before libsame:amd64 configure'
 test "$LS_U_INT" -lt "$LS_C_AMD" && msgpass || msgfail
 
 msgtest 'Test if' 'libsame:i386 unpack is before libsame:amd64 configure'
 test "$LS_U_INT" -lt "$LS_C_AMD" && msgpass || msgfail

+
+# lets see if it really does work out
+testsuccess apt dist-upgrade -y

diff --git a/test/integration/test-crossgrades b/test/integration/test-crossgrades
index d412546c1b1dda1d1f02445f6276e73aa0cec2ea..30195d30f1c6c4403bb5456e88bb3ec4deeb09d9 100755 (executable)
--- a/test/integration/test-crossgrades
+++ b/test/integration/test-crossgrades
@@ -11,6 +11,7 @@ configdpkgnoopchroot
 buildsimplenativepackage 'unrelated' 'amd64' '1' 'stable'
 buildsimplenativepackage 'crosser' 'i386,armel' '1' 'stable' 'Multi-Arch: same'
 buildsimplenativepackage 'crosser' 'amd64' '2' 'unstable'
 buildsimplenativepackage 'unrelated' 'amd64' '1' 'stable'
 buildsimplenativepackage 'crosser' 'i386,armel' '1' 'stable' 'Multi-Arch: same'
 buildsimplenativepackage 'crosser' 'amd64' '2' 'unstable'

+buildsimplenativepackage 'crosser' 'i386,armel' '3' 'experimental' 'Multi-Arch: same'

 setupaptarchive
 
 singleinstance() {
 setupaptarchive
 
 singleinstance() {


@@ -43,7 +44,15 @@ multiinstance() {
        testdpkgnotinstalled 'crosser:i386' 'crosser:armel' 'unrelated'
        testdpkginstalled 'crosser:amd64'
 
        testdpkgnotinstalled 'crosser:i386' 'crosser:armel' 'unrelated'
        testdpkginstalled 'crosser:amd64'
 

-       testsuccess apt purge crosser:amd64 -y --planner $1
-       testdpkgnotinstalled 'crosser:amd64'
+       testsuccess apt install crosser:i386 crosser:armel -t experimental -y -o Debug::pkgDpkgPm=1 -o Dpkg::Use-Pty=0 --purge --planner $1
+       cp -a rootdir/tmp/testsuccess.output crosser.output
+       testsuccess grep -- '--remove.*crosser.*' crosser.output
+       testsuccess grep -- '--purge' crosser.output
+       testsuccess apt install crosser:i386 crosser:armel -t experimental -y -o Debug::pkgDPkgProgressReporting=1 -o Dpkg::Use-Pty=0 --purge --planner $1
+       testdpkgnotinstalled 'crosser:amd64' 'unrelated'
+       testdpkginstalled 'crosser:i386' 'crosser:armel'
+
+       testsuccess apt purge crosser:i386 crosser:armel -y --planner $1
+       testdpkgnotinstalled 'crosser:i386' 'crosser:armel' 'unrelated'

 }
 multiinstance 'internal'
 }
 multiinstance 'internal'