Subject: bug#59684: closed (Re: bug#59684: 29.0.50; [PATCH] package.el: Insert news for installed packages only)

From: [email-masked] (GNU bug Tracking System)
Date: Wed, 30 Nov 2022 19:55:02 +0000

Your bug report

#59684: 29.0.50; [PATCH] package.el: Insert news for installed packages only

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
59684: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=59684
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#59684: 29.0.50; [PATCH] package.el: Insert news for installed packages only
From: Philip Kaludercic <[email-masked]>
To: Andrii Kolomoiets <[email-masked]>
Cc: [email-masked]
Date: Wed, 30 Nov 2022 19:53:54 +0000
List-Id: <debbugs-submit.debbugs.gnu.org>

Andrii Kolomoiets <[email-masked]> writes:

> Philip Kaludercic <[email-masked]> 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.

Subject: 29.0.50; [PATCH] package.el: Insert news for installed packages only
From: Andrii Kolomoiets <[email-masked]>
To: [email-masked]
Cc: Philip Kaludercic <[email-masked]>
Date: Tue, 29 Nov 2022 18:54:44 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

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

>From 9bd185609643ec861b02b4327295bb6db3b82974 Mon Sep 17 00:00:00 2001
From: muffinmad <[email-masked]>
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)))
                  file)))
     (when (string= status "avail-obso")
       (setq status "available obsolete"))
-- 
2.32.1 (Apple Git-133)