On 05/06/15 at 12:32am, David Macek wrote:
On 4. 5. 2015 14:55, Andrew Gregory wrote:
I don't think this is the right solution. The comments in dload_progress_cb suggest that the callback is not intended to be called until we have actually downloaded something. So, file_xfered should always equal 0 exactly once per downloaded file. I think it makes more sense to fix dload_progress_cb. We might also consider using the event callback rather than the download callback when noprogressbar is set.
Okay. I was thinking and came up with another idea, along the lines of this incomplete patch:
--- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -127,15 +127,17 @@ static int dload_progress_cb(void *file, double dltotal, double dlnow,
/* initialize the progress bar here to avoid displaying it when * a repo is up to date and nothing gets downloaded */ - if(payload->prevprogress == 0) { + if(payload->prevprogress == -1) { payload->handle->dlcb(payload->remote_name, 0, (off_t)dltotal); + payload->prevprogress = payload->initial_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; + if(payload->prevprogress != current_size) { + payload->handle->dlcb(payload->remote_name, (off_t)dlnow, (off_t)dltotal); + payload->prevprogress = current_size; + }
return 0; }
If putting -1 into prevprogress is undesirable, can we introduce a new field into the dload_payload type? (If introducing a new field is a big deal for libalpm clients, maybe we can future-proof the struct by adding a `struct dload_payload_internal *_alpm_internal` field instead and keeping `struct dload_payload_internal` unexported.)
There's an easier solution: just check for dlnow > 0 at the same time we check for the existence of dlcb. apg