Hi
On Mon, Jan 11, 2021 at 4:27 AM morganamilo <[email protected]> wrote:
>
> When downloading a package with -U, alpm only checks if the package
> itself is in cache when deciding whether anything needs to be
> downloaded. So if for some reason the package is in cache but the
> signature file is not, there's be no attempt to download the signature
> and instead just throw an error.
>
> morganamilo@Octavia ~git/pacman % rm /var/cache/pacman/pkg/*.sig
> morganamilo@Octavia ~git/pacman % sudo ./build/pacman -U
> https://mirrors.ims.nksc.lt/archlinux/extra/os/x86_64/xterm-363-1-x86_64.pkg.tar.zst
> loading packages...
> error: '/var/cache/pacman/pkg/xterm-363-1-x86_64.pkg.tar.zst': package
> missing required signature
>
> So let's just make sure to check that the package and sig file is there
> before downloading. Like how the -S codepath already does.
Thanks for fixing the issue. The commit looks good to me. A few questions below.
> Also, I think the way signature downloading is a bit weird. You can't
> just download a signature. You have to say you want to download the
> package then the downloader will download the sig after the package
> finishes downloading.
Is this question resolved so it can be removed from the commit message?
> I think it would make more sense for signatures to be their own
> payloads and then have a dlsigcb.
>
> This would go towards fixing FS#67813
Could you please add a reference to FS#33992?
>
> If totaldlcb reports 0 packages to download, then we can show the
> progress bars for the sigs instead of the packages.
> ---
> lib/libalpm/dload.c | 21 ++++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
> index df5e8be7..66ebeae9 100644
> --- a/lib/libalpm/dload.c
> +++ b/lib/libalpm/dload.c
> @@ -863,8 +863,27 @@ int SYMEXPORT alpm_fetch_pkgurl(alpm_handle_t *handle,
> const alpm_list_t *urls,
> char *url = i->data;
>
> /* attempt to find the file in our pkgcache */
> +
> char *filepath = filecache_find_url(handle, url);
> - if(filepath) {
> + int need_download = !filepath;
> + /* even if the package file in the cache we need to check for
> + * accompanion *.sig file as well.
> + * If *.sig is not cached then force download the package +
> its signature file.
> + */
> + if(!need_download && (handle->siglevel & ALPM_SIG_PACKAGE)) {
> + char *sig_filename = NULL;
> + int len = strlen(filepath) + 5;
> +
> + MALLOC(sig_filename, len, RET_ERR(handle,
> ALPM_ERR_MEMORY, -1));
> + snprintf(sig_filename, len, "%s.sig", filepath);
> +
> + need_download = !_alpm_filecache_exists(handle,
> sig_filename);
> +
> + FREE(sig_filename);
> + }
> +
> +
> + if(!need_download) {
> /* the file is locally cached so add it to the output
> right away */
> alpm_list_append(fetched, filepath);
> } else {
Is there a chance of a memory leak for 'filepath' here? What if 'url'
is in the cache, but its signature is not. It looks like in this case
we do not free 'filepath' anywhere.
