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

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

[No Wayman]

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.

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

[No Wayman]

No Wayman <iarchivedmywholelife@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. 

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

[Philip Kaludercic]

[-- Attachment #1: Type: text/plain, Size: 2500 bytes --]

No Wayman <iarchivedmywholelife@gmail.com> writes:

> No Wayman <iarchivedmywholelife@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@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@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?


[-- Attachment #2: Type: text/plain, Size: 663 bytes --]

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)))

[-- Attachment #3: Type: text/plain, Size: 812 bytes --]


>
> 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

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

[Joseph Turner via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 943 bytes --]

Philip Kaludercic <philipk@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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-Use-lm-version-instead-of-lm-header-version.patch --]
[-- Type: text/x-diff, Size: 998 bytes --]

From e83ee369ae90e5e15b3adca9eab1ded4db864427 Mon Sep 17 00:00:00 2001
From: Joseph Turner <joseph@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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-Check-Package-Version-header-in-lm-version-also.patch --]
[-- Type: text/x-diff, Size: 785 bytes --]

From 20db8c9afcb03d8a5acb750fa738c5066e204401 Mon Sep 17 00:00:00 2001
From: Joseph Turner <joseph@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
-- 
2.41.0

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

[Joseph Turner via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Joseph Turner <joseph@breatheoutbreathe.in> writes:

> Philip Kaludercic <philipk@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!

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

[Eli Zaretskii]

Ping! Ping!  Philip, can you please chime in?

> Cc: 69528@debbugs.gnu.org, iarchivedmywholelife@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@gnu.org>
> 
> Joseph Turner <joseph@breatheoutbreathe.in> writes:
> 
> > Philip Kaludercic <philipk@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

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

[Philip Kaludercic]

Joseph Turner <joseph@breatheoutbreathe.in> writes:

> Philip Kaludercic <philipk@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@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@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@breatheoutbreathe.in> writes:

> Joseph Turner <joseph@breatheoutbreathe.in> writes:
>
>> Philip Kaludercic <philipk@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@gnu.org> writes:

> Ping! Ping!  Philip, can you please chime in?
>
>> Cc: 69528@debbugs.gnu.org, iarchivedmywholelife@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@gnu.org>
>> 
>> Joseph Turner <joseph@breatheoutbreathe.in> writes:
>> 
>> > Philip Kaludercic <philipk@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

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

[Joseph Turner via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Philip Kaludercic <philipk@posteo.net> writes:

> Joseph Turner <joseph@breatheoutbreathe.in> writes:
>
>> Philip Kaludercic <philipk@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@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@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@breatheoutbreathe.in> writes:
>
>> Joseph Turner <joseph@breatheoutbreathe.in> writes:
>>
>>> Philip Kaludercic <philipk@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@gnu.org> writes:
>
>> Ping! Ping!  Philip, can you please chime in?
>>
>>> Cc: 69528@debbugs.gnu.org, iarchivedmywholelife@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@gnu.org>
>>> 
>>> Joseph Turner <joseph@breatheoutbreathe.in> writes:
>>> 
>>> > Philip Kaludercic <philipk@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
>>
>>
>>
>>

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

[Philip Kaludercic]

Joseph Turner <joseph@breatheoutbreathe.in> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> Joseph Turner <joseph@breatheoutbreathe.in> writes:
>>
>>> Philip Kaludercic <philipk@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

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

[Eli Zaretskii]

> Cc: 69528@debbugs.gnu.org, iarchivedmywholelife@gmail.com
> From: Philip Kaludercic <philipk@posteo.net>
> Date: Sat, 25 May 2024 08:04:24 +0000
> 
> Joseph Turner <joseph@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.

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

[Joseph Turner via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Philip Kaludercic <philipk@posteo.net> writes:

> Joseph Turner <joseph@breatheoutbreathe.in> writes:
>
>> Philip Kaludercic <philipk@posteo.net> writes:
>>
>>> Joseph Turner <joseph@breatheoutbreathe.in> writes:
>>>
>>>> Philip Kaludercic <philipk@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.

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

[Stefan Kangas]

Eli Zaretskii <eliz@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?

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

[Philip Kaludercic]

Stefan Kangas <stefankangas@gmail.com> writes:

> Eli Zaretskii <eliz@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

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

[Stefan Kangas]

Philip Kaludercic <philipk@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

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

[Philip Kaludercic]

Stefan Kangas <stefankangas@gmail.com> writes:

> Philip Kaludercic <philipk@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

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

[Joseph Turner via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 1712 bytes --]

Philip Kaludercic <philipk@posteo.net> writes:

> Stefan Kangas <stefankangas@gmail.com> writes:
>
>> Philip Kaludercic <philipk@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?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Check-Package-Version-header-in-lm-version-also.patch --]
[-- Type: text/x-diff, Size: 912 bytes --]

From a666581f2a58568bb7f83a369e1040920a6b2c14 Mon Sep 17 00:00:00 2001
From: Joseph Turner <joseph@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 | 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."
   (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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Use-lm-version-instead-of-lm-header-version.patch --]
[-- Type: text/x-diff, Size: 998 bytes --]

From 6c4262d7236c64bbc938f7b4e76988d95049b7c1 Mon Sep 17 00:00:00 2001
From: Joseph Turner <joseph@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 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

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

[Philip Kaludercic]

Joseph Turner <joseph@breatheoutbreathe.in> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> Stefan Kangas <stefankangas@gmail.com> writes:
>>
>>> Philip Kaludercic <philipk@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

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

[Stefan Kangas]

Joseph Turner <joseph@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@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@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

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> * 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

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

[Philip Kaludercic]

Stefan Monnier <monnier@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

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>>> * 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

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

[Stefan Kangas]

Stefan Monnier <monnier@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")

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

[Joseph Turner via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Stefan Kangas <stefankangas@gmail.com> writes:

> Joseph Turner <joseph@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

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>>>>> * 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