[pacman-dev] [PATCH] Add a fetch callback to allow front-end download support

[Dan McGee]

On Fri, Apr 3, 2009 at 11:20 AM, Sebastian Nowicki <sebnow at gmail.com> wrote:
> On 29/03/2009, at 5:34 AM, Dan McGee wrote:
>
>> On Fri, Feb 20, 2009 at 2:31 AM, Sebastian Nowicki <sebnow at gmail.com>
>> wrote:
>>>
>>> This allows a frontend to define its own download algorithm so that the
>>> libfetch dependency can be omitted without using an external process.
>>>  The
>>> callback will be used if it is defined, otherwise the internal method
>>> (libfetch) is used, if available.
>>>
>>> The external download method was moved to pacman and is set as the fetch
>>> callback, if the command is defined in the configuration file. As a
>>> result,
>>> alpm_option_get_xfercommand() and alpm_option_set_xfercommand() have been
>>> removed.
>>>
>>> Signed-off-by: Sebastian Nowicki <sebnow at gmail.com>
>>
>> cc1: warnings being treated as errors
>> dload.c: In function ‘download’:
>> dload.c:274: error: passing argument 4 of ‘handle->fetchcb’ makes
>> pointer from integer without a cast
>> (you used mtimeold twice)
>>
>> handle.c: In function ‘_alpm_handle_free’:
>> handle.c:81: error: ‘pmhandle_t’ has no member named ‘xfercommand’
>> handle.c:81: error: ‘pmhandle_t’ has no member named ‘xfercommand’
>> (just kill the line)
>>
>> In file included from pacman.c:52:
>> conf.h:26:1: error: "PATH_MAX" redefined
>> In file included from /usr/include/bits/local_lim.h:39,
>>                from /usr/include/bits/posix1_lim.h:153,
>>                from /usr/include/limits.h:145,
>>                from
>> /usr/lib/gcc/x86_64-unknown-linux-gnu/4.3.3/include-fixed/limits.h:122,
>>                from
>> /usr/lib/gcc/x86_64-unknown-linux-gnu/4.3.3/include-fixed/syslimits.h:7,
>>                from
>> /usr/lib/gcc/x86_64-unknown-linux-gnu/4.3.3/include-fixed/limits.h:11,
>>                from pacman.c:30:
>> /usr/include/linux/limits.h:12:1: error: this is the location of the
>> previous definition
>>
>> Don't mess with that path_max #ifdef junk- although it does look like
>> we interchange <sys/syslimits.h> and <limits.h> to get the definition
>> of PATH_MAX. Which one is correct?
>>
>> Do you guys compile these patches first? :P
>
> I compiled it and tested. I must have not compiled with --enable-debug or
> something. Perhaps my last change broke it (I might have forgotten to test,
> oops), which is more likely. Sorry either way.
>
>> I'm assuming the below code didn't really change from what was in the
>> backend- can you verify this, or point out what you did modify?
>
> For the most part, yes. I did "inline" some functions, which were static.
> There might have been other minor changes.

Was this strictly necessary? I much prefer things being split out in
functions as it is much easier to read. gcc will *always* optimize
these back in if they are called as infrequently as they probably are
(or in a tight loop), so don't try to outsmart the compiler.

I'll try to do a comparison diff sometime to see what changed once you
resubmit the patch.

> Quickly looking over the log, I changed this:
>
>>       filename = get_filename(url);
>>       if(!filename) {
>>               RET_ERR(PM_ERR_EXTERNAL_DOWNLOAD, -1);
>>       }
>>       destfile = get_destfile(localpath, filename);
>>       tempfile = get_tempfile(localpath, filename);
>
>
> To this:
>
>>       filename = strrchr(url, '/');
>>       if(!filename) {
>>               return -1;
>>       } else {
>>               filename++; /* omit leading slash */
>>       }
>>
>>       len = strlen(localpath) + strlen(filename) + 1;
>>       destfile = calloc(len, sizeof(*destfile));
>>       snprintf(destfile, len, "%s%s", localpath, filename);
>>
>>       len += 5; /* ".part" */
>>       tempfile = calloc(len, sizeof(*tempfile));
>>       snprintf(tempfile, len, "%s.part", destfile);
>
>
> I'll fix the patch up tomorrow.
> _______________________________________________
> pacman-dev mailing list
> pacman-dev at archlinux.org
> http://www.archlinux.org/mailman/listinfo/pacman-dev
>