GNU bug report logs - #46841
[PATCH] Make package downloading in inversion.el obsolete

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 46841 in the body.
You can then email your comments to 46841 AT debbugs.gnu.org in the normal way.

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

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

From: Stefan Kangas <stefan <at> marxist.se>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Make package downloading in inversion.el obsolete
Date: Sun, 28 Feb 2021 20:30:29 -0800

[Message part 1 (text/plain, inline)]

Severity: wishlist

There is some code to download packages in cedet/inversion.el which
seems completely redundant now that we have package.el.  I suggest
marking that part as obsolete, see the attached patch.

To be honest, I don't exactly see that there is any need for
inversion.el since the problem it tries to solve is already solved by
package.el.  Perhaps we should just move the entire library to
obsolete/.

[0001-Make-package-downloading-in-inversion.el-obsolete.patch (text/x-diff, attachment)]

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 46841 <at> debbugs.gnu.org
Subject: Re: bug#46841: [PATCH] Make package downloading in inversion.el
 obsolete
Date: Mon, 01 Mar 2021 14:00:36 +0100

Stefan Kangas <stefan <at> marxist.se> writes:

> There is some code to download packages in cedet/inversion.el which
> seems completely redundant now that we have package.el.  I suggest
> marking that part as obsolete, see the attached patch.
>
> To be honest, I don't exactly see that there is any need for
> inversion.el since the problem it tries to solve is already solved by
> package.el.  Perhaps we should just move the entire library to
> obsolete/.

Utility functions like `inversion-package-version' are used in the rest
of CEDET, though?  (I just did a quick grep, but didn't actually read
the call sites.)

But, yes, the download bits don't seem very useful.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: "Eric M. Ludlam" <zappo <at> gnu.org>, 46841 <at> debbugs.gnu.org
Subject: Re: bug#46841: [PATCH] Make package downloading in inversion.el
 obsolete
Date: Mon, 1 Mar 2021 10:08:48 -0600

(Copying in Eric Ludlam in case he has something to add.)

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Stefan Kangas <stefan <at> marxist.se> writes:
>
>> There is some code to download packages in cedet/inversion.el which
>> seems completely redundant now that we have package.el.  I suggest
>> marking that part as obsolete, see the attached patch.
>>
>> To be honest, I don't exactly see that there is any need for
>> inversion.el since the problem it tries to solve is already solved by
>> package.el.  Perhaps we should just move the entire library to
>> obsolete/.
>
> Utility functions like `inversion-package-version' are used in the rest
> of CEDET, though?  (I just did a quick grep, but didn't actually read
> the call sites.)

Yes.  I took a closer look.  The uses all seem to be of dubious value
these days, and could probably themselves be obsoleted or changed to not
need inversion.  Here is what I could find:

./lisp/cedet/cedet.el106:		(filever (car (inversion-find-version sym)))

The function `cedet-version' produces a screen like this:

    CEDET Version:	2.0
      			Requested	File		Loaded
      Package		Version		Version		Version
      ----------------------------------------------------------
      cedet:		2.0		ok		ok
      eieio:		1.4		ok		ok
      semantic:		2.2		ok		Not Loaded
      srecode:		1.2		ok		Not Loaded
      ede:			1.2		2.0		Not Loaded


    C-h f cedet-version RET
      for details on output format.

But all these packages are distributed with Emacs itself.

./lisp/cedet/semantic/ede-grammar.el165:    (list "eieio" "semantic"
"inversion" "ede")))
./lisp/cedet/semantic/ede-grammar.el168:  ;; Inversion for versioning system.

Add inversion to loadpath in Makefiles produced by EDE, for some reason.
If inversion is obsolete, that would not be useful.

./lisp/cedet/semantic/db-file.el176:	  (if (not (inversion-test
'semanticdb-file fv))
./lisp/cedet/semantic/db-file.el177:	      (when (inversion-test
'semantic-tag tv)
./lisp/cedet/cedet-cscope.el156:	(if (inversion-check-version rev nil
cedet-cscope-min-version)
./lisp/cedet/cedet-idutils.el185:	(if (inversion-check-version rev nil
cedet-idutils-min-version)
./lisp/cedet/cedet-global.el160:	(if (inversion-check-version rev nil
cedet-global-min-version)

Several similar functions that just check for the installed version,
which is either the one distributed with Emacs or the latest released
version.  The latter would normally be the one you automatically
installed with package.el.

So it's all just about making sure the correct version of a package is
installed.  AFAICT, that is fully obsoleted by `package-install'.

./lisp/cedet/ede/make.el76:	(setq ans (not (inversion-check-version
rev nil ede-make-min-version))))

This parses the make version and ensures its recent enough.  Perhaps
useful, but could likely be replaced by the built-in `version<' or
somesuch.

./lisp/cedet/semantic.el61:  (inversion-test 'semantic

Here we have a function `semantic-require-version' that allows callers
to check if this is a recent enough version of Semantic.

They should instead be checking for the major version of Emacs, or if
Semantic is installed from the external CEDET repository deal with it
themselves, or if Semantic is turned into a core package and installed
that way they should just make sure their dependencies are correct.

Am I missing something here?  Eric?

> But, yes, the download bits don't seem very useful.

Thanks.

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

From: Eric Ludlam <ericludlam <at> gmail.com>
To: Stefan Kangas <stefan <at> marxist.se>, Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: "Eric M. Ludlam" <zappo <at> gnu.org>, 46841 <at> debbugs.gnu.org
Subject: Re: bug#46841: [PATCH] Make package downloading in inversion.el
 obsolete
