modus-themes ELPA and etc/themes

modus-themes ELPA and etc/themes

[Stefan Monnier]

I just noticed that Emacs is not aware that it comes bundled with
`modus-themes`, so it may unnecessarily fetch `modus-themes` from the GNU
ELPA archive.

The problem is due to the fact that the file is under `etc/themes` and
is hence not processed by our autoloads machinery which would otherwise
add something like

    (push (purecopy '(modus-themes 1 2 4)) package--builtin-versions)

to lisp/loaddefs.el.

There are various ways to fix this, but to me the simplest would be to
move `etc/themes` to `lisp` (or at least to move `modus-themes.el`).


        Stefan

Re: modus-themes ELPA and etc/themes

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Sat, 20 Mar 2021 15:52:39 -0400
> 
> The problem is due to the fact that the file is under `etc/themes` and
> is hence not processed by our autoloads machinery which would otherwise
> add something like
> 
>     (push (purecopy '(modus-themes 1 2 4)) package--builtin-versions)
> 
> to lisp/loaddefs.el.
> 
> There are various ways to fix this, but to me the simplest would be to
> move `etc/themes` to `lisp` (or at least to move `modus-themes.el`).

I'd rather we didn't move files around.  If nothing else, it makes
forensics harder down the line.  Besides, we had themes in that place
for too long to make such changes now, and for this minor reason on
top of that.

Why can't we take any of the other ways to fix this?

Re: modus-themes ELPA and etc/themes

[Stefan Monnier]

>> The problem is due to the fact that the file is under `etc/themes` and
>> is hence not processed by our autoloads machinery which would otherwise
>> add something like
>> 
>>     (push (purecopy '(modus-themes 1 2 4)) package--builtin-versions)
>> 
>> to lisp/loaddefs.el.
>> 
>> There are various ways to fix this, but to me the simplest would be to
>> move `etc/themes` to `lisp` (or at least to move `modus-themes.el`).
>
> I'd rather we didn't move files around.  If nothing else, it makes
> forensics harder down the line.  Besides, we had themes in that place
> for too long to make such changes now, and for this minor reason on
> top of that.

OK, so it's a definite no for moving `etc/themes`.

How 'bout moving just `modus-themes.el` (it's a fresh new file, so the
forensics aspect shouldn't matter that much, especially since the real
Git history is in the upstream repository rather than in emacs.git
anyway)?  Part of the reason why `modus-themes.el` could be treated
differently is that it actually contains a fair bit of ELisp code, so it
would make sense to compile it.

> Why can't we take any of the other ways to fix this?

Oh, we definitely can, I'm just trying to figure out which solution is best.


        Stefan

Re: modus-themes ELPA and etc/themes

[Basil L. Contovounesios]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> OK, so it's a definite no for moving `etc/themes`.
>
> How 'bout moving just `modus-themes.el` (it's a fresh new file, so the
> forensics aspect shouldn't matter that much, especially since the real
> Git history is in the upstream repository rather than in emacs.git
> anyway)?  Part of the reason why `modus-themes.el` could be treated
> differently is that it actually contains a fair bit of ELisp code, so it
> would make sense to compile it.

[Just for posterity - this was touched on in bug#45068 to an extent.]

-- 
Basil

Re: modus-themes ELPA and etc/themes

[Protesilaos Stavrou]

On 2021-03-20, 21:31 +0000, "Basil L. Contovounesios" <contovob@tcd.ie> wrote:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>> OK, so it's a definite no for moving `etc/themes`.
>>
>> How 'bout moving just `modus-themes.el` (it's a fresh new file, so the
>> forensics aspect shouldn't matter that much, especially since the real
>> Git history is in the upstream repository rather than in emacs.git
>> anyway)?  Part of the reason why `modus-themes.el` could be treated
>> differently is that it actually contains a fair bit of ELisp code, so it
>> would make sense to compile it.
>
> [Just for posterity - this was touched on in bug#45068 to an extent.]

A very short summary of that bug, as pertains to this point:

+ A file that resides in etc/themes cannot be loaded with just a
  (require) call because it is not in the load-path.

+ We need such a functionality to load the main modus-themes.el from
  within modus-{operandi,vivendi}-theme.el.  The former contains the
  bulk of the code, while the latter two expand the relevant parts into
  actual themes.

+ To make everything work inside etc/themes we had to use the new
  'require-theme' function, which accounts for that directory.

-- 
Protesilaos Stavrou
protesilaos.com

Re: modus-themes ELPA and etc/themes

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Sat, 20 Mar 2021 16:39:31 -0400
> 
> How 'bout moving just `modus-themes.el` (it's a fresh new file, so the
> forensics aspect shouldn't matter that much, especially since the real
> Git history is in the upstream repository rather than in emacs.git
> anyway)?  Part of the reason why `modus-themes.el` could be treated
> differently is that it actually contains a fair bit of ELisp code, so it
> would make sense to compile it.

I envision that more themes will go the way of modus-themes (and at
the time you more or less said the same), so I think it would be good
to design and implement support for such themes in package.el now,
that we have a real-life example.  Otherwise I'm afraid we will have
this discussion many times in the future, and we eventually will end
up with some themes under Lisp and others under etc/themes, which I
think is undesirable.

We recently added some simple infrastructure to support themes that,
like modus-themes, want to dynamically load their parts.  How about
introducing special support for those in package.el as well?

> > Why can't we take any of the other ways to fix this?
> 
> Oh, we definitely can, I'm just trying to figure out which solution is best.

I'd like to keep themes in their current location.  Where this
requires additional infrastructure for themes that do something
special, I think we should add such an infrastructure.

Thanks.

Re: modus-themes ELPA and etc/themes

[Stefan Monnier]

> I envision that more themes will go the way of modus-themes (and at
> the time you more or less said the same), so I think it would be good
> to design and implement support for such themes in package.el now,
> that we have a real-life example.

I'm not sure what extra support for it you want in `package.el`.
`package.el` already works great to install modus-themes as well as
many other themes available from ELPA.

The problem here is with the themes we bundle with Emacs, because these
are not installed in the normal way (e.g. they're not `load-path` and
they're not scrubbed by autoload.el).

> I'd like to keep themes in their current location.  Where this
> requires additional infrastructure for themes that do something
> special, I think we should add such an infrastructure.

The current problem is not due to modus-themes doing something special,
but to the need to record the version of modus-themes that we bundle so
that we can compare it to the versions available from ELPA.  The problem
already existed for other themes, such as `leuven` which we bundle
without telling package.el.

I opened a bug for that.


        Stefan

Re: modus-themes ELPA and etc/themes

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Fri, 26 Mar 2021 17:41:34 -0400
> 
> > I envision that more themes will go the way of modus-themes (and at
> > the time you more or less said the same), so I think it would be good
> > to design and implement support for such themes in package.el now,
> > that we have a real-life example.
> 
> I'm not sure what extra support for it you want in `package.el`.
> `package.el` already works great to install modus-themes as well as
> many other themes available from ELPA.
> 
> The problem here is with the themes we bundle with Emacs, because these
> are not installed in the normal way (e.g. they're not `load-path` and
> they're not scrubbed by autoload.el).

They are in custom-theme-load-path, right?  So that's what I meant by
supporting themes in package.el: teach it about
custom-theme-load-path, so it could know which themes are bundled.

> > I'd like to keep themes in their current location.  Where this
> > requires additional infrastructure for themes that do something
> > special, I think we should add such an infrastructure.
> 
> The current problem is not due to modus-themes doing something special,
> but to the need to record the version of modus-themes that we bundle so
> that we can compare it to the versions available from ELPA.  The problem
> already existed for other themes, such as `leuven` which we bundle
> without telling package.el.
> 
> I opened a bug for that.

Thanks.