GNU bug report logs - #75065
Upon archive download failure print the original error

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 75065 in the body.
You can then email your comments to 75065 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
To: bug-gnu-emacs <at> gnu.org
Subject: Upon archive download failure print the original error
Date: Tue, 24 Dec 2024 18:25:30 +0300

[Message part 1 (text/plain, inline)]

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

[1.patch (text/x-patch, attachment)]

Message #10 received at 75065 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>,
 Philip Kaludercic <philipk <at> posteo.net>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 75065 <at> debbugs.gnu.org
Subject: Re: bug#75065: Upon archive download failure print the original error
Date: Thu, 26 Dec 2024 10:55:21 +0200

> From: Konstantin Kharlamov <Hi-Angel <at> 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 <at> 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?

Message #13 received at 75065 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 75065 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
Subject: Re: bug#75065: Upon archive download failure print the original error
Date: Thu, 26 Dec 2024 18:13:00 +0000

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Konstantin Kharlamov <Hi-Angel <at> 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 <at> 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.

Message #16 received at 75065 <at> debbugs.gnu.org (full text, mbox):

From: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
To: Philip Kaludercic <philipk <at> posteo.net>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 75065 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#75065: Upon archive download failure print the original error
Date: Thu, 26 Dec 2024 21:17:00 +0300

On Thu, 2024-12-26 at 18:13 +0000, Philip Kaludercic wrote:
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > > From: Konstantin Kharlamov <Hi-Angel <at> 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 <at> 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.

Message #19 received at 75065 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
Cc: 75065 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#75065: Upon archive download failure print the original error
Date: Thu, 26 Dec 2024 18:31:41 +0000

Konstantin Kharlamov <Hi-Angel <at> yandex.ru> writes:

> On Thu, 2024-12-26 at 18:13 +0000, Philip Kaludercic wrote:
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> 
>> > > From: Konstantin Kharlamov <Hi-Angel <at> 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 <at> 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.

In that case I think %S is preferable, and the patch is fine with me.

Message #22 received at 75065 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 75065 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
Subject: Re: bug#75065: Upon archive download failure print the original error
Date: Thu, 26 Dec 2024 14:17:46 -0500

>>>    (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))))))
>>>  
>> Stefan and Philip, is this okay to install?

I agree with the idea behind the patch, but printing just `(cdr err)`
doesn't seem right, it should print the whole of `err`.

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

`%s` to print `err` or `(cdr err)` would be wrong, since `%s` is for use
with strings rather than lists.  IOW, IMO, it should be either

    ...%S" ... err)

or

    ...%s" ... (error-message-string err))

where the first is a bit more "debugging/developer" friendly and the second
is a bit more "user" friendly.


        Stefan

Message #25 received at 75065 <at> debbugs.gnu.org (full text, mbox):

From: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>, Philip Kaludercic
 <philipk <at> posteo.net>
Cc: 75065 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#75065: Upon archive download failure print the original error
Date: Thu, 26 Dec 2024 23:14:10 +0300

On Thu, 2024-12-26 at 14:17 -0500, Stefan Monnier wrote:
> > > >    (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))))))
> > > >
> > > Stefan and Philip, is this okay to install?
>
> I agree with the idea behind the patch, but printing just `(cdr err)`
> doesn't seem right, it should print the whole of `err`.

The `car` seems to just contain word error. Here's how both compare:

• current patch with `(cdr err)`:
    Failed to download ‘melpa’ archive. Error: ("Location melpa.org/packages/ is not a url nor an absolute file name")

• suggested change with `err`:
    Failed to download ‘melpa’ archive. Error: (error "Location melpa.org/packages/ is not a url nor an absolute file name")

I can of course remove the word `Error` in the second case. My question
then is: will `(car err)` always be the word "error"? Or may there be
another content?

> > It seems harmless, I am just uncertain if we should prefer %S or %s
> > to
> > format the error message.
>
> `%s` to print `err` or `(cdr err)` would be wrong, since `%s` is for
> use
> with strings rather than lists.  IOW, IMO, it should be either
>
>     ...%S" ... err)
>
> or
>
>     ...%s" ... (error-message-string err))
>
> where the first is a bit more "debugging/developer" friendly and the
> second
> is a bit more "user" friendly.

