Re: [pacman-dev] Extending download callback interface
-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 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-
Re: [pacman-dev] Extending download callback interface
On 11/2/20 6:21 pm, Anatol Pomozov wrote: > Hello folks > > While working on multiplexed download API I hit one issue that requires > some alpm API changes. > Note this is an ALPM breaking API change. It means it can be done only > with major version bump. We break some part of the API almost every major release. This is not an issue. > What do you think about it? While I have not had a chance to look into this in detail, your proposal seems fine. A
Re: [pacman-dev] Extending download callback interface
Hello Eric
On Tue, Feb 11, 2020 at 1:49 AM Erich Eckner 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.
Re: [pacman-dev] Extending download callback interface
-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").
Or is this, what "const char *filename" gets used for in the callback?
regards,
Erich
-BEGIN PGP SIGNATURE-
iQIzBAEBCAAdFiEE3p92iMrPBP64GmxZCu7JB1Xae1oFAl5CeKEACgkQCu7JB1Xa
e1ogXA//ecAXHyhvNW8vhlcMLkiSIkctdF+w394tkfH7XSHxf9IaYxNSuBD/qoBd
IllSA5Fw7uQ0V1CK3mGMBUi/WGBIrR4qVOBNaP3CAuYKfubgrvz2qxTKeIfg9xs5
5rpfcglFh/hXd4qjeHWgCwdDOZFhM9PeKcj4PnA8ONrzcAzFzCE6C9ELNSfUkiP4
PQTlIVKCdukzbX0FTL6a7ak2bWrbufREhQ2YG4QRWnvN0FtkbG6z6iU7anC0t/gR
F5STdkuSdkEoCgCra0Ng/h5i/htJXYfZM/rve5xlQ4i7CgvPQ74s2lGNPMm0jEW/
UhJJ8eGnRoLAP1LdUO4EadrNhOGE6kyNTcYRjoTYVZWD56plZDTDkjnu0EXacfIf
i4hFNLA0VKo/bSjIWPVwzQs3YuGe9DkwH8D+0pz0ob5KB19pbuXUYfPiDQk7w2mD
a63yhiYmKlsu84o6ws9Rm4VyajM+R+1FwmuFhVoxuKFWVPmw36jxrrcR7dSo3fhq
Fg10VlQoOPQzgWBCXOy7qYHqWhxNa/oodMzeUsci04jYlxSS8DGr4wmIR6G5FL7l
OYC+th2/TknRARQ5tv2+Unc/qgDSfzosNEWV3u/xlcqTEoQlnh1F+mn7WrVfBiAG
t+3uwsgsXTOWzgho4r0yexs1piFRXe2fpspB11Y++fChrea4lFA=
=Igd3
-END PGP SIGNATURE-
[pacman-dev] Extending download callback interface
Hello folks
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?
