[pacman-dev] [PATCH] compute package download size outside _alpm_sync_prepare
Thanks to Dan for pointing out a mistake I made. --- lib/libalpm/db.h | 3 ++- lib/libalpm/sync.c | 14 +++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/lib/libalpm/db.h b/lib/libalpm/db.h index 9b78ad4..1851b5c 100644 --- a/lib/libalpm/db.h +++ b/lib/libalpm/db.h @@ -34,7 +34,8 @@ typedef enum _pmdbinfrq_t { INFRQ_FILES = (1 << 3), INFRQ_SCRIPTLET = (1 << 4), INFRQ_DELTAS = (1 << 5), - /* ALL should be sum of all above */ + INFRQ_DSIZE = (1 << 6), + /* ALL should be info stored in the package or database */ INFRQ_ALL = 0x3F } pmdbinfrq_t; diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 4996920..13d39aa 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -356,6 +356,7 @@ static int compute_download_size(pmpkg_t *newpkg) off_t size = 0; if(newpkg->origin == PKG_FROM_FILE) { + newpkg->infolevel |= INFRQ_DSIZE; newpkg->download_size = 0; return(0); } @@ -392,6 +393,7 @@ static int compute_download_size(pmpkg_t *newpkg) _alpm_log(PM_LOG_DEBUG, "setting download size %jd for pkg %s\n", (intmax_t)size, alpm_pkg_get_name(newpkg)); + newpkg->infolevel |= INFRQ_DSIZE; newpkg->download_size = size; return(0); } @@ -649,6 +651,9 @@ cleanup: */ off_t SYMEXPORT alpm_pkg_download_size(pmpkg_t *newpkg) { + if(!(newpkg->infolevel & INFRQ_DSIZE)) { + compute_download_size(newpkg); + } return(newpkg->download_size); } @@ -850,7 +855,14 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data) if(files) { EVENT(trans, PM_TRANS_EVT_RETRIEVE_START, current->treename, NULL); - if(_alpm_download_files(files, current->servers, cachedir)) { + errors = _alpm_download_files(files, current->servers, cachedir); + + for(j = trans->add; j; j = j->next) { + j->infolevel &= ~INFRQ_DSIZE; + j->download_size = 0; + } + + if (errors) { _alpm_log(PM_LOG_WARNING, _("failed to retrieve some files from %s\n"), current->treename); if(pm_errno == 0) { -- 1.7.1
On Wed, May 5, 2010 at 5:07 PM, Jonathan Conder <j@skurvy.no-ip.org> wrote:
Thanks to Dan for pointing out a mistake I made.
Looks good now, except for one other mistake that I've fixed already locally but I should point out to you- the patch doesn't even compile. :) You tried treating a list pointer as a package object directly rather than get the data pointer off of it first. Please always add "Signed-off-by" lines as well, I've also done that this time for you (but just this once). Thanks! diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 13d39aa..6bc0b37 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -858,8 +858,9 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data) errors = _alpm_download_files(files, current->servers, cachedir); for(j = trans->add; j; j = j->next) { - j->infolevel &= ~INFRQ_DSIZE; - j->download_size = 0; + pmpkg_t *pkg = j->data; + pkg->infolevel &= ~INFRQ_DSIZE; + pkg->download_size = 0; } if (errors) {
--- lib/libalpm/db.h | 3 ++- lib/libalpm/sync.c | 14 +++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/lib/libalpm/db.h b/lib/libalpm/db.h index 9b78ad4..1851b5c 100644 --- a/lib/libalpm/db.h +++ b/lib/libalpm/db.h @@ -34,7 +34,8 @@ typedef enum _pmdbinfrq_t { INFRQ_FILES = (1 << 3), INFRQ_SCRIPTLET = (1 << 4), INFRQ_DELTAS = (1 << 5), - /* ALL should be sum of all above */ + INFRQ_DSIZE = (1 << 6), + /* ALL should be info stored in the package or database */ INFRQ_ALL = 0x3F } pmdbinfrq_t;
diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 4996920..13d39aa 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -356,6 +356,7 @@ static int compute_download_size(pmpkg_t *newpkg) off_t size = 0;
if(newpkg->origin == PKG_FROM_FILE) { + newpkg->infolevel |= INFRQ_DSIZE; newpkg->download_size = 0; return(0); } @@ -392,6 +393,7 @@ static int compute_download_size(pmpkg_t *newpkg) _alpm_log(PM_LOG_DEBUG, "setting download size %jd for pkg %s\n", (intmax_t)size, alpm_pkg_get_name(newpkg));
+ newpkg->infolevel |= INFRQ_DSIZE; newpkg->download_size = size; return(0); } @@ -649,6 +651,9 @@ cleanup: */ off_t SYMEXPORT alpm_pkg_download_size(pmpkg_t *newpkg) { + if(!(newpkg->infolevel & INFRQ_DSIZE)) { + compute_download_size(newpkg); + } return(newpkg->download_size); }
@@ -850,7 +855,14 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)
if(files) { EVENT(trans, PM_TRANS_EVT_RETRIEVE_START, current->treename, NULL); - if(_alpm_download_files(files, current->servers, cachedir)) { + errors = _alpm_download_files(files, current->servers, cachedir); + + for(j = trans->add; j; j = j->next) { + j->infolevel &= ~INFRQ_DSIZE; + j->download_size = 0; + } + + if (errors) { _alpm_log(PM_LOG_WARNING, _("failed to retrieve some files from %s\n"), current->treename); if(pm_errno == 0) { -- 1.7.1
On Wed, 2010-05-05 at 18:18 -0500, Dan McGee wrote:
Looks good now, except for one other mistake that I've fixed already locally but I should point out to you- the patch doesn't even compile. :) You tried treating a list pointer as a package object directly rather than get the data pointer off of it first.
Oops, sorry about that :S. Next time I'll try to test it myself.
Please always add "Signed-off-by" lines as well, I've also done that this time for you (but just this once). Thanks!
Ok, I wasn't aware of that, will do next time. Jonathan
Thanks to Dan for pointing out a mistake I made.
Personally I prefer the old patch over this. If I read this patch correctly, you have completely removed the on-demand computation from current pacman GUI. The old patch was better for -Sp for example... I think we should catch the PM_ERR_PKG_INVALID_NAME error outside of compute_download_size(). Bye
On Thu, 2010-05-06 at 01:33 +0200, Nagy Gabor wrote:
Personally I prefer the old patch over this. If I read this patch correctly, you have completely removed the on-demand computation from current pacman GUI. The old patch was better for -Sp for example...
You can still do on-demand computation with alpm_pkg_download_size. _alpm_sync_commit does not use this because they are guaranteed to be valid by _alpm_sync_prepare (and I wanted to make as few changes as possible this time round).
I think we should catch the PM_ERR_PKG_INVALID_NAME error outside of compute_download_size().
Maybe that would prevent mistakes like mine in the future, but really I should not have overlooked the return code of compute_download_size. Jonathan
participants (3)
-
Dan McGee
-
Jonathan Conder
-
Nagy Gabor