In my tests there seems to be no difference in the output between %s
and %S. I would presume doing `(message "%s" '(a b))` would result in
error as the param isn't a string, but it works.

Message #28 received at 75065 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 
 Philip Kaludercic <philipk <at> posteo.net>
Cc: 75065 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#75065: Upon archive download failure print the original error
Date: Thu, 26 Dec 2024 20:32:08 +0000

Konstantin Kharlamov <Hi-Angel <at> yandex.ru> writes:

> On Thu, 2024-12-26 at 14:17 -0500, Stefan Monnier wrote:
>> > > >    (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))))))
>> > > >
>> > > Stefan and Philip, is this okay to install?
>>
>> I agree with the idea behind the patch, but printing just `(cdr err)`
>> doesn't seem right, it should print the whole of `err`.
>
> The `car` seems to just contain word error. Here's how both compare:
>
> • current patch with `(cdr err)`:
>     Failed to download ‘melpa’ archive. Error: ("Location melpa.org/packages/ is not a url nor an absolute file name")
>
> • suggested change with `err`:
>     Failed to download ‘melpa’ archive. Error: (error "Location melpa.org/packages/ is not a url nor an absolute file name")

The "Error:" part is redundant, so I think it could be shortened to
something like:

    Failed to download ‘melpa’ archive: Location melpa.org/packages/ is
    not a url nor an absolute file name

Message #31 received at 75065 <at> debbugs.gnu.org (full text, mbox):

From: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
To: Stefan Kangas <stefankangas <at> gmail.com>, Stefan Monnier
 <monnier <at> iro.umontreal.ca>, Philip Kaludercic <philipk <at> posteo.net>
Cc: 75065 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#75065: Upon archive download failure print the original error
Date: Thu, 26 Dec 2024 23:37:31 +0300

