[pacman-dev] [PATCH] compute package download size outside _alpm_sync_prepare
Hi Here is a patch to resolve FS#18769. I created a new INFRQ/infolevel to describe package download size, so that it could be lazily calculated like most of the other package info. This differs slightly from most of the other infolevels, which correspond to information actually read by _alpm_db_read and alpm_pkg_load. However, I felt it better to do it this way than setting the download size to -1 for invalid, as this would make it the only field in pmpkg_t that needs to be initialized to something other than 0. It is also possible that the download size will be reported incorrectly if a package download size is read, and then that same package is downloaded with alpm_fetch_pkgurl. However, this is better than the present situation where download sizes aren't reported at all, and furthermore this is unlikely since alpm_fetch_pkgurl would probably not be used to download a package that is in the repositories. Thanks for your time, Jonathan
Hi! I like this patch. Sometimes (-Sp, for example) it is needless to compute this info, so this little speed-up is one more minor argument for on-demand download-size computing.
Here is a patch to resolve FS#18769. I created a new INFRQ/infolevel to describe package download size, so that it could be lazily calculated like most of the other package info. This differs slightly from most of the other infolevels, which correspond to information actually read by _alpm_db_read and alpm_pkg_load. However, I felt it better to do it this way than setting the download size to -1 for invalid, as this would make it the only field in pmpkg_t that needs to be initialized to something other than 0.
I also came up with these two possible solutions earlier, see 5. here: http://mailman.archlinux.org/pipermail/pacman-dev/2008-August/007512.html Now I also think that infolevel is better.
It is also possible that the download size will be reported incorrectly if a package download size is read, and then that same package is downloaded with alpm_fetch_pkgurl. However, this is better than the present situation where download sizes aren't reported at all, and furthermore this is unlikely since alpm_fetch_pkgurl would probably not be used to download a package that is in the repositories.
I agree, alpm_fetch_pkgurl is not used by alpm. Moreover, in my opinion (see also the thread above), our download code is quite messy. That's why this is needed: + for(j = trans->add; j; j = j->next) { + j->infolevel &= ~INFRQ_DSIZE; + j->download_size = 0; + } Clearly we should somehow restructure our download code to handle pmpkg_t's. Bye
Hi again
I like this patch. Sometimes (-Sp, for example) it is needless to compute this info, so this little speed-up is one more minor argument for on-demand download-size computing.
Glad to hear it.
I also came up with these two possible solutions earlier, see 5. here: http://mailman.archlinux.org/pipermail/pacman-dev/2008-August/007512.html Now I also think that infolevel is better.
Yeah, well, I did read your link before doing this patch :).
Clearly we should somehow restructure our download code to handle pmpkg_t's.
I would like to see something like this too. At least, my life would be easier if a pmpkg_t gets passed to the download callback, with perhaps the download_size for that package along with the current file, size of that file, etc. Maybe I will look into it when I have more time. Thanks again, Jonathan
On Thu, Mar 25, 2010 at 9:38 PM, Jonathan Conder <j@skurvy.no-ip.org> wrote:
Hi again
I like this patch. Sometimes (-Sp, for example) it is needless to compute this info, so this little speed-up is one more minor argument for on-demand download-size computing.
Glad to hear it.
I also came up with these two possible solutions earlier, see 5. here: http://mailman.archlinux.org/pipermail/pacman-dev/2008-August/007512.html Now I also think that infolevel is better.
Yeah, well, I did read your link before doing this patch :).
Clearly we should somehow restructure our download code to handle pmpkg_t's.
I would like to see something like this too. At least, my life would be easier if a pmpkg_t gets passed to the download callback, with perhaps the download_size for that package along with the current file, size of that file, etc. Maybe I will look into it when I have more time.
I think you broke something unintentionally, see this commit: 6d79ba2db0f37f46b925a509ef83724fc0f61184 You completely removed the loop in _alpm_sync_prepare that makes sure we have a filename for each participating package. Does that still need to be there in some capacity? Otherwise I like this changeset, thanks for the work. -Dan
On Wed, 2010-05-05 at 11:47 -0500, Dan McGee wrote:
I think you broke something unintentionally, see this commit: 6d79ba2db0f37f46b925a509ef83724fc0f61184
You completely removed the loop in _alpm_sync_prepare that makes sure we have a filename for each participating package. Does that still need to be there in some capacity?
I think you're right, yeah. My bad. I'll send in a new patch shortly.
Otherwise I like this changeset, thanks for the work.
Thanks, you're welcome. Jonathan
participants (3)
-
Dan McGee
-
Jonathan Conder
-
Nagy Gabor