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

Add plugins conditionally at compile time #687

Merged
merged 1 commit into from
Dec 18, 2020
Merged

Conversation

jneira
Copy link
Member

@jneira jneira commented Dec 17, 2020

To assemble the desired hls version you could do:

  • cabal install: all plugins and formatters by default like the actual behaviour
  • cabal install -f-all-plugins -f-all-formatters: only ghcide plugin (it is fixed)
  • cabal install -f-all-formatters -f+floskell: all non formatting plugins and only floskell as formatter
  • cabal install -f-all-plugins -f-all-formatters -f+hlint -f+fourmolu: only ghcide (fixed) and hlint plugins and fourmolu as formatter

The plan would be change the install script to take in account plugins and:

  • let user list the concrete list of plugins
  • prepare some predefined plugins sets and set the appropiate cabal flags underneath:
    • all
    • none
    • floskell (all non formatter plugins and floskell). Each formatter would have a equivalent.
    • etc

The .cabal file has grown with quite boilerplate and add a plugin involves insert more locs, but it is almost mechanical. So add a new one it could be automated if we find it too cumbersome.

@jneira jneira requested a review from alanz December 17, 2020 19:38
@pepeiborra
Copy link
Collaborator

pepeiborra commented Dec 17, 2020

It is so unavoidably ugly.

Did you consider Backpack? My limited understanding is that an external module system is the best possible solution to this problem, but I have never used backpack myself.

@jneira
Copy link
Member Author

jneira commented Dec 17, 2020

@pepeiborra maybe backpack could work but stack does not support it for now so we could not use it. And i am afraid there are a bunch of not resolved issues in cabal.
And yeah, it is ugly but it is "only" boilerplate. We could generate it automatically using dhall-to-cabal or a custom preprocessor, for example. I would not say that it is unavoidable. And it is quite usable imo for an external user and based in a known and reliable build tool feature.

I am afraid i can't think off in other, better, alternative that would be very very welcomed

Copy link
Collaborator

@alanz alanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's horribly ugly, but in line with the approach we should be following in time.

I wonder if we shouldn't default to default True, so we don't break expectations for people just building as normal.

@jneira
Copy link
Member Author

jneira commented Dec 17, 2020

I wonder if we shouldn't default to default True

The default is build all plugins and formatters due to

flag all-plugins
  description: Enable all non formatter plugins
  default:     True
  manual:      True

flag all-formatters
  description: Enable all fomatters
  default:     True
  manual:      True

you have to disable those flags to build plugins one by one explicitly

@jneira
Copy link
Member Author

jneira commented Dec 18, 2020

I am gonna wait for a while just in case someone comes with a better alternative.
I myself continue thinking about to find it. For me that alternative, in addition to be more elegant, should fullfill

And it is quite usable imo for an external user and based in a known and reliable build tool feature.

@wz1000
Copy link
Collaborator

wz1000 commented Dec 18, 2020

I think we could dynamically load HLS plugins (like we dynamically load GHC plugins).

Then the user won't even need to recompile HLS in order to change their plugins.

@berberman
Copy link
Collaborator

Will using dynamic loading increase cross-platform maintenance costs? Even so, I think it would be a better way than the conditional compilation...

@jneira
Copy link
Member Author

jneira commented Dec 18, 2020

@wz1000 i mentioned that in #1 (comment) and #1 (comment) and we discarded it at that point (#1 (comment))

@wz1000
Copy link
Collaborator

wz1000 commented Dec 18, 2020

I'm not convinced, since we already support this for GHC plugins. initPlugins dynamically loads GHC plugins and inserts them into the cachedPlugins field of DynFlags. I don't see how it would be any different from dynamically loading HLS plugins.

/cc @ndmitchell

@jneira
Copy link
Member Author

jneira commented Dec 18, 2020

If dynamic plugins are reliable i think they could be a better alternative for sure. Some thoughts:

  • Not an expert in ghc plugins but want to note that we have some open issues related with them (https://github.com/haskell/ghcide/issues/868 for example). I suppose they should be eventually fixed and maybe they would be not applicable to hls plugins.
  • What could be the interface for final users willing to use some or all non formatter plugins and only one formatter (the usual setup)? Would be as straightforward as run cabal install -fxxxx -fyyyy?

@ndmitchell
Copy link
Collaborator

I maintain a GHC plugin, record-dot-preprocessor. Getting it working on Windows without segfaults is basically impossible - I debug it via CI, and developed it on a Mac. Dynamic plugins would be a great way to go, but currently, are not practical.

Likewise, Backpack is poorly supported on Stack, and leads to numerous problems.

CPP and Cabal flags is the only way to go :(

@alanz
Copy link
Collaborator

alanz commented Dec 18, 2020

CPP and Cabal flags is the only way to go :(

I actually think a custom tool to generate a "local" cabal package that has just the plugins a given site wants is the way to go.

And if the generated packages are popular enough,they can be published in hackage. This means we bring in one novel step, a converter from a config file to a package, and everything else is ecosystem standard.

@jneira
Copy link
Member Author

jneira commented Dec 18, 2020

I actually think a custom tool to generate a "local" cabal package that has just the plugins a given site wants is the way to go.

That could be a good idea, close to my proposal on automate the generation of the boilerplate imo, but doing it in a new .cabal config file instead in update the main haskell-language-server.cabal. The new .cabal config files will look nicer but if we dont want to touch the source file they should have the cpp flags.

We would exchange the boilerplate for a program that generates it.

And if the generated packages are popular enough,they can be published in hackage

That would be the weaker point, cause with this pr you'll have all possible variants (and there would be a lot of them, minimum one for formatter) in one package "for free" (well, paying the boilerplate tax). And users could assemble it at will at the call site, without waiting for us to release the package (or to have to release it themselves).


Given that we will not publish this in hackage yet (so we will not have our hands tied yet), i would merge it, as the custom tool has to be implemented, what do you think?

Of course if anybody wants to work in that tool i would not merge.

@alanz
Copy link
Collaborator

alanz commented Dec 18, 2020

I am happy with this as an interim step, it does not get in the way of possible future alternatives.

@jneira
Copy link
Member Author

jneira commented Dec 18, 2020

ok I would not mention it in docs and no change the install script to make easy its use, to add it as a feature for advanced users

@jneira jneira merged commit 4ea51f6 into haskell:master Dec 18, 2020
@jneira jneira deleted the cpplugins branch January 15, 2021 06:52
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.

6 participants