[pacman-dev] [PATCH] 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
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
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
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.
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.
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.
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
participants (3)
-
Dan McGee
-
Dave Reisner
-
Lukas Fleischer