[pacman-dev] [PATCH] Don't initialize progress to zero before calling curl_easy_perform().

[Dan McGee]

On Mon, Mar 21, 2011 at 5:01 PM, Lukas Fleischer
<archlinux at cryptocrack.de> wrote:
> On Mon, Mar 21, 2011 at 09:24:35AM -0400, Dave Reisner wrote:
>> On Mon, Mar 21, 2011 at 07:50:37AM -0500, Dan McGee wrote:
>> > On Mon, Mar 21, 2011 at 5:53 AM, Lukas Fleischer
>> > <archlinux at cryptocrack.de> wrote:
>> > > Drawing progress bars before calling curl_easy_perform() is needless as
>> > > the curl progress callback is called with zero progress before actually
>> > > downloading the file anyways. Fixes display of "0%" progress bars when
>> > > sync'ing package databases that are already up to date.
>> >
>> > I'll take this for now, but there is still some hackjob action going
>> > on in here- comparing doubles via equality to ints, etc. I figure
>> > these will get worked out in time though.
>> >
>> > > Signed-off-by: Lukas Fleischer <archlinux at cryptocrack.de>
>> > > ---
>> > >  lib/libalpm/dload.c |    7 +++----
>> > >  1 files changed, 3 insertions(+), 4 deletions(-)
>> > >
>> > > diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
>> > > index ebfd042..a14d1d1 100644
>> > > --- a/lib/libalpm/dload.c
>> > > +++ b/lib/libalpm/dload.c
>> > > @@ -212,10 +212,9 @@ static int curl_download_internal(const char *url, const char *localpath,
>> > >        sigaction(SIGINT, NULL, &sig_int[OLD]);
>> > >        sigaction(SIGINT, &sig_int[NEW], NULL);
>> > >
>> > > -       /* Progress 0 - initialize */
>> > > -       if(handle->dlcb) {
>> > > -               handle->dlcb(filename, 0, 1);
>> > > -       }
>> > > +       /* set initial value of prevprogress to -1 which causes curl_progress() to
>> > > +        * initialize the progress bar with 0% once. */
>> > > +       prevprogress = -1;
>> > >
>> > >        /* perform transfer */
>> > >        handle->curlerr = curl_easy_perform(handle->curl);
>> > > --
>> > > 1.7.4.1
>> >
>>
>> I appreciate the help, but this patch breaks more than it fixes.
>>
>> http://sprunge.us/DIED
>>
>> I already had a working fix for this on my working branch.
>
> Weird, I cannot reproduce that. Just tested it again with some test
> cases and that didn't happen to me once. Is that any special case?
>
> The fix in your working tree actually even looks like it does exactly
> the same thing, except that it calls handle->dlcb() twice when
> curl_progess() is called for the first time. But I trust you having a
> better overview of the code and given that your working tree contains
> some more cleanups, I'd say Dan might just pull in your changes so this
> gets fixed if there's any bugs remaining.

Yeah, we're not done in this arena yet. I'd like to remove the need
for three (omg!) download callback functions, progress bars, total
download, and all this other BS we currently have so anything right
now is a primarily a stopgap to appease those of us running
pacman-git.

With that said, if you see bugs in the cURL download (but not
progress) side of things, then definitely raise concerns here, as that
I reintegrated it to hopefully get some more testing.

-Dan