From 4b10240cca0dc0a4e82e42959545d2ae7e622d29 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Wed, 23 Nov 2016 17:32:20 +0100 Subject: [PATCH] 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. --- apt-pkg/deb/dpkgpm.cc | 216 +++++++++++++----- apt-pkg/deb/dpkgpm.h | 1 + .../test-bug-618288-multiarch-same-lockstep | 3 + test/integration/test-crossgrades | 13 +- 4 files changed, 169 insertions(+), 64 deletions(-) diff --git a/apt-pkg/deb/dpkgpm.cc b/apt-pkg/deb/dpkgpm.cc index 70e7592a7..abf91fe2a 100644 --- 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" - // 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]); - } 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") { - 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") { - d->progress->ConffilePrompt(pkgname, PackagesDone, PackagesTotal, - list[3]); + d->progress->ConffilePrompt(pkgname, PackagesDone, PackagesTotal, list[3]); 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) { - // 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); - 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 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; - pkgname = std::move(fullname); + pkgname = P.FullName(); 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 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 = ""; @@ -706,36 +795,7 @@ void pkgDPkgPM::ProcessDpkgStatusLine(char *line) 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; } @@ -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) { diff --git a/apt-pkg/deb/dpkgpm.h b/apt-pkg/deb/dpkgpm.h index 193754644..d1c2bcf41 100644 --- 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); + APT_HIDDEN void handleCrossUpgradeAction(std::string const &pkgname); protected: int pkgFailures; diff --git a/test/integration/test-bug-618288-multiarch-same-lockstep b/test/integration/test-bug-618288-multiarch-same-lockstep index d9582f6b6..eeecb40ee 100755 --- 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 + +# 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 d412546c1..30195d30f 100755 --- 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 'crosser' 'i386,armel' '3' 'experimental' 'Multi-Arch: same' setupaptarchive singleinstance() { @@ -43,7 +44,15 @@ multiinstance() { 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' -- 2.45.2