Re: [pacman-dev] [PATCH] Move signature payload creation to download engine
Hi
On Mon, Jun 22, 2020 at 11:35 PM Anatol Pomozov
wrote:
>
> Hi
>
> On Wed, Jun 10, 2020 at 9:33 PM Allan McRae wrote:
> >
> > On 14/5/20 8:15 am, Anatol Pomozov wrote:
> > > Until now callee of ALPM download functionality has been in charge of
> > > payload creation both for the main file (e.g. *.pkg) and for the
> > > accompanied
> > > *.sig file. One advantage of such solution is that all payloads are
> > > independent and can be fetched in parallel thus exploiting the maximum
> > > level of download parallelism.
> > >
> > > To build *.sig file url we've been using a simple string concatenation:
> > > $requested_url + ".sig". Unfortunately there are cases when it does not
> > > work. For example an archlinux.org "Download From Mirror" link looks like
> > > this https://www.archlinux.org/packages/core/x86_64/bash/download/ and
> > > it gets redirected to some mirror. But if we append ".sig" to the end of
> > > the link url and try to download it then archlinux.org returns 404 error.
> > >
> > > To overcome this issue we need to follow redirects for the main payload
> > > first, find the final url and only then append '.sig' suffix.
> > > This implies 2 things:
> > > - the signature payload initialization need to be moved to dload.c
> > > as it is the place where we have access to the resolved url
> > > - *.sig is downloaded serially with the main payload and this reduces
> > > level of parallelism
> > >
> > > Move *.sig payload creation to dload.c. Once the main payload is fetched
> > > successfully we check if the callee asked to download the accompanied
> > > signature. If yes - create a new payload and add it to mcurl.
> > >
> > > *.sig payload does not use server list of the main payload and thus does
> > > not support mirror failover. *.sig file comes from the same server as
> > > the main payload.
> > >
> > > Refactor event loop in curl_multi_download_internal() a bit. Instead of
> > > relying on curl_multi_check_finished_download() to return number of new
> > > payloads we simply rerun the loop iteration one more time to check if
> > > there are any active downloads left.
> > > ---
> > > lib/libalpm/be_sync.c | 34 +++-
> > > lib/libalpm/dload.c | 91 ++-
> > > lib/libalpm/dload.h | 4 +-
> > > 3 files changed, 65 insertions(+), 64 deletions(-)
> > >
> > > diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
> > > index 82018e15..41675d21 100644
> > > --- a/lib/libalpm/be_sync.c
> > > +++ b/lib/libalpm/be_sync.c
> > > @@ -180,12 +180,10 @@ int SYMEXPORT alpm_db_update(alpm_handle_t *handle,
> > > alpm_list_t *dbs, int force)
> > > dbforce = 1;
> > > }
> > >
> > > - CALLOC(payload, 1, sizeof(*payload), GOTO_ERR(handle,
> > > ALPM_ERR_MEMORY, cleanup));
> > > + siglevel = alpm_db_get_siglevel(db);
> > >
> > > - /* set hard upper limit of 128 MiB */
> > > - payload->max_size = 128 * 1024 * 1024;
> > > + CALLOC(payload, 1, sizeof(*payload), GOTO_ERR(handle,
> > > ALPM_ERR_MEMORY, cleanup));
> > > payload->servers = db->servers;
> > > -
> > > /* print server + filename into a buffer */
> > > len = strlen(db->treename) + strlen(dbext) + 1;
> > > MALLOC(payload->filepath, len,
> > > @@ -194,31 +192,11 @@ int SYMEXPORT alpm_db_update(alpm_handle_t *handle,
> > > alpm_list_t *dbs, int force)
> > > payload->handle = handle;
> > > payload->force = dbforce;
> > > payload->unlink_on_fail = 1;
> > > -
> > > + payload->download_signature = (siglevel &
> > > ALPM_SIG_DATABASE);
> > > + payload->signature_optional = (siglevel &
> > > ALPM_SIG_DATABASE_OPTIONAL);
> > > + /* set hard upper limit of 128 MiB */
> > > + payload->max_size = 128 * 1024 * 1024;
> > > payloads = alpm_list_add(payloads, payload);
> >
> > OK.
> >
> > > -
> > > - siglevel = alpm_db_get_siglevel(db);
> > > - if(siglevel & ALPM_SIG_DATABASE) {
> > > - struct dload_payload *sig_payload;
> > > - CALLOC(sig_payload, 1, sizeof(*sig_payload),
> > > GOTO_ERR(handle, ALPM_ERR_MEMORY, cleanup));
> > > - sig_payload->signature = 1;
> > > -
> > > - /* print filename into a buffer (leave space for
> > > separator and .sig) */
> > > - len = strlen(db->treename) + strlen(dbext) + 5;
> > > - MALLOC(sig_payload->filepath, len,
> > > - FREE(sig_payload); GOTO_ERR(handle,
> > > ALPM_ERR_MEMORY, cleanup));
> > > - snprintf(sig_payload->filepath, len, "%s%s.sig",
> > > db->treename, dbext);
> > > -
> > > - sig_payload->handle = handle;
> > > - sig_payload->force = dbforce;
> > > - si
Re: [pacman-dev] [PATCH] Move signature payload creation to download engine
Hi
On Wed, Jun 10, 2020 at 9:33 PM Allan McRae wrote:
>
> On 14/5/20 8:15 am, Anatol Pomozov wrote:
> > Until now callee of ALPM download functionality has been in charge of
> > payload creation both for the main file (e.g. *.pkg) and for the accompanied
> > *.sig file. One advantage of such solution is that all payloads are
> > independent and can be fetched in parallel thus exploiting the maximum
> > level of download parallelism.
> >
> > To build *.sig file url we've been using a simple string concatenation:
> > $requested_url + ".sig". Unfortunately there are cases when it does not
> > work. For example an archlinux.org "Download From Mirror" link looks like
> > this https://www.archlinux.org/packages/core/x86_64/bash/download/ and
> > it gets redirected to some mirror. But if we append ".sig" to the end of
> > the link url and try to download it then archlinux.org returns 404 error.
> >
> > To overcome this issue we need to follow redirects for the main payload
> > first, find the final url and only then append '.sig' suffix.
> > This implies 2 things:
> > - the signature payload initialization need to be moved to dload.c
> > as it is the place where we have access to the resolved url
> > - *.sig is downloaded serially with the main payload and this reduces
> > level of parallelism
> >
> > Move *.sig payload creation to dload.c. Once the main payload is fetched
> > successfully we check if the callee asked to download the accompanied
> > signature. If yes - create a new payload and add it to mcurl.
> >
> > *.sig payload does not use server list of the main payload and thus does
> > not support mirror failover. *.sig file comes from the same server as
> > the main payload.
> >
> > Refactor event loop in curl_multi_download_internal() a bit. Instead of
> > relying on curl_multi_check_finished_download() to return number of new
> > payloads we simply rerun the loop iteration one more time to check if
> > there are any active downloads left.
> > ---
> > lib/libalpm/be_sync.c | 34 +++-
> > lib/libalpm/dload.c | 91 ++-
> > lib/libalpm/dload.h | 4 +-
> > 3 files changed, 65 insertions(+), 64 deletions(-)
> >
> > diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
> > index 82018e15..41675d21 100644
> > --- a/lib/libalpm/be_sync.c
> > +++ b/lib/libalpm/be_sync.c
> > @@ -180,12 +180,10 @@ int SYMEXPORT alpm_db_update(alpm_handle_t *handle,
> > alpm_list_t *dbs, int force)
> > dbforce = 1;
> > }
> >
> > - CALLOC(payload, 1, sizeof(*payload), GOTO_ERR(handle,
> > ALPM_ERR_MEMORY, cleanup));
> > + siglevel = alpm_db_get_siglevel(db);
> >
> > - /* set hard upper limit of 128 MiB */
> > - payload->max_size = 128 * 1024 * 1024;
> > + CALLOC(payload, 1, sizeof(*payload), GOTO_ERR(handle,
> > ALPM_ERR_MEMORY, cleanup));
> > payload->servers = db->servers;
> > -
> > /* print server + filename into a buffer */
> > len = strlen(db->treename) + strlen(dbext) + 1;
> > MALLOC(payload->filepath, len,
> > @@ -194,31 +192,11 @@ int SYMEXPORT alpm_db_update(alpm_handle_t *handle,
> > alpm_list_t *dbs, int force)
> > payload->handle = handle;
> > payload->force = dbforce;
> > payload->unlink_on_fail = 1;
> > -
> > + payload->download_signature = (siglevel & ALPM_SIG_DATABASE);
> > + payload->signature_optional = (siglevel &
> > ALPM_SIG_DATABASE_OPTIONAL);
> > + /* set hard upper limit of 128 MiB */
> > + payload->max_size = 128 * 1024 * 1024;
> > payloads = alpm_list_add(payloads, payload);
>
> OK.
>
> > -
> > - siglevel = alpm_db_get_siglevel(db);
> > - if(siglevel & ALPM_SIG_DATABASE) {
> > - struct dload_payload *sig_payload;
> > - CALLOC(sig_payload, 1, sizeof(*sig_payload),
> > GOTO_ERR(handle, ALPM_ERR_MEMORY, cleanup));
> > - sig_payload->signature = 1;
> > -
> > - /* print filename into a buffer (leave space for
> > separator and .sig) */
> > - len = strlen(db->treename) + strlen(dbext) + 5;
> > - MALLOC(sig_payload->filepath, len,
> > - FREE(sig_payload); GOTO_ERR(handle,
> > ALPM_ERR_MEMORY, cleanup));
> > - snprintf(sig_payload->filepath, len, "%s%s.sig",
> > db->treename, dbext);
> > -
> > - sig_payload->handle = handle;
> > - sig_payload->force = dbforce;
> > - sig_payload->errors_ok = (siglevel &
> > ALPM_SIG_DATABASE_OPTIONAL);
> > -
> > - /* set hard upper limit of 16 KiB */
> > - sig_payload->max_size = 16 * 1024;
> > - sig_payload->servers = db->servers;
> > -
> > -
Re: [pacman-dev] [PATCH] Move signature payload creation to download engine
On 14/5/20 8:15 am, Anatol Pomozov wrote:
> Until now callee of ALPM download functionality has been in charge of
> payload creation both for the main file (e.g. *.pkg) and for the accompanied
> *.sig file. One advantage of such solution is that all payloads are
> independent and can be fetched in parallel thus exploiting the maximum
> level of download parallelism.
>
> To build *.sig file url we've been using a simple string concatenation:
> $requested_url + ".sig". Unfortunately there are cases when it does not
> work. For example an archlinux.org "Download From Mirror" link looks like
> this https://www.archlinux.org/packages/core/x86_64/bash/download/ and
> it gets redirected to some mirror. But if we append ".sig" to the end of
> the link url and try to download it then archlinux.org returns 404 error.
>
> To overcome this issue we need to follow redirects for the main payload
> first, find the final url and only then append '.sig' suffix.
> This implies 2 things:
> - the signature payload initialization need to be moved to dload.c
> as it is the place where we have access to the resolved url
> - *.sig is downloaded serially with the main payload and this reduces
> level of parallelism
>
> Move *.sig payload creation to dload.c. Once the main payload is fetched
> successfully we check if the callee asked to download the accompanied
> signature. If yes - create a new payload and add it to mcurl.
>
> *.sig payload does not use server list of the main payload and thus does
> not support mirror failover. *.sig file comes from the same server as
> the main payload.
>
> Refactor event loop in curl_multi_download_internal() a bit. Instead of
> relying on curl_multi_check_finished_download() to return number of new
> payloads we simply rerun the loop iteration one more time to check if
> there are any active downloads left.
> ---
> lib/libalpm/be_sync.c | 34 +++-
> lib/libalpm/dload.c | 91 ++-
> lib/libalpm/dload.h | 4 +-
> 3 files changed, 65 insertions(+), 64 deletions(-)
>
> diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
> index 82018e15..41675d21 100644
> --- a/lib/libalpm/be_sync.c
> +++ b/lib/libalpm/be_sync.c
> @@ -180,12 +180,10 @@ int SYMEXPORT alpm_db_update(alpm_handle_t *handle,
> alpm_list_t *dbs, int force)
> dbforce = 1;
> }
>
> - CALLOC(payload, 1, sizeof(*payload), GOTO_ERR(handle,
> ALPM_ERR_MEMORY, cleanup));
> + siglevel = alpm_db_get_siglevel(db);
>
> - /* set hard upper limit of 128 MiB */
> - payload->max_size = 128 * 1024 * 1024;
> + CALLOC(payload, 1, sizeof(*payload), GOTO_ERR(handle,
> ALPM_ERR_MEMORY, cleanup));
> payload->servers = db->servers;
> -
> /* print server + filename into a buffer */
> len = strlen(db->treename) + strlen(dbext) + 1;
> MALLOC(payload->filepath, len,
> @@ -194,31 +192,11 @@ int SYMEXPORT alpm_db_update(alpm_handle_t *handle,
> alpm_list_t *dbs, int force)
> payload->handle = handle;
> payload->force = dbforce;
> payload->unlink_on_fail = 1;
> -
> + payload->download_signature = (siglevel & ALPM_SIG_DATABASE);
> + payload->signature_optional = (siglevel &
> ALPM_SIG_DATABASE_OPTIONAL);
> + /* set hard upper limit of 128 MiB */
> + payload->max_size = 128 * 1024 * 1024;
> payloads = alpm_list_add(payloads, payload);
OK.
> -
> - siglevel = alpm_db_get_siglevel(db);
> - if(siglevel & ALPM_SIG_DATABASE) {
> - struct dload_payload *sig_payload;
> - CALLOC(sig_payload, 1, sizeof(*sig_payload),
> GOTO_ERR(handle, ALPM_ERR_MEMORY, cleanup));
> - sig_payload->signature = 1;
> -
> - /* print filename into a buffer (leave space for
> separator and .sig) */
> - len = strlen(db->treename) + strlen(dbext) + 5;
> - MALLOC(sig_payload->filepath, len,
> - FREE(sig_payload); GOTO_ERR(handle,
> ALPM_ERR_MEMORY, cleanup));
> - snprintf(sig_payload->filepath, len, "%s%s.sig",
> db->treename, dbext);
> -
> - sig_payload->handle = handle;
> - sig_payload->force = dbforce;
> - sig_payload->errors_ok = (siglevel &
> ALPM_SIG_DATABASE_OPTIONAL);
> -
> - /* set hard upper limit of 16 KiB */
> - sig_payload->max_size = 16 * 1024;
> - sig_payload->servers = db->servers;
> -
> - payloads = alpm_list_add(payloads, sig_payload);
> - }
> }
>
> event.type = ALPM_EVENT_DB_RETRIEVE_START;
OK.
> diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
> index 4dbb011f..b68dcf6d 100644
> --- a/l
