Re: having emacs-matlab in ELPA, finally. FSF paper signed

[Philip Kaludercic <philipk@posteo.net>]

Stefan Kangas <stefankangas@gmail.com> writes:

> Uwe Brauer <oub@mat.ucm.es> writes:
>
>>>>> "SK" == Stefan Kangas <stefankangas@gmail.com> writes:
>>
>>> Philip Kaludercic <philipk@posteo.net> writes:
>>>> +(defconst matlab-mode-version (package-get-version)
>>>> +  "Current version of MATLAB(R) mode.")
>>
>>> We have tried to discourage the addition of such variables and commands,
>>> given that one can always find that information by other means,
>>> e.g. interactively with `describe-package` or from Lisp:
>>
>>>     (package-desc-version (cadr (assq 'foo package-alist)))
>>
>> So you want me use just that string
>> well (package-desc-version (cadr (assq 'matlab-mode package-alist)))
>> I presume
>>
>> in matlab.el (and matlab-mode.el)?
>
> That's what I would do, indeed.  Well, I'd use this if you need the
> string form:
>
>     (package-version-join
>      (package-desc-version (cadr (assq 'matlab-mode package-alist))))

We should keep in mind that this will break if the package is not
installed using package.el, as then

  (cadr (assq 'matlab-mode package-alist)) ;=> nil

and thus

  (package-desc-version nil) ;!> (wrong-type-argument package-desc nil)

> Yeah, it's a bit of a mouthful.  Most packages don't bother, and just
> rely on `describe-package`, I think.
>
> That said, there's nothing wrong with keeping it around; it just seems
> redundant now that you have a "Version" header.  I mostly mentioned it
> for the record, and in case you wanted to get rid of some possible
> redundancy.
>
> (Personally, I'm not sure if the `matlab-show-version` command is needed
> either given that we have `describe-package`.  In emacs.git, we tend to
> make such `foo-package-version` commands obsolete.)

1+ to this.  The custom commands for version numbers are an anti-pattern IMO.

>
>> What's about our current setting
>>
>> (defconst matlab-mode-version "6.2"
>>   "Current version of MATLAB(R) mode.")
>>
>> Which we need for MELPA?
>
> Hmm, are you sure that you need it for MELPA even with the addition of
> the "Version" header?
>
> In any case, this is an extremely minor nit either way.

Uwe Brauer via Matlab-emacs-discuss
<matlab-emacs-discuss-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
writes:

>> Uwe Brauer <oub-YB6e1s5WF/He5aOfsHch1g@public.gmane.org> writes:
>
>> You don't have to do anything*, I just have to add a package
>> specification (similar to the "recipe file" for MELPA) to elpa.git.
>
>> * As soon as it works.  The main issues right now, are as you noticed
>>   that ELPA doesn't release a new tarball for every commit, but just
>>   when a new version is released.  We track this by checking for commits
>>   that bump the "Version" header in the main file (in your case
>>   matlab.el).  You currently don't have any version number, so the build
>>   fails.  I would suggest applying a change like this, if it doesn't
>>   break any assumptions you have regarding the minimal Emacs version:
>
>> diff --git a/matlab.el b/matlab.el
>> index 8ca0ee24a2..b82229a672 100644
>> --- a/matlab.el
>> +++ b/matlab.el
>> @@ -5,11 +5,7 @@
>>  ;; Maintainer: Eric M. Ludlam <eludlam-I5WecO5yM8GakBO8gow8eQ@public.gmane.org>
>>  ;; Created: 04 Jan 91
>>  ;; Keywords: MATLAB(R)
>> -;; Version:
>> -
>> -(defconst matlab-mode-version "5.0"
>> -  "Current version of MATLAB(R) mode.")
>> -
>> +;; Version: 5.0
>>  ;;
>>  ;; Copyright (C) 1997-2022 Eric M. Ludlam
>>  ;; Copyright (C) 1991-1997 Matthew R. Wette
>> @@ -50,6 +46,10 @@
>  
>>  ;;; Code:
>  
>> +(defconst matlab-mode-version (package-get-version)
>> +  "Current version of MATLAB(R) mode.")
>> +
>> +
>>  (require 'matlab-compat)
>>  (require 'matlab-syntax)
>>  (require 'matlab-scan)
>
> A couple of remarks, 
>
>     1. this diff seems against an older matlab-mode version (may the one
>        in sourceforge), the latest github version is 6.2
>        https://github.com/mathworks/Emacs-MATLAB-Mode.git

That might very well be, I used a local checkout that ELPA made when
experimenting with the package, and it appears that was not the newest
version.

>     2. Jonas Bernoulli one of the MELPA maintainers, recommended (but
>        did not demand) to add file called matlab-mode.el which is
>        basically a dummy file but also contains the current version 

What is the name of the MELPA package?  If it is called "matlab", then
by default ELPA will consult the "matlab.el" file for version
information.  Otherwise it will consult "matlab-mode.el".  We can adjust
this though if need be in the package specification.

>>   The other issue is that ELPA checks the copyright string and wants to
>>   see that all packages in GNU ELPA have their copyright assigned to the
>>   FSF.
>
> There is an automated process for checking this. If I had known, it
> would have saved my quite a bit of time and work. I eyeballed the logs
> and if there was an unknown author I checked the diff, and if necessary
> contacted him. 

It doesn't go through the logs, it just searches for a copyright string
somewhere in the header.

>>   If you can fix these two things, then everything should go through.
>>   You can then decide to filter out files out of the final tarball by
>>   adding an .elpaignore file that lists what files to exclude.
>
>
>     1. I will add an elpaignore file, which is urgent,
>
>     2. And concerning the version number, what shall I do, your proposal
>        or Stefan's?
>
> For the moment being I would like to keep the MELPA version as well till
> I know ELPA works

Inherently there is no reason against having both, especially if people
have already installed the package via MELPA.

> Uwe 

Uwe Brauer <oub@mat.ucm.es> writes:

>> Uwe Brauer <oub@mat.ucm.es> writes:
>
>  
>> +(defconst matlab-mode-version (package-get-version)
>> +  "Current version of MATLAB(R) mode.")
>> +
>> +
>>  (require 'matlab-compat)
>>  (require 'matlab-syntax)
>>  (require 'matlab-scan)
>
>
>>   The other issue is that ELPA checks the copyright string and wants to
>>   see that all packages in GNU ELPA have their copyright assigned to the
>>   FSF.
>
>>   If you can fix these two things, then everything should go through.
>>   You can then decide to filter out files out of the final tarball by
>>   adding an .elpaignore file that lists what files to exclude.
>
> I added .elpaignore, 
> and pushed the commit to the github repository.

1+

> Concerning the version header, I got two different recommendation so I
> prefer to wait till this is settled, and also what to do with the
> original string that is needed for MELPA
>
>
>> We have tried to discourage the addition of such variables and commands,
>> given that one can always find that information by other means,
>> e.g. interactively with `describe-package` or from Lisp:
>
>>     (package-desc-version (cadr (assq 'foo package-alist)))