* [devel] PATCH for apt: custom callbacks @ 2021-06-30 13:09 Ivan Zakharyaschev 2021-06-30 13:54 ` Dmitry V. Levin 0 siblings, 1 reply; 16+ messages in thread From: Ivan Zakharyaschev @ 2021-06-30 13:09 UTC (permalink / raw) To: Oleg Solovyov; +Cc: devel [-- Attachment #1: Type: text/plain, Size: 12236 bytes --] Hello! Предлагается патч на apt (API), добавляющий custom callbacks. После чтения в общих чертах у меня не появилось замечаний по архитектуре (а также оформлению, стилю: git show --check; отдельные места, где возможны разные стилистические решения вполне соответствуют окружающему коду, а enforced style guide у нас отсутствует). apt имеет свой тип для callback-ов, особенности rpm скрыты, что соответствует общему подходу в apt. "Переводом" для rpm занимается функция pkgRPMLibPM::customCallback из apt-pkg/rpm/rpmpm.{h,cc}: static void * customCallback(const void * h, const rpmCallbackType what, const uint64_t amount, const uint64_t total, const void * pkgKey, void * data); Потом через неё rpm работает с apt-овыми callback-ами в apt-pkg/rpm/rpmpm.cc: struct CallbackData data; if (callback != nullptr ) { data.callback = callback; data.callbackData = callbackData; rc = rpmtsSetNotifyCallback(TS, customCallback, &data); } else { rc = rpmtsSetNotifyCallback(TS, rpmShowProgress, (void *) (unsigned long) notifyFlags); } Я ещё сейчас почитаю внимательнее, в первую очередь, чтобы изменения не приводили к изменению старого поведения. Вот патч -- вдруг у кого-то будут ценные замечания: $ git --no-pager show --reverse gears/sisyphus..276207.200/master commit 07dfdf44e9df71646815dcdf83f8e405bf6c988b Author: Oleg Solovyov <mcpain@altlinux.org> Date: Wed Dec 11 12:15:00 2019 +0300 Implemented generic callback system for package manager transactions We're introducing custom callback for higher layers (like packagekit), letting them pass their own callbacks to APT instead of using rpmShowProgress when it's necessary. It's useful in particular case of offline updating when packagekit can send messages to plymouth letting user know about transaction progress but because APT does not send anything since it's using rpmShowProgress, packagekit reports nothing because it's just nothing to report. diff --git a/apt-pkg/packagemanager.cc b/apt-pkg/packagemanager.cc index c2e2f08be..50bb304ea 100644 --- a/apt-pkg/packagemanager.cc +++ b/apt-pkg/packagemanager.cc @@ -661,11 +661,11 @@ pkgPackageManager::OrderResult pkgPackageManager::OrderInstall() // --------------------------------------------------------------------- /* This uses the filenames in FileNames and the information in the DepCache to perform the installation of packages.*/ -pkgPackageManager::OrderResult pkgPackageManager::DoInstall() +pkgPackageManager::OrderResult pkgPackageManager::DoInstall(PackageManagerCallback_t callback, void *callbackData) { OrderResult Res = OrderInstall(); if (Res != Failed) - if (Go() == false) + if (Go(callback, callbackData) == false) return Failed; return Res; } diff --git a/apt-pkg/packagemanager.h b/apt-pkg/packagemanager.h index 9c5f04825..dce84811f 100644 --- a/apt-pkg/packagemanager.h +++ b/apt-pkg/packagemanager.h @@ -45,6 +45,32 @@ class pkgDepCache; class pkgSourceList; class pkgOrderList; class pkgRecords; + +typedef enum aptCallbackType_e { + APTCALLBACK_UNKNOWN = 0, + APTCALLBACK_INST_PROGRESS, + APTCALLBACK_INST_START, + APTCALLBACK_INST_STOP, + APTCALLBACK_TRANS_PROGRESS, + APTCALLBACK_TRANS_START, + APTCALLBACK_TRANS_STOP, + APTCALLBACK_UNINST_PROGRESS, + APTCALLBACK_UNINST_START, + APTCALLBACK_UNINST_STOP, + APTCALLBACK_UNPACK_ERROR, + APTCALLBACK_CPIO_ERROR, + APTCALLBACK_SCRIPT_ERROR, + APTCALLBACK_SCRIPT_START, + APTCALLBACK_SCRIPT_STOP, + APTCALLBACK_ELEM_PROGRESS, +} aptCallbackType; + +typedef void (*PackageManagerCallback_t)(const char *nevra, + const aptCallbackType what, + const uint64_t amount, + const uint64_t total, + void *callbackData); + class pkgPackageManager : protected pkgCache::Namespace { public: @@ -76,7 +102,7 @@ class pkgPackageManager : protected pkgCache::Namespace virtual bool Install(PkgIterator /*Pkg*/,const string &/*File*/) {return false;} virtual bool Configure(PkgIterator /*Pkg*/) {return false;} virtual bool Remove(PkgIterator /*Pkg*/,bool /*Purge*/=false) {return false;} - virtual bool Go() {return true;} + virtual bool Go(PackageManagerCallback_t /*callback*/ = nullptr, void * /*callbackData*/ = nullptr) {return true;} virtual void Reset() {} public: @@ -84,7 +110,7 @@ class pkgPackageManager : protected pkgCache::Namespace // Main action members bool GetArchives(pkgAcquire *Owner,pkgSourceList *Sources, pkgRecords *Recs); - OrderResult DoInstall(); + OrderResult DoInstall(PackageManagerCallback_t callback = nullptr, void *callbackData = nullptr); bool FixMissing(); // If marks updating not supported, skip this step diff --git a/apt-pkg/rpm/rpmpm.cc b/apt-pkg/rpm/rpmpm.cc index 287de77e2..700031e1b 100644 --- a/apt-pkg/rpm/rpmpm.cc +++ b/apt-pkg/rpm/rpmpm.cc @@ -270,7 +270,7 @@ bool pkgRPMPM::RunScriptsWithPkgs(const char *Cnf) // RPMPM::Go - Run the sequence /*{{{*/ // --------------------------------------------------------------------- /* This globs the operations and calls rpm */ -bool pkgRPMPM::Go() +bool pkgRPMPM::Go(PackageManagerCallback_t callback, void *callbackData) { if (List.empty() == true) return true; @@ -356,7 +356,7 @@ bool pkgRPMPM::Go() } #endif - if (Process(install, upgrade, uninstall) == false) + if (Process(install, upgrade, uninstall, callback, callbackData) == false) Ret = false; #ifdef WITH_LUA @@ -685,7 +685,8 @@ bool pkgRPMExtPM::ExecRPM(Item::RPMOps op, const std::vector<apt_item> &files) bool pkgRPMExtPM::Process(const std::vector<apt_item> &install, const std::vector<apt_item> &upgrade, - const std::vector<apt_item> &uninstall) + const std::vector<apt_item> &uninstall, + PackageManagerCallback_t callback, void *callbackData) { if (uninstall.empty() == false) ExecRPM(Item::RPMErase, uninstall); @@ -786,9 +787,82 @@ bool pkgRPMLibPM::AddToTransaction(Item::RPMOps op, const std::vector<apt_item> return true; } +struct CallbackData +{ + PackageManagerCallback_t callback; + void *callbackData; +}; + +void * pkgRPMLibPM::customCallback(const void * h, + const rpmCallbackType what, + const uint64_t amount, + const uint64_t total, + const void * pkgKey, + void * data) +{ + /* When invoking rpmShowProgress, the last parameter is notifyFlags, + which ain't used when callback type is OPEN_FILE or CLOSE_FILE + so it's safe to just pass zero. */ + if (what == RPMCALLBACK_INST_OPEN_FILE || what == RPMCALLBACK_INST_CLOSE_FILE) + return rpmShowProgress(h, what, amount, total, pkgKey, 0); + + CallbackData *s = (CallbackData *) data; + PackageManagerCallback_t func = s->callback; + rpmtd td = nullptr; + + const char* nevra = nullptr; + if (h != nullptr) { + td = rpmtdNew(); + + // Get NEVRA for package + int rc = headerGet((rpmHeader) h, RPMTAG_NEVRA, td, HEADERGET_DEFAULT); + if (rc == 1) + nevra = rpmtdGetString(td); + } + +#define DEF_CASE(name) case RPMCALLBACK_##name: callbackType = APTCALLBACK_##name; break + + aptCallbackType callbackType = APTCALLBACK_UNKNOWN; + switch (what) { + DEF_CASE(INST_PROGRESS); + DEF_CASE(INST_START); + DEF_CASE(INST_STOP); + DEF_CASE(TRANS_PROGRESS); + DEF_CASE(TRANS_START); + DEF_CASE(TRANS_STOP); + DEF_CASE(UNINST_PROGRESS); + DEF_CASE(UNINST_START); + DEF_CASE(UNINST_STOP); + DEF_CASE(UNPACK_ERROR); + DEF_CASE(CPIO_ERROR); + DEF_CASE(SCRIPT_ERROR); + DEF_CASE(SCRIPT_START); + DEF_CASE(SCRIPT_STOP); + DEF_CASE(ELEM_PROGRESS); + +#undef DEF_CASE + default: + break; + } + + try { + func(nevra, callbackType, amount, total, s->callbackData); + } + catch (...) + { + } + + if (h != nullptr) { + rpmtdFreeData(td); + rpmtdFree(td); + } + return nullptr; +} + bool pkgRPMLibPM::Process(const std::vector<apt_item> &install, const std::vector<apt_item> &upgrade, - const std::vector<apt_item> &uninstall) + const std::vector<apt_item> &uninstall, + PackageManagerCallback_t callback, void *callbackData) { int rc = 0; bool Success = false; @@ -906,8 +980,17 @@ bool pkgRPMLibPM::Process(const std::vector<apt_item> &install, probFilter |= rpmtsFilterFlags(TS); rpmtsSetFlags(TS, (rpmtransFlags)(rpmtsFlags(TS) | tsFlags)); rpmtsClean(TS); - rc = rpmtsSetNotifyCallback(TS, rpmShowProgress, - (void *) (unsigned long) notifyFlags); + struct CallbackData data; + + if (callback != nullptr ) { + data.callback = callback; + data.callbackData = callbackData; + + rc = rpmtsSetNotifyCallback(TS, customCallback, &data); + } else { + rc = rpmtsSetNotifyCallback(TS, rpmShowProgress, + (void *) (unsigned long) notifyFlags); + } rc = rpmtsRun(TS, NULL, (rpmprobFilterFlags)probFilter); probs = rpmtsProblems(TS); diff --git a/apt-pkg/rpm/rpmpm.h b/apt-pkg/rpm/rpmpm.h index 8a01b80cc..157e113ee 100644 --- a/apt-pkg/rpm/rpmpm.h +++ b/apt-pkg/rpm/rpmpm.h @@ -49,9 +49,10 @@ class pkgRPMPM : public pkgPackageManager virtual bool Process(const std::vector<apt_item> &install, const std::vector<apt_item> &upgrade, - const std::vector<apt_item> &uninstall) {return false;} + const std::vector<apt_item> &uninstall, + PackageManagerCallback_t callback, void *callbackData) {return false;} - virtual bool Go() override; + virtual bool Go(PackageManagerCallback_t callback, void *callbackData) override; virtual void Reset() override; public: @@ -66,7 +67,8 @@ class pkgRPMExtPM : public pkgRPMPM bool ExecRPM(Item::RPMOps op, const std::vector<apt_item> &files); virtual bool Process(const std::vector<apt_item> &install, const std::vector<apt_item> &upgrade, - const std::vector<apt_item> &uninstall) override; + const std::vector<apt_item> &uninstall, + PackageManagerCallback_t callback, void *callbackData) override; public: pkgRPMExtPM(pkgDepCache *Cache); @@ -80,9 +82,13 @@ class pkgRPMLibPM : public pkgRPMPM bool ParseRpmOpts(const char *Cnf, int *tsFlags, int *probFilter); bool AddToTransaction(Item::RPMOps op, const std::vector<apt_item> &files); + static void * customCallback(const void * h, const rpmCallbackType what, + const uint64_t amount, const uint64_t total, + const void * pkgKey, void * data); virtual bool Process(const std::vector<apt_item> &install, const std::vector<apt_item> &upgrade, - const std::vector<apt_item> &uninstall) override; + const std::vector<apt_item> &uninstall, + PackageManagerCallback_t callback, void *callbackData) override; public: commit 1403c34da1bc0e22762146010a002114c2ac88da (HEAD -> callbacks, tag: 276207.200/0.5.15lorg2-alt73, 276207.200/master) Author: Oleg Solovyov <mcpain@altlinux.org> Date: Mon Jun 28 14:47:56 2021 +0300 0.5.15lorg2-alt73 - implement generic callback system diff --git a/apt.spec b/apt.spec index deee52a23..ddda19609 100644 --- a/apt.spec +++ b/apt.spec @@ -7,7 +7,7 @@ Name: apt Version: 0.5.15lorg2 -Release: alt72 +Release: alt73 Summary: Debian's Advanced Packaging Tool with RPM support Summary(ru_RU.UTF-8): Debian APT - Усовершенствованное средство управления пакетами с поддержкой RPM @@ -406,6 +406,9 @@ popd %_datadir/%name/tests/ %changelog +* Mon Jun 28 2021 Oleg Solovyov <mcpain@altlinux.org> 0.5.15lorg2-alt73 +- implement generic callback system + * Thu Mar 18 2021 Ivan Zakharyaschev <imz@altlinux.org> 0.5.15lorg2-alt72 - Cleaned up the code (thx Dmitry V. Levin ldv@; including quite a few commits cherry-picked from http://apt-rpm.org/scm/apt.git): -- Best regards, Ivan ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [devel] PATCH for apt: custom callbacks 2021-06-30 13:09 [devel] PATCH for apt: custom callbacks Ivan Zakharyaschev @ 2021-06-30 13:54 ` Dmitry V. Levin 2021-07-05 11:31 ` Oleg Solovyov 2021-07-06 10:53 ` Ivan Zakharyaschev 0 siblings, 2 replies; 16+ messages in thread From: Dmitry V. Levin @ 2021-06-30 13:54 UTC (permalink / raw) To: ALT Devel discussion list On Wed, Jun 30, 2021 at 04:09:40PM +0300, Ivan Zakharyaschev wrote: > Hello! > > Предлагается патч на apt (API), добавляющий custom callbacks. > > После чтения в общих чертах у меня не появилось замечаний по архитектуре > (а также оформлению, стилю: git show --check; отдельные места, где > возможны разные стилистические решения вполне соответствуют окружающему > коду, а enforced style guide у нас отсутствует). > > apt имеет свой тип для callback-ов, особенности rpm скрыты, что > соответствует общему подходу в apt. "Переводом" для rpm занимается функция > pkgRPMLibPM::customCallback из apt-pkg/rpm/rpmpm.{h,cc}: Кажется, апстрим apt-rpm (когда он ещё был) сильно переписал этот код по сравнению с тем, что есть у нас. Но лучше проверить, это может быть ложная память. -- ldv ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [devel] PATCH for apt: custom callbacks 2021-06-30 13:54 ` Dmitry V. Levin @ 2021-07-05 11:31 ` Oleg Solovyov 2021-07-06 10:53 ` Ivan Zakharyaschev 1 sibling, 0 replies; 16+ messages in thread From: Oleg Solovyov @ 2021-07-05 11:31 UTC (permalink / raw) To: devel В письме от среда, 30 июня 2021 г. 16:54:39 MSK пользователь Dmitry V. Levin написал: > Кажется, апстрим apt-rpm (когда он ещё был) сильно переписал этот код по > сравнению с тем, что есть у нас. Но лучше проверить, это может быть > ложная память. Появились какие-то новости? ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [devel] PATCH for apt: custom callbacks 2021-06-30 13:54 ` Dmitry V. Levin 2021-07-05 11:31 ` Oleg Solovyov @ 2021-07-06 10:53 ` Ivan Zakharyaschev 2021-07-06 10:58 ` Ivan Zakharyaschev 1 sibling, 1 reply; 16+ messages in thread From: Ivan Zakharyaschev @ 2021-07-06 10:53 UTC (permalink / raw) To: ALT Linux Team development discussions [-- Attachment #1: Type: text/plain, Size: 39060 bytes --] Hello! On Wed, 30 Jun 2021, Dmitry V. Levin wrote: > On Wed, Jun 30, 2021 at 04:09:40PM +0300, Ivan Zakharyaschev wrote: > > Предлагается патч на apt (API), добавляющий custom callbacks. > > > > После чтения в общих чертах у меня не появилось замечаний по архитектуре > > (а также оформлению, стилю: git show --check; отдельные места, где > > возможны разные стилистические решения вполне соответствуют окружающему > > коду, а enforced style guide у нас отсутствует). > > > > apt имеет свой тип для callback-ов, особенности rpm скрыты, что > > соответствует общему подходу в apt. "Переводом" для rpm занимается функция > > pkgRPMLibPM::customCallback из apt-pkg/rpm/rpmpm.{h,cc}: > > Кажется, апстрим apt-rpm (когда он ещё был) сильно переписал этот код по > сравнению с тем, что есть у нас. Но лучше проверить, это может быть > ложная память. Это правда. Для начала заметим эти коммиты: $ git --no-pager log -S rpmShowProgress apt-rpm/master commit 521c705aaff16ce8df5418d7a94fee4dc523cc8b Author: pmatilai <pmatilai> Date: Fri May 26 15:58:30 2006 +0000 - add repackage progress callbacks - report full progress, erasures included - still NeedsWork (tm) commit b6c71b30209f52af6fe7f4ae75744905fa3a5d79 Author: pmatilai <pmatilai> Date: Thu May 25 13:14:44 2006 +0000 - always use our own progress meter with internal pm - show arch for packages in progress (to clarify multilib situations) commit 7142d7fe26e117310b31a0f38a2e74e0bdc05618 Author: pmatilai <pmatilai> Date: Mon Jan 16 23:10:47 2006 +0000 - initial import of revision 374 from cnc У меня есть ещё в точности та же (по патчам) история apt-rpm, но получше оформленная, которую я нашёл на github (как описано в spec-файле); ещё graft-ы через replace на неё, так что дальше мне будет удобнее её исследовать, и там другие номера у коммитов: $ git --no-pager log -S rpmShowProgress apt-rpm@github/master ^gears/sisyphus commit a5de6ef3b8f156ddfdff8a5b110803bb7c362844 Author: Panu Matilainen <pmatilai@laiskiainen.org> Date: Fri May 26 15:58:30 2006 +0000 - add repackage progress callbacks - report full progress, erasures included - still NeedsWork (tm) commit 7a4bf4fae645462b27fbfbde940a98039eb7e045 Author: Panu Matilainen <pmatilai@laiskiainen.org> Date: Thu May 25 13:14:44 2006 +0000 - always use our own progress meter with internal pm - show arch for packages in progress (to clarify multilib situations) Подробности про код: $ git --no-pager log -S rpmShowProgress apt-rpm@github/master ^gears/sisyphus --reverse -p commit 7a4bf4fae645462b27fbfbde940a98039eb7e045 Author: Panu Matilainen <pmatilai@laiskiainen.org> Date: Thu May 25 13:14:44 2006 +0000 - always use our own progress meter with internal pm - show arch for packages in progress (to clarify multilib situations) diff --git a/apt-pkg/rpm/rpmpm.cc b/apt-pkg/rpm/rpmpm.cc index 13b0131f6..5df695b6c 100644 --- a/apt-pkg/rpm/rpmpm.cc +++ b/apt-pkg/rpm/rpmpm.cc @@ -36,7 +36,6 @@ #if RPM_VERSION >= 0x040100 #include <rpm/rpmdb.h> -#define packagesTotal rpmcliPackagesTotal #else #define rpmpsPrint(a,b) rpmProblemSetPrint(a,b) #define rpmpsFree(a) rpmProblemSetFree(a) @@ -44,9 +43,9 @@ #if RPM_VERSION < 0x040000 #define rpmtransFlags int #define rpmprobFilterFlags int -#include "rpmshowprogress.h" #endif #endif +#include "rpmshowprogress.h" // RPMPM::pkgRPMPM - Constructor /*{{{*/ // --------------------------------------------------------------------- @@ -874,11 +873,11 @@ bool pkgRPMLibPM::Process(vector<const char*> &install, probFilter |= rpmtsFilterFlags(TS); rpmtsSetFlags(TS, (rpmtransFlags)(rpmtsFlags(TS) | tsFlags)); rpmtsClean(TS); - rc = rpmtsSetNotifyCallback(TS, rpmShowProgress, (void *)notifyFlags); + rc = rpmtsSetNotifyCallback(TS, rpmpmShowProgress, (void *)notifyFlags); rc = rpmtsRun(TS, NULL, (rpmprobFilterFlags)probFilter); probs = rpmtsProblems(TS); #else - rc = rpmRunTransactions(TS, rpmShowProgress, (void *)notifyFlags, NULL, + rc = rpmRunTransactions(TS, rpmpmShowProgress, (void *)notifyFlags, NULL, &probs, (rpmtransFlags)tsFlags, (rpmprobFilterFlags)probFilter); #endif diff --git a/apt-pkg/rpm/rpmshowprogress.h b/apt-pkg/rpm/rpmshowprogress.h index ddc26a63d..41dd3fae8 100644 --- a/apt-pkg/rpm/rpmshowprogress.h +++ b/apt-pkg/rpm/rpmshowprogress.h @@ -46,12 +46,20 @@ static void printHash(const unsigned long amount, const unsigned long total) } } -void * rpmShowProgress(const Header h, +#if RPM_VERSION < 0x040000 +void * rpmpmShowProgress(const Header h, +#else +void * rpmpmShowProgress(const void * arg, +#endif const rpmCallbackType what, const unsigned long amount, const unsigned long total, const void * pkgKey, void * data) { +#if RPM_VERSION >= 0x040000 + Header h = (Header) arg; +#endif + char * s; int flags = (int) ((long)data); void * rc = NULL; @@ -81,7 +89,7 @@ void * rpmShowProgress(const Header h, if (h == NULL || !(flags & INSTALL_LABEL)) break; if (flags & INSTALL_HASH) { - s = headerSprintf(h, "%{NAME}", + s = headerSprintf(h, "%{NAME}.%{ARCH}", rpmTagTable, rpmHeaderFormats, NULL); if (isatty (STDOUT_FILENO)) fprintf(stdout, "%4d:%-23.23s", progressCurrent + 1, s); @@ -91,7 +99,7 @@ void * rpmShowProgress(const Header h, free(s); s = NULL; } else { - s = headerSprintf(h, "%{NAME}-%{VERSION}-%{RELEASE}", + s = headerSprintf(h, "%{NAME}-%{VERSION}-%{RELEASE}.%{ARCH}", rpmTagTable, rpmHeaderFormats, NULL); fprintf(stdout, "%s\n", s); (void) fflush(stdout); commit a5de6ef3b8f156ddfdff8a5b110803bb7c362844 Author: Panu Matilainen <pmatilai@laiskiainen.org> Date: Fri May 26 15:58:30 2006 +0000 - add repackage progress callbacks - report full progress, erasures included - still NeedsWork (tm) diff --git a/apt-pkg/rpm/rpmpm.cc b/apt-pkg/rpm/rpmpm.cc index 5df695b6c..ffdcd70a6 100644 --- a/apt-pkg/rpm/rpmpm.cc +++ b/apt-pkg/rpm/rpmpm.cc @@ -808,9 +808,9 @@ bool pkgRPMLibPM::Process(vector<const char*> &install, if (_config->FindI("quiet",0) >= 1) notifyFlags |= INSTALL_LABEL; - else if (Interactive == true) + else if (Interactive == true) notifyFlags |= INSTALL_LABEL | INSTALL_HASH; - else + else notifyFlags |= INSTALL_LABEL | INSTALL_PERCENT; if (uninstall.empty() == false) @@ -820,8 +820,10 @@ bool pkgRPMLibPM::Process(vector<const char*> &install, if (upgrade.empty() == false) AddToTransaction(Item::RPMUpgrade, upgrade); - // Setup the gauge used by rpmShowProgress. - packagesTotal = install.size()+upgrade.size(); + // FIXME: This ain't right because most things show up in upgrade + // even if they're really just installs, and repackaging isn't taken + // into account either. + packagesTotal = install.size() + upgrade.size() * 2 + uninstall.size(); #if RPM_VERSION >= 0x040100 if (_config->FindB("RPM::NoDeps", false) == false) { Здесь имя функции заменилось на rpmpmShowProgress; поищем дальше по нему: $ git --no-pager log -S rpmpmShowProgress apt-rpm@github/master ^gears/sisyphus --reverse --stat -p commit 7a4bf4fae645462b27fbfbde940a98039eb7e045 Author: Panu Matilainen <pmatilai@laiskiainen.org> Date: Thu May 25 13:14:44 2006 +0000 - always use our own progress meter with internal pm - show arch for packages in progress (to clarify multilib situations) --- apt-pkg/rpm/rpmpm.cc | 7 +++---- apt-pkg/rpm/rpmshowprogress.h | 14 +++++++++++--- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/apt-pkg/rpm/rpmpm.cc b/apt-pkg/rpm/rpmpm.cc index 13b0131f6..5df695b6c 100644 --- a/apt-pkg/rpm/rpmpm.cc +++ b/apt-pkg/rpm/rpmpm.cc @@ -36,7 +36,6 @@ #if RPM_VERSION >= 0x040100 #include <rpm/rpmdb.h> -#define packagesTotal rpmcliPackagesTotal #else #define rpmpsPrint(a,b) rpmProblemSetPrint(a,b) #define rpmpsFree(a) rpmProblemSetFree(a) @@ -44,9 +43,9 @@ #if RPM_VERSION < 0x040000 #define rpmtransFlags int #define rpmprobFilterFlags int -#include "rpmshowprogress.h" #endif #endif +#include "rpmshowprogress.h" // RPMPM::pkgRPMPM - Constructor /*{{{*/ // --------------------------------------------------------------------- @@ -874,11 +873,11 @@ bool pkgRPMLibPM::Process(vector<const char*> &install, probFilter |= rpmtsFilterFlags(TS); rpmtsSetFlags(TS, (rpmtransFlags)(rpmtsFlags(TS) | tsFlags)); rpmtsClean(TS); - rc = rpmtsSetNotifyCallback(TS, rpmShowProgress, (void *)notifyFlags); + rc = rpmtsSetNotifyCallback(TS, rpmpmShowProgress, (void *)notifyFlags); rc = rpmtsRun(TS, NULL, (rpmprobFilterFlags)probFilter); probs = rpmtsProblems(TS); #else - rc = rpmRunTransactions(TS, rpmShowProgress, (void *)notifyFlags, NULL, + rc = rpmRunTransactions(TS, rpmpmShowProgress, (void *)notifyFlags, NULL, &probs, (rpmtransFlags)tsFlags, (rpmprobFilterFlags)probFilter); #endif diff --git a/apt-pkg/rpm/rpmshowprogress.h b/apt-pkg/rpm/rpmshowprogress.h index ddc26a63d..41dd3fae8 100644 --- a/apt-pkg/rpm/rpmshowprogress.h +++ b/apt-pkg/rpm/rpmshowprogress.h @@ -46,12 +46,20 @@ static void printHash(const unsigned long amount, const unsigned long total) } } -void * rpmShowProgress(const Header h, +#if RPM_VERSION < 0x040000 +void * rpmpmShowProgress(const Header h, +#else +void * rpmpmShowProgress(const void * arg, +#endif const rpmCallbackType what, const unsigned long amount, const unsigned long total, const void * pkgKey, void * data) { +#if RPM_VERSION >= 0x040000 + Header h = (Header) arg; +#endif + char * s; int flags = (int) ((long)data); void * rc = NULL; @@ -81,7 +89,7 @@ void * rpmShowProgress(const Header h, if (h == NULL || !(flags & INSTALL_LABEL)) break; if (flags & INSTALL_HASH) { - s = headerSprintf(h, "%{NAME}", + s = headerSprintf(h, "%{NAME}.%{ARCH}", rpmTagTable, rpmHeaderFormats, NULL); if (isatty (STDOUT_FILENO)) fprintf(stdout, "%4d:%-23.23s", progressCurrent + 1, s); @@ -91,7 +99,7 @@ void * rpmShowProgress(const Header h, free(s); s = NULL; } else { - s = headerSprintf(h, "%{NAME}-%{VERSION}-%{RELEASE}", + s = headerSprintf(h, "%{NAME}-%{VERSION}-%{RELEASE}.%{ARCH}", rpmTagTable, rpmHeaderFormats, NULL); fprintf(stdout, "%s\n", s); (void) fflush(stdout); commit 8b1651fb32c7b93a0cd8454896f818343d2dda12 Author: Panu Matilainen <pmatilai@laiskiainen.org> Date: Thu May 25 13:31:38 2006 +0000 - move showprogress implementation out of the header file so that changes dont trigger rebuilding the whole dang thing --- apt-pkg/rpm/rpmshowprogress.cc | 152 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 152 insertions(+) diff --git a/apt-pkg/rpm/rpmshowprogress.cc b/apt-pkg/rpm/rpmshowprogress.cc new file mode 100644 index 000000000..ccae011d7 --- /dev/null +++ b/apt-pkg/rpm/rpmshowprogress.cc @@ -0,0 +1,152 @@ + +#include <stdio.h> +#include <rpm/rpmlib.h> +#include <apti18n.h> + +#include "rpmshowprogress.h" +// +// This code was originally from rpm 4.0.3. +// +static void printHash(const unsigned long amount, const unsigned long total) +{ + int hashesNeeded; + int hashesTotal = 50; + + if (isatty (STDOUT_FILENO)) + hashesTotal = 44; + + if (hashesPrinted != hashesTotal) { + hashesNeeded = (int)(hashesTotal*(total?(((float)amount)/total):1)); + while (hashesNeeded > hashesPrinted) { + if (isatty (STDOUT_FILENO)) { + int i; + for (i = 0; i < hashesPrinted; i++) (void) putchar ('#'); + for (; i < hashesTotal; i++) (void) putchar (' '); + fprintf(stdout, "(%3d%%)", + (int)(100 * (total ? (((float) amount) / total) : 1))); + for (i = 0; i < (hashesTotal + 6); i++) (void) putchar ('\b'); + } else + fprintf(stdout, "#"); + + hashesPrinted++; + } + (void) fflush(stdout); + hashesPrinted = hashesNeeded; + + if (hashesPrinted == hashesTotal) { + int i; + progressCurrent++; + if (isatty(STDOUT_FILENO)) { + for (i = 1; i < hashesPrinted; i++) (void) putchar ('#'); + fprintf(stdout, " [%3d%%]", (int)(100 * (progressTotal ? + (((float) progressCurrent) / progressTotal) : 1))); + } + fprintf(stdout, "\n"); + } + (void) fflush(stdout); + } +} + +#if RPM_VERSION < 0x040000 +void * rpmpmShowProgress(const Header h, +#else +void * rpmpmShowProgress(const void * arg, +#endif + const rpmCallbackType what, + const unsigned long amount, + const unsigned long total, + const void * pkgKey, void * data) +{ +#if RPM_VERSION >= 0x040000 + Header h = (Header) arg; +#endif + + char * s; + int flags = (int) ((long)data); + void * rc = NULL; + const char * filename = (const char *) pkgKey; + static FD_t fd = NULL; + + switch (what) { + case RPMCALLBACK_INST_OPEN_FILE: + if (filename == NULL || filename[0] == '\0') + return NULL; + fd = Fopen(filename, "r.ufdio"); + if (fd) + fd = fdLink(fd, "persist (showProgress)"); + return fd; + /*@notreached@*/ break; + + case RPMCALLBACK_INST_CLOSE_FILE: + fd = fdFree(fd, "persist (showProgress)"); + if (fd) { + (void) Fclose(fd); + fd = NULL; + } + break; + + case RPMCALLBACK_INST_START: + hashesPrinted = 0; + if (h == NULL || !(flags & INSTALL_LABEL)) + break; + if (flags & INSTALL_HASH) { + s = headerSprintf(h, "%{NAME}.%{ARCH}", + rpmTagTable, rpmHeaderFormats, NULL); + if (isatty (STDOUT_FILENO)) + fprintf(stdout, "%4d:%-23.23s", progressCurrent + 1, s); + else + fprintf(stdout, "%-28.28s", s); + (void) fflush(stdout); + free(s); + s = NULL; + } else { + s = headerSprintf(h, "%{NAME}-%{VERSION}-%{RELEASE}.%{ARCH}", + rpmTagTable, rpmHeaderFormats, NULL); + fprintf(stdout, "%s\n", s); + (void) fflush(stdout); + free(s); + s = NULL; + } + break; + + case RPMCALLBACK_TRANS_PROGRESS: + case RPMCALLBACK_INST_PROGRESS: + if (flags & INSTALL_PERCENT) + fprintf(stdout, "%%%% %f\n", (double) (total + ? ((((float) amount) / total) * 100) + : 100.0)); + else if (flags & INSTALL_HASH) + printHash(amount, total); + (void) fflush(stdout); + break; + + case RPMCALLBACK_TRANS_START: + hashesPrinted = 0; + progressTotal = 1; + progressCurrent = 0; + if (!(flags & INSTALL_LABEL)) + break; + if (flags & INSTALL_HASH) + fprintf(stdout, "%-28s", _("Preparing...")); + else + fprintf(stdout, "%s\n", _("Preparing...")); + (void) fflush(stdout); + break; + + case RPMCALLBACK_TRANS_STOP: + if (flags & INSTALL_HASH) + printHash(1, 1); /* Fixes "preparing..." progress bar */ + progressTotal = packagesTotal; + progressCurrent = 0; + break; + + case RPMCALLBACK_UNINST_PROGRESS: + case RPMCALLBACK_UNINST_START: + case RPMCALLBACK_UNINST_STOP: + /* ignore */ + break; + } + + return rc; +} + commit 8f940eff2b3061c3a80e8e92ef0cf0ba4c9deb02 Author: Panu Matilainen <pmatilai@laiskiainen.org> Date: Sat Feb 24 15:25:52 2007 +0200 - beginnings of install progress API --- apt-pkg/rpm/rpmpm.cc | 21 ++++++++------- apt-pkg/rpm/rpmshowprogress.cc | 227 ------------------------------------------------------------------------------------------------------------------------------------------------------------- apt-pkg/rpm/rpmshowprogress.h | 17 ------------ 3 files changed, 11 insertions(+), 254 deletions(-) diff --git a/apt-pkg/rpm/rpmpm.cc b/apt-pkg/rpm/rpmpm.cc index 61044bd57..91eeb3027 100644 --- a/apt-pkg/rpm/rpmpm.cc +++ b/apt-pkg/rpm/rpmpm.cc @@ -45,7 +45,9 @@ #define rpmprobFilterFlags int #endif #endif -#include "rpmshowprogress.h" +#include "rpmcallback.h" + +extern int packagesTotal; // RPMPM::pkgRPMPM - Constructor /*{{{*/ // --------------------------------------------------------------------- @@ -236,7 +238,7 @@ bool pkgRPMPM::RunScriptsWithPkgs(const char *Cnf) // RPMPM::Go - Run the sequence /*{{{*/ // --------------------------------------------------------------------- /* This globs the operations and calls rpm */ -bool pkgRPMPM::Go() +bool pkgRPMPM::Go(InstProgress &Prog) { if (List.empty() == true) return true; @@ -339,7 +341,7 @@ bool pkgRPMPM::Go() } #endif - if (Process(install, upgrade, uninstall) == false) + if (Process(Prog, install, upgrade, uninstall) == false) Ret = false; #ifdef APT_WITH_LUA @@ -638,7 +640,8 @@ bool pkgRPMExtPM::ExecRPM(Item::RPMOps op, vector<const char*> &files) return true; } -bool pkgRPMExtPM::Process(vector<const char*> &install, +bool pkgRPMExtPM::Process(InstProgress &Prog, + vector<const char*> &install, vector<const char*> &upgrade, vector<const char*> &uninstall) { @@ -742,7 +745,8 @@ bool pkgRPMLibPM::AddToTransaction(Item::RPMOps op, vector<const char*> &files) return true; } -bool pkgRPMLibPM::Process(vector<const char*> &install, +bool pkgRPMLibPM::Process(InstProgress &Prog, + vector<const char*> &install, vector<const char*> &upgrade, vector<const char*> &uninstall) { @@ -887,13 +891,12 @@ bool pkgRPMLibPM::Process(vector<const char*> &install, goto exit; } - cout << _("Committing changes...") << endl << flush; - + Prog.OverallProgress(0, 1, 1, "Committing changes..."); #if RPM_VERSION >= 0x040100 probFilter |= rpmtsFilterFlags(TS); rpmtsSetFlags(TS, (rpmtransFlags)(rpmtsFlags(TS) | tsFlags)); rpmtsClean(TS); - rc = rpmtsSetNotifyCallback(TS, rpmpmShowProgress, (void *)notifyFlags); + rc = rpmtsSetNotifyCallback(TS, rpmCallback, &Prog); rc = rpmtsRun(TS, NULL, (rpmprobFilterFlags)probFilter); probs = rpmtsProblems(TS); #else @@ -910,8 +913,6 @@ bool pkgRPMLibPM::Process(vector<const char*> &install, Success = true; if (rc < 0) _error->Warning(_("Some errors occurred while running transaction")); - else if (Interactive == true) - cout << _("Done.") << endl; } rpmpsFree(probs); diff --git a/apt-pkg/rpm/rpmshowprogress.cc b/apt-pkg/rpm/rpmshowprogress.cc deleted file mode 100644 index d3d61eb6a..000000000 --- a/apt-pkg/rpm/rpmshowprogress.cc +++ /dev/null @@ -1,227 +0,0 @@ - -#include <stdio.h> -#include <rpm/rpmlib.h> -#include <apti18n.h> - -#include "rpmshowprogress.h" - -static int hashesTotal = 0; -static int hashesCurrent = 0; -static int progressCurrent = 0; -static int progressTotal = 0; - -int packagesTotal; - -static void printHash(const unsigned long amount, const unsigned long total) -{ - int hashesNeeded; - - hashesTotal = (isatty (STDOUT_FILENO) ? 44 : 50); - - if (hashesCurrent != hashesTotal) { - float pct = (total ? (((float) amount) / total) : 1.0); - hashesNeeded = (int) ((hashesTotal * pct) + 0.5); - while (hashesNeeded > hashesCurrent) { - if (isatty (STDOUT_FILENO)) { - int i; - for (i = 0; i < hashesCurrent; i++) - (void) putchar ('#'); - for (; i < hashesTotal; i++) - (void) putchar (' '); - fprintf(stdout, "(%3d%%)", (int)((100 * pct) + 0.5)); - for (i = 0; i < (hashesTotal + 6); i++) - (void) putchar ('\b'); - } else - fprintf(stdout, "#"); - - hashesCurrent++; - } - (void) fflush(stdout); - - if (hashesCurrent == hashesTotal) { - int i; - progressCurrent++; - if (isatty(STDOUT_FILENO)) { - for (i = 1; i < hashesCurrent; i++) - (void) putchar ('#'); - pct = (progressTotal - ? (((float) progressCurrent) / progressTotal) - : 1); - fprintf(stdout, " [%3d%%]", (int)((100 * pct) + 0.5)); - } - fprintf(stdout, "\n"); - } - (void) fflush(stdout); - } -} - -#if RPM_VERSION < 0x040000 -void * rpmpmShowProgress(const Header h, -#else -void * rpmpmShowProgress(const void * arg, -#endif - const rpmCallbackType what, -#if RPM_VERSION >= 0x040405 - const unsigned long long amount, - const unsigned long long total, -#else - const unsigned long amount, - const unsigned long total, -#endif - const void * pkgKey, void * data) - -{ -#if RPM_VERSION >= 0x040000 - Header h = (Header) arg; -#endif - - char * s; - int flags = (int) ((long)data); - void * rc = NULL; - const char * filename = (const char *) pkgKey; - static FD_t fd = NULL; - static rpmCallbackType state; - static bool repackage; - - switch (what) { - case RPMCALLBACK_INST_OPEN_FILE: - if (filename == NULL || filename[0] == '\0') - return NULL; - fd = Fopen(filename, "r.ufdio"); - if (fd) - fd = fdLink(fd, "persist (showProgress)"); - return fd; - /*@notreached@*/ break; - - case RPMCALLBACK_INST_CLOSE_FILE: - fd = fdFree(fd, "persist (showProgress)"); - if (fd) { - (void) Fclose(fd); - fd = NULL; - } - break; - - case RPMCALLBACK_INST_START: - hashesCurrent = 0; - if (h == NULL || !(flags & INSTALL_LABEL)) - break; - - if (state != what && repackage == false) { - state = what; - fprintf(stdout, "%s\n", _("Installing / Updating...")); - (void) fflush(stdout); - } - - if (flags & INSTALL_HASH) { - s = headerSprintf(h, "%{NAME}.%{ARCH}", - rpmTagTable, rpmHeaderFormats, NULL); - if (isatty (STDOUT_FILENO)) - fprintf(stdout, "%4d:%-23.23s", progressCurrent + 1, s); - else - fprintf(stdout, "%-28.28s", s); - (void) fflush(stdout); - free(s); - s = NULL; - } else { - s = headerSprintf(h, "%{NAME}-%{VERSION}-%{RELEASE}.%{ARCH}", - rpmTagTable, rpmHeaderFormats, NULL); - fprintf(stdout, "%s\n", s); - (void) fflush(stdout); - free(s); - s = NULL; - } - break; - - case RPMCALLBACK_TRANS_PROGRESS: - case RPMCALLBACK_INST_PROGRESS: - if (flags & INSTALL_PERCENT) - fprintf(stdout, "%%%% %f\n", (double) (total - ? ((((float) amount) / total) * 100) - : 100.0)); - else if (flags & INSTALL_HASH) - printHash(amount, total); - (void) fflush(stdout); - break; - - case RPMCALLBACK_TRANS_START: - state = what; - repackage = false; - hashesCurrent = 0; - progressTotal = 1; - progressCurrent = 0; - if (!(flags & INSTALL_LABEL)) - break; - if (flags & INSTALL_HASH) - fprintf(stdout, "%-28s", _("Preparing...")); - else - fprintf(stdout, "%s\n", _("Preparing...")); - (void) fflush(stdout); - break; - - case RPMCALLBACK_TRANS_STOP: - if (flags & INSTALL_HASH) - printHash(1, 1); /* Fixes "preparing..." progress bar */ - progressTotal = packagesTotal; - progressCurrent = 0; - break; - - case RPMCALLBACK_REPACKAGE_START: - hashesCurrent = 0; - progressCurrent = 0; - repackage = true; - if (!(flags & INSTALL_LABEL)) - break; - if (flags & INSTALL_HASH) - fprintf(stdout, "%-28s\n", _("Repackaging...")); - else - fprintf(stdout, "%s\n", _("Repackaging...")); - (void) fflush(stdout); - break; - - case RPMCALLBACK_REPACKAGE_PROGRESS: - if (amount && (flags & INSTALL_HASH)) - printHash(1, 1); /* Fixes "preparing..." progress bar */ - break; - - case RPMCALLBACK_REPACKAGE_STOP: - progressTotal = total; - progressCurrent = total; - if (flags & INSTALL_HASH) - printHash(1, 1); /* Fixes "preparing..." progress bar */ - progressTotal = packagesTotal; - repackage = false; - break; - - case RPMCALLBACK_UNINST_PROGRESS: - break; - case RPMCALLBACK_UNINST_START: - hashesCurrent = 0; - if (!(flags & INSTALL_LABEL)) - break; - if (state != what) { - state = what; - fprintf(stdout, "%s\n", _("Removing / Cleaning up...")); - (void) fflush(stdout); - } - break; - - case RPMCALLBACK_UNINST_STOP: - if (h == NULL || !(flags & INSTALL_LABEL)) - break; - s = headerSprintf(h, "%{NAME}.%{ARCH}", rpmTagTable, rpmHeaderFormats, NULL); - if (flags & INSTALL_HASH) { - fprintf(stdout, "%4d:%-23.23s", progressCurrent + 1, s); - printHash(1, 1); - } else { - fprintf(stdout, "%-28.28s", s); - } - fflush(stdout); - s = NULL; - break; - default: // Fall through - break; - } - - return rc; -} - diff --git a/apt-pkg/rpm/rpmshowprogress.h b/apt-pkg/rpm/rpmshowprogress.h deleted file mode 100644 index 9c33f962e..000000000 --- a/apt-pkg/rpm/rpmshowprogress.h +++ /dev/null @@ -1,17 +0,0 @@ - -extern int packagesTotal; - -#if RPM_VERSION < 0x040000 -void * rpmpmShowProgress(const Header h, -#else -void * rpmpmShowProgress(const void * arg, -#endif - const rpmCallbackType what, -#if RPM_VERSION >= 0x040405 - const unsigned long long amount, - const unsigned long long total, -#else - const unsigned long amount, - const unsigned long total, -#endif - const void * pkgKey, void * data); commit fa650dbfb5757dceffb269cbd0c34d2e2e44ef7e Author: Panu Matilainen <pmatilai@laiskiainen.org> Date: Mon Mar 5 23:39:29 2007 +0200 - More progress meter work. Broken in various entertaining ways at the moment - Unbreak API by allowing DoInstall() to create "compatibility" progress meters. Compatibility inteded mostly for Synaptic, which has dirrrrrrrrrty hacks that break the pkgPackageManager class protections, invalidating our compat hacks. Oh well... --- apt-pkg/rpm/rpmpm.cc | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/apt-pkg/rpm/rpmpm.cc b/apt-pkg/rpm/rpmpm.cc index 91eeb3027..f7ab343d9 100644 --- a/apt-pkg/rpm/rpmpm.cc +++ b/apt-pkg/rpm/rpmpm.cc @@ -238,7 +238,7 @@ bool pkgRPMPM::RunScriptsWithPkgs(const char *Cnf) // RPMPM::Go - Run the sequence /*{{{*/ // --------------------------------------------------------------------- /* This globs the operations and calls rpm */ -bool pkgRPMPM::Go(InstProgress &Prog) +bool pkgRPMPM::Go() { if (List.empty() == true) return true; @@ -341,7 +341,7 @@ bool pkgRPMPM::Go(InstProgress &Prog) } #endif - if (Process(Prog, install, upgrade, uninstall) == false) + if (Process(install, upgrade, uninstall) == false) Ret = false; #ifdef APT_WITH_LUA @@ -640,10 +640,9 @@ bool pkgRPMExtPM::ExecRPM(Item::RPMOps op, vector<const char*> &files) return true; } -bool pkgRPMExtPM::Process(InstProgress &Prog, - vector<const char*> &install, - vector<const char*> &upgrade, - vector<const char*> &uninstall) +bool pkgRPMExtPM::Process(vector<const char*> &install, + vector<const char*> &upgrade, + vector<const char*> &uninstall) { if (uninstall.empty() == false) ExecRPM(Item::RPMErase, uninstall); @@ -745,8 +744,7 @@ bool pkgRPMLibPM::AddToTransaction(Item::RPMOps op, vector<const char*> &files) return true; } -bool pkgRPMLibPM::Process(InstProgress &Prog, - vector<const char*> &install, +bool pkgRPMLibPM::Process(vector<const char*> &install, vector<const char*> &upgrade, vector<const char*> &uninstall) { @@ -832,7 +830,8 @@ bool pkgRPMLibPM::Process(InstProgress &Prog, if (upgrade.empty() == false) AddToTransaction(Item::RPMUpgrade, upgrade); - packagesTotal = 0; + // XXX temp stuff.. + int packagesTotal = 0; // Figure out exactly how many rpm operations we're going to process, // repackages and all. int repackage = (tsFlags & RPMTRANS_FLAG_REPACKAGE) ? 1 : 0; @@ -891,16 +890,16 @@ bool pkgRPMLibPM::Process(InstProgress &Prog, goto exit; } - Prog.OverallProgress(0, 1, 1, "Committing changes..."); + Progress->OverallProgress(0, 1, 1, "Committing changes..."); #if RPM_VERSION >= 0x040100 probFilter |= rpmtsFilterFlags(TS); rpmtsSetFlags(TS, (rpmtransFlags)(rpmtsFlags(TS) | tsFlags)); rpmtsClean(TS); - rc = rpmtsSetNotifyCallback(TS, rpmCallback, &Prog); + rc = rpmtsSetNotifyCallback(TS, rpmCallback, Progress); rc = rpmtsRun(TS, NULL, (rpmprobFilterFlags)probFilter); probs = rpmtsProblems(TS); #else - rc = rpmRunTransactions(TS, rpmpmShowProgress, (void *)notifyFlags, NULL, + rc = rpmRunTransactions(TS, rpmCallback, Progress, NULL, &probs, (rpmtransFlags)tsFlags, (rpmprobFilterFlags)probFilter); #endif rpmpmShowProgress заменяется на rpmCallback. Поищем по нему: $ git --no-pager log -S rpmCallback apt-rpm@github/master ^gears/sisyphus --reverse --stat --pickaxe-all commit 8b1651fb32c7b93a0cd8454896f818343d2dda12 Author: Panu Matilainen <pmatilai@laiskiainen.org> Date: Thu May 25 13:31:38 2006 +0000 - move showprogress implementation out of the header file so that changes dont trigger rebuilding the whole dang thing apt-pkg/Makefile.am | 1 + apt-pkg/Makefile.in | 22 ++++++++++++++-------- apt-pkg/rpm/rpmshowprogress.cc | 152 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ apt-pkg/rpm/rpmshowprogress.h | 140 +++----------------------------------------------------------------------------------------------------------------------------------------- 4 files changed, 170 insertions(+), 145 deletions(-) commit a1a543f32bff361343759d08a85661a69f748726 Author: Panu Matilainen <pmatilai@laiskiainen.org> Date: Thu May 25 14:15:26 2006 +0000 - initial erasure callbacks apt-pkg/rpm/rpmshowprogress.cc | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) commit 8f940eff2b3061c3a80e8e92ef0cf0ba4c9deb02 Author: Panu Matilainen <pmatilai@laiskiainen.org> Date: Sat Feb 24 15:25:52 2007 +0200 - beginnings of install progress API apt-pkg/Makefile.am | 4 +-- apt-pkg/Makefile.in | 17 ++++++------ apt-pkg/contrib/progress.cc | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ apt-pkg/contrib/progress.h | 27 +++++++++++++++++++ apt-pkg/packagemanager.cc | 4 +-- apt-pkg/packagemanager.h | 5 ++-- apt-pkg/rpm/rpmpm.cc | 21 ++++++++------- apt-pkg/rpm/rpmpm.h | 11 +++++--- apt-pkg/rpm/rpmshowprogress.cc | 227 ------------------------------------------------------------------------------------------------------------------------------------------------------------- apt-pkg/rpm/rpmshowprogress.h | 17 ------------ cmdline/apt-get.cc | 6 +++-- cmdline/apt-shell.cc | 6 +++-- 12 files changed, 153 insertions(+), 277 deletions(-) commit a6b3e3642cd9635a887d8f56b3275684e8602c4a Author: Panu Matilainen <pmatilai@laiskiainen.org> Date: Mon Feb 26 08:53:20 2007 +0200 - remember to add rpmcallback files to git, duh apt-pkg/rpm/rpmcallback.cc | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ apt-pkg/rpm/rpmcallback.h | 20 ++++++++++++++++++++ 2 files changed, 150 insertions(+) commit fa650dbfb5757dceffb269cbd0c34d2e2e44ef7e Author: Panu Matilainen <pmatilai@laiskiainen.org> Date: Mon Mar 5 23:39:29 2007 +0200 - More progress meter work. Broken in various entertaining ways at the moment - Unbreak API by allowing DoInstall() to create "compatibility" progress meters. Compatibility inteded mostly for Synaptic, which has dirrrrrrrrrty hacks that break the pkgPackageManager class protections, invalidating our compat hacks. Oh well... apt-pkg/contrib/progress.cc | 129 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------------------------------------- apt-pkg/contrib/progress.h | 51 +++++++++++++++++++++++++++++++++++++-------------- apt-pkg/packagemanager.cc | 18 ++++++++++++++++-- apt-pkg/packagemanager.h | 6 ++++-- apt-pkg/rpm/rpmcallback.cc | 70 ++++++++++++++++++++++++++++++++++++++++++++++------------------------ apt-pkg/rpm/rpmpm.cc | 23 +++++++++++------------ apt-pkg/rpm/rpmpm.h | 23 ++++++++++------------- cmdline/apt-get.cc | 6 ++---- cmdline/apt-shell.cc | 6 ++---- 9 files changed, 195 insertions(+), 137 deletions(-) commit 882ff480abef8ebf88398335a7392db85272cf4a Author: Panu Matilainen <pmatilai@laiskiainen.org> Date: Sun Jun 8 09:16:24 2008 +0300 Use raptFoo types as necessary in rpmhandler & friends apt-pkg/rpm/aptcallback.cc | 4 ++-- apt-pkg/rpm/aptcallback.h | 11 +++-------- apt-pkg/rpm/rpmhandler.cc | 42 ++++++++++++++++++------------------------ apt-pkg/rpm/rpmhandler.h | 5 +++-- apt-pkg/rpm/rpmversion.cc | 5 +++-- 5 files changed, 29 insertions(+), 38 deletions(-) commit 502243cccdf802d31e998afb679e02bb56e48f43 Author: Panu Matilainen <pmatilai@laiskiainen.org> Date: Tue Nov 23 21:45:28 2010 +0200 Rip out support for RPM v3. Time to let go of something... apt-pkg/rpm/aptcallback.cc | 6 ------ apt-pkg/rpm/aptcallback.h | 4 ---- apt-pkg/rpm/raptheader.cc | 2 -- apt-pkg/rpm/rpmhandler.cc | 45 +-------------------------------------------- apt-pkg/rpm/rpmhandler.h | 2 -- apt-pkg/rpm/rpmpm.cc | 28 ---------------------------- apt-pkg/rpm/rpmpm.h | 3 --- tools/genpkglist.cc | 2 -- tools/gensrclist.cc | 2 -- 9 files changed, 1 insertion(+), 93 deletions(-) commit 8c88e2177a7b29cc36eca603eef0937116c514c0 Author: Panu Matilainen <pmatilai@laiskiainen.org> Date: Thu Apr 7 18:25:48 2011 +0300 Chainsaw support for rpm < 4.6.0 out for real apt-pkg/rpm/raptheader.cc | 73 +------------------------------------------------------------------------ apt-pkg/rpm/raptheader.h | 4 ---- apt-pkg/rpm/rapttypes.h | 18 ------------------ apt-pkg/rpm/rpmhandler.cc | 126 ++---------------------------------------------------------------------------------------------------------------------------- apt-pkg/rpm/rpmhandler.h | 6 ------ apt-pkg/rpm/rpmpackagedata.cc | 2 -- apt-pkg/rpm/rpmpm.cc | 89 +++++------------------------------------------------------------------------------------ apt-pkg/rpm/rpmpm.h | 7 ------- apt-pkg/rpm/rpmsystem.cc | 11 ----------- apt-pkg/rpm/rpmversion.cc | 10 ---------- configure.ac | 27 ++------------------------- tools/genpkglist.cc | 13 ------------- tools/gensrclist.cc | 16 ---------------- 13 files changed, 10 insertions(+), 392 deletions(-) Тут можно заметить, основные релефантные файлы такие, можно по ним историю отфильтровать: $ git --no-pager log --oneline apt-rpm@github/master -- apt-pkg/{contrib/progress\*,apt-pkg/rpm/rpmcallback\*} 462c64e8d Add bunch of missing includes to fix build with gcc 4.7 ae1584ad3 progress.cc (OpProgress::CheckChange): optimize redundant gettimeofday() calls f51e0bac4 Remove all interface/implementation #pragmas - hardly useful, eliminated from debian apt too d16036df4 Remove bogus semicolons in inline method definitions (Bernhard Rosenkränzer) f7db55202 Preliminaries for quiet option in progress class 32a76471d Silly install progress cosmetics c4da25f05 Don't call installprogress Done() from destructor, not a very good idea to begin with and causes crash n burn in some situations (rhbz#419811). Since rpm doesn't issue "all done" callback, manually call Done() after transaction finishes. c782c159b Explicitly include <cstring> where needed, in gcc 4.3 <string> no longer automatically includes it. 5f5fe0a6f - more descriptive progress messages - make more space for package nevra info in hash progress meter e3089a009 - working install, remove and upgrade progress .. kinda ef49b0639 - progress meter cosmetics fa650dbfb - More progress meter work. Broken in various entertaining ways at the moment - Unbreak API by allowing DoInstall() to create "compatibility" progress meters. Compatibility inteded mostly for Synaptic, which has dirrrrrrrrrty hacks that break the pkgPackageManager class protections, invalidating our compat hacks. Oh well... 11c8ba548 - show package name when upgrading/removing - remove debug junk 8f940eff2 - beginnings of install progress API fffc37671 - various "cosmetics" cleanups to shut up gcc complaints on higher warning levels (Ralf) 794485f9c - fix opening *.cc and *.h files in proper C++ mode in xemacs as well (Ville Skyttä) c27ced7a9 Merging apt-rpm 0.5.5cnc1 over upstream APT in development trunk. А у нас в истории это место трогается вот где: $ git --no-pager log --oneline gears/sisyphus -- {apt/,}apt-pkg/{contrib/progress\*,apt-pkg/rpm/rpmcallback\*} 4baf89c7a optimize: Avoid copying objects (Might affect ABI; but tiny visible API changes) 1e8f85a1c Remove bogus semicolons in inline method definitions (Bernhard Rosenkränzer) 1a3583734 API changes: Use 'override' keyword 4597a9c3d Remove all interface/implementation #pragmas 702382b05 Fix remaining whitespace issues 2bcb8712b Strip trailing whitespaces b9f992810 Eliminate apt subdirectory 6f66568cc Revert a 0.5.15lorg2-alt70 helpful stylistic change to be applied later: "Use 'override' keyword" 093aa4083 Revert a 0.5.15lorg2-alt70 optimization change to be rebased over other cleanup: "Avoid copying objects" 8dfa25bde Revert a 0.5.15lorg2-alt70 change, which is too rude: "Support large files" 297a12d92 (refs/ATTIC/fix-longlong0_/BASE) Support large files c742cecda (refs/ATTIC/avoid-copy_/REVIEW-BASE) Avoid copying objects ec437de01 Use 'override' keyword b631357d8 (for Emacs) add apt/.dir-locals.el; remove unknown mode specifications in files 79b66f684 Apply apt-0.5.15lorg2-alt-lfs.patch 6bbc09c43 Apply apt-0.5.15lorg2-alt-gcc4.3.patch 321234252 (tag: 0.5.5cnc1-alt3) 0.5.5cnc1-alt3 e3ad0ef69 (tag: 0.3.19cnc55-alt3) 0.3.19cnc55-alt3 У меня сложилось представление (может быть, я ошибаюсь), что там не две разные функции rpmShowProgress vs rpmCallback используются, а один путь, и особая обработка прогресса предполагается что будет реализовываться через расширение класса OpProgress из contrib/progress.h. -- Best regards, Ivan ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [devel] PATCH for apt: custom callbacks 2021-07-06 10:53 ` Ivan Zakharyaschev @ 2021-07-06 10:58 ` Ivan Zakharyaschev 2021-07-15 12:24 ` Ivan Zakharyaschev 2021-11-09 12:14 ` Dmitry V. Levin 0 siblings, 2 replies; 16+ messages in thread From: Ivan Zakharyaschev @ 2021-07-06 10:58 UTC (permalink / raw) To: ALT Linux Team development discussions [-- Attachment #1: Type: text/plain, Size: 2050 bytes --] Небольший вывод, к которому я пришёл (может быть, ошибочно), я сформулировал в конце письма. На случай, если кто-то не дочитал до конца, вот он: On Tue, 6 Jul 2021, Ivan Zakharyaschev wrote: > On Wed, 30 Jun 2021, Dmitry V. Levin wrote: > > > Предлагается патч на apt (API), добавляющий custom callbacks. > > > > > > После чтения в общих чертах у меня не появилось замечаний по архитектуре > > > (а также оформлению, стилю: git show --check; отдельные места, где > > > возможны разные стилистические решения вполне соответствуют окружающему > > > коду, а enforced style guide у нас отсутствует). > > > > > > apt имеет свой тип для callback-ов, особенности rpm скрыты, что > > > соответствует общему подходу в apt. "Переводом" для rpm занимается функция > > > pkgRPMLibPM::customCallback из apt-pkg/rpm/rpmpm.{h,cc}: > > > > Кажется, апстрим apt-rpm (когда он ещё был) сильно переписал этот код по > > сравнению с тем, что есть у нас. Но лучше проверить, это может быть > > ложная память. > > Это правда. > У меня сложилось представление (может быть, я ошибаюсь), что там не > две разные функции rpmShowProgress vs rpmCallback используются, а один > путь, и особая обработка прогресса предполагается что будет > реализовываться через расширение класса OpProgress из contrib/progress.h. > > -- > Best regards, > Ivan ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [devel] PATCH for apt: custom callbacks 2021-07-06 10:58 ` Ivan Zakharyaschev @ 2021-07-15 12:24 ` Ivan Zakharyaschev 2021-07-15 12:38 ` Oleg Solovyov 2021-11-09 11:57 ` Dmitry V. Levin 2021-11-09 12:14 ` Dmitry V. Levin 1 sibling, 2 replies; 16+ messages in thread From: Ivan Zakharyaschev @ 2021-07-15 12:24 UTC (permalink / raw) To: ALT Linux Team development discussions [-- Attachment #1: Type: text/plain, Size: 18635 bytes --] Здравствуйте! Пока ясного согласия мейнтейнеров по общей архитектуре custom callbacks в apt нет, уделю внимание двум частным вопросам. Во-первых, виртуальные методы с дефолтными параметрами -- запутывающая фича в C++. (Потому что дефолтные значения подбираются не через механизм таблиц виртуальных методов, т.е. не так, как реализация метода.) Пример из обсуждаемого патча: @@ -75,17 +101,17 @@ class pkgPackageManager : protected pkgCache::Namespace // The Actual installation implementation virtual bool Install(PkgIterator /*Pkg*/,const string &/*File*/) {return false;} virtual bool Configure(PkgIterator /*Pkg*/) {return false;} virtual bool Remove(PkgIterator /*Pkg*/,bool /*Purge*/=false) {return false;} - virtual bool Go() {return true;} + virtual bool Go(PackageManagerCallback_t /*callback*/ = nullptr, void * /*callbackData*/ = nullptr) {return true;} virtual void Reset() {} public: Так что я в целом в коде поизбавлялся от всех таких старых мест, и новое переписал аналогично. См. ветку callbacks у меня. (Вставлю патчи в конце сообщения.) Во-вторых, сигнатура метода меняется (при этом дефолтные параметры не надо учитывать, потому что они другим механизмом подбираются), поэтому обычное поведение мейнтейнеров было бы изменить soname. Однако, это всё (что связано с package manager и взиамодействием с rpm) -- довольно внутренняя часть apt, нечасто используемая внешними клиентами библиотеки (насколько я понимаю). А также: часть изменений функций у нас в ALT поймает rpm с set-versions на библиотеки. Эти два соображения могли бы быть аргументом, чтобы не менять soname и не пересобирать весь набор клиентов. Что думаете? Менять soname? Ну и другие мейнтейнеры apt -- за то, чтбы сделать по-своему пока это API без забирания нарботок из apt-rpm, против, или им всё равно? Если за сближение с apt-rpm, то кто возьмётся? Мои патчи с избавлением от дефолтных параметров в виртуаьных методах, которые я предлагаю оставить в любом случае (за исключением обсуждаемого вопроса с soname): $ git --no-pager log -p --reverse test-pk..virtual-has-no-default-param commit 8f35f2dff70dfe39b853fdef0ac4516dae93f1cb Author: Ivan Zakharyaschev <imz@altlinux.org> Date: Fri Jul 9 20:57:31 2021 +0300 (.spec) move some (non-idempotent) actions into %%prep diff --git a/apt.spec b/apt.spec index deee52a23..3506f3560 100644 --- a/apt.spec +++ b/apt.spec @@ -225,7 +225,6 @@ This package contains test suite for APT. %prep %setup -n %name-%version-%release -%build # Fix url. sed -i 's,/usr/share/common-licenses/GPL,/usr/share/license/GPL,' COPYING @@ -235,6 +234,7 @@ sed -i 's, > /dev/null 2>&1,,' buildlib/tools.m4 # Add trivial arch translation. printf '%_target_cpu\t%_target_cpu' >> buildlib/archtable +%build gettextize --force --quiet --no-changelog --symlink %autoreconf commit 350aaf2dc5fdca23dd447cc3213f091912830f4b Author: Ivan Zakharyaschev <imz@altlinux.org> Date: Fri Jul 9 21:04:01 2021 +0300 (.spec) add a hook to verify that the source conforms to some policies Will be useful when rebasing our patches (on newer apt-rpm or Debian releases) and enforcing the policies earlier than in our patches now. And in other ways during maintenance. diff --git a/apt.spec b/apt.spec index 3506f3560..97db63ea6 100644 --- a/apt.spec +++ b/apt.spec @@ -225,6 +225,8 @@ This package contains test suite for APT. %prep %setup -n %name-%version-%release +./verify-src.sh + # Fix url. sed -i 's,/usr/share/common-licenses/GPL,/usr/share/license/GPL,' COPYING diff --git a/verify-src.sh b/verify-src.sh new file mode 100755 index 000000000..3897da4b2 --- /dev/null +++ b/verify-src.sh @@ -0,0 +1,26 @@ +#!/bin/sh -euC +set -o pipefail +shopt -s nullglob + +# Verify that the source code conforms to some policies. +# (Some policies--other or the same--are verified by compiler flags.) +# It's to be invoked from the .spec file; and one could invoke it +# from a Git hook on commit. + +MY="$(dirname -- "$(realpath -- "$0")")" +readonly MY + +for f in "$MY"/verify-src.d/*; do + [ -x "$f" ] || continue + case "$f" in + *~) + continue + ;; + *.verify) + "$f" + ;; + *) # ignore errors + "$f" ||: + ;; + esac +done commit a1b9a98c7de2eac657e84a3f8fb98d9c16abc56d Author: Ivan Zakharyaschev <imz@altlinux.org> Date: Fri Jul 9 21:22:38 2021 +0300 add verify-src.d/virtual-has-no-default-param.sh (result ignored) diff --git a/verify-src.d/virtual-has-no-default-param.sh b/verify-src.d/virtual-has-no-default-param.sh new file mode 100755 index 000000000..eb3e5cb60 --- /dev/null +++ b/verify-src.d/virtual-has-no-default-param.sh @@ -0,0 +1,19 @@ +#!/bin/sh -efuC +set -o pipefail + +# sed is simpler than grep (especially, under xarg) w.r.t. the exit status. +# However, it's more complex to output the filename and line number. +# (= outputs a line number, but it counts total lines in all files.) +virtual_with_default_param="$( + find -type f -'(' -name '*.h' -or -name '*.cc' -')' -print0 \ + | xargs -0 --no-run-if-empty \ + sed -nEe '/virtual [^(]*\([^)]*=|=.*\).*override/ {F; p; }' +)" +readonly virtual_with_default_param + +[ -z "$virtual_with_default_param" ] || { + printf '%s: Found default parameters in virtual methods:\n%s\n' \ + "${0##*/}" \ + "$virtual_with_default_param" + exit 1 +} >&2 commit d531f9f49e491fb9ca7b6bb87fcf9e3f446388dd Author: Ivan Zakharyaschev <imz@altlinux.org> Date: Fri Jul 9 22:44:43 2021 +0300 bump soversion before changing virtual methods' signatures In general, if a virtual method is added or deleted or modified (the signature), the table of virtual methods is modified, and old positions of any methods might be invalid. So, any code using this base class or derived classes (and calling its virtual methods) would be invalid without recompilation. Another upcoming change is in the default parameters of virtual methods. The default values are taken from the type known at the place of the invocation (i.e., they are not inherited or looked up through the table of virtual methods). For them to be seen and take effect, the code that uses these methods must be recompiled. And in any case, it's preferable not have default parameters in virtual methods (because of the confusion described above: different values can be declared in different derived classes, but they are used differently from how the definitions of virtual methods are looked up). So, I want to rewrite this API. Luckily, it is mostly present only in the API related to package manager (so, mostly internal to APT), as can be seen from a run of verify-src.d/virtual-has-no-default-param.sh. The classes (and their methods) that will be affected in the upcoming changes: [only moving the default parameters out of virtual methods declaration; not modifying the signatures and hence probably not affecting ABI and not requiring the bump of the soname] * virtual pkgSystem::UnLock() (from pkgsystem.h) * virtual pkgPackageManager::Remove() (from packagemanager.h) * virtual RPMHandler::DepsList() (from rpm/rpmhandler.h) * virtual pkgIndexFile::Describe() (from indexfile.h) [modifying the signature--if the new API is accepted after a review; default parameters shouldn't be taken into account when deciding whether this affects ABI] * virtual pkgPackageManager::Go(), non-virtual pkgPackageManager::DoInstall() (from packagemanager.h) * virtual pkgRPMPM::Process() (from rpm/rpmpm.h) Let's decide whether a modification of the methods in these classes deserves a change of the soname... (Usually, when a function is deleted or its signature is modified or a structure is modified, the soversion is bumped. But in ALT, the deletion of a function can be detected by RPM's set-versions for libraries, so it's not that necessary. However, I'm in doubts concerning the modification of virtual methods, i.e., will a deletion of the method with the old signature be detected by RPM if this method is used in other packages.) diff --git a/apt-pkg/Makefile.am b/apt-pkg/Makefile.am index 171d4b771..c940c567c 100644 --- a/apt-pkg/Makefile.am +++ b/apt-pkg/Makefile.am @@ -2,7 +2,7 @@ lib_LTLIBRARIES = libapt-pkg.la libapt_pkg_la_LIBADD = @RPMLIBS@ -libapt_pkg_la_LDFLAGS = -version-info 10:0:1 -release @GLIBC_VER@-@LIBSTDCPP_VER@@FILE_OFFSET_BITS_SUFFIX@ +libapt_pkg_la_LDFLAGS = -version-info 11:0:1 -release @GLIBC_VER@-@LIBSTDCPP_VER@@FILE_OFFSET_BITS_SUFFIX@ AM_CPPFLAGS = -DLIBDIR=\"$(libdir)\" AM_CFLAGS = $(WARN_CFLAGS) commit a82df01b332a7a6977e7a5ece342733189c338d1 Author: Ivan Zakharyaschev <imz@altlinux.org> Date: Thu Jul 15 12:24:01 2021 +0300 move a default parameter from virtual pkgSystem::UnLock() to avoid confusion Default parameters in virtual methods are confusing, because they are not looked up in the same way as a virtual method's implementation (through the table of virtual methods of the object; but rather from the "known" type in the code that uses the method). Luckily, this occurred in pkgSystem class (a thing mostly internal for APT). This change probably doesn't affect ABI, since the table of virtual methods is not modified. After recompilation, the new code will behave equivalently to the old one; but if one changes the default value, this will probably take effect only after recompilation (because the new UnLock() looks like an inlined method defined in the header). diff --git a/apt-pkg/deb/debsystem.h b/apt-pkg/deb/debsystem.h index c056cc9bf..75e396c47 100644 --- a/apt-pkg/deb/debsystem.h +++ b/apt-pkg/deb/debsystem.h @@ -24,7 +24,7 @@ class debSystem : public pkgSystem public: virtual bool Lock(); - virtual bool UnLock(bool NoErrors = false); + virtual bool UnLock(bool NoErrors); virtual pkgPackageManager *CreatePM(pkgDepCache *Cache) const; virtual bool Initialize(Configuration &Cnf); virtual bool ArchiveSupported(const char *Type); diff --git a/apt-pkg/pkgsystem.h b/apt-pkg/pkgsystem.h index 0e63ab984..e20212726 100644 --- a/apt-pkg/pkgsystem.h +++ b/apt-pkg/pkgsystem.h @@ -62,7 +62,9 @@ class pkgSystem /* Prevent other programs from touching shared data not covered by other locks (cache or state locks) */ virtual bool Lock() = 0; - virtual bool UnLock(bool NoErrors = false) = 0; + virtual bool UnLock(bool NoErrors) = 0; + /* a virtual method with default parameter is confusing; instead, define: */ + bool UnLock() { return UnLock(false); } // CNC:2002-07-06 virtual bool LockRead() {return true;} diff --git a/apt-pkg/rpm/rpmsystem.h b/apt-pkg/rpm/rpmsystem.h index e52490bf4..8847c531e 100644 --- a/apt-pkg/rpm/rpmsystem.h +++ b/apt-pkg/rpm/rpmsystem.h @@ -34,7 +34,7 @@ class rpmSystem : public pkgSystem virtual bool LockRead() override; virtual bool Lock() override; - virtual bool UnLock(bool NoErrors = false) override; + virtual bool UnLock(bool NoErrors) override; virtual pkgPackageManager *CreatePM(pkgDepCache *Cache) const override; virtual bool Initialize(Configuration &Cnf) override; virtual bool ArchiveSupported(const char *Type) override; commit 8015eabc1c4273244a5e722a4e31a308e19192e9 Author: Ivan Zakharyaschev <imz@altlinux.org> Date: Thu Jul 15 12:50:35 2021 +0300 move a default parameter from virtual pkgPackageManager::Remove() to avoid confusion Default parameters in virtual methods are confusing, because they are not looked up in the same way as a virtual method's implementation (through the table of virtual methods of the object; but rather from the "known" type in the code that uses the method). Luckily, this occurred in pkgPackageManager class (a thing mostly internal for APT). This change probably doesn't affect ABI, since the table of virtual methods is not modified. After recompilation, the new code will behave equivalently to the old one; but if one changes the default value, this will probably take effect only after recompilation (because the new Remove() looks like an inlined method defined in the header). diff --git a/apt-pkg/deb/dpkgpm.h b/apt-pkg/deb/dpkgpm.h index b768850bf..fc4683fbf 100644 --- a/apt-pkg/deb/dpkgpm.h +++ b/apt-pkg/deb/dpkgpm.h @@ -39,7 +39,7 @@ class pkgDPkgPM : public pkgPackageManager // The Actuall installation implementation virtual bool Install(PkgIterator Pkg,string File); virtual bool Configure(PkgIterator Pkg); - virtual bool Remove(PkgIterator Pkg,bool Purge = false); + virtual bool Remove(PkgIterator Pkg,bool Purge); virtual bool Go(); virtual void Reset(); diff --git a/apt-pkg/packagemanager.h b/apt-pkg/packagemanager.h index 9c5f04825..aeb1c8a02 100644 --- a/apt-pkg/packagemanager.h +++ b/apt-pkg/packagemanager.h @@ -75,7 +75,9 @@ class pkgPackageManager : protected pkgCache::Namespace // The Actual installation implementation virtual bool Install(PkgIterator /*Pkg*/,const string &/*File*/) {return false;} virtual bool Configure(PkgIterator /*Pkg*/) {return false;} - virtual bool Remove(PkgIterator /*Pkg*/,bool /*Purge*/=false) {return false;} + virtual bool Remove(PkgIterator /*Pkg*/,bool /*Purge*/) {return false;} + /* a virtual method with default parameter is confusing; instead, define: */ + bool Remove(const PkgIterator Pkg) {return Remove(Pkg,false);} virtual bool Go() {return true;} virtual void Reset() {} diff --git a/apt-pkg/rpm/rpmpm.h b/apt-pkg/rpm/rpmpm.h index 8a01b80cc..693bb1e1f 100644 --- a/apt-pkg/rpm/rpmpm.h +++ b/apt-pkg/rpm/rpmpm.h @@ -45,7 +45,7 @@ class pkgRPMPM : public pkgPackageManager // The Actuall installation implementation virtual bool Install(PkgIterator Pkg,const string &File) override; virtual bool Configure(PkgIterator Pkg) override; - virtual bool Remove(PkgIterator Pkg,bool Purge = false) override; + virtual bool Remove(PkgIterator Pkg,bool Purge) override; virtual bool Process(const std::vector<apt_item> &install, const std::vector<apt_item> &upgrade, commit acf303214c77dc401f5d6bdb8cb5f85a2a4a6b25 Author: Ivan Zakharyaschev <imz@altlinux.org> Date: Thu Jul 15 13:05:13 2021 +0300 move a default parameter from virtual RPMHandler::DepsList() to avoid confusion Default parameters in virtual methods are confusing, because they are not looked up in the same way as a virtual method's implementation (through the table of virtual methods of the object; but rather from the "known" type in the code that uses the method). Luckily, this occurred in RPMHandler class (a thing internal for apt-rpm). This change probably doesn't affect ABI, since the table of virtual methods is not modified. After recompilation, the new code will behave equivalently to the old one; but if one changes the default value, this will probably take effect only after recompilation (because the new DepsList() looks like an inlined method defined in the header). diff --git a/apt-pkg/rpm/rpmhandler.h b/apt-pkg/rpm/rpmhandler.h index cd899058c..54dcc4d50 100644 --- a/apt-pkg/rpm/rpmhandler.h +++ b/apt-pkg/rpm/rpmhandler.h @@ -87,7 +87,10 @@ class RPMHandler virtual string SourceRpm() const = 0; virtual bool DepsList(unsigned int Type, std::vector<Dependency*> &Deps, - bool checkInternalDep = true) const = 0; + bool checkInternalDep) const = 0; + /* a virtual method with default parameter is confusing; instead, define: */ + bool DepsList(const unsigned int Type, std::vector<Dependency*> &Deps) const + { return DepsList(Type,Deps,true); } virtual bool FileList(std::vector<string> &FileList) const = 0; virtual bool HasFile(const char *File) const; @@ -127,7 +130,7 @@ class RPMHdrHandler : public RPMHandler virtual string SourceRpm() const override {return GetSTag(RPMTAG_SOURCERPM);} virtual bool DepsList(unsigned int Type, std::vector<Dependency*> &Deps, - bool checkInternalDep = true) const override; + bool checkInternalDep) const override; virtual bool FileList(std::vector<string> &FileList) const override; RPMHdrHandler() : RPMHandler(), HeaderP(0) {} commit cc3e7bd160ee521f52f12cd789b766ff888dd71d Author: Ivan Zakharyaschev <imz@altlinux.org> Date: Thu Jul 15 13:13:05 2021 +0300 move a default parameter from virtual pkgIndexFile::Describe() to avoid confusion Default parameters in virtual methods are confusing, because they are not looked up in the same way as a virtual method's implementation (through the table of virtual methods of the object; but rather from the "known" type in the code that uses the method). Not sure how much this pkgIndexFile class is used from outside APT (by the clients of the library)... This change probably doesn't affect ABI, since the table of virtual methods is not modified. After recompilation, the new code will behave equivalently to the old one; but if one changes the default value, this will probably take effect only after recompilation (because the new Describe() looks like an inlined method defined in the header). diff --git a/apt-pkg/indexfile.h b/apt-pkg/indexfile.h index 8e000b419..0b26ffdad 100644 --- a/apt-pkg/indexfile.h +++ b/apt-pkg/indexfile.h @@ -55,7 +55,9 @@ class pkgIndexFile virtual string ArchiveInfo(pkgCache::VerIterator Ver) const; virtual string SourceInfo(pkgSrcRecords::Parser const &Record, pkgSrcRecords::File const &File) const; - virtual string Describe(bool Short = false) const = 0; + virtual string Describe(bool Short) const = 0; + /* a virtual method with default parameter is confusing; instead, define: */ + string Describe() const { return Describe(false); } // Interface for acquire virtual string ArchiveURI(const string &/*File*/) const {return string();} commit 80558860d80b2e44b11761f2f589bf63dee2dc0b (@ALT/virtual-has-no-default-param, virtual-has-no-default-param) Author: Ivan Zakharyaschev <imz@altlinux.org> Date: Fri Jul 9 21:25:42 2021 +0300 verify-src.d/virtual-has-no-default-param.sh: turn on diff --git a/verify-src.d/virtual-has-no-default-param.sh b/verify-src.d/virtual-has-no-default-param.verify similarity index 100% rename from verify-src.d/virtual-has-no-default-param.sh rename to verify-src.d/virtual-has-no-default-param.verify -- Best regards, Ivan ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [devel] PATCH for apt: custom callbacks 2021-07-15 12:24 ` Ivan Zakharyaschev @ 2021-07-15 12:38 ` Oleg Solovyov 2021-07-15 12:46 ` Ivan Zakharyaschev 2021-11-09 11:57 ` Dmitry V. Levin 1 sibling, 1 reply; 16+ messages in thread From: Oleg Solovyov @ 2021-07-15 12:38 UTC (permalink / raw) To: ALT Linux Team development discussions; +Cc: Ivan Zakharyaschev В письме от четверг, 15 июля 2021 г. 15:24:48 MSK пользователь Ivan Zakharyaschev написал: > Мои патчи с избавлением от дефолтных параметров в виртуаьных методах, > которые я предлагаю оставить в любом случае (за исключением обсуждаемого > вопроса с soname): Можно это куда-нибудь запушить? Без подсветки лично мне это очень тяжело читать ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [devel] PATCH for apt: custom callbacks 2021-07-15 12:38 ` Oleg Solovyov @ 2021-07-15 12:46 ` Ivan Zakharyaschev 0 siblings, 0 replies; 16+ messages in thread From: Ivan Zakharyaschev @ 2021-07-15 12:46 UTC (permalink / raw) To: ALT Linux Team development discussions [-- Attachment #1: Type: text/plain, Size: 522 bytes --] On Thu, 15 Jul 2021, Oleg Solovyov wrote: > В письме от четверг, 15 июля 2021 г. 15:24:48 MSK пользователь Ivan > Zakharyaschev написал: > > Мои патчи с избавлением от дефолтных параметров в виртуаьных методах, > > которые я предлагаю оставить в любом случае (за исключением обсуждаемого > > вопроса с soname): > > Можно это куда-нибудь запушить? > Без подсветки лично мне это очень тяжело читать Да, это всё есть в истории ветки callbacks у меня в git.altlinux.org/people/imz/packages/apt.git -- Best regards, Ivan ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [devel] PATCH for apt: custom callbacks 2021-07-15 12:24 ` Ivan Zakharyaschev 2021-07-15 12:38 ` Oleg Solovyov @ 2021-11-09 11:57 ` Dmitry V. Levin 1 sibling, 0 replies; 16+ messages in thread From: Dmitry V. Levin @ 2021-11-09 11:57 UTC (permalink / raw) To: devel Hi, On Thu, Jul 15, 2021 at 03:24:48PM +0300, Ivan Zakharyaschev wrote: [...] > Во-вторых, сигнатура метода меняется (при этом дефолтные параметры не надо > учитывать, потому что они другим механизмом подбираются), поэтому обычное > поведение мейнтейнеров было бы изменить soname. Однако, это всё (что > связано с package manager и взиамодействием с rpm) -- довольно внутренняя > часть apt, нечасто используемая внешними клиентами библиотеки (насколько я > понимаю). А также: часть изменений функций у нас в ALT поймает rpm с > set-versions на библиотеки. Эти два соображения могли бы быть аргументом, > чтобы не менять soname и не пересобирать весь набор клиентов. Что думаете? > Менять soname? Я предлагаю в таких случаях менять soname, пересборка зависимых пакетов не сложнее, чем task add remeet, экономить на этом не вижу смысла. -- ldv ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [devel] PATCH for apt: custom callbacks 2021-07-06 10:58 ` Ivan Zakharyaschev 2021-07-15 12:24 ` Ivan Zakharyaschev @ 2021-11-09 12:14 ` Dmitry V. Levin 2021-11-09 13:41 ` Ivan Zakharyaschev 1 sibling, 1 reply; 16+ messages in thread From: Dmitry V. Levin @ 2021-11-09 12:14 UTC (permalink / raw) To: ALT Devel discussion list On Tue, Jul 06, 2021 at 01:58:48PM +0300, Ivan Zakharyaschev wrote: > Небольший вывод, к которому я пришёл (может быть, ошибочно), я > сформулировал в конце письма. На случай, если кто-то не дочитал до конца, > вот он: > > On Tue, 6 Jul 2021, Ivan Zakharyaschev wrote: > > > On Wed, 30 Jun 2021, Dmitry V. Levin wrote: > > > > > Предлагается патч на apt (API), добавляющий custom callbacks. > > > > > > > > После чтения в общих чертах у меня не появилось замечаний по архитектуре > > > > (а также оформлению, стилю: git show --check; отдельные места, где > > > > возможны разные стилистические решения вполне соответствуют окружающему > > > > коду, а enforced style guide у нас отсутствует). > > > > > > > > apt имеет свой тип для callback-ов, особенности rpm скрыты, что > > > > соответствует общему подходу в apt. "Переводом" для rpm занимается функция > > > > pkgRPMLibPM::customCallback из apt-pkg/rpm/rpmpm.{h,cc}: > > > > > > Кажется, апстрим apt-rpm (когда он ещё был) сильно переписал этот код по > > > сравнению с тем, что есть у нас. Но лучше проверить, это может быть > > > ложная память. > > > > Это правда. > > > У меня сложилось представление (может быть, я ошибаюсь), что там не > > две разные функции rpmShowProgress vs rpmCallback используются, а один > > путь, и особая обработка прогресса предполагается что будет > > реализовываться через расширение класса OpProgress из contrib/progress.h. Коллеги говорят, что им очень нужна какая-нибудь реализация custom callbacks. Давайте мы всё-таки её сделаем. -- ldv ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [devel] PATCH for apt: custom callbacks 2021-11-09 12:14 ` Dmitry V. Levin @ 2021-11-09 13:41 ` Ivan Zakharyaschev 2021-11-09 14:39 ` Oleg Solovyov 0 siblings, 1 reply; 16+ messages in thread From: Ivan Zakharyaschev @ 2021-11-09 13:41 UTC (permalink / raw) To: ALT Linux Team development discussions [-- Attachment #1: Type: text/plain, Size: 1960 bytes --] Hello! On Tue, 9 Nov 2021, Dmitry V. Levin wrote: > On Tue, Jul 06, 2021 at 01:58:48PM +0300, Ivan Zakharyaschev wrote: > > Небольший вывод, к которому я пришёл (может быть, ошибочно), я > > сформулировал в конце письма. На случай, если кто-то не дочитал до конца, > > вот он: > > > > On Tue, 6 Jul 2021, Ivan Zakharyaschev wrote: > > > > > On Wed, 30 Jun 2021, Dmitry V. Levin wrote: > > > > > > > Предлагается патч на apt (API), добавляющий custom callbacks. > > > > > > > > > > После чтения в общих чертах у меня не появилось замечаний по архитектуре > > > > > (а также оформлению, стилю: git show --check; отдельные места, где > > > > > возможны разные стилистические решения вполне соответствуют окружающему > > > > > коду, а enforced style guide у нас отсутствует). > > > > > > > > > > apt имеет свой тип для callback-ов, особенности rpm скрыты, что > > > > > соответствует общему подходу в apt. "Переводом" для rpm занимается функция > > > > > pkgRPMLibPM::customCallback из apt-pkg/rpm/rpmpm.{h,cc}: > > > > > > > > Кажется, апстрим apt-rpm (когда он ещё был) сильно переписал этот код по > > > > сравнению с тем, что есть у нас. Но лучше проверить, это может быть > > > > ложная память. > > > > > > Это правда. > > > > > У меня сложилось представление (может быть, я ошибаюсь), что там не > > > две разные функции rpmShowProgress vs rpmCallback используются, а один > > > путь, и особая обработка прогресса предполагается что будет > > > реализовываться через расширение класса OpProgress из contrib/progress.h. > > Коллеги говорят, что им очень нужна какая-нибудь реализация custom callbacks. > Давайте мы всё-таки её сделаем. Я бы хотел подготовить варианты сейчас: на основе того предложения или с большей ориентацией на apt-rpm (в чём я уже разобрался чуть получше). Ещё у меня появился вопрос: может кто-нибудь предложить какие-нибудь тесты для этой части? Через command-line apt или внешние пакеты? -- Best regards, Ivan ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [devel] PATCH for apt: custom callbacks 2021-11-09 13:41 ` Ivan Zakharyaschev @ 2021-11-09 14:39 ` Oleg Solovyov 2021-11-09 14:44 ` Dmitry V. Levin 2021-11-09 14:45 ` Anton Farygin 0 siblings, 2 replies; 16+ messages in thread From: Oleg Solovyov @ 2021-11-09 14:39 UTC (permalink / raw) To: ALT Linux Team development discussions В письме от вторник, 9 ноября 2021 г. 16:41:58 MSK пользователь Ivan Zakharyaschev написал: > Ещё у меня появился вопрос: может кто-нибудь предложить какие-нибудь тесты > для этой части? Через command-line apt или внешние пакеты? Могу предложить только use-case, который привел к необходимости реализовать эти callback'и: использование offline update через plasma5-discover. Без этих callback индикатор прогресса обновления "замирает" на время обновления на 100%, что с точки зрения дизайна не соответствует действительности и создаёт предпосылки к "hard reset" со стороны пользователя, следуя логике "Да он же почти всё поставил, завис просто, что такого может случиться?" А случится куча установленных дубликатов пакетов, которые пользователю придётся разгребать либо самостоятельно, либо с помощью третьих лиц (особенно когда обновление на 2000+ пакетов) ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [devel] PATCH for apt: custom callbacks 2021-11-09 14:39 ` Oleg Solovyov @ 2021-11-09 14:44 ` Dmitry V. Levin 2021-11-09 14:45 ` Anton Farygin 1 sibling, 0 replies; 16+ messages in thread From: Dmitry V. Levin @ 2021-11-09 14:44 UTC (permalink / raw) To: devel On Tue, Nov 09, 2021 at 05:39:06PM +0300, Oleg Solovyov wrote: > В письме от вторник, 9 ноября 2021 г. 16:41:58 MSK пользователь Ivan > Zakharyaschev написал: > > Ещё у меня появился вопрос: может кто-нибудь предложить какие-нибудь тесты > > для этой части? Через command-line apt или внешние пакеты? > Могу предложить только use-case, который привел к необходимости реализовать > эти callback'и: использование offline update через plasma5-discover. Насколько я понял вопрос, нужно что-нибудь такое, что можно вставить в %check пакета. -- ldv ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [devel] PATCH for apt: custom callbacks 2021-11-09 14:39 ` Oleg Solovyov 2021-11-09 14:44 ` Dmitry V. Levin @ 2021-11-09 14:45 ` Anton Farygin 2021-11-09 15:03 ` Oleg Solovyov 1 sibling, 1 reply; 16+ messages in thread From: Anton Farygin @ 2021-11-09 14:45 UTC (permalink / raw) To: devel On 09.11.2021 17:39, Oleg Solovyov wrote: > В письме от вторник, 9 ноября 2021 г. 16:41:58 MSK пользователь Ivan > Zakharyaschev написал: >> Ещё у меня появился вопрос: может кто-нибудь предложить какие-нибудь тесты >> для этой части? Через command-line apt или внешние пакеты? > Могу предложить только use-case, который привел к необходимости реализовать > эти callback'и: использование offline update через plasma5-discover. > > Без этих callback индикатор прогресса обновления "замирает" на время > обновления на 100%, что с точки зрения дизайна не соответствует > действительности и создаёт предпосылки к "hard reset" со стороны пользователя, > следуя логике "Да он же почти всё поставил, завис просто, что такого может > случиться?" > > А случится куча установленных дубликатов пакетов, которые пользователю > придётся разгребать либо самостоятельно, либо с помощью третьих лиц (особенно > когда обновление на 2000+ пакетов) Т.е. - для примера нужно будет пропатчить packagekitd ? У тебя есть работающий вариант такого патча для твоей версии apt callbacks ? ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [devel] PATCH for apt: custom callbacks 2021-11-09 14:45 ` Anton Farygin @ 2021-11-09 15:03 ` Oleg Solovyov 2021-11-09 15:38 ` Aleksei Nikiforov 0 siblings, 1 reply; 16+ messages in thread From: Oleg Solovyov @ 2021-11-09 15:03 UTC (permalink / raw) To: devel В письме от вторник, 9 ноября 2021 г. 17:45:14 MSK пользователь Anton Farygin написал: > Т.е. - для примера нужно будет пропатчить packagekitd ? > > У тебя есть работающий вариант такого патча для твоей версии apt callbacks ? Близко такого варианта нет, не уверен что он вообще сохранился. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [devel] PATCH for apt: custom callbacks 2021-11-09 15:03 ` Oleg Solovyov @ 2021-11-09 15:38 ` Aleksei Nikiforov 0 siblings, 0 replies; 16+ messages in thread From: Aleksei Nikiforov @ 2021-11-09 15:38 UTC (permalink / raw) To: devel 09.11.2021 18:03, Oleg Solovyov пишет: > В письме от вторник, 9 ноября 2021 г. 17:45:14 MSK пользователь Anton Farygin > написал: >> Т.е. - для примера нужно будет пропатчить packagekitd ? >> >> У тебя есть работающий вариант такого патча для твоей версии apt callbacks ? > Близко такого варианта нет, не уверен что он вообще сохранился. А как же задание 276207? Оно успело устареть, но необходимые изменения там есть. С уважением, Алексей Никифоров ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2021-11-09 15:38 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-30 13:09 [devel] PATCH for apt: custom callbacks Ivan Zakharyaschev 2021-06-30 13:54 ` Dmitry V. Levin 2021-07-05 11:31 ` Oleg Solovyov 2021-07-06 10:53 ` Ivan Zakharyaschev 2021-07-06 10:58 ` Ivan Zakharyaschev 2021-07-15 12:24 ` Ivan Zakharyaschev 2021-07-15 12:38 ` Oleg Solovyov 2021-07-15 12:46 ` Ivan Zakharyaschev 2021-11-09 11:57 ` Dmitry V. Levin 2021-11-09 12:14 ` Dmitry V. Levin 2021-11-09 13:41 ` Ivan Zakharyaschev 2021-11-09 14:39 ` Oleg Solovyov 2021-11-09 14:44 ` Dmitry V. Levin 2021-11-09 14:45 ` Anton Farygin 2021-11-09 15:03 ` Oleg Solovyov 2021-11-09 15:38 ` Aleksei Nikiforov
ALT Linux Team development discussions This inbox may be cloned and mirrored by anyone: git clone --mirror http://lore.altlinux.org/devel/0 devel/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 devel devel/ http://lore.altlinux.org/devel \ devel@altlinux.org devel@altlinux.ru devel@lists.altlinux.org devel@lists.altlinux.ru devel@linux.iplabs.ru mandrake-russian@linuxteam.iplabs.ru sisyphus@linuxteam.iplabs.ru public-inbox-index devel Example config snippet for mirrors. Newsgroup available over NNTP: nntp://lore.altlinux.org/org.altlinux.lists.devel AGPL code for this site: git clone https://public-inbox.org/public-inbox.git