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

[Eli Schwartz]

On 5/2/21 3:55 PM, Guillaume Benoit wrote:
> 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.
> 


Help me out here a bit, and repeat that?

-- 
Eli Schwartz
Bug Wrangler and Trusted User

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.archlinux.org/pipermail/pacman-dev/attachments/20210502/eb4f1836/attachment.sig>