[pacman-dev] Extending download callback interface

[Anatol Pomozov]

Hello Eric

On Tue, Feb 11, 2020 at 1:49 AM Erich Eckner <arch at eckner.net> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
>
> On Tue, 11 Feb 2020, Anatol Pomozov wrote:
>
> > Hello folks
>
> Hi Anatol,
>
> >
> > While working on multiplexed download API I hit one issue that requires
> > some alpm API changes.
> >
> > Current ALPM download api handles one file at a time. And interaction
> > between pacman and ALPM looks like:
> >
> > - pacman iterates over list of files to download
> > - pacman calls alpm API to download one file
> > - alpm initializes curl with progress callback implemented by pacman
> > - during the download curl calls the pacman callback
> > - once download is done alpm returns a result code of the download
> > transaction
> >
> > In this single-download scenario pacman knows when the download starts,
> > progresses and finished.
> >
> > With multiplexed download feature alpm_download() will change its API. It
> > will get a list of files as a parameter and handle downloads for all of
> > them with a single function call.
> >
> > This unfortunately makes impossible to intercept "start file download"
> and
> > "complete file download" events. These events are needed because we want
> to
> > render UI correctly and print download information like "file up to
> date",
> > "failed to download"... at the exact moment when the event happens.
> >
> > To mitigate this problem I propose to extend the callback API to make it
> > possible for ALPM to provide other types of download events.
> >
> > The signature will change from:
> > typedef void (*alpm_cb_download)(const char *filename, off_t xfered,
> off_t
> > total);
> > that implies only download progress events, to:
> > typedef void (*alpm_cb_download)(const char *filename,
> > alpm_download_event_type_t event, void *data);
> >
> > where alpm_download_event_type_t is enum
> >
> > typedef enum {
> >       ALPM_DOWNLOAD_INIT,
> >       ALPM_DOWNLOAD_PROGRESS,
> >       ALPM_DOWNLOAD_ERROR,
> >       ALPM_DOWNLOAD_COMPLETED
> > } alpm_download_event_type_t;
> >
> > and *data is a pointer to event specific structure (different for each
> type
> > of event), e.g.:
> > typedef struct {
> >       int result; /* 1 - file is uptodate, 0 - download completed
> > successfully, -1 failed */
> > } alpm_download_event_completed_t;
> >
> > Note this is an ALPM breaking API change. It means it can be done only
> with
> > major version bump.
> >
> > What do you think about it?
> >
>
> I'm sure, you have considered this, but: Why not only(?) add a
> "this_event_refers_to_that_file" pointer of some kind to the callback (the
> index of the file in the parameter array, for example)? This might be
> nice, even if you change the interface in the direction you proposed. Then
> the frontend could update the progress/status of the respective file
> separately (xfered==0 would be "start", xfered==total would be
> "finished").
>

I am not fully understand your question. To answer it better could you
please give a signature of the function you propose?

Or is this, what "const char *filename" gets used for in the callback?


filename is a name of the file we download. It is unique for a given
multiplexed_download transaction so it can be used as a key to lookup the
state of this particular download.