dundalek / lazy-lsp.nvim

Neovim plugin to auto install LSP servers
MIT License
203 stars 17 forks source link

Flakes support #33

Open dundalek opened 8 months ago

dundalek commented 8 months ago

This is an issue to collect feedback and notes about Flakes. We should consider using them in the future, but there might be a few things that need to be figured out.

Motivation:

Obstacles:

Questions:

I've seen a proof-of-concept in a fork. Is this a good solution we could build upon? https://github.com/kamalmarhubi/lazy-lsp.nvim/commit/ebb15438fa1736e53f82ac45b4844968f7aa4f7b

jkmdn-dev commented 8 months ago

I love this this plugin, I think you have done a great job! I see lots of potential, and I'd love to contribute.

My initial thoughts is that this should be a step-by-step process, and maybe using flakes for everything might be too ambitious to start with?

I think a good approach would be to start with converting this plugin to a Nix derivation and a flake. I have a feeling that this plugin will be used by in the nix-ecosystem.

And by converting, I really mean to just add it, everyone could still develop in the same manner as before, and the ppl that like to use the flake while developing can so that.

I also read somewhere that you'd like to add tests, that would be a suitable problem to solve with nix and experiment with how to use flakes with the LSPs.

An easy win would be to add the option to use a local nix-shell or a flake. This would probably be a pretty easy thing to implement and be completely backwards compatible. Just have an option like prefer_local_{shell, flake} and have it be false by default. The real issue would then be to find the root of the workspace to look for shell.nix or flake.nix.

The really big win of this is the ability to easily define a flake that determines the LSP that should be used for the current project. I have noticed that lazy-lsp together with zero-lsp loads many LSPs, and sometimes they interact poorly. This would make it easy to use only one LSP without having to change the Neovim plugin settings.

I feel like this would be such a great improvement.

What are your thoughts? @dundalek

dundalek commented 8 months ago

I love this this plugin, I think you have done a great job! I see lots of potential, and I'd love to contribute.

Very cool!

My initial thoughts is that this should be a step-by-step process, and maybe using flakes for everything might be too ambitious to start with?

The philosophy of the plugin is to make things work as much out-of-the-box as possible with fewest options as possible. So I always try to start with a problem statement, then coming up with possible solutions and benefits.

What I had in mind was just using flakes shell instead of nix-shell in case there are upsides for it. But I am open to other ideas, as I said I am not that familiar with flakes.

I think a good approach would be to start with converting this plugin to a Nix derivation and a flake. I have a feeling that this plugin will be used by in the nix-ecosystem.

And by converting, I really mean to just add it, everyone could still develop in the same manner as before, and the ppl that like to use the flake while developing can so that.

I am not sure I follow how it would work as a flake. Also the plugin is already in nixpkgs for people who prefer to install plugins with nix. Maybe we are confusing two things together here?

  1. loading lsp servers via flakes - I could see some benefits, but also potential downsides
  2. installing this plugin via flakes - I am not sure I see benefits, please expand

Or is there something else entirely?

An easy win would be to add the option to use a local nix-shell or a flake. This would probably be a pretty easy thing to implement and be completely backwards compatible. Just have an option like prefer_local_{shell, flake} and have it be false by default. The real issue would then be to find the root of the workspace to look for shell.nix or flake.nix.

I think I would prefer not to expose this and try to have things work out-of-the-box as possible without additional options. If we find good reasons run flakes shell over nix-shell, I would just rather make that decision and switched internally.

The really big win of this is the ability to easily define a flake that determines the LSP that should be used for the current project. I have noticed that lazy-lsp together with zero-lsp loads many LSPs, and sometimes they interact poorly. This would make it easy to use only one LSP without having to change the Neovim plugin settings.

You can have a look at the excluded_servers or preferred_servers options to control which lsps are loaded which might already improve the situation for you.

I think the biggest value of the plugin is that it works for projects written in various languages that don't use nix. If a project already uses shell.nix/flake.nix then one just start neovim in that shell so a plugin wouldn't add much?

jkmdn-dev commented 8 months ago

Yeah, you are right. I just get so riled up :)

