GNU bug report logs - #59684
29.0.50; [PATCH] package.el: Insert news for installed packages only

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 59684 in the body.
You can then email your comments to 59684 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: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Cc: Philip Kaludercic <philipk <at> posteo.net>
Subject: 29.0.50; [PATCH] package.el: Insert news for installed packages only
Date: Tue, 29 Nov 2022 18:54:44 +0200

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

Hi,

In some folder, e.g. "/tmp":
1. Create folder named "news"
2. emacs -Q
3. M-x list-packages
4. RET on any available not installed package

This will produce "Read error: Is a directory, /tmp/news" message.

In GNU Emacs 29.0.50
Repository revision: 2772ebe3667f28cefd0d6134204ce6a3c7a8c323
Repository branch: master

Adding news to package description was introuced in
dd98fedd0c7f27bfba046d761042c19181cb461d

Adding Philip to Cc as the author of the original commit.

Attached patch solves issue by checking that:
1. pkg-dir is not nil
2. pkg-dir is not 'builtin
3. "news" is a regular file

[0001-Insert-news-for-installed-packages-only.patch (text/x-patch, attachment)]

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Cc: bug-gnu-emacs <at> gnu.org
Subject: Re: 29.0.50; [PATCH] package.el: Insert news for installed packages
 only
Date: Tue, 29 Nov 2022 21:06:33 +0000

Andrii Kolomoiets <andreyk.mad <at> gmail.com> writes:

> Hi,
>
> In some folder, e.g. "/tmp":
> 1. Create folder named "news"
> 2. emacs -Q
> 3. M-x list-packages
> 4. RET on any available not installed package
>
> This will produce "Read error: Is a directory, /tmp/news" message.
>
> In GNU Emacs 29.0.50
> Repository revision: 2772ebe3667f28cefd0d6134204ce6a3c7a8c323
> Repository branch: master
>
> Adding news to package description was introuced in
> dd98fedd0c7f27bfba046d761042c19181cb461d
>
> Adding Philip to Cc as the author of the original commit.
>
> Attached patch solves issue by checking that:
> 1. pkg-dir is not nil
> 2. pkg-dir is not 'builtin
> 3. "news" is a regular file

This sounds good.

> From 9bd185609643ec861b02b4327295bb6db3b82974 Mon Sep 17 00:00:00 2001
> From: muffinmad <andreyk.mad <at> gmail.com>
> Date: Mon, 28 Nov 2022 23:09:59 +0200
> Subject: [PATCH] Insert news for installed packages only
>
> * lisp/emacs-lisp/package.el (describe-package-1): Check package is
> installed and "news" is a regular file.
> ---
>  lisp/emacs-lisp/package.el | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
> index 8d44fae30a0..f722eb8fae9 100644
> --- a/lisp/emacs-lisp/package.el
> +++ b/lisp/emacs-lisp/package.el
> @@ -2696,8 +2696,10 @@ describe-package-1
>           (signed (if desc (package-desc-signed desc)))
>           (maintainer (cdr (assoc :maintainer extras)))
>           (authors (cdr (assoc :authors extras)))
> -         (news (and-let* ((file (expand-file-name "news" pkg-dir))
> -                          ((file-readable-p file)))
> +         (news (and-let* (pkg-dir
> +                          ((not built-in))
> +                          (file (expand-file-name "news" pkg-dir))
> +                          ((file-regular-p file)))

Shouldn't we keep `file-readable-p'?

>                   file)))
>      (when (string= status "avail-obso")
>        (setq status "available obsolete"))

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

From: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: bug-gnu-emacs <at> gnu.org
Subject: Re: 29.0.50; [PATCH] package.el: Insert news for installed packages
 only
Date: Tue, 29 Nov 2022 23:16:00 +0200

Philip Kaludercic <philipk <at> posteo.net> writes:

>> +                          ((file-regular-p file)))
>
> Shouldn't we keep `file-readable-p'?

I think we should use `file-regular-p' so even if the pkg-dir will
contain directory named "news" we will not try to insert it with
`insert-file-contents'.

-- 
Andrii

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Cc: bug-gnu-emacs <at> gnu.org
Subject: Re: 29.0.50; [PATCH] package.el: Insert news for installed packages
 only
Date: Tue, 29 Nov 2022 21:25:32 +0000

Andrii Kolomoiets <andreyk.mad <at> gmail.com> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>>> +                          ((file-regular-p file)))
>>
>> Shouldn't we keep `file-readable-p'?
>
> I think we should use `file-regular-p' so even if the pkg-dir will
> contain directory named "news" we will not try to insert it with
> `insert-file-contents'.

I agree that `file-regular-p' is sensible, what I meant was using
`file-readable-p' in addition to that.  But I guess that all in all,
this is a rare circumstance...

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

From: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 59684 <at> debbugs.gnu.org
Subject: Re: bug#59684: 29.0.50; [PATCH] package.el: Insert news for
 installed packages only
Date: Tue, 29 Nov 2022 23:38:24 +0200

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

Philip Kaludercic <philipk <at> posteo.net> writes:

>>>> +                          ((file-regular-p file)))
>>>
>>> Shouldn't we keep `file-readable-p'?
>>
>> I think we should use `file-regular-p' so even if the pkg-dir will
>> contain directory named "news" we will not try to insert it with
>> `insert-file-contents'.
>
> I agree that `file-regular-p' is sensible, what I meant was using
> `file-readable-p' in addition to that.  But I guess that all in all,
> this is a rare circumstance...

Oh, right.  Makes sense.  I've updated the patch.

[0001-Insert-news-for-installed-packages-only.patch (text/x-patch, attachment)]

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Cc: 59684-done <at> debbugs.gnu.org
Subject: Re: bug#59684: 29.0.50; [PATCH] package.el: Insert news for
 installed packages only
Date: Wed, 30 Nov 2022 19:53:54 +0000

Andrii Kolomoiets <andreyk.mad <at> gmail.com> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>>>>> +                          ((file-regular-p file)))
>>>>
>>>> Shouldn't we keep `file-readable-p'?
>>>
>>> I think we should use `file-regular-p' so even if the pkg-dir will
>>> contain directory named "news" we will not try to insert it with
>>> `insert-file-contents'.
>>
>> I agree that `file-regular-p' is sensible, what I meant was using
>> `file-readable-p' in addition to that.  But I guess that all in all,
>> this is a rare circumstance...
>
> Oh, right.  Makes sense.  I've updated the patch.

Thanks, I've pushed the change.

Previous Next

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