REGRESSION: "Don't un-exclude an excluded package" prevents including explicitly

[aaronjensen]

Description

#6338 prevents explicitly including packages that have been included like evil-terminal-cursor-changer.

Reproduction guide 🪲

• Start Emacs
• Add evil-terminal-cursor-changer to dotspacemacs-additional-pacages
• SPC f e R

Observed behaviour: 👀 💔
The package is not installed

Expected behaviour: ❤️ 😄
The package should be installed

System Info 💻

• OS: darwin
• Emacs: 25.0.95.1
• Spacemacs: 0.105.21
• Spacemacs branch: nil (rev. 45afa3b)
• Graphic display: t
• Distribution: spacemacs
• Editing style: hybrid
• Completion: helm
• Layers:

(better-defaults spacemacs-layouts helm emacs-lisp markdown
                 (syntax-checking :variables syntax-checking-enable-tooltips nil)
                 (auto-completion :variables auto-completion-enable-sort-by-usage nil auto-completion-enable-snippets-in-popup nil)
                 erlang elixir git dash html clojure
                 (org :variables org-enable-github-support t)
                 colors osx github javascript react deft floobits ruby
                 (shell :variables shell-default-shell 'ansi-term shell-default-height 30 shell-default-position 'bottom)
                 spell-checking ranger version-control rcirc tmux yaml docker elm evil-little-word auto-correct frame-geometry cleverparens-lispy)

[darkfeline]

I think the issue here is that Spacemacs shouldn't be explicitly excluding any packages, not that you should be able to include a previously excluded package. This latter behavior (include a previously excluded package) is bad since behavior should be independent of layer load order.

[aaronjensen]

I don't think I agree with this. I'm fine with packages not overriding one another, but dotspacemacs-additional-packages should trump all. That's where I say, "no really, include these"

[darkfeline]

dotspacemacs-additional-packages includes packages without you having to define layer/init-package, but it still uses the same package loading mechanism as, e.g., layers.

In other words, dotspacemacs-additional-packages is functionally equivalent to defining the package in a layer.

[syl20bnr]

I think we are too strict about the current rule, owners of packages + dotfile should be allowed to override the exclude property. Does it make sense ?

[darkfeline]

@syl20bnr I disagree. :excluded should be an absolute blacklist.

Implementing this would require adding at least two different "levels" of exclusion and inclusion, such that packages included by level 1 can be excluded by level 2, and packages excluded by level 1 can be included by level 2, and perhaps other precedence rules (what happens if a package is both included and excluded at the same level?). I don't think it's worth it (although since I probably won't be the one implementing this, I'll let the implementer decide).

(As someone who loves doing really bad things with macros and metaprogramming, I actually really love this idea (infinite levels of inclusion/exclusion, toggling dozens of layers that all include and exclude things at different levels, just imagine!), but I don't think it will be good for Spacemacs because it adds unnecessary complexity.)

When would a user need to override an excluded package? When would a Spacemacs-bundled layer need to exclude a package? It seems to me that exclusion is solely used by users to exclude packages from Spacemacs-bundled layers or to ban packages that simply cannot work with the functionality provided by a layer (indicating that the package is poorly written).

Here's the current documentation for reference:

Packages may be /excluded/ by setting the =:excluded= property to true. This
will prevent the package from being installed even if it is used by another
layer.

[syl20bnr]

@darkfeline l I agree with you that we should not exclude package just for the sake of it, we should comment evil-terminal-cursor-changer and not exclude it.

But sometimes a layer needs to exclude a package because it replaces some package by another. Maybe we could replace the :exclude keyword at layer level by :replace <package> which would prevent <package> from being loaded without excluding it. And then the only way to exclude a package is to use dotspacemacs-excluded-packages.

[syl20bnr]

To quickly fix the issue, I commented the package instead of excluding it: 9499e24

[darkfeline]

What would the semantics for :replace be? The package would be installed but the <layer>/init-<package> function is not called? Then the user "overrides" this by initializing the package in their config. Users cannot do this in their own my-layer/packages.el since this would also be subject to :replace, but instead in .spacemacs or my-layer/config.el.

I'm not familiar with the interaction between package.el and use-package, but for some poorly written packages simply loading the package (by package.el) might have unwanted side effects.

[syl20bnr]

We need to get back to the root for this case. :-)

@darkfeline you wrote in the PR:

If a layer using a package is loaded after a layer excluding it, the
exclude flag should not be overwritten.

It does not tell me what the PR actually fixes, what is the concrete "bug" you fixed with this PR? Did you encounter it in practice?

[darkfeline]

The bug is that I excluded a package in my personal configuration layer, but that package was being re-included by a Spacemacs layer, although I can't recall the details now, since it was a while back.

[aaronjensen]

The bug is that I excluded a package in my personal configuration layer, but that package was being re-included by a Spacemacs layer, although I can't recall the details now, since it was a while back.

This seems fine to prevent. My problem is treating dotspacemacs-additional-pacages the same as a layer.

Another option would be the equivalent of css's !important. Making it possible to include a package even if another layer had excluded it.

[syl20bnr]

The bug is that I excluded a package in my personal configuration layer, but that package was being re-included by a Spacemacs layer, although I can't recall the details now, since it was a while back.

How did you exclude it ?
The layer system is made like this: the last layer wins. If you want to make sure to override a property then put your layer at the end of the list.

I believe I made a mistake by not asking you what problem you wanted to solve in the first place.

[darkfeline]

Sorry, I've been busy, so I haven't had time to revert everything and reproduce this, but from memory:

I had excluded mu4e-maildirs-extension by (mu4e-maildirs-extension :excluded t) in my private configuration layer. I always keep my private configuration layers at the bottom of my layers list.

When I debugged the issue, I was surprised that this was somehow getting overridden by the mu4e layer, but I was also surprised that this was even happening to begin with because I intuitively assumed that :excluded was a blacklist, and then (wrongly?) assumed that most people would also assume this behavior.

In retrospect, this may have been caused by using configuration-layer/declare-layers. From memory, I had in my dotfile layers:

...
mir-rc-mail

in mir-rc-mail's configuration-layer/declare-layers:

mir-rc-mu4e

in mir-rc-mu4e's packages:

...
(mu4e-maildirs-extension :excluded t)
...

[syl20bnr]

I always keep my private configuration layers at the bottom of my layers list.

Then we have a bug somewhere, your private layer should have the last word on mu4e-maildirs-extension :exclude property.

I suppose that configuration-layer/declare-layers was in config.el file ?

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Please let us know if this issue is still valid!