Maybe there is a way of using flakes, as you mentioned, they might provide a performance boost, but I would probably require quite a substantial effort. Might need to write a flake for each LSP that doesn't already have one. They are probably not hard to write, but it's a lot of work anyway. Maybe one could write a script that takes the list of supported LSPs and search nixpkgs, and if they exist, generate some code to wrap it in a flake?

Btw, does all the LSPs spawn each own nix-shell sub-process? I'm not very familiar with Lua threading model. My guess would be that this is the case, otherwise one could end up with nested nix-shell, which might be no issues anyway. Or maybe all the LSPs applicable load together in a shared nix-shell? I'm guessing that each LSP is started in a way to pipe stdout, stdin, stderr in an easy way? Or are some other IPC-communication used? My thinking was that with flakes, or even with shell.nix files, one would have more control of IPC, but using anything other than stdout, etc, is probably just more work than benefit anyway.

One way to deal with the experimental issue is to also provide shell.nix for alll the LSPs as a fallback. One might be able to do this in a smart way such that the flake and the shell.nix are similar enough to be generated almost the same way.

After some thought, this might be to much work for little pay of? There are other ways of speeding up the startup of nix-shells. I think the biggest benefit of flakes would be to allow users to somehow provide their own, or override parts of them, stuff that nix does really well. And, as you mentioned, adding LSPs that's a part of nixpkgs yet. However, this might also be to much work. The LSPs already available probably covers most use cases, and without the possibility to easily provide their own flake it would fall on the maintainers to create this flake.

Maybe a companion plugin is a better idea? Something that's meant to be used with lazy-lsp but adds stuff like the ability to use locally provided flakes? Then there might be an internal ifdef that makes sure to use the sub-proc like they are now, and are only switched out with a forward declared method, something like that. I don't know.

Edit: Spelling, sorry about that, Im way to tired to be thinking about this :)

dundalek commented 8 months ago

My understanding is that nixpkgs can be used from flakes to have it as a base and then have individual flakes as additions.

That's right, each lsp server is spawned as a separate process and usually communicates via stdio, and we wrap each in its own nix-shell that pipes it through. From the resources used perspective, it does not matter if processes run in a single shell or multiple shells. If there are shared executables or libraries then OS shares them in memory.

The initial nix-shell start up that downloads the packages takes time. But repeated starts are negligible, the lsp servers usually take more time themselves to catch and re-index files. But of course, if there is a way to reduce startup for "free" I am open to it.

For the per-project configuration, I don't know, I haven't encountered the need in practice yet. One needs to control versions of compilers to produce the right output, but for tooling like lsp picking the latest one seems to be good enough.

jkmdn-dev commented 8 months ago

Yeah, I agree, it's probably very niche to require an per-project-lsp setup.

I'm an expert at finding useless things to solve with unnecessarily complicated solutions. Just see my repo where I build Neovim from scratch using Nix derivations. I couldn't get my Lua configs to work since I used some stuff from nightly, so instead of learning how to use flake overlays I converted everything to a Nix derivation. The funny thing is that I needed to learn overlays to use my work anyway! 🤣

The per-project-lsp is probably just one of my stupid ideas that doesn't really solve anything, but I'm probably going to end up doing it anyway, and discover why it's useless the hard way.

I'm dreaming of a plug-in that basically highjack Neovim's IO to make everything Neovim does run through a nix-shell. I think neovim uses a Lua library for this, so this can probably not be a plugin, since Neovim statically links it's dependencies. The only way is probably to patch the library before it's linked.

Anyway, here I go again!

Think I'm out of ideas on this subject, I'll let you know if I find something that might interest this subject, and maybe I'll submit a PR in the future.

As a side note, what's your thoughts on using fennel instead of pure Lua?

clamydo commented 8 months ago

For a project that is a flake itself and manages its dependencies and dev-shells, wouldn't it make sense to call nix shell --inputs-from=. nixpkgs#lsp_foo -c lsp_foo_whatever instead?

Then I had control, what version of an lsp would actually be called. This is especially important, if a lsp is tight to a specific compiler version used in the project, like ghc for example.

dundalek commented 8 months ago

For a project that is a flake itself and manages its dependencies and dev-shells, wouldn't it make sense to call nix shell --inputs-from=. nixpkgs#lsp_foo -c lsp_foo_whatever instead?

