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

[Dan McGee]

On Sat, Mar 28, 2009 at 4:51 PM, Aaron Griffin <aaronmgriffin at gmail.com> wrote:
> On Sat, Mar 28, 2009 at 4:47 PM, Dan McGee <dpmcgee at gmail.com> wrote:
>> On Sat, Mar 28, 2009 at 4:44 PM, Aaron Griffin <aaronmgriffin at gmail.com> wrote:
>>> On Sat, Mar 28, 2009 at 4:34 PM, Dan McGee <dpmcgee at gmail.com> wrote:
>>>> On Fri, Feb 20, 2009 at 2:31 AM, Sebastian Nowicki <sebnow at gmail.com> wrote:
>>>>> +/** A callback for downloading files
>>>>> + * @param url the URL of the file to be downloaded
>>>>> + * @param localpath the directory to which the file should be downloaded
>>>>> + * @param mtimeold the modification time of the file previously downloaded
>>>>> + * @param mtimenew the modification time of the newly downloaded file.
>>>>> + * This should be set by the callback.
>>>>> + * @return 0 on success, 1 if the modification times are identical, -1 on
>>>>> + * error.
>>>>> + */
>>>>> +typedef int (*alpm_cb_fetch)(const char *url, const char *localpath,
>>>>> +               time_t mtimeold, time_t *mtimenew);
>>>> Does everyone thing this is a good enough interface? I don't want to
>>>> have to change this later after we release once.
>>>
>>> Hmmm I wonder... why do we need to pass the mtime stuff in? Doesn't it
>>> make sense that most fetch callbacks would do the exact same
>>> comparison? I can't see any other need for these - except maybe
>>> mtimenew to set the download file's mtime to match the server's,
>>> but... do we care that they are exact? If we don't set it, it should
>>> always be greater than the server anyway.
>>>
>>> I'm kinda just talking here, not putting lots of thought into it, but
>>> are those two points (mtime compare, and setting local file mtime)
>>> sane? Is there something I'm missing?
>>
>> The mtime stuff is mainly used for database downloads, where we don't
>> actually have the old file to compare against- we simply have the
>> mtime stored in:
>> $ cat /var/lib/pacman/sync/core/.lastupdate
>> 1238199239
>>
>> We could probably simplify this by always keeping the db file we
>> download around and just using the stat-ed mtime from that...
>
> What about the compare? if(mtimenew > mtimeold) will be done in every
> fetch callback, correct? Why don't e just do that before we call the
> fetch callback?

Because we don't have mtimenew- thats a pointer being passed in that
gets filled by the method so we can use the value when we come back
from the download callback. The fetch method has to do something (such
as a HEAD request) first before the compare can be done.

-Dan