GNU bug report logs - #69528
30.0.50; [BUG] transient.el is not a member of package--builtin-versions

Previous Next

Package: emacs;

Reported by: No Wayman <iarchivedmywholelife <at> gmail.com>

Date: Sun, 3 Mar 2024 17:26:02 UTC

Severity: normal

Found in version 30.0.50

To reply to this bug, email your comments to 69528 AT debbugs.gnu.org.

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#69528; Package emacs. (Sun, 03 Mar 2024 17:26:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to No Wayman <iarchivedmywholelife <at> gmail.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Sun, 03 Mar 2024 17:26:02 GMT) Full text and rfc822 format available.

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

From: No Wayman <iarchivedmywholelife <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 30.0.50; [BUG] transient.el is not a member of
 package--builtin-versions
Date: Sun, 03 Mar 2024 12:25:16 -0500
Transient.el was added in 28.1 according to NEWS.
It is not a member of package--builtin-versions for any of the 
following Emacs versions: 28.1, 28.2, 29.1, 29.2.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69528; Package emacs. (Mon, 04 Mar 2024 17:23:02 GMT) Full text and rfc822 format available.

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

From: No Wayman <iarchivedmywholelife <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: 30.0.50; [BUG] transient.el is not a member of
 package--builtin-versions
Date: Mon, 04 Mar 2024 12:22:00 -0500
No Wayman <iarchivedmywholelife <at> gmail.com> writes:

> Transient.el was added in 28.1 according to NEWS.
> It is not a member of package--builtin-versions for any of the 
> following Emacs
> versions: 28.1, 28.2, 29.1, 29.2.

I believe the behavior described here is due to this:
https://www.reddit.com/r/emacs/comments/1b69v1b/let_magit_330_use_builtin_transient/

To summarize, the user has a built-in version of transient.el 
shipped with Emacs 29.2.
They installed magit 3.3.0, which requires transient 0.3.6. 
Instead of package.el seeing magit's transient.el dependency as 
satisfied by the built-in version, it installed the latest 
version.

I've patched Elpaca, which relies on package--builtin-vesrions, 
due to similar complaints about transient.el being pulled in 
despite the built-in version satisfying a dependency. 




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69528; Package emacs. (Mon, 04 Mar 2024 18:43:02 GMT) Full text and rfc822 format available.

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: No Wayman <iarchivedmywholelife <at> gmail.com>
Cc: 69528 <at> debbugs.gnu.org
Subject: Re: bug#69528: 30.0.50; [BUG] transient.el is not a member of
 package--builtin-versions
Date: Mon, 04 Mar 2024 18:41:26 +0000
[Message part 1 (text/plain, inline)]
No Wayman <iarchivedmywholelife <at> gmail.com> writes:

> No Wayman <iarchivedmywholelife <at> gmail.com> writes:
>
>> Transient.el was added in 28.1 according to NEWS.
>> It is not a member of package--builtin-versions for any of the
>> following Emacs
>> versions: 28.1, 28.2, 29.1, 29.2.

