bug#48404: 28.0.50; "Invalid version syntax" errors with read-extended-command

[Tassilo Horn]

Eli Zaretskii <eliz@gnu.org> writes:

>> For example, M-x mag TAB gives
>> 
>>   completion--some: Invalid version syntax: ‘Magit 3.0.0’ (must start with a 
>> number)
>
> That commit uses byte-obsolete-info property of a symbol, and expects
> its caddr to be a valid version number.  It sounds like in some cases
> it is not an Emacs version, but something else.  So, questions:
>
>   . can you find out what kind of value does the byte-obsolete-info
>     property is there on Magit functions? and

It's not just Magit.  I've grepped my ~/.emacs.d/elpa/ and here are some
values:

  (make-obsolete-variable 'magit-refresh-args nil "Magit 3.0.0")
  (make-obsolete-variable backported sym "yasnippet 0.8")
  (make-obsolete-variable 'which-key-key-replacement-alist
                          'which-key-replacement-alist "2016-11-21")

So at least magit and yasnippet use "<package> <version>" syntax here,
and which-key uses a date (which seems fine for version-to-list).

>   . how come these properties were added to something that is not an
>     Emacs core code?

It seems to be added by `make-obsolete', no?  Otherwise, there is no
writing menion of byte-obsolete-info.

> In any case, it sounds like blindly trusting the value of that
> property is not a good idea, and we should wrap the call to
> version-to-list there in condition-case.  Could you try that?

I've tried this

--8<---------------cut here---------------start------------->8---
diff --git a/lisp/simple.el b/lisp/simple.el
index 0255f69e42..573e344fea 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -2020,8 +2020,10 @@ read-extended-command
                                     ;; Has a current-name.
                                     (functionp (car obsolete))
                                     ;; when >= emacs-major-version
-                                    (>= (car (version-to-list (caddr 
obsolete)))
-                                        emacs-major-version))))))
+                                    (condition-case nil
+                                        (>= (car (version-to-list (caddr 
obsolete)))
+                                            emacs-major-version)
+                                      (t nil)))))))
                     pred)))
              (complete-with-action action obarray string pred))))
--8<---------------cut here---------------end--------------->8---

which does the trick.

Should I write bug reports for Magit and Yasnippets so that they should
only specify the version number in `make-obsolete' /
`make-obsolete-variable'?  Or should emacs support the "<package>
<version>" syntax?

Bye,
Tassilo