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

[Dan McGee]

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

-Dan