FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > ArchLinux > ArchLinux Pacman Development

 
 
LinkBack Thread Tools
 
Old 03-21-2011, 09:53 AM
Lukas Fleischer
 
Default Don't initialize progress to zero before calling curl_easy_perform().

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.

Signed-off-by: Lukas Fleischer <archlinux@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
 
Old 03-21-2011, 11:50 AM
Dan McGee
 
Default Don't initialize progress to zero before calling curl_easy_perform().

On Mon, Mar 21, 2011 at 5:53 AM, Lukas Fleischer
<archlinux@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@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
 
Old 03-21-2011, 12:24 PM
Dave Reisner
 
Default Don't initialize progress to zero before calling curl_easy_perform().

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@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@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.

dave
 
Old 03-21-2011, 09:01 PM
Lukas Fleischer
 
Default Don't initialize progress to zero before calling curl_easy_perform().

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@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@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.
 
Old 03-21-2011, 09:08 PM
Dan McGee
 
Default Don't initialize progress to zero before calling curl_easy_perform().

On Mon, Mar 21, 2011 at 5:01 PM, Lukas Fleischer
<archlinux@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@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@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
 

Thread Tools




All times are GMT. The time now is 11:32 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org