Date: Tue, 2 Mar 2021 17:13:09 -0500

Thanks for checking in about inversion.el.

I used to have a lot of issues with different versions of things when 
CEDET was
separate from Emacs and new versions of CEDET could overlay on top of an 
Emacs
with an earlier version of CEDET included.  Inversion was a way of getting
things tied together, and upgrading save files to new versions, and 
things like that.

None of those old issues exist anymore since external CEDET is too hard 
to merge back
into Emacs and I now post patches direct to Emacs instead, so it makes 
sense to clean this up.

Thanks
Eric

On 3/1/21 11:08 AM, Stefan Kangas wrote:
> (Copying in Eric Ludlam in case he has something to add.)
>
> Lars Ingebrigtsen <larsi <at> gnus.org> writes:
>
>> Stefan Kangas <stefan <at> marxist.se> writes:
>>
>>> There is some code to download packages in cedet/inversion.el which
>>> seems completely redundant now that we have package.el.  I suggest
>>> marking that part as obsolete, see the attached patch.
>>>
>>> To be honest, I don't exactly see that there is any need for
>>> inversion.el since the problem it tries to solve is already solved by
>>> package.el.  Perhaps we should just move the entire library to
>>> obsolete/.
>> Utility functions like `inversion-package-version' are used in the rest
>> of CEDET, though?  (I just did a quick grep, but didn't actually read
>> the call sites.)
> Yes.  I took a closer look.  The uses all seem to be of dubious value
> these days, and could probably themselves be obsoleted or changed to not
> need inversion.  Here is what I could find:
>
> ./lisp/cedet/cedet.el106:		(filever (car (inversion-find-version sym)))
>
> The function `cedet-version' produces a screen like this:
>
>      CEDET Version:	2.0
>        			Requested	File		Loaded
>        Package		Version		Version		Version
>        ----------------------------------------------------------
>        cedet:		2.0		ok		ok
>        eieio:		1.4		ok		ok
>        semantic:		2.2		ok		Not Loaded
>        srecode:		1.2		ok		Not Loaded
>        ede:			1.2		2.0		Not Loaded
>
>
>      C-h f cedet-version RET
>        for details on output format.
>
> But all these packages are distributed with Emacs itself.
>
> ./lisp/cedet/semantic/ede-grammar.el165:    (list "eieio" "semantic"
> "inversion" "ede")))
> ./lisp/cedet/semantic/ede-grammar.el168:  ;; Inversion for versioning system.
>
> Add inversion to loadpath in Makefiles produced by EDE, for some reason.
> If inversion is obsolete, that would not be useful.
>
> ./lisp/cedet/semantic/db-file.el176:	  (if (not (inversion-test
> 'semanticdb-file fv))
> ./lisp/cedet/semantic/db-file.el177:	      (when (inversion-test
> 'semantic-tag tv)
> ./lisp/cedet/cedet-cscope.el156:	(if (inversion-check-version rev nil
> cedet-cscope-min-version)
> ./lisp/cedet/cedet-idutils.el185:	(if (inversion-check-version rev nil
> cedet-idutils-min-version)
> ./lisp/cedet/cedet-global.el160:	(if (inversion-check-version rev nil
> cedet-global-min-version)
>
> Several similar functions that just check for the installed version,
> which is either the one distributed with Emacs or the latest released
> version.  The latter would normally be the one you automatically
> installed with package.el.
>
> So it's all just about making sure the correct version of a package is
> installed.  AFAICT, that is fully obsoleted by `package-install'.
>
> ./lisp/cedet/ede/make.el76:	(setq ans (not (inversion-check-version
> rev nil ede-make-min-version))))
>
> This parses the make version and ensures its recent enough.  Perhaps
> useful, but could likely be replaced by the built-in `version<' or
> somesuch.
>
> ./lisp/cedet/semantic.el61:  (inversion-test 'semantic
>
> Here we have a function `semantic-require-version' that allows callers
> to check if this is a recent enough version of Semantic.
>
> They should instead be checking for the major version of Emacs, or if
> Semantic is installed from the external CEDET repository deal with it
> themselves, or if Semantic is turned into a core package and installed
> that way they should just make sure their dependencies are correct.
>
> Am I missing something here?  Eric?
>
>> But, yes, the download bits don't seem very useful.
> Thanks.

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Eric Ludlam <ericludlam <at> gmail.com>, Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: "Eric M. Ludlam" <zappo <at> gnu.org>, 46841 <at> debbugs.gnu.org
Subject: Re: bug#46841: [PATCH] Make package downloading in inversion.el
 obsolete
Date: Wed, 3 Mar 2021 13:02:00 -0600

[Message part 1 (text/plain, inline)]

tags 46841 + fixed
close 46841 28.1
thanks

Eric Ludlam <ericludlam <at> gmail.com> writes:

> I used to have a lot of issues with different versions of things when
> CEDET was
> separate from Emacs and new versions of CEDET could overlay on top of an
> Emacs
> with an earlier version of CEDET included.  Inversion was a way of getting
> things tied together, and upgrading save files to new versions, and
> things like that.
>
> None of those old issues exist anymore since external CEDET is too hard
> to merge back
> into Emacs and I now post patches direct to Emacs instead, so it makes
> sense to clean this up.

Thanks for giving the background.  Since you agree, I think we should
make the entire library obsolete.

So I've installed the attached patch on master and I'm closing this bug.

[0001-Make-inversion.el-obsolete-Bug-46841.patch (text/x-diff, attachment)]

Previous Next

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