bug#75065: Upon archive download failure print the original error

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Konstantin Kharlamov <Hi-Angel@yandex.ru>
>> Date: Tue, 24 Dec 2024 18:25:30 +0300
>> 
>> I was recently helping out a new Emacs user with package installation¹,
>> and I found an interesting thing: if you put to `package-archives` a
>> URL without `https` prefix, download will fail. Long story short, the
>> reason turns out that `package-archives` also supports local paths,
>> which the URL being considered as. However, Emacs never prints a
>> message about that, even though such message exists in the code.
>> Instead it just says that download failed, leaving a user wondering
>> why.
>> 
>> That happens because (package--download-and-read-archives) ignores the
>> exception message, and always just prints generic failure message.
>> 
>> This code fixes this, so now the actual failure message will be
>> correctly shown.
>> 
>> 1:
>> https://emacs.stackexchange.com/questions/82828/is-installing-deadgrep-fron-melpa-still-possible/82829#82829
>> 
>> 
>> From fb4685238726a79599f6388318916d2962da93ae Mon Sep 17 00:00:00 2001
>> From: Konstantin Kharlamov <Hi-Angel@yandex.ru>
>> Date: Tue, 24 Dec 2024 18:16:41 +0300
>> Subject: [PATCH] Upon archive download failure print the original error
>> 
>> * lisp/emacs-lisp/package.el (package--download-and-read-archives):
>> upon catching exception, use the exception message as part of the
>> error to provide more context about the failure.
>> ---
>>  lisp/emacs-lisp/package.el | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>> 
>> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
>> index 5f785071ea3..cb81efc71f0 100644
>> --- a/lisp/emacs-lisp/package.el
>> +++ b/lisp/emacs-lisp/package.el
>> @@ -1829,10 +1829,10 @@ Populate `package-archive-contents' with the result.
>>  If optional argument ASYNC is non-nil, perform the downloads
>>  asynchronously."
>>    (dolist (archive package-archives)
>> -    (condition-case-unless-debug nil
>> +    (condition-case-unless-debug err
>>          (package--download-one-archive archive "archive-contents" async)
>> -      (error (message "Failed to download `%s' archive."
>> -               (car archive))))))
>> +      (error (message "Failed to download `%s' archive. Error: %S"
>> +               (car archive) (cdr err))))))
>>  
>>  (defvar package-refresh-contents-hook (list 
>> #'package--download-and-read-archives)
>>    "List of functions to call to refresh the package archive.
>> -- 
>> 2.47.1
>
> Thanks.
>
> Stefan and Philip, is this okay to install?

It seems harmless, I am just uncertain if we should prefer %S or %s to
format the error message.