On Thu, 2024-12-26 at 20:32 +0000, Stefan Kangas wrote:
> Konstantin Kharlamov <Hi-Angel <at> yandex.ru> writes:
> 
> > On Thu, 2024-12-26 at 14:17 -0500, Stefan Monnier wrote:
> > > > > >    (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))))))
> > > > > > 
> > > > > Stefan and Philip, is this okay to install?
> > > 
> > > I agree with the idea behind the patch, but printing just `(cdr
> > > err)`
> > > doesn't seem right, it should print the whole of `err`.
> > 
> > The `car` seems to just contain word error. Here's how both
> > compare:
> > 
> > • current patch with `(cdr err)`:
> >     Failed to download ‘melpa’ archive. Error: ("Location
> > melpa.org/packages/ is not a url nor an absolute file name")
> > 
> > • suggested change with `err`:
> >     Failed to download ‘melpa’ archive. Error: (error "Location
> > melpa.org/packages/ is not a url nor an absolute file name")
> 
> The "Error:" part is redundant, so I think it could be shortened to
> something like:
> 
>     Failed to download ‘melpa’ archive: Location melpa.org/packages/
> is
>     not a url nor an absolute file name

Please note that in the above examples everything after the word
`Error: ` is taken from the exception message, which includes the
parentheses and (if I follow Stefan's advice) the downcased word
"error".

The reason in my patch I used `Error:` is because I have no control
over the text that will follow, so I need to make sure it's clear that
what follows is an error message. Hence my question to Stefan above: if
the downcase word `error` will always be there, then removing `Error:`
and replacing `(cdr err)` with `err` will works just as well.

Message #34 received at 75065 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
Cc: 75065 <at> debbugs.gnu.org, Philip Kaludercic <philipk <at> posteo.net>,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#75065: Upon archive download failure print the original error
Date: Thu, 26 Dec 2024 15:40:21 -0500

> The `car` seems to just contain word error. Here's how both compare:
>
> • current patch with `(cdr err)`:
>     Failed to download ‘melpa’ archive. Error: ("Location melpa.org/packages/ is not a url nor an absolute file name")
>
> • suggested change with `err`:
>     Failed to download ‘melpa’ archive. Error: (error "Location melpa.org/packages/ is not a url nor an absolute file name")
>
> I can of course remove the word `Error` in the second case.
> My question then is: will `(car err)` always be the word "error"? Or
> may there be another content?

The shape and content of `err` depends on the actual error that caused
the download to fail.  `(car err)` contains the error "type", which can
be `error` but can also be more specific such as
`wrong-number-of-arguments`, `file-error`, ...

>> > It seems harmless, I am just uncertain if we should prefer %S or %s
>> > to format the error message.
>>
>> `%s` to print `err` or `(cdr err)` would be wrong, since `%s` is for
>> use with strings rather than lists.  IOW, IMO, it should be either
>>
>>     ...%S" ... err)
>>
>> or
>>
>>     ...%s" ... (error-message-string err))
>>
>> where the first is a bit more "debugging/developer" friendly and the
>> second is a bit more "user" friendly.
>
> In my tests there seems to be no difference in the output between %s
> and %S.

Try (format "%S" '(a "b")) vs (format "%s" '(a "b"))

> I would presume doing `(message "%s" '(a b))` would result in
> error as the param isn't a string, but it works.

I agree that in an ideal world it should signal an error, but here
we are.


        Stefan

Message #37 received at 75065 <at> debbugs.gnu.org (full text, mbox):

From: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 75065 <at> debbugs.gnu.org, Philip Kaludercic <philipk <at> posteo.net>,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#75065: Upon archive download failure print the original error
Date: Thu, 26 Dec 2024 23:51:30 +0300

[Message part 1 (text/plain, inline)]

On Thu, 2024-12-26 at 15:40 -0500, Stefan Monnier wrote:
> > The `car` seems to just contain word error. Here's how both
> > compare:
> > 
> > • current patch with `(cdr err)`:
> >     Failed to download ‘melpa’ archive. Error: ("Location
> > melpa.org/packages/ is not a url nor an absolute file name")
> > 
> > • suggested change with `err`:
> >     Failed to download ‘melpa’ archive. Error: (error "Location
> > melpa.org/packages/ is not a url nor an absolute file name")
> > 
> > I can of course remove the word `Error` in the second case.
> > My question then is: will `(car err)` always be the word "error"?
> > Or
> > may there be another content?
> 
> The shape and content of `err` depends on the actual error that
> caused
> the download to fail.  `(car err)` contains the error "type", which
> can
> be `error` but can also be more specific such as
> `wrong-number-of-arguments`, `file-error`, ...

Thank you, I see!

Please check the attached patch. With this change the error of
accessing https-less address is shown as follows:

	Failed to download ‘melpa’ archive: (error "Location melpa.org/packages/ is not a url nor an absolute file name")

[1.patch (text/x-patch, attachment)]

Message #40 received at 75065 <at> debbugs.gnu.org (full text, mbox):

From: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 75065 <at> debbugs.gnu.org, Philip Kaludercic <philipk <at> posteo.net>,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#75065: Upon archive download failure print the original error
Date: Thu, 26 Dec 2024 23:53:41 +0300

Oh, actually, let me fix the other place as well then…

On Thu, 2024-12-26 at 23:51 +0300, Konstantin Kharlamov wrote:
> On Thu, 2024-12-26 at 15:40 -0500, Stefan Monnier wrote:
> > > The `car` seems to just contain word error. Here's how both
> > > compare:
> > > 
> > > • current patch with `(cdr err)`:
> > >     Failed to download ‘melpa’ archive. Error: ("Location
> > > melpa.org/packages/ is not a url nor an absolute file name")
> > > 
> > > • suggested change with `err`:
> > >     Failed to download ‘melpa’ archive. Error: (error "Location
> > > melpa.org/packages/ is not a url nor an absolute file name")
> > > 
> > > I can of course remove the word `Error` in the second case.
> > > My question then is: will `(car err)` always be the word "error"?
> > > Or
> > > may there be another content?
> > 
> > The shape and content of `err` depends on the actual error that
> > caused
> > the download to fail.  `(car err)` contains the error "type", which
> > can
> > be `error` but can also be more specific such as
> > `wrong-number-of-arguments`, `file-error`, ...
> 
> Thank you, I see!
> 
> Please check the attached patch. With this change the error of
> accessing https-less address is shown as follows:
> 
> 	Failed to download ‘melpa’ archive: (error "Location
> melpa.org/packages/ is not a url nor an absolute file name")

Message #43 received at 75065 <at> debbugs.gnu.org (full text, mbox):

From: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 75065 <at> debbugs.gnu.org, Philip Kaludercic <philipk <at> posteo.net>,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#75065: Upon archive download failure print the original error
Date: Fri, 27 Dec 2024 00:03:59 +0300

[Message part 1 (text/plain, inline)]

On Thu, 2024-12-26 at 23:53 +0300, Konstantin Kharlamov wrote:
> Oh, actually, let me fix the other place as well then…
> 
> On Thu, 2024-12-26 at 23:51 +0300, Konstantin Kharlamov wrote:
> > On Thu, 2024-12-26 at 15:40 -0500, Stefan Monnier wrote:
> > > > The `car` seems to just contain word error. Here's how both
> > > > compare:
> > > > 
> > > > • current patch with `(cdr err)`:
> > > >     Failed to download ‘melpa’ archive. Error: ("Location
> > > > melpa.org/packages/ is not a url nor an absolute file name")
> > > > 
> > > > • suggested change with `err`:
> > > >     Failed to download ‘melpa’ archive. Error: (error "Location
> > > > melpa.org/packages/ is not a url nor an absolute file name")
> > > > 
> > > > I can of course remove the word `Error` in the second case.
> > > > My question then is: will `(car err)` always be the word
> > > > "error"?
> > > > Or
> > > > may there be another content?
> > > 
> > > The shape and content of `err` depends on the actual error that
> > > caused
> > > the download to fail.  `(car err)` contains the error "type",
> > > which
> > > can
> > > be `error` but can also be more specific such as
> > > `wrong-number-of-arguments`, `file-error`, ...
> > 
> > Thank you, I see!
> > 
> > Please check the attached patch. With this change the error of
> > accessing https-less address is shown as follows:
> > 
> > 	Failed to download ‘melpa’ archive: (error "Location
> > melpa.org/packages/ is not a url nor an absolute file name")
> 

Done, please review 😊

[1.patch (text/x-patch, attachment)]

Message #46 received at 75065 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 75065 <at> debbugs.gnu.org, philipk <at> posteo.net, Hi-Angel <at> yandex.ru
Subject: Re: bug#75065: Upon archive download failure print the original error
Date: Fri, 27 Dec 2024 09:22:19 +0200

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: Philip Kaludercic <philipk <at> posteo.net>,  Eli Zaretskii <eliz <at> gnu.org>,
>   75065 <at> debbugs.gnu.org
> Date: Thu, 26 Dec 2024 15:40:21 -0500
> 
> > The `car` seems to just contain word error. Here's how both compare:
> >
> > • current patch with `(cdr err)`:
> >     Failed to download ‘melpa’ archive. Error: ("Location melpa.org/packages/ is not a url nor an absolute file name")
> >
> > • suggested change with `err`:
> >     Failed to download ‘melpa’ archive. Error: (error "Location melpa.org/packages/ is not a url nor an absolute file name")
> >
> > I can of course remove the word `Error` in the second case.
> > My question then is: will `(car err)` always be the word "error"? Or
> > may there be another content?
> 
> The shape and content of `err` depends on the actual error that caused
> the download to fail.  `(car err)` contains the error "type", which can
> be `error` but can also be more specific such as
> `wrong-number-of-arguments`, `file-error`, ...

Yes, the car part is not redundant, it can include useful information
that we should not lose.

Message #51 received at 75065-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 75065-done <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
Subject: Re: bug#75065: Upon archive download failure print the original error
Date: Fri, 27 Dec 2024 12:01:14 -0500

[Message part 1 (text/plain, inline)]

> `%s` to print `err` or `(cdr err)` would be wrong, since `%s` is for use
> with strings rather than lists.  IOW, IMO, it should be either
>
>     ...%S" ... err)
>
> or
>
>     ...%s" ... (error-message-string err))
>
> where the first is a bit more "debugging/developer" friendly and the second
> is a bit more "user" friendly.

Revising the code of `package.el` I see that those errors&messages are
meant to be exposed to the user, so they should preferably use
`error-message-string`.

So I installed the patch below into `master`.
Thank you!
Closing,


        Stefan

[package.patch (text/x-diff, inline)]

commit a99e1cc745047977b0b7cbd25fe9df66e9c86a39
Author: Stefan Monnier <monnier <at> iro.umontreal.ca>
Date:   Fri Dec 27 11:58:30 2024 -0500

    (package--download-and-read-archives): Fix bug#75065
    
    * lisp/emacs-lisp/package.el (package--download-and-read-archives):
    Include the error info in the error message.
    Suggested by Konstantin Kharlamov <Hi-Angel <at> yandex.ru>.
    (package-refresh-contents, package-menu--perform-transaction):
    Use `error-message-string`.

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 5f785071ea3..bff45d57b07 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -1829,10 +1829,11 @@ package--download-and-read-archives
 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: %s"
+                      (car archive)
+                      (error-message-string err))))))
 
 (defvar package-refresh-contents-hook (list #'package--download-and-read-archives)
   "List of functions to call to refresh the package archive.
@@ -1856,7 +1857,8 @@ package-refresh-contents
     (when (and (package-check-signature) (file-exists-p default-keyring))
       (condition-case-unless-debug error
           (package-import-keyring default-keyring)
-        (error (message "Cannot import default keyring: %S" (cdr error))))))
+        (error (message "Cannot import default keyring: %s"
+                        (error-message-string error))))))
   (run-hook-with-args 'package-refresh-contents-hook async))
 
 
@@ -3989,8 +3991,9 @@ package-menu--perform-transaction
               (package-delete elt nil 'nosave))
           (error
            (push (package-desc-full-name elt) errors)
-           (message "Error trying to delete `%s': %S"
-                    (package-desc-full-name elt) err)))))
+           (message "Error trying to delete `%s': %s"
+                    (package-desc-full-name elt)
+                    (error-message-string err))))))
     errors))
 
 (defun package--update-selected-packages (add remove)

Message #54 received at 75065-done <at> debbugs.gnu.org (full text, mbox):

From: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>, Philip Kaludercic
 <philipk <at> posteo.net>
Cc: 75065-done <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#75065: Upon archive download failure print the original error
Date: Fri, 27 Dec 2024 21:34:30 +0300

On Fri, 2024-12-27 at 12:01 -0500, Stefan Monnier wrote:
> > `%s` to print `err` or `(cdr err)` would be wrong, since `%s` is
> > for use
> > with strings rather than lists.  IOW, IMO, it should be either
> > 
> >     ...%S" ... err)
> > 
> > or
> > 
> >     ...%s" ... (error-message-string err))
> > 
> > where the first is a bit more "debugging/developer" friendly and
> > the second
> > is a bit more "user" friendly.
> 
> Revising the code of `package.el` I see that those errors&messages
> are
> meant to be exposed to the user, so they should preferably use
> `error-message-string`.
> 
> So I installed the patch below into `master`.
> Thank you!

This is not fair: despite me actively answering on my patch, you took
my changes, just changed one function call to another, and rewritten
the author like if it were you who were writing and debugging the code
and the issue rather than me.

Message #57 received at 75065 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
Cc: 75065 <at> debbugs.gnu.org, Philip Kaludercic <philipk <at> posteo.net>,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#75065: Upon archive download failure print the original error
Date: Fri, 27 Dec 2024 22:09:21 -0500

> This is not fair: despite me actively answering on my patch, you took
> my changes, just changed one function call to another, and rewritten
> the author like if it were you who were writing and debugging the code
> and the issue rather than me.

You're right, I'm sorry.


        Stefan

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.