At least on Emacs 30, (assq 'transient package--builtin-versions) gives
me a non-nil value.  I can confirm that this is the case on the emacs-29
branch.

I suspect this commit resolved the issue, since
`loaddefs-generate--parse-file' only checks the version header, not the
package-version header.

--8<---------------cut here---------------start------------->8---
commit fa5f06c1251ff717d661f05fcd240b4792054aae
Author: Jonas Bernoulli <jonas <at> bernoul.li>
Date:   Tue Dec 5 20:01:44 2023 +0100

    ; * lisp/transient.el: Set Version instead of Package-Version
    
    `finder-compile-keywords' only considers the "Version" header.

diff --git a/lisp/transient.el b/lisp/transient.el
--- a/lisp/transient.el
+++ b/lisp/transient.el
@@ -1,35 +1,35 @@
 ;;; transient.el --- Transient commands  -*- lexical-binding:t -*-
 
 ;; Copyright (C) 2018-2023 Free Software Foundation, Inc.
 
 ;; Author: Jonas Bernoulli <jonas <at> bernoul.li>
 ;; Homepage: https://github.com/magit/transient
 ;; Keywords: extensions
 
-;; Package-Version: 0.5.2
+;; Version: 0.5.2
 ;; Package-Requires: ((emacs "26.1") (compat "29.1.4.4") (seq "2.24"))
 
 ;; SPDX-License-Identifier: GPL-3.0-or-later
 
 ;; This file is part of GNU Emacs.
 
 ;; GNU Emacs is free software: you can redistribute it and/or modify
 ;; it under the terms of the GNU General Public License as published
 ;; by the Free Software Foundation, either version 3 of the License,
 ;; or (at your option) any later version.
 ;;
 ;; GNU Emacs is distributed in the hope that it will be useful,
 ;; but WITHOUT ANY WARRANTY; without even the implied warranty of
 ;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 ;; GNU General Public License for more details.
 ;;
 ;; You should have received a copy of the GNU General Public License
 ;; along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
 ;;; Commentary:
 
 ;; Transient is the library used to implement the keyboard-driven menus
 ;; in Magit.  It is distributed as a separate package, so that it can be
 ;; used to implement similar menus in other packages.
 
 ;;; Code:

--8<---------------cut here---------------end--------------->8---

So in general, this patch might be appropriate?

[Message part 2 (text/plain, inline)]
diff --git a/lisp/emacs-lisp/loaddefs-gen.el b/lisp/emacs-lisp/loaddefs-gen.el
index 581053f6304..42f386933dc 100644
--- a/lisp/emacs-lisp/loaddefs-gen.el
+++ b/lisp/emacs-lisp/loaddefs-gen.el
@@ -433,7 +433,8 @@ loaddefs-generate--parse-file
           ;; loaddefs for packages so that `syntax-ppss' later gives
           ;; correct results.
           (emacs-lisp-mode)
-        (let ((version (lm-header "version"))
+        (let ((version (or (lm-header "package-version")
+                           (lm-header "version")))
               package)
           (when (and version
                      (setq version (ignore-errors (version-to-list version)))
[Message part 3 (text/plain, inline)]
>
> I believe the behavior described here is due to this:
> https://www.reddit.com/r/emacs/comments/1b69v1b/let_magit_330_use_builtin_transient/
>
> To summarize, the user has a built-in version of transient.el shipped
> with Emacs 29.2.
> They installed magit 3.3.0, which requires transient 0.3.6. Instead of
> package.el seeing magit's transient.el dependency as satisfied by the
> built-in version, it installed the latest version.
>
> I've patched Elpaca, which relies on package--builtin-vesrions, due to
> similar complaints about transient.el being pulled in despite the
> built-in version satisfying a dependency. 

I suspect that whatever you did, won't help us since this specific issue
will be resolved by the time any package.el-related fix would be published?

-- 
	Philip Kaludercic on peregrine

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69528; Package emacs. (Tue, 05 Mar 2024 06:21:01 GMT) Full text and rfc822 format available.

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

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: bug-gnu-emacs <at> gnu.org, No Wayman <iarchivedmywholelife <at> gmail.com>,
 69528 <at> debbugs.gnu.org
Subject: Re: bug#69528: 30.0.50; [BUG] transient.el is not a member of
 package--builtin-versions
Date: Mon, 04 Mar 2024 22:17:30 -0800
[Message part 1 (text/plain, inline)]
Philip Kaludercic <philipk <at> posteo.net> writes:
> So in general, this patch might be appropriate?
>
> diff --git a/lisp/emacs-lisp/loaddefs-gen.el b/lisp/emacs-lisp/loaddefs-gen.el
> index 581053f6304..42f386933dc 100644
> --- a/lisp/emacs-lisp/loaddefs-gen.el
> +++ b/lisp/emacs-lisp/loaddefs-gen.el
> @@ -433,7 +433,8 @@ loaddefs-generate--parse-file
>            ;; loaddefs for packages so that `syntax-ppss' later gives
>            ;; correct results.
>            (emacs-lisp-mode)
> -        (let ((version (lm-header "version"))
> +        (let ((version (or (lm-header "package-version")
> +                           (lm-header "version")))
>                package)
>            (when (and version
>                       (setq version (ignore-errors (version-to-list version)))
>
>

What about making `lm-version' handle the "package-version" header then
using `lm-version' in loaddefs-generate--parse-file?  See patches.

Joseph

[0002-Use-lm-version-instead-of-lm-header-version.patch (text/x-diff, attachment)]
[0001-Check-Package-Version-header-in-lm-version-also.patch (text/x-diff, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69528; Package emacs. (Tue, 05 Mar 2024 06:21:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69528; Package emacs. (Thu, 09 May 2024 06:55:01 GMT) Full text and rfc822 format available.

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

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: bug-gnu-emacs <at> gnu.org, No Wayman <iarchivedmywholelife <at> gmail.com>,
 69528 <at> debbugs.gnu.org
Subject: Re: bug#69528: 30.0.50; [BUG] transient.el is not a member of
 package--builtin-versions
Date: Wed, 08 May 2024 23:53:38 -0700
Joseph Turner <joseph <at> breatheoutbreathe.in> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>> So in general, this patch might be appropriate?
>>
>> diff --git a/lisp/emacs-lisp/loaddefs-gen.el b/lisp/emacs-lisp/loaddefs-gen.el
>> index 581053f6304..42f386933dc 100644
>> --- a/lisp/emacs-lisp/loaddefs-gen.el
>> +++ b/lisp/emacs-lisp/loaddefs-gen.el
>> @@ -433,7 +433,8 @@ loaddefs-generate--parse-file
>>            ;; loaddefs for packages so that `syntax-ppss' later gives
>>            ;; correct results.
>>            (emacs-lisp-mode)
>> -        (let ((version (lm-header "version"))
>> +        (let ((version (or (lm-header "package-version")
>> +                           (lm-header "version")))
>>                package)
>>            (when (and version
>>                       (setq version (ignore-errors (version-to-list version)))
>>
>>
>
> What about making `lm-version' handle the "package-version" header then
> using `lm-version' in loaddefs-generate--parse-file?  See patches.
>
> Joseph
>
> [2. text/x-diff; 0002-Use-lm-version-instead-of-lm-header-version.patch]...
>
> [3. text/x-diff; 0001-Check-Package-Version-header-in-lm-version-also.patch]...

Ping!




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69528; Package emacs. (Thu, 09 May 2024 06:55:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69528; Package emacs. (Sat, 25 May 2024 07:40:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: philipk <at> posteo.net, 69528 <at> debbugs.gnu.org, iarchivedmywholelife <at> gmail.com
Subject: Re: bug#69528: 30.0.50;
 [BUG] transient.el is not a member of package--builtin-versions
Date: Sat, 25 May 2024 10:39:16 +0300
Ping! Ping!  Philip, can you please chime in?

> Cc: 69528 <at> debbugs.gnu.org, iarchivedmywholelife <at> gmail.com
> Date: Wed, 08 May 2024 23:53:38 -0700
> From:  Joseph Turner via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> Joseph Turner <joseph <at> breatheoutbreathe.in> writes:
> 
> > Philip Kaludercic <philipk <at> posteo.net> writes:
> >> So in general, this patch might be appropriate?
> >>
> >> diff --git a/lisp/emacs-lisp/loaddefs-gen.el b/lisp/emacs-lisp/loaddefs-gen.el
> >> index 581053f6304..42f386933dc 100644
> >> --- a/lisp/emacs-lisp/loaddefs-gen.el
> >> +++ b/lisp/emacs-lisp/loaddefs-gen.el
> >> @@ -433,7 +433,8 @@ loaddefs-generate--parse-file
> >>            ;; loaddefs for packages so that `syntax-ppss' later gives
> >>            ;; correct results.
> >>            (emacs-lisp-mode)
> >> -        (let ((version (lm-header "version"))
> >> +        (let ((version (or (lm-header "package-version")
> >> +                           (lm-header "version")))
> >>                package)
> >>            (when (and version
> >>                       (setq version (ignore-errors (version-to-list version)))
> >>
> >>
> >
> > What about making `lm-version' handle the "package-version" header then
> > using `lm-version' in loaddefs-generate--parse-file?  See patches.
> >
> > Joseph




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69528; Package emacs. (Sat, 25 May 2024 08:05:02 GMT) Full text and rfc822 format available.

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: 69528 <at> debbugs.gnu.org, iarchivedmywholelife <at> gmail.com
Subject: Re: bug#69528: 30.0.50; [BUG] transient.el is not a member of
 package--builtin-versions
Date: Sat, 25 May 2024 08:04:24 +0000
Joseph Turner <joseph <at> breatheoutbreathe.in> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>> So in general, this patch might be appropriate?
>>
>> diff --git a/lisp/emacs-lisp/loaddefs-gen.el b/lisp/emacs-lisp/loaddefs-gen.el
>> index 581053f6304..42f386933dc 100644
>> --- a/lisp/emacs-lisp/loaddefs-gen.el
>> +++ b/lisp/emacs-lisp/loaddefs-gen.el
>> @@ -433,7 +433,8 @@ loaddefs-generate--parse-file
>>            ;; loaddefs for packages so that `syntax-ppss' later gives
>>            ;; correct results.
>>            (emacs-lisp-mode)
>> -        (let ((version (lm-header "version"))
>> +        (let ((version (or (lm-header "package-version")
>> +                           (lm-header "version")))
>>                package)
>>            (when (and version
>>                       (setq version (ignore-errors (version-to-list version)))
>>
>>
>
> What about making `lm-version' handle the "package-version" header then
> using `lm-version' in loaddefs-generate--parse-file?  See patches.

My main concern was if we want to have Package-Version always override
Version, but if my patch modified loaddefs-gen, then I don't think there
is much of a difference if we change lisp-mnt instead (in terms of the
generality of the change).

So I am fine with the change, and think we can merge it.  Eli: Is master
still fine for these kinds of changes?

> Joseph
>
>>From e83ee369ae90e5e15b3adca9eab1ded4db864427 Mon Sep 17 00:00:00 2001
> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
> Date: Mon, 4 Mar 2024 22:15:50 -0800
> Subject: [PATCH 2/2] Use lm-version instead of lm-header "version"
>
> bug#69528
>
> * lisp/emacs-lisp/loaddefs-gen.el (loaddefs-generate--parse-file)
> ---
>  lisp/emacs-lisp/loaddefs-gen.el | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lisp/emacs-lisp/loaddefs-gen.el b/lisp/emacs-lisp/loaddefs-gen.el
> index 581053f6304..6b24f7dc8c7 100644
> --- a/lisp/emacs-lisp/loaddefs-gen.el
> +++ b/lisp/emacs-lisp/loaddefs-gen.el
> @@ -433,7 +433,7 @@ loaddefs-generate--parse-file
>            ;; loaddefs for packages so that `syntax-ppss' later gives
>            ;; correct results.
>            (emacs-lisp-mode)
> -        (let ((version (lm-header "version"))
> +        (let ((version (lm-version))
>                package)
>            (when (and version
>                       (setq version (ignore-errors (version-to-list version)))
> -- 
> 2.41.0
>
>
>>From 20db8c9afcb03d8a5acb750fa738c5066e204401 Mon Sep 17 00:00:00 2001
> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
> Date: Mon, 4 Mar 2024 22:14:26 -0800
> Subject: [PATCH 1/2] Check Package-Version: header in lm-version also
>
> * lisp/emacs-lisp/lisp-mnt.el (lm-version)
> ---
>  lisp/emacs-lisp/lisp-mnt.el | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/lisp/emacs-lisp/lisp-mnt.el b/lisp/emacs-lisp/lisp-mnt.el
> index f111a77663c..12b23853801 100644
> --- a/lisp/emacs-lisp/lisp-mnt.el
> +++ b/lisp/emacs-lisp/lisp-mnt.el
> @@ -416,6 +416,7 @@ lm-version
>  This can be found in an RCS or SCCS header."
>    (lm-with-file file
>      (or (lm-header "version")
> +        (lm-header "package-version")
>          (let ((header-max (lm-code-start)))
>  	  (goto-char (point-min))
>  	  (cond

Joseph Turner <joseph <at> breatheoutbreathe.in> writes:

> Joseph Turner <joseph <at> breatheoutbreathe.in> writes:
>
>> Philip Kaludercic <philipk <at> posteo.net> writes:
>>> So in general, this patch might be appropriate?
>>>
>>> diff --git a/lisp/emacs-lisp/loaddefs-gen.el b/lisp/emacs-lisp/loaddefs-gen.el
>>> index 581053f6304..42f386933dc 100644
>>> --- a/lisp/emacs-lisp/loaddefs-gen.el
>>> +++ b/lisp/emacs-lisp/loaddefs-gen.el
>>> @@ -433,7 +433,8 @@ loaddefs-generate--parse-file
>>>            ;; loaddefs for packages so that `syntax-ppss' later gives
>>>            ;; correct results.
>>>            (emacs-lisp-mode)
>>> -        (let ((version (lm-header "version"))
>>> +        (let ((version (or (lm-header "package-version")
>>> +                           (lm-header "version")))
>>>                package)
>>>            (when (and version
>>>                       (setq version (ignore-errors (version-to-list version)))
>>>
>>>
>>
>> What about making `lm-version' handle the "package-version" header then
>> using `lm-version' in loaddefs-generate--parse-file?  See patches.
>>
>> Joseph
>>
>> [2. text/x-diff; 0002-Use-lm-version-instead-of-lm-header-version.patch]...
>>
>> [3. text/x-diff; 0001-Check-Package-Version-header-in-lm-version-also.patch]...
>
> Ping!
>
>
>
>

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

> Ping! Ping!  Philip, can you please chime in?
>
>> Cc: 69528 <at> debbugs.gnu.org, iarchivedmywholelife <at> gmail.com
>> Date: Wed, 08 May 2024 23:53:38 -0700
>> From:  Joseph Turner via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>> 
>> Joseph Turner <joseph <at> breatheoutbreathe.in> writes:
>> 
>> > Philip Kaludercic <philipk <at> posteo.net> writes:
>> >> So in general, this patch might be appropriate?
>> >>
>> >> diff --git a/lisp/emacs-lisp/loaddefs-gen.el b/lisp/emacs-lisp/loaddefs-gen.el
>> >> index 581053f6304..42f386933dc 100644
>> >> --- a/lisp/emacs-lisp/loaddefs-gen.el
>> >> +++ b/lisp/emacs-lisp/loaddefs-gen.el
>> >> @@ -433,7 +433,8 @@ loaddefs-generate--parse-file
>> >>            ;; loaddefs for packages so that `syntax-ppss' later gives
>> >>            ;; correct results.
>> >>            (emacs-lisp-mode)
>> >> -        (let ((version (lm-header "version"))
>> >> +        (let ((version (or (lm-header "package-version")
>> >> +                           (lm-header "version")))
>> >>                package)
>> >>            (when (and version
>> >>                       (setq version (ignore-errors (version-to-list version)))
>> >>
>> >>
>> >
>> > What about making `lm-version' handle the "package-version" header then
>> > using `lm-version' in loaddefs-generate--parse-file?  See patches.
>> >
>> > Joseph
>
>
>
>

-- 
	Philip Kaludercic on peregrine




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69528; Package emacs. (Sat, 25 May 2024 08:09:02 GMT) Full text and rfc822 format available.

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

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: eliz <at> gnu.org, 69528 <at> debbugs.gnu.org, iarchivedmywholelife <at> gmail.com
Subject: Re: bug#69528: 30.0.50; [BUG] transient.el is not a member of
 package--builtin-versions
Date: Sat, 25 May 2024 01:08:08 -0700
Philip Kaludercic <philipk <at> posteo.net> writes:

> Joseph Turner <joseph <at> breatheoutbreathe.in> writes:
>
>> Philip Kaludercic <philipk <at> posteo.net> writes:
>>> So in general, this patch might be appropriate?
>>>
>>> diff --git a/lisp/emacs-lisp/loaddefs-gen.el b/lisp/emacs-lisp/loaddefs-gen.el
>>> index 581053f6304..42f386933dc 100644
>>> --- a/lisp/emacs-lisp/loaddefs-gen.el
>>> +++ b/lisp/emacs-lisp/loaddefs-gen.el
>>> @@ -433,7 +433,8 @@ loaddefs-generate--parse-file
>>>            ;; loaddefs for packages so that `syntax-ppss' later gives
>>>            ;; correct results.
>>>            (emacs-lisp-mode)
>>> -        (let ((version (lm-header "version"))
>>> +        (let ((version (or (lm-header "package-version")
>>> +                           (lm-header "version")))
>>>                package)
>>>            (when (and version
>>>                       (setq version (ignore-errors (version-to-list version)))
>>>
>>>
>>
>> What about making `lm-version' handle the "package-version" header then
>> using `lm-version' in loaddefs-generate--parse-file?  See patches.
>
> My main concern was if we want to have Package-Version always override
> Version, but if my patch modified loaddefs-gen, then I don't think there
> is much of a difference if we change lisp-mnt instead (in terms of the
> generality of the change).

If it would be more appropriate, I can resubmit another patch with
"Version" used preferentially over "Package-Version".

> So I am fine with the change, and think we can merge it.  Eli: Is master
> still fine for these kinds of changes?
>
>> Joseph
>>
>>>From e83ee369ae90e5e15b3adca9eab1ded4db864427 Mon Sep 17 00:00:00 2001
>> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
>> Date: Mon, 4 Mar 2024 22:15:50 -0800
>> Subject: [PATCH 2/2] Use lm-version instead of lm-header "version"
>>
>> bug#69528
>>
>> * lisp/emacs-lisp/loaddefs-gen.el (loaddefs-generate--parse-file)
>> ---
>>  lisp/emacs-lisp/loaddefs-gen.el | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lisp/emacs-lisp/loaddefs-gen.el b/lisp/emacs-lisp/loaddefs-gen.el
>> index 581053f6304..6b24f7dc8c7 100644
>> --- a/lisp/emacs-lisp/loaddefs-gen.el
>> +++ b/lisp/emacs-lisp/loaddefs-gen.el
>> @@ -433,7 +433,7 @@ loaddefs-generate--parse-file
>>            ;; loaddefs for packages so that `syntax-ppss' later gives
>>            ;; correct results.
>>            (emacs-lisp-mode)
>> -        (let ((version (lm-header "version"))
>> +        (let ((version (lm-version))
>>                package)
>>            (when (and version
>>                       (setq version (ignore-errors (version-to-list version)))
>> -- 
>> 2.41.0
>>
>>
>>>From 20db8c9afcb03d8a5acb750fa738c5066e204401 Mon Sep 17 00:00:00 2001
>> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
>> Date: Mon, 4 Mar 2024 22:14:26 -0800
>> Subject: [PATCH 1/2] Check Package-Version: header in lm-version also
>>
>> * lisp/emacs-lisp/lisp-mnt.el (lm-version)
>> ---
>>  lisp/emacs-lisp/lisp-mnt.el | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/lisp/emacs-lisp/lisp-mnt.el b/lisp/emacs-lisp/lisp-mnt.el
>> index f111a77663c..12b23853801 100644
>> --- a/lisp/emacs-lisp/lisp-mnt.el
>> +++ b/lisp/emacs-lisp/lisp-mnt.el
>> @@ -416,6 +416,7 @@ lm-version
>>  This can be found in an RCS or SCCS header."
>>    (lm-with-file file
>>      (or (lm-header "version")
>> +        (lm-header "package-version")
>>          (let ((header-max (lm-code-start)))
>>  	  (goto-char (point-min))
>>  	  (cond
>
> Joseph Turner <joseph <at> breatheoutbreathe.in> writes:
>
>> Joseph Turner <joseph <at> breatheoutbreathe.in> writes:
>>
>>> Philip Kaludercic <philipk <at> posteo.net> writes:
>>>> So in general, this patch might be appropriate?
>>>>
>>>> diff --git a/lisp/emacs-lisp/loaddefs-gen.el b/lisp/emacs-lisp/loaddefs-gen.el
>>>> index 581053f6304..42f386933dc 100644
>>>> --- a/lisp/emacs-lisp/loaddefs-gen.el
>>>> +++ b/lisp/emacs-lisp/loaddefs-gen.el
>>>> @@ -433,7 +433,8 @@ loaddefs-generate--parse-file
>>>>            ;; loaddefs for packages so that `syntax-ppss' later gives
>>>>            ;; correct results.
>>>>            (emacs-lisp-mode)
>>>> -        (let ((version (lm-header "version"))
>>>> +        (let ((version (or (lm-header "package-version")
>>>> +                           (lm-header "version")))
>>>>                package)
>>>>            (when (and version
>>>>                       (setq version (ignore-errors (version-to-list version)))
>>>>
>>>>
>>>
>>> What about making `lm-version' handle the "package-version" header then
>>> using `lm-version' in loaddefs-generate--parse-file?  See patches.
>>>
>>> Joseph
>>>
>>> [2. text/x-diff; 0002-Use-lm-version-instead-of-lm-header-version.patch]...
>>>
>>> [3. text/x-diff; 0001-Check-Package-Version-header-in-lm-version-also.patch]...
>>
>> Ping!
>>
>>
>>
>>
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>> Ping! Ping!  Philip, can you please chime in?
>>
>>> Cc: 69528 <at> debbugs.gnu.org, iarchivedmywholelife <at> gmail.com
>>> Date: Wed, 08 May 2024 23:53:38 -0700
>>> From:  Joseph Turner via "Bug reports for GNU Emacs,
>>>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>>> 
>>> Joseph Turner <joseph <at> breatheoutbreathe.in> writes:
>>> 
>>> > Philip Kaludercic <philipk <at> posteo.net> writes:
>>> >> So in general, this patch might be appropriate?
>>> >>
>>> >> diff --git a/lisp/emacs-lisp/loaddefs-gen.el b/lisp/emacs-lisp/loaddefs-gen.el
>>> >> index 581053f6304..42f386933dc 100644
>>> >> --- a/lisp/emacs-lisp/loaddefs-gen.el
>>> >> +++ b/lisp/emacs-lisp/loaddefs-gen.el
>>> >> @@ -433,7 +433,8 @@ loaddefs-generate--parse-file
>>> >>            ;; loaddefs for packages so that `syntax-ppss' later gives
>>> >>            ;; correct results.
>>> >>            (emacs-lisp-mode)
>>> >> -        (let ((version (lm-header "version"))
>>> >> +        (let ((version (or (lm-header "package-version")
>>> >> +                           (lm-header "version")))
>>> >>                package)
>>> >>            (when (and version
>>> >>                       (setq version (ignore-errors (version-to-list version)))
>>> >>
>>> >>
>>> >
>>> > What about making `lm-version' handle the "package-version" header then
>>> > using `lm-version' in loaddefs-generate--parse-file?  See patches.
>>> >
>>> > Joseph
>>
>>
>>
>>




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69528; Package emacs. (Sat, 25 May 2024 08:48:01 GMT) Full text and rfc822 format available.

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: eliz <at> gnu.org, 69528 <at> debbugs.gnu.org, iarchivedmywholelife <at> gmail.com
Subject: Re: bug#69528: 30.0.50; [BUG] transient.el is not a member of
 package--builtin-versions
Date: Sat, 25 May 2024 08:47:36 +0000
Joseph Turner <joseph <at> breatheoutbreathe.in> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> Joseph Turner <joseph <at> breatheoutbreathe.in> writes:
>>
>>> Philip Kaludercic <philipk <at> posteo.net> writes:
>>>> So in general, this patch might be appropriate?
>>>>
>>>> diff --git a/lisp/emacs-lisp/loaddefs-gen.el b/lisp/emacs-lisp/loaddefs-gen.el
>>>> index 581053f6304..42f386933dc 100644
>>>> --- a/lisp/emacs-lisp/loaddefs-gen.el
>>>> +++ b/lisp/emacs-lisp/loaddefs-gen.el
>>>> @@ -433,7 +433,8 @@ loaddefs-generate--parse-file
>>>>            ;; loaddefs for packages so that `syntax-ppss' later gives
>>>>            ;; correct results.
>>>>            (emacs-lisp-mode)
>>>> -        (let ((version (lm-header "version"))
>>>> +        (let ((version (or (lm-header "package-version")
>>>> +                           (lm-header "version")))
>>>>                package)
>>>>            (when (and version
>>>>                       (setq version (ignore-errors (version-to-list version)))
>>>>
>>>>
>>>
>>> What about making `lm-version' handle the "package-version" header then
>>> using `lm-version' in loaddefs-generate--parse-file?  See patches.
>>
>> My main concern was if we want to have Package-Version always override
>> Version, but if my patch modified loaddefs-gen, then I don't think there
>> is much of a difference if we change lisp-mnt instead (in terms of the
>> generality of the change).
>
> If it would be more appropriate, I can resubmit another patch with
> "Version" used preferentially over "Package-Version".

No, I believe that would be wrong, at least going by these quotes from
the manual:

(elisp) Simple Packages:

     The version number comes from the ‘Package-Version’ header, if it
  exists, or from the ‘Version’ header otherwise.  One or the other _must_
  be present.  Here, the version number is 1.3.

(elisp) Library Headers:

  ‘Package-Version’
       If ‘Version’ is not suitable for use by the package manager, then a
       package can define ‘Package-Version’; it will be used instead.
       This is handy if ‘Version’ is an RCS id or something else that
       cannot be parsed by ‘version-to-list’.

So the patch is fine as it is.

-- 
	Philip Kaludercic on peregrine




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69528; Package emacs. (Sat, 25 May 2024 10:50:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Stefan Kangas <stefankangas <at> gmail.com>, Andrea Corallo <acorallo <at> gnu.org>
Cc: iarchivedmywholelife <at> gmail.com, 69528 <at> debbugs.gnu.org,
 joseph <at> breatheoutbreathe.in
Subject: Re: bug#69528: 30.0.50;
 [BUG] transient.el is not a member of package--builtin-versions
Date: Sat, 25 May 2024 13:49:01 +0300
> Cc: 69528 <at> debbugs.gnu.org, iarchivedmywholelife <at> gmail.com
> From: Philip Kaludercic <philipk <at> posteo.net>
> Date: Sat, 25 May 2024 08:04:24 +0000
> 
> Joseph Turner <joseph <at> breatheoutbreathe.in> writes:
> 
> > What about making `lm-version' handle the "package-version" header then
> > using `lm-version' in loaddefs-generate--parse-file?  See patches.
> 
> My main concern was if we want to have Package-Version always override
> Version, but if my patch modified loaddefs-gen, then I don't think there
> is much of a difference if we change lisp-mnt instead (in terms of the
> generality of the change).
> 
> So I am fine with the change, and think we can merge it.  Eli: Is master
> still fine for these kinds of changes?

I think so, yes.  But maybe I don't fully understand the effect of
this change?  Can you describe it?

I also added the other maintainers, in case they have opinions on
this.

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69528; Package emacs. (Sun, 26 May 2024 00:47:01 GMT) Full text and rfc822 format available.

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

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: eliz <at> gnu.org, 69528 <at> debbugs.gnu.org, iarchivedmywholelife <at> gmail.com
Subject: Re: bug#69528: 30.0.50; [BUG] transient.el is not a member of
 package--builtin-versions
Date: Sat, 25 May 2024 17:45:21 -0700
Philip Kaludercic <philipk <at> posteo.net> writes:

> Joseph Turner <joseph <at> breatheoutbreathe.in> writes:
>
>> Philip Kaludercic <philipk <at> posteo.net> writes:
>>
>>> Joseph Turner <joseph <at> breatheoutbreathe.in> writes:
>>>
>>>> Philip Kaludercic <philipk <at> posteo.net> writes:
>>>>> So in general, this patch might be appropriate?
>>>>>
>>>>> diff --git a/lisp/emacs-lisp/loaddefs-gen.el b/lisp/emacs-lisp/loaddefs-gen.el
>>>>> index 581053f6304..42f386933dc 100644
>>>>> --- a/lisp/emacs-lisp/loaddefs-gen.el
>>>>> +++ b/lisp/emacs-lisp/loaddefs-gen.el
>>>>> @@ -433,7 +433,8 @@ loaddefs-generate--parse-file
>>>>>            ;; loaddefs for packages so that `syntax-ppss' later gives
>>>>>            ;; correct results.
>>>>>            (emacs-lisp-mode)
>>>>> -        (let ((version (lm-header "version"))
>>>>> +        (let ((version (or (lm-header "package-version")
>>>>> +                           (lm-header "version")))
>>>>>                package)
>>>>>            (when (and version
>>>>>                       (setq version (ignore-errors (version-to-list version)))
>>>>>
>>>>>
>>>>
>>>> What about making `lm-version' handle the "package-version" header then
>>>> using `lm-version' in loaddefs-generate--parse-file?  See patches.
>>>
>>> My main concern was if we want to have Package-Version always override
>>> Version, but if my patch modified loaddefs-gen, then I don't think there
>>> is much of a difference if we change lisp-mnt instead (in terms of the
>>> generality of the change).
>>
>> If it would be more appropriate, I can resubmit another patch with
>> "Version" used preferentially over "Package-Version".
>
> No, I believe that would be wrong, at least going by these quotes from
> the manual:
>
> (elisp) Simple Packages:
>
>      The version number comes from the ‘Package-Version’ header, if it
>   exists, or from the ‘Version’ header otherwise.  One or the other _must_
>   be present.  Here, the version number is 1.3.
>
> (elisp) Library Headers:
>
>   ‘Package-Version’
>        If ‘Version’ is not suitable for use by the package manager, then a
>        package can define ‘Package-Version’; it will be used instead.
>        This is handy if ‘Version’ is an RCS id or something else that
>        cannot be parsed by ‘version-to-list’.
>
> So the patch is fine as it is.

Thanks for double-checking.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69528; Package emacs. (Sun, 02 Jun 2024 10:38:01 GMT) Full text and rfc822 format available.

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Philip Kaludercic <philipk <at> posteo.net>, 
 Stefan Monnier <monnier <at> iro.umontreal.ca>, Andrea Corallo <acorallo <at> gnu.org>
Cc: iarchivedmywholelife <at> gmail.com, 69528 <at> debbugs.gnu.org,
 joseph <at> breatheoutbreathe.in
Subject: Re: bug#69528: 30.0.50;
 [BUG] transient.el is not a member of package--builtin-versions
Date: Sun, 2 Jun 2024 10:36:26 +0000
Eli Zaretskii <eliz <at> gnu.org> writes:

>> > What about making `lm-version' handle the "package-version" header then
>> > using `lm-version' in loaddefs-generate--parse-file?  See patches.
>>
>> My main concern was if we want to have Package-Version always override
>> Version, but if my patch modified loaddefs-gen, then I don't think there
>> is much of a difference if we change lisp-mnt instead (in terms of the
>> generality of the change).
>>
>> So I am fine with the change, and think we can merge it.  Eli: Is master
>> still fine for these kinds of changes?
>
> I think so, yes.  But maybe I don't fully understand the effect of
> this change?  Can you describe it?
>
> I also added the other maintainers, in case they have opinions on
> this.

I think the first patch is right, i.e. to use

    (lm-version)

instead of

    (lm-header "version")

So let's install that one, I think.

The effect of the second patch is to change `lm-version` to look for a
"Package-Version" header if there is no "Version" header.

This has two problems:

1. We didn't do that until now, and it's not clear to me what is the
   issue that is prompting this change.  The transient.el issue seems to
   have been fixed already.

2. The way I read the manual, it seems like "Package-Version" should be
   preferred over "Version", if it exists:

        ‘Package-Version’
             If ‘Version’ is not suitable for use by the package manager, then a
             package can define ‘Package-Version’; it will be used instead.
             This is handy if ‘Version’ is an RCS id or something else that
             cannot be parsed by ‘version-to-list’.

   I'm also not sure we need to support packages with unusual versions
   like RCS id's these days.  Is that use case still relevant?  Perhaps
   we should simply deprecate the "Package-Version" header?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69528; Package emacs. (Sun, 02 Jun 2024 11:08:02 GMT) Full text and rfc822 format available.

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: iarchivedmywholelife <at> gmail.com, joseph <at> breatheoutbreathe.in,
 69528 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Andrea Corallo <acorallo <at> gnu.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#69528: 30.0.50; [BUG] transient.el is not a member of
 package--builtin-versions
Date: Sun, 02 Jun 2024 11:07:02 +0000
Stefan Kangas <stefankangas <at> gmail.com> writes:

> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>>> > What about making `lm-version' handle the "package-version" header then
>>> > using `lm-version' in loaddefs-generate--parse-file?  See patches.
>>>
>>> My main concern was if we want to have Package-Version always override
>>> Version, but if my patch modified loaddefs-gen, then I don't think there
>>> is much of a difference if we change lisp-mnt instead (in terms of the
>>> generality of the change).
>>>
>>> So I am fine with the change, and think we can merge it.  Eli: Is master
>>> still fine for these kinds of changes?
>>
>> I think so, yes.  But maybe I don't fully understand the effect of
>> this change?  Can you describe it?

Sorry for the late response, but Stefan summaries the situation well
below.

>> I also added the other maintainers, in case they have opinions on
>> this.
>
> I think the first patch is right, i.e. to use
>
>     (lm-version)
>
> instead of
>
>     (lm-header "version")
>
> So let's install that one, I think.

I agree.

> The effect of the second patch is to change `lm-version` to look for a
> "Package-Version" header if there is no "Version" header.
>
> This has two problems:
>
> 1. We didn't do that until now, and it's not clear to me what is the
>    issue that is prompting this change.  The transient.el issue seems to
>    have been fixed already.
>
> 2. The way I read the manual, it seems like "Package-Version" should be
>    preferred over "Version", if it exists:
>
>         ‘Package-Version’
>              If ‘Version’ is not suitable for use by the package manager, then a
>              package can define ‘Package-Version’; it will be used instead.
>              This is handy if ‘Version’ is an RCS id or something else that
>              cannot be parsed by ‘version-to-list’.
>
>    I'm also not sure we need to support packages with unusual versions
>    like RCS id's these days.  Is that use case still relevant?  Perhaps
>    we should simply deprecate the "Package-Version" header?

FWIW I use this for some of my own scripts that I version using RCS, so
I'd appreciate it if that functionality would stay.

-- 
	Philip Kaludercic on peregrine




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69528; Package emacs. (Sun, 02 Jun 2024 12:11:01 GMT) Full text and rfc822 format available.

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: iarchivedmywholelife <at> gmail.com, joseph <at> breatheoutbreathe.in,
 69528 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Andrea Corallo <acorallo <at> gnu.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#69528: 30.0.50;
 [BUG] transient.el is not a member of package--builtin-versions
Date: Sun, 2 Jun 2024 05:08:58 -0700
Philip Kaludercic <philipk <at> posteo.net> writes:

>> 2. The way I read the manual, it seems like "Package-Version" should be
>>    preferred over "Version", if it exists:
>>
>>         ‘Package-Version’
>>              If ‘Version’ is not suitable for use by the package manager, then a
>>              package can define ‘Package-Version’; it will be used instead.
>>              This is handy if ‘Version’ is an RCS id or something else that
>>              cannot be parsed by ‘version-to-list’.
>
> FWIW I use this for some of my own scripts that I version using RCS, so
> I'd appreciate it if that functionality would stay.

OK, so let's keep it.  But shouldn't the below be the correct order
according to the above quoted documentation?

diff --git a/lisp/emacs-lisp/lisp-mnt.el b/lisp/emacs-lisp/lisp-mnt.el
index f111a77663c..5db0b50adc3 100644
--- a/lisp/emacs-lisp/lisp-mnt.el
+++ b/lisp/emacs-lisp/lisp-mnt.el
@@ -415,7 +415,8 @@ lm-version
   "Return the version listed in file FILE, or current buffer if FILE is nil.
 This can be found in an RCS or SCCS header."
   (lm-with-file file
-    (or (lm-header "version")
+    (or (lm-header "package-version")
+        (lm-header "version")
         (let ((header-max (lm-code-start)))
 	  (goto-char (point-min))
 	  (cond




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69528; Package emacs. (Sun, 02 Jun 2024 13:13:02 GMT) Full text and rfc822 format available.

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: iarchivedmywholelife <at> gmail.com, joseph <at> breatheoutbreathe.in,
 69528 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Andrea Corallo <acorallo <at> gnu.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#69528: 30.0.50; [BUG] transient.el is not a member of
 package--builtin-versions
Date: Sun, 02 Jun 2024 13:11:38 +0000
Stefan Kangas <stefankangas <at> gmail.com> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>>> 2. The way I read the manual, it seems like "Package-Version" should be
>>>    preferred over "Version", if it exists:
>>>
>>>         ‘Package-Version’
>>>              If ‘Version’ is not suitable for use by the package manager, then a
>>>              package can define ‘Package-Version’; it will be used instead.
>>>              This is handy if ‘Version’ is an RCS id or something else that
>>>              cannot be parsed by ‘version-to-list’.
>>
>> FWIW I use this for some of my own scripts that I version using RCS, so
>> I'd appreciate it if that functionality would stay.
>
> OK, so let's keep it.  But shouldn't the below be the correct order
> according to the above quoted documentation?
>
> diff --git a/lisp/emacs-lisp/lisp-mnt.el b/lisp/emacs-lisp/lisp-mnt.el
> index f111a77663c..5db0b50adc3 100644
> --- a/lisp/emacs-lisp/lisp-mnt.el
> +++ b/lisp/emacs-lisp/lisp-mnt.el
> @@ -415,7 +415,8 @@ lm-version
>    "Return the version listed in file FILE, or current buffer if FILE is nil.
>  This can be found in an RCS or SCCS header."
>    (lm-with-file file
> -    (or (lm-header "version")
> +    (or (lm-header "package-version")
> +        (lm-header "version")
>          (let ((header-max (lm-code-start)))
>  	  (goto-char (point-min))
>  	  (cond

Of course, that was also the change proposed in my first patch but I
didn't notice the change in Joseph's suggestion.

-- 
	Philip Kaludercic on peregrine




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69528; Package emacs. (Sun, 02 Jun 2024 18:28:01 GMT) Full text and rfc822 format available.

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

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: iarchivedmywholelife <at> gmail.com, 69528 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Andrea Corallo <acorallo <at> gnu.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#69528: 30.0.50; [BUG] transient.el is not a member of
 package--builtin-versions
Date: Sun, 02 Jun 2024 11:26:42 -0700
[Message part 1 (text/plain, inline)]
Philip Kaludercic <philipk <at> posteo.net> writes:

> Stefan Kangas <stefankangas <at> gmail.com> writes:
>
>> Philip Kaludercic <philipk <at> posteo.net> writes:
>>
>>>> 2. The way I read the manual, it seems like "Package-Version" should be
>>>>    preferred over "Version", if it exists:
>>>>
>>>>         ‘Package-Version’
>>>>              If ‘Version’ is not suitable for use by the package manager, then a
>>>>              package can define ‘Package-Version’; it will be used instead.
>>>>              This is handy if ‘Version’ is an RCS id or something else that
>>>>              cannot be parsed by ‘version-to-list’.
>>>
>>> FWIW I use this for some of my own scripts that I version using RCS, so
>>> I'd appreciate it if that functionality would stay.
>>
>> OK, so let's keep it.  But shouldn't the below be the correct order
>> according to the above quoted documentation?
>>
>> diff --git a/lisp/emacs-lisp/lisp-mnt.el b/lisp/emacs-lisp/lisp-mnt.el
>> index f111a77663c..5db0b50adc3 100644
>> --- a/lisp/emacs-lisp/lisp-mnt.el
>> +++ b/lisp/emacs-lisp/lisp-mnt.el
>> @@ -415,7 +415,8 @@ lm-version
>>    "Return the version listed in file FILE, or current buffer if FILE is nil.
>>  This can be found in an RCS or SCCS header."
>>    (lm-with-file file
>> -    (or (lm-header "version")
>> +    (or (lm-header "package-version")
>> +        (lm-header "version")
>>          (let ((header-max (lm-code-start)))
>>  	  (goto-char (point-min))
>>  	  (cond
>
> Of course, that was also the change proposed in my first patch but I
> didn't notice the change in Joseph's suggestion.

Thanks for the correction.  Are the attached patches appropriate?

[0001-Check-Package-Version-header-in-lm-version-also.patch (text/x-diff, attachment)]
[0002-Use-lm-version-instead-of-lm-header-version.patch (text/x-diff, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69528; Package emacs. (Sun, 02 Jun 2024 18:41:01 GMT) Full text and rfc822 format available.

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: iarchivedmywholelife <at> gmail.com, 69528 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Andrea Corallo <acorallo <at> gnu.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#69528: 30.0.50; [BUG] transient.el is not a member of
 package--builtin-versions
Date: Sun, 02 Jun 2024 18:40:13 +0000
Joseph Turner <joseph <at> breatheoutbreathe.in> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> Stefan Kangas <stefankangas <at> gmail.com> writes:
>>
>>> Philip Kaludercic <philipk <at> posteo.net> writes:
>>>
>>>>> 2. The way I read the manual, it seems like "Package-Version" should be
>>>>>    preferred over "Version", if it exists:
>>>>>
>>>>>         ‘Package-Version’
>>>>>              If ‘Version’ is not suitable for use by the package manager, then a
>>>>>              package can define ‘Package-Version’; it will be used instead.
>>>>>              This is handy if ‘Version’ is an RCS id or something else that
>>>>>              cannot be parsed by ‘version-to-list’.
>>>>
>>>> FWIW I use this for some of my own scripts that I version using RCS, so
>>>> I'd appreciate it if that functionality would stay.
>>>
>>> OK, so let's keep it.  But shouldn't the below be the correct order
>>> according to the above quoted documentation?
>>>
>>> diff --git a/lisp/emacs-lisp/lisp-mnt.el b/lisp/emacs-lisp/lisp-mnt.el
>>> index f111a77663c..5db0b50adc3 100644
>>> --- a/lisp/emacs-lisp/lisp-mnt.el
>>> +++ b/lisp/emacs-lisp/lisp-mnt.el
>>> @@ -415,7 +415,8 @@ lm-version
>>>    "Return the version listed in file FILE, or current buffer if FILE is nil.
>>>  This can be found in an RCS or SCCS header."
>>>    (lm-with-file file
>>> -    (or (lm-header "version")
>>> +    (or (lm-header "package-version")
>>> +        (lm-header "version")
>>>          (let ((header-max (lm-code-start)))
>>>  	  (goto-char (point-min))
>>>  	  (cond
>>
>> Of course, that was also the change proposed in my first patch but I
>> didn't notice the change in Joseph's suggestion.
>
> Thanks for the correction.  Are the attached patches appropriate?

Unless I am forgetting something again, it looks good.

-- 
	Philip Kaludercic on peregrine




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69528; Package emacs. (Mon, 03 Jun 2024 17:26:01 GMT) Full text and rfc822 format available.

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>,
 Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Andrea Corallo <acorallo <at> gnu.org>,
 iarchivedmywholelife <at> gmail.com, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 69528 <at> debbugs.gnu.org
Subject: Re: bug#69528: 30.0.50;
 [BUG] transient.el is not a member of package--builtin-versions
Date: Mon, 3 Jun 2024 13:24:36 -0400
Joseph Turner <joseph <at> breatheoutbreathe.in> writes:

>> Of course, that was also the change proposed in my first patch but I
>> didn't notice the change in Joseph's suggestion.

Ah, right.  I somehow missed that part.

> Thanks for the correction.  Are the attached patches appropriate?

Looks good to me, except for a few comments below.

> From a666581f2a58568bb7f83a369e1040920a6b2c14 Mon Sep 17 00:00:00 2001
> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
> Date: Mon, 4 Mar 2024 22:14:26 -0800
> Subject: [PATCH 1/2] Check Package-Version: header in lm-version also

Bonus points if you add a test for this one.

> * lisp/emacs-lisp/lisp-mnt.el (lm-version)

See CONTRIBUTE for details, but this should read:

* lisp/emacs-lisp/lisp-mnt.el (lm-version): Prefer version in the
"Package-Version:" header.  (Bug#69528)

> ---
>  lisp/emacs-lisp/lisp-mnt.el | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lisp/emacs-lisp/lisp-mnt.el b/lisp/emacs-lisp/lisp-mnt.el
> index f111a77663c..5db0b50adc3 100644
> --- a/lisp/emacs-lisp/lisp-mnt.el
> +++ b/lisp/emacs-lisp/lisp-mnt.el
> @@ -415,7 +415,8 @@ lm-version
>    "Return the version listed in file FILE, or current buffer if FILE is nil.
>  This can be found in an RCS or SCCS header."

It would be good to fix the docstring here to clarify that it can come
from the "Version" or "Package-Version" headers too.

>    (lm-with-file file
> -    (or (lm-header "version")
> +    (or (lm-header "package-version")
> +        (lm-header "version")
>          (let ((header-max (lm-code-start)))
>  	  (goto-char (point-min))
>  	  (cond
> --
> 2.41.0
>
> From 6c4262d7236c64bbc938f7b4e76988d95049b7c1 Mon Sep 17 00:00:00 2001
> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
> Date: Mon, 4 Mar 2024 22:15:50 -0800
> Subject: [PATCH 2/2] Use lm-version instead of lm-header "version"
>
> bug#69528
>
> * lisp/emacs-lisp/loaddefs-gen.el (loaddefs-generate--parse-file)

This should read something like this instead:

* lisp/emacs-lisp/loaddefs-gen.el (loaddefs-generate--parse-file):
Prefer 'lm-version'.  (Bug#69528)

> ---
>  lisp/emacs-lisp/loaddefs-gen.el | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lisp/emacs-lisp/loaddefs-gen.el b/lisp/emacs-lisp/loaddefs-gen.el
> index 50e90cdf94c..f0355b25f57 100644
> --- a/lisp/emacs-lisp/loaddefs-gen.el
> +++ b/lisp/emacs-lisp/loaddefs-gen.el
> @@ -433,7 +433,7 @@ loaddefs-generate--parse-file
>            ;; loaddefs for packages so that `syntax-ppss' later gives
>            ;; correct results.
>            (emacs-lisp-mode)
> -        (let ((version (lm-header "version"))
> +        (let ((version (lm-version))
>                package)
>            (when (and version
>                       (setq version (ignore-errors (version-to-list version)))
> --
> 2.41.0




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69528; Package emacs. (Mon, 03 Jun 2024 19:26:01 GMT) Full text and rfc822 format available.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: Philip Kaludercic <philipk <at> posteo.net>, iarchivedmywholelife <at> gmail.com,
 Joseph Turner <joseph <at> breatheoutbreathe.in>, 69528 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Andrea Corallo <acorallo <at> gnu.org>
Subject: Re: bug#69528: 30.0.50; [BUG] transient.el is not a member of
 package--builtin-versions
Date: Mon, 03 Jun 2024 15:24:59 -0400
> * lisp/emacs-lisp/lisp-mnt.el (lm-version): Prefer version in the
> "Package-Version:" header.  (Bug#69528)

BTW, I think this is a backward-incompatible change.

Whether we want `lm-version` to return the info from `Version:` or from
`Package-Version:` depends on what we want to do with it.


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69528; Package emacs. (Mon, 03 Jun 2024 19:59:02 GMT) Full text and rfc822 format available.

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: iarchivedmywholelife <at> gmail.com, Joseph Turner <joseph <at> breatheoutbreathe.in>,
 69528 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Andrea Corallo <acorallo <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#69528: 30.0.50; [BUG] transient.el is not a member of
 package--builtin-versions
Date: Mon, 03 Jun 2024 19:58:35 +0000
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>> * lisp/emacs-lisp/lisp-mnt.el (lm-version): Prefer version in the
>> "Package-Version:" header.  (Bug#69528)
>
> BTW, I think this is a backward-incompatible change.
>
> Whether we want `lm-version` to return the info from `Version:` or from
> `Package-Version:` depends on what we want to do with it.

When do we want lm-version to return Version and not Package-Version,
where a (lm-header "version") wouldn't serve as a more specific
replacement?

FWIW the function is used in a single place (in the core, lm-report-bug)
and both on ELPA and NonGNU ELPA, all instances appear to might as well
be using `package-get-version' (if it were not for the version of Emacs
they are depending on).

-- 
	Philip Kaludercic on peregrine




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69528; Package emacs. (Mon, 03 Jun 2024 20:39:01 GMT) Full text and rfc822 format available.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: iarchivedmywholelife <at> gmail.com, Joseph Turner <joseph <at> breatheoutbreathe.in>,
 69528 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Andrea Corallo <acorallo <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#69528: 30.0.50; [BUG] transient.el is not a member of
 package--builtin-versions
Date: Mon, 03 Jun 2024 16:38:17 -0400
>>> * lisp/emacs-lisp/lisp-mnt.el (lm-version): Prefer version in the
>>> "Package-Version:" header.  (Bug#69528)
>> BTW, I think this is a backward-incompatible change.
>> Whether we want `lm-version` to return the info from `Version:` or from
>> `Package-Version:` depends on what we want to do with it.
> When do we want lm-version to return Version and not Package-Version,
> where a (lm-header "version") wouldn't serve as a more specific
> replacement?

I don't know, but if we never want to return the value of `Version:`
when there's a `Package-Version:` then we don't need `Package-Version:`
either (we should just replace the `Version:` field with the content of
`Package-Version:`).

IOW the very existence of `Package-Version:` is predicated on the desire
to distinguish the two.


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69528; Package emacs. (Tue, 04 Jun 2024 22:22:02 GMT) Full text and rfc822 format available.

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Andrea Corallo <acorallo <at> gnu.org>,
 69528 <at> debbugs.gnu.org, iarchivedmywholelife <at> gmail.com,
 Joseph Turner <joseph <at> breatheoutbreathe.in>
Subject: Re: bug#69528: 30.0.50;
 [BUG] transient.el is not a member of package--builtin-versions
Date: Tue, 4 Jun 2024 18:19:50 -0400
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>>>> * lisp/emacs-lisp/lisp-mnt.el (lm-version): Prefer version in the
>>>> "Package-Version:" header.  (Bug#69528)
>>> BTW, I think this is a backward-incompatible change.
>>> Whether we want `lm-version` to return the info from `Version:` or from
>>> `Package-Version:` depends on what we want to do with it.

AFAICT, we currently use it in `lm-report-bug' and with Joseph's patch
we will use it also for `loaddefs-generate--parse-file'.

>> When do we want lm-version to return Version and not Package-Version,
>> where a (lm-header "version") wouldn't serve as a more specific
>> replacement?
>
> I don't know, but if we never want to return the value of `Version:`
> when there's a `Package-Version:` then we don't need `Package-Version:`
> either (we should just replace the `Version:` field with the content of
> `Package-Version:`).

I don't have a strong opinion, but there seems to be a mismatch between
what the code does and what the documentation says.

    "The version number comes from the ‘Package-Version’ header, if it
    exists, or from the ‘Version’ header otherwise."

    (info "(elisp) Simple Packages")




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69528; Package emacs. (Tue, 04 Jun 2024 22:24:02 GMT) Full text and rfc822 format available.

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

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: Philip Kaludercic <philipk <at> posteo.net>, iarchivedmywholelife <at> gmail.com,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, Eli Zaretskii <eliz <at> gnu.org>,
 Andrea Corallo <acorallo <at> gnu.org>, 69528 <at> debbugs.gnu.org
Subject: Re: bug#69528: 30.0.50; [BUG] transient.el is not a member of
 package--builtin-versions
Date: Tue, 04 Jun 2024 15:22:50 -0700
Stefan Kangas <stefankangas <at> gmail.com> writes:

> Joseph Turner <joseph <at> breatheoutbreathe.in> writes:
>
>>> Of course, that was also the change proposed in my first patch but I
>>> didn't notice the change in Joseph's suggestion.
>
> Ah, right.  I somehow missed that part.
>
>> Thanks for the correction.  Are the attached patches appropriate?
>
> Looks good to me, except for a few comments below.

[...]

I'll be happy to make those changes if we decide to move forward with
the general idea of this patch.

Thanks!

Joseph




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69528; Package emacs. (Tue, 04 Jun 2024 22:56:02 GMT) Full text and rfc822 format available.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: Philip Kaludercic <philipk <at> posteo.net>, iarchivedmywholelife <at> gmail.com,
 Joseph Turner <joseph <at> breatheoutbreathe.in>, 69528 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>, Andrea Corallo <acorallo <at> gnu.org>
Subject: Re: bug#69528: 30.0.50; [BUG] transient.el is not a member of
 package--builtin-versions
Date: Tue, 04 Jun 2024 18:34:35 -0400
>>>>> * lisp/emacs-lisp/lisp-mnt.el (lm-version): Prefer version in the
>>>>> "Package-Version:" header.  (Bug#69528)
>>>> BTW, I think this is a backward-incompatible change.
>>>> Whether we want `lm-version` to return the info from `Version:` or from
>>>> `Package-Version:` depends on what we want to do with it.
> AFAICT, we currently use it in `lm-report-bug' and with Joseph's patch
> we will use it also for `loaddefs-generate--parse-file'.

`lm-report-bug` does not seem directly related to ELPA packaging, so it
makes sense to use just `Version:` there, which is presumably the format
that the maintainer favors (where the `Package-Version:` header is
instead the format that the maintainer was forced to add to accommodate
the restrictions of the ELPA protocol).

In contrast, `loaddefs-generate--parse-file' is about generating info
for `package.el`, so this one *does* want to use `Package-Version:`
if it's present.

Of course `lm-report-bug` would work likely fine as well if it uses
`Package-Version:`.  The distinction is probably not that important in
that case.

> I don't have a strong opinion, but there seems to be a mismatch between
> what the code does and what the documentation says.
>
>     "The version number comes from the ‘Package-Version’ header, if it
>     exists, or from the ‘Version’ header otherwise."
>
>     (info "(elisp) Simple Packages")

Definitely.  My only point was that the patch changed `lm-version` in
a backward incompatible way (tho arguably a minor one) without even
mentioning it.
Maybe it's OK to do that, but let's do it consciously.
If not, then we'll presumably add a new `lm-package-version` (which
wouldn't look at RCS keywords either).


        Stefan





This bug report was last modified 1 year and 11 days ago.

Previous Next


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