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

[Konstantin Kharlamov]

On Thu, 2024-12-26 at 18:13 +0000, Philip Kaludercic wrote:
> 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.

The reason I used %S is that 2x lines below there's similar
construction with (error …) and it's using %S, so the usage here goes
in line with the other code.

That said, I have no preference, I can change it to %s, even in both
locations if you want.