Greetings,
I've been working on and off on a curl-based internal downloader to replace the libfetch based implementation. Why?
- libcurl is more widely used in the arch repos. fetch is used for one and only package: pacman. - support for compression over http connections. I see proposals for adding this to fetch, but never any acceptance. Please correct me if I'm mistaken. - personal bias. not really a reason, but I think the implementation is cleaner and more readable than the fetch equivalent. - dan wanted me to. (the devil made me do it?)
In it's current form, I'd say this patch set is 90% complete. I've been using it on top of pacman-git for about a week now and haven't come across any functional issues. I've done a fair bit of testing with ftp, http, and file based sync'ing.
So what's remaining?
- setting pm_errno: libfetch provides a global variable fetchLastErrString, whereas curl returns error codes (CURLcode) from operations and you feed them to curl_easy_strerror() to get a char* back. I'm not sure what the best way is to get back an error string from curl when (what will be) PM_ERR_LIBCURL is set. could the global handle be used to store the CURLcode (it's just a typedef'd long). Yeah, on the handle probably works although that seems like a slight crosscutting concern. Of course I don't have a much more spectacular idea. The other option is to call easy_strerror() immediately after a cURL error occurs and store a pointer to the string there instead of
On Sun, Jan 2, 2011 at 7:13 PM, Dave Reisner <d@falconindy.com> wrote: the error code- but not sure of the memory conventions surrounding that and if you need to free the string, who knows what. And it doesn't matter one iota what the typedef is, because you are going to use their type of course. :)
- the progress bar does _not_ play well with curl. The bar itself is fine, but the time remaining jumps around a fair bit. You'll see a fair bit of hackery in curl_progress() (lib/libalpm/dload.c) where I'm fighting with the progress bar. Perhaps this belongs in the progress bar voodoo itself?
- my autoconf-fu sucks. Big time. I'm not sure what I've done is best practice, but after applying patch 3/4, compiling without extra flags links in curl and leaves out fetch. There is, however, no checking for both --with-curl and --with-fetch being given, which will break the build. Fixing this would be kinda cool so we can at least test both side by side for a while; but I haven't looked near close enough to see what is going on. I'd probably start by confusing yourself less and renaming them to download_fetch and download_curl or something, rather than the ifdef madness and only having one of them at a time. And you could shoehorn another "temporary" patch in here that checked an env var or something to see which internal function to use.
Dan (and anyone else), I look forward to your tower of constructive criticism.
dave
configure.ac | 33 ++++--- lib/libalpm/alpm.c | 13 +-- lib/libalpm/dload.c | 263 ++++++++++++++++++++++++++++++--------------------- 3 files changed, 176 insertions(+), 133 deletions(-)