Split HLS plugins into internal libraries

[isovector]

While working through #391, and now attempting to do followup work, I'm continually running into issues of project structure. Today, HLS consists of a small internal library providing common functionality to plugins, and then one monolithic executable providing all of the plugins.

Due to the inaccessibility of the source code in cabal executables, the only sort of testing that can be done against the HLS exe is at the integration-level. And this testing must be mediated via LSP, aggressively limiting it's usefulness for any sort of unit test of internal-facing behavior.

Instead, I'd suggest moving plugins to internal libraries which eventually get linked by the exe. This would allow for the code to be reused in tests, and improve incremental build times for everyone.

[alanz]

FYI that is the next step as envisioned in #164

[isovector]

Excellent. Is there a timeline on #164? I'm willing to throw some elbow grease at it if necessary.

[alanz]

The goal is to have it done before we release the rest to hackage. And no blockers, just not got to yet. I suggest pinging @jneira and @fendor to coordinate any activity though.

[jneira]

I am afraid that we had issues with cabal private libs in the past:

• Due to a nasty cabal bug, the presence of private libs drives to cabal store corruption. The unique workaround is wipe out the entire store or the offending packages inside and run cabal-store-check --repair
• The stack explicit cradle cant load private libs: stack cradle fails to load private lib #114

We had a private lib and we converted it to a common stanza to avoid those problems: #136.

For the hlint plugin that must live in its own lib to being even buildable (due to ghc-lib) we opted to create a subpackage, with its own .cabal file: #166 (comment)

[jneira]

I would separate plugins one by one in their own subpackages, if we consider that it would give us something in return.
Bigger plugins, like tactic are good candidates to live in its own package, but simpler ones maybe will not need it.

[isovector]

@jneira how do you suggest I move forward in that case?

[jneira]

Well, as first step i would move the tactic plugin in its own subpackage (hls-tactic-plugin?) cause it will improve its development process and the extra maintaining effort would worth it for sure.
We did it for the hlint plugin, although it is not merged yet: https://github.com/jneira/haskell-language-server/tree/hlint-plugin-ghc-lib/plugins/hls-hlint-plugin.

[alanz]

@jneira part of the context is refactoring the build etc process to enable testing of subcomponents. Will this do that?

[jneira]

Mmm, good question, i supposed the advantages of having a private lib are the same as having a subpackage. The plugin could have its own (unit?) test suite or a hls test suite could have the plugin as dependency.

[isovector]

We tried using an internal library but as anticipated, none of the tooling could handle it. @TOTBWF and I are running into some extremely subtle bugs in tactics right now that don't show up outside of HLS. Being unable to write tests is extremely painful --- how can we move forward on this issue?

[jneira]

@isovector could split the plugin in its own subpackage as commented above help? It is an almost mechanic change

[fendor]

I am in favour of splitting them into sub-packages. I think this will help with future work, too, e.g. when we have compile-time plugin composition.

[gdevanla]

Of late I have been spending time reading/understanding hls/ghcide code. This conversation here makes me wonder about the following questions that come to mind (just based on my initial understanding of the code base).

1. I see a lot Completion/CodeActions provided by ghcide and then wrapped as a plug-in from hls. Is this also going to change?
2. There are 'Rules' current ghcide/hls provides and other plugins- could share. If plugins-were to live by themselves, they might not be able to leverage the HLS provided infrastructure that easily. Is that a correct understanding. Here I assume, hls has visibility to the plug-in but not the other way around.
3. Perhaps, (2) could be addressed by makes sharing of Rules more easier.

I may be off the mark with these questions. But, answers to these questions could help me understand the current and future architecture better.

Thanks

[isovector]

I made some progress today on what splitting tactics out would look like. It's easy to rip out the code into a separate package, but the testing infrastructure doesn't yet seem to be flexible enough to survive the same porting.

https://github.com/isovector/hls-tactics-plugin/commit/c052eff383e1db7a9b8807128792d605a2dd1187

[jneira]

@gdevanla there is a subpackage hls-plugin-api that depends on ghcide, independent of hls itself and published to hackage.
It should have the common functionality needed by the plugins so

• plugins would depend on hls-plugin-api and ghcide, so they have access to ghcide definitions
  • @pepeiborra suggested hls-plugin-api could export definitions from ghcide and then plugins only would depend on it
• hls, which at this point it is mainly a thin executable and its lib, depends on the plugins and ghcide

[jneira]

@isovector When hlint-plugin was moved to its own package, functional tests that needs a hls version with the plugin included continued living in hls, cause they dont test the plugin in isolation but its integration within hls.
If hlint-hls-plugin would have unit tests, with no executable involved they could live directly in its own package.

What do you miss from the hls tests infrastructure, hls-test-utils?

[isovector]

@jneira that's an unfortunate state of affairs. We'd like to run gold tests in our separate package --- they're really and truly tests of the plugin, not the integration. I copied and pasted the test infrastructure into the package, but it fails being unable to find the haskell-language-server binary.

[jneira]

mmm not sure if i am understanding it correctly: if we are using the hls executable to run functional test we are testing the integration of the plugin and the executable even if that integration is trivial. But i get you really want to have those tests close to the plugin and no in hls (imho they could live in hls too, if the plugin is a subpackage in the same project than hls)

Note that although use private libraries would fit that concrete use case, the goal is plugins (even private ones) could live (and be tested) outside hls. So find out a way to do functional tests would be necessary for sure.

I can think off in some paths:

• move test infrastructure into hls-plugin-api if possible to avoid copy it in each plugin
  • to avoid to create another package hls-plugin-api-test
• depend at runtime on hls executable without having it as component of the package (i think it should be possible somehow, but i have not checked it)
  • or create a dummy lsp server plugin container and put it in hls-plugin-api (or in lsp-test @bubba?)

We always could let the funcional tests live in hls, at least temporary, while we find out a final solution.

[jneira]

Corrections:

• To move the test infrastructure (hls-test-utils) we will need to copy modules directly to hls-plugin-api (not sure if it is a good idea) or definitely create a new package hls-plugin-test or similar
  • Cause we cant have two public libraries in the existing hls-plugin-api
• I just realized your plan is separate the plugin in its own package and project, so makes more sense to have tests in the package itself
  • hlint will live as a subpackage of hls project

[isovector]

@jneira yeah, that sums it up I think. We'd like to split tactics into a subrepo, so that I can work on it without requiring reviews on plugin-internal code. Right now this is a bit of a blocker --- is there any quick and dirty solution to the testing situation?

[alanz]

In my view we should not put test infra into hls-plugin-utils, rather do it in a separate library, if that is needed.

Would it make sense to be able to run a plugin's tests in some sort of composable way, so have a slimmed down Main,hs that only calls in the plugin, and runs the functional tests against that. This part would ideally run via some sort of exposed part of hls.

This might mean that a plugin in its own repo needs to either have a flag to fully enable the tests, or publish its tests in a separate package (which is functionally the same effect, just different admin process).

[jneira]

I think the quick one is keep functional tests in hls while we dont have a definitive solution, the hls test suite could import the plugin lib if needed. It is far from ideal cause changes in the repo usually supposes add or modify tests.
Maybe we could keep it as a subpackage in the main repo and give you permission to modify the repo to speed up your development process.

[jneira]

I think we close this issue and open a new one about test infra for plugins