[pacman-dev] [PATCH] alpm: Fix wrong xferred/total sizes when resuming downloads
When a package is already partially downloaded in the cache, its download size will only be of what's left to be downloaded. Since pkg->download_size is what's used when calculating the total download size for the totaldl callback, same thing apply. However, the download progress callback was including this initial size, which would thus lead to invalid values (and percentage) used in frontends. That is, the progress bar could e.g. go further than 100% In the case of pacman, there is a sanity check for different historical reason (44a57c89), so before the possible "overflow" was noticed, the total download size/progress reported was wrong. Once caught, the TotalDownload option was ignored and it would use individual file download values as fallback instead. Signed-off-by: Olivier Brunel <jjk@jjacky.com> --- Including the initial size during the dl progress cb was added in 283bf7e8, which clearly states the goal of having the progress bar bumped when resuming a download. I'm not sure if this is really the best thing to do anyway though, since the download size announced by pacman prior is that of what's left to be downloaded only, but the progress bar is then of the package size instead, which is a bit wrong/misleading, not to mention totally bogus when it comes to TotalDownload, since the package size might be greater than the actual total download size. With this fix, the progress bar will simply go from 0% to 100% and only refer to the download size, and everything remains correct with TotalDownload. lib/libalpm/dload.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index 88ea427..3e72bd4 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -131,7 +131,9 @@ static int dload_progress_cb(void *file, double dltotal, double dlnow, payload->handle->dlcb(payload->remote_name, 0, (off_t)dltotal); } - payload->handle->dlcb(payload->remote_name, current_size, total_size); + /* do NOT include initial_size since it wasn't part of the package's + * download_size (nor included in the total download size callback) */ + payload->handle->dlcb(payload->remote_name, (off_t)dlnow, (off_t)dltotal); payload->prevprogress = current_size; -- 2.0.3
On 01/08/14 03:42, Olivier Brunel wrote:
When a package is already partially downloaded in the cache, its download size will only be of what's left to be downloaded. Since pkg->download_size is what's used when calculating the total download size for the totaldl callback, same thing apply.
However, the download progress callback was including this initial size, which would thus lead to invalid values (and percentage) used in frontends. That is, the progress bar could e.g. go further than 100%
In the case of pacman, there is a sanity check for different historical reason (44a57c89), so before the possible "overflow" was noticed, the total download size/progress reported was wrong. Once caught, the TotalDownload option was ignored and it would use individual file download values as fallback instead.
Can I clarify that this issue on more than 100% being "shown" only applied to TotalDownload?
Signed-off-by: Olivier Brunel <jjk@jjacky.com> --- Including the initial size during the dl progress cb was added in 283bf7e8, which clearly states the goal of having the progress bar bumped when resuming a download.
I'm not sure if this is really the best thing to do anyway though, since the download size announced by pacman prior is that of what's left to be downloaded only, but the progress bar is then of the package size instead, which is a bit wrong/misleading, not to mention totally bogus when it comes to TotalDownload, since the package size might be greater than the actual total download size.
With this fix, the progress bar will simply go from 0% to 100% and only refer to the download size, and everything remains correct with TotalDownload.
lib/libalpm/dload.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index 88ea427..3e72bd4 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -131,7 +131,9 @@ static int dload_progress_cb(void *file, double dltotal, double dlnow, payload->handle->dlcb(payload->remote_name, 0, (off_t)dltotal); }
- payload->handle->dlcb(payload->remote_name, current_size, total_size); + /* do NOT include initial_size since it wasn't part of the package's + * download_size (nor included in the total download size callback) */ + payload->handle->dlcb(payload->remote_name, (off_t)dlnow, (off_t)dltotal);
payload->prevprogress = current_size;
On 08/01/14 01:24, Allan McRae wrote:
On 01/08/14 03:42, Olivier Brunel wrote:
When a package is already partially downloaded in the cache, its download size will only be of what's left to be downloaded. Since pkg->download_size is what's used when calculating the total download size for the totaldl callback, same thing apply.
However, the download progress callback was including this initial size, which would thus lead to invalid values (and percentage) used in frontends. That is, the progress bar could e.g. go further than 100%
In the case of pacman, there is a sanity check for different historical reason (44a57c89), so before the possible "overflow" was noticed, the total download size/progress reported was wrong. Once caught, the TotalDownload option was ignored and it would use individual file download values as fallback instead.
Can I clarify that this issue on more than 100% being "shown" only applied to TotalDownload?
Absolutely; without TotalDownload there was no going over 100% or anything like that. (Just as mentioned below the fact that the size & progress bar wouldn't be related to the download size but the package size, but nothing actually incorrect.)
Signed-off-by: Olivier Brunel <jjk@jjacky.com> --- Including the initial size during the dl progress cb was added in 283bf7e8, which clearly states the goal of having the progress bar bumped when resuming a download.
I'm not sure if this is really the best thing to do anyway though, since the download size announced by pacman prior is that of what's left to be downloaded only, but the progress bar is then of the package size instead, which is a bit wrong/misleading, not to mention totally bogus when it comes to TotalDownload, since the package size might be greater than the actual total download size.
With this fix, the progress bar will simply go from 0% to 100% and only refer to the download size, and everything remains correct with TotalDownload.
lib/libalpm/dload.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index 88ea427..3e72bd4 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -131,7 +131,9 @@ static int dload_progress_cb(void *file, double dltotal, double dlnow, payload->handle->dlcb(payload->remote_name, 0, (off_t)dltotal); }
- payload->handle->dlcb(payload->remote_name, current_size, total_size); + /* do NOT include initial_size since it wasn't part of the package's + * download_size (nor included in the total download size callback) */ + payload->handle->dlcb(payload->remote_name, (off_t)dlnow, (off_t)dltotal);
payload->prevprogress = current_size;
participants (2)
-
Allan McRae
-
Olivier Brunel