[pacman-dev] [PATCH] alpm: Fix wrong xferred/total sizes when resuming downloads

[Olivier Brunel]

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 at 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;
>>  
>>
>