Then I had control, what version of an lsp would actually be called. This is especially important, if a lsp is tight to a specific compiler version used in the project, like ghc for example.

It does make sense. Would you help elaborate on details how this could be implemented?

As a starter there are few questions in order to make sure the existing workflow would not break:

clamydo commented 8 months ago

You raise valid points.

Is there a single command that will work if flake dependencies are defined and fallback to default channel otherwise?

  • If not and we would need two different commands, how do we detect that?
  • Is checking for presence of flake.nix in the lsp root sufficient?

Not that I know of. However, it is still true, that there are projects using flake and those without. Can you elaborate, what an 'lsp root' is? I guess nix shell walks the directory up and looks for a flake.nix? If inside a git repo, it requires the flake.nix to be tracked by git. That might add some comlexity 🤔

Would it work on default Nix installation, e.g. when users have not enabled flakes explicitly?

I was rather thinking of an explicit option to enable this, because nix flakes are experimental themselves. So, somebody that has not enabled flakes, but enables the suggested option in lazy-lsp is doing it wrong :).

In the flake dependencies mode, would LSPs not specified in dependencies still work? For example jsonls might not be considered important to add to dependencies, but it would still be good to get LSP for JSON files if it is enabled globally.

I was assuming, at least for a first draft, lazy-lsp would work exactly the same except for using the flake-enabled commands. That is, you wouldn't need to specify the lsp in the dependencies.

I believe that is also what we want as we cannot assume that every developer uses nvim.

dundalek commented 8 months ago

Is there a single command that will work if flake dependencies are defined and fallback to default channel otherwise?

  • If not and we would need two different commands, how do we detect that?
  • Is checking for presence of flake.nix in the lsp root sufficient?

Not that I know of. However, it is still true, that there are projects using flake and those without. Can you elaborate, what an 'lsp root' is? I guess nix shell walks the directory up and looks for a flake.nix? If inside a git repo, it requires the flake.nix to be tracked by git. That might add some comlexity 🤔

Root dir is to determine in which directory to start the server, see :h lspconfig-root-dir. It starts at the current file path and walks the dir tree up. It depends on the sever, but usually language configs like package.json or Cargo.toml have precedence, then it looks for .git.

I can imagine there could be a situation when the repo contains multiple modules, so the lsp root might not match git root. But it should probably just work to walk up to the nearest flake.nix anyway.

I was rather thinking of an explicit option to enable this, because nix flakes are experimental themselves. So, somebody that has not enabled flakes, but enables the suggested option in lazy-lsp is doing it wrong :).

I like to avoid options as much as possible and just try to make it work the right way :) I also want to be mindful to folks who were just barely able to install Nix. Things should not blow up in their face if they open a repo with a flake in it :).

I was assuming, at least for a first draft, lazy-lsp would work exactly the same except for using the flake-enabled commands. That is, you wouldn't need to specify the lsp in the dependencies.

Yeah, it would be ideal if it worked like that.

cognivore commented 1 month ago

I think that a lot of developers use direnv with a flake.

Some languages have very dirty toolchains. For example, elixir requires provisioning of third-party tools into directories specified by MIX_DIR and HEX_DIR environment variables.

In this case, pure shell won't cut it, whereas detecting a flake and dropping there would.

Obvious workaround is to tell lazy-lsp to prefer local language servers (which is already possible) and add elixirls to flake at home, but it's not lazy enough!

But I want to say that when lazy-lsp works, it really works. I love zero-configuration tools and I love the genius usage of nix toolchain you have discovered! Great plugin, great job.

dundalek commented 1 month ago

In this case, pure shell won't cut it, whereas detecting a flake and dropping there would.

@cognivore My current opinion is that looking up the flake and using it would make sense by default. If we discover some issues in the practice we can iterate on it or as a last resort introduce an opt-out option.

The main obstacle is that flakes are not declared as stable and I want the plugin to also work for people with default installation that do not enable them. Maybe there is a way to do some feature detection and fallback to old mechanism, but I am not that compelled to dig into that.

I am thinking to wait it out, but am open to contribution if anyone wants to do the exploration.

But I want to say that when lazy-lsp works, it really works. I love zero-configuration tools and I love the genius usage of nix toolchain you have discovered! Great plugin, great job.

Thanks, I agree and appreciate that :smiley: