[pacman-dev] [PATCH] libalpm: fix alpm_fetch_pkgurl with a fetch callback

[Allan McRae]

On 2/5/21 12:33 am, Guillaume Benoit wrote:
> Le 01/05/2021 à 05:53, Andrew Gregory a écrit :
>> On 04/30/21 at 12:09pm, Guillaume Benoit wrote:
>>> After download, alpm_fetch_pkgurl uses payload->destfile_name
>>> and payload->tempfile_name in order to find the downloaded file path.
>>> Those fields are not set if a custom fetch callback is defined.
>>> Use filecache_find_url instead, like in the beginning of the function.
>>>
>>> ---
>>>   lib/libalpm/dload.c | 13 ++++---------
>>>   1 file changed, 4 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
>>> index 6f33451a..d45c7707 100644
>>> --- a/lib/libalpm/dload.c
>>> +++ b/lib/libalpm/dload.c
>>> @@ -1007,16 +1007,11 @@ int SYMEXPORT alpm_fetch_pkgurl(alpm_handle_t
>>> *handle, const alpm_list_t *urls,
>>>               EVENT(handle, &event);
>>>           }
>>>   -        for(i = payloads; i; i = i->next) {
>>> -            struct dload_payload *payload = i->data;
>>> -            char *filepath;
>>> +        for(i = urls; i; i = i->next) {
>>> +            char *url = i->data;
>>>   -            if(payload->destfile_name) {
>>> -                const char *filename =
>>> mbasename(payload->destfile_name);
>>> -                filepath = _alpm_filecache_find(handle, filename);
>>> -            } else {
>>> -                STRDUP(filepath, payload->tempfile_name,
>>> GOTO_ERR(handle, ALPM_ERR_MEMORY, err));
>>> -            }
>>> +            /* attempt again to find the file in our pkgcache */
>>> +            char *filepath = filecache_find_url(handle, url);
>>>               if(filepath) {
>>>                   alpm_list_append(fetched, filepath);
>>>               } else {
>>
>> Without testing since this needs to be rebased now; this looks broken.
>> filecache_find_url with fail for any url where destfile_name doesn't
>> match the file name in the url.  Notably, that includes the download
>> links on Arch's package pages:
>> https://archlinux.org/packages/core/x86_64/pacman/download/
>>
> Yes, with or without this patch, the current implementation doesn't work
> for this kind of url when using a fetch callback.
> The solution I see is to change the fetch callback signature by making
> it return the path of the downloaded file.
> Otherwise this patch works for standard mirror urls.
> .

While this patch does improve the current situation, it is a step
backwards in terms the full fix, which will need to pass the full
payload to the front-end so it can fill in the needed values.

On that basis, I will not be accepting this patch.

Given fetching a URL with XferCommand has not worked for a long time (or
ever?), this issue is not a blocker for 6.0.

Allan