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

[Guillaume Benoit]

Le 02/05/2021 à 11:47, Allan McRae a écrit :> 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> Yes, with pacman 5, fetching a URL with XferCommand doesn't work.With pacman 6, it doesn't work with the current code but it works with this patch,except for the special URLs given by Andrew Gregory.I can try to provide a different patch that pass the full payload to the front-endbut it can't help making pacman 6 correctly fetching a URL with XferCommand when thefilename can be guessed from the URL.