Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

POC: push plugin enablement into plugin components #4156

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

michaelpj
Copy link
Collaborator

This implements a version of what I describe in [this comment] on the fourmolu plugin as a POC. The goal here is that:

  • CPP moves to plugin components, rather than the integration point
  • Plugin components always export a plugin descriptor, it just might provide nothing and say it is disabled
    • This can be helpful for our users, to tell them that things are disabled
  • Plugin components always build, even if the flag is turned off
    • This is useful since cabal actually always tries to solve for all the buildable components, pulling in unwanted dependencies. This way that will happen, but it will be fine. (An alternative would be to just mark them as not buildable in this situation)
  • Plugin tests are not buildable if the flag is off.
    • We could try and make them always buildable, and just have no tests, but that seems worse?

I haven't tried to make "being disabled" a first-class status or anything, a disabled plugin is just a plugin with no handlers and a description that calls that out. Maybe that's a mistake? I'm not sure.

@maralorn
Copy link
Contributor

As a downstream consumer I like this a lot. It will make things easier for us.

In regard to the code I just wonder how good this scales. Does this mean that every file in every plugin contains a big CPP statement. I wonder if that is annoying to maintain?

@michaelpj
Copy link
Collaborator Author

I pushed an alternative version that instead has multiple different source directories.

@michaelpj
Copy link
Collaborator Author

WDYT @fendor ? It turns one module into four, which is not that nice...

@soulomoon
Copy link
Collaborator

soulomoon commented Apr 4, 2024

An alternative would be to just mark them as not buildable in this situation

Will it improve the build time if most of the plugins are disabled.
It would be helpful to create the minimal HLS described in #2979 ?

michaelpj added a commit that referenced this pull request Apr 6, 2024
This ensures that cabal does not consider them at all, and won't try to
solve for their dependencies. So if we turn off the fourmolu plugin, cabal
really won't consider fourmolu at all.

This gets us some of the benefits of #4156 with much less work.

Fixes #4100.
michaelpj added a commit that referenced this pull request Apr 6, 2024
This ensures that cabal does not consider them at all, and won't try to
solve for their dependencies. So if we turn off the fourmolu plugin, cabal
really won't consider fourmolu at all.

This gets us some of the benefits of #4156 with much less work.

Fixes #4100.
fendor pushed a commit that referenced this pull request Apr 6, 2024
* Mark plugins as not buildable if the flag is disabled

This ensures that cabal does not consider them at all, and won't try to
solve for their dependencies. So if we turn off the fourmolu plugin, cabal
really won't consider fourmolu at all.

This gets us some of the benefits of #4156 with much less work.

Fixes #4100.

* Stick to no space after flag for consistency
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants