[pacman-dev] Extending download callback interface

[Erich Eckner]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On Tue, 11 Feb 2020, Anatol Pomozov wrote:

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

My idea was to extend the current interface by just additionally 
referencing the file to which the callback-call refers to:

typedef void (*alpm_cb_download)(const char *filename, int file_idx, off_t 
xfered, off_t total);

so the callback function would know, for which file the reported progress 
is. But your interface seems superior to this, because it's more flexible.

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

regards,
Erich

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE3p92iMrPBP64GmxZCu7JB1Xae1oFAl5DnlIACgkQCu7JB1Xa
e1pq3g//UGfLS7PdGKbjguetHRb4upxVv3NDenssLHJRUrlfEBO70VtcgeP35mt/
8OD8CvWsPZM3InctkppZdBhbL/lkdpG989Tlo3kMQEUnVFP5ay7c+Whvza/6yzVV
KKmw5VbIM0J00UYMLNC0ps002YLSXnO/PQsHZZPca78vqUtDkALaauNgP6v6nGae
JWMiJ3vNTWE70ZneFJ3jxvNdh44JYiYqLUAoaK784ZYjDubfCHGPwf5O4nrMV+Cw
buW0vxuntvj4vxdyIcgppqQ+O151f/AeOjXwGtwjW9Rh5+VxfA46hrIcP2lE0PiU
hqHItCalioLsyxCuQyAszRNtfNRAKM5PkQ5MeIHoqVzIlMhZ4ubGlN0VMqBbqsue
b8hRACh83rQk8X+/XmRY3PbRDBbP5gSJKqbr/pgT7aD+VLiiJGPQhdpMtHjLWFDN
TL1dSRTUsnRd7lOtrMBtwD+o6aIeoF9Jt7CAeyXrspfIH2el2gxy5k6o++9QkaNU
6Ktd3zc6vktUi71PPYjJBDcHwYmrQdxrAI2CzhuSmE32fE2av4z7+1ZIBwasyV24
PFP7O+JwukTQEWfq/FH4G8mVGyXyRmYNCEgrGQu2elFSLXvUsDoq9atex4bMp+O9
embh5muVSqqMmS9UEProoLVDTClS4pRjycDwiH21gbGbzZzoXyc=
=2QhA
-----END PGP SIGNATURE-----