kabouzeid / nvim-lspinstall

Provides the missing :LspInstall for nvim-lspconfig
MIT License
526 stars 67 forks source link

Document how to run omnisharp server with system mono rather than bundled mono #127

Open insidewhy opened 2 years ago

insidewhy commented 2 years ago

I'm using Unity which needs to use .NET Framework. On archlinux this is available to the system installed mono, but not the mono that is installed and used by the omnisharp plugin.

After installing this plugin I've been doing this:

cd ~/.local/share/nvim/lspinstall/csharp/omnisharp/bin
mv mono mono.backup
ln -s /usr/bin/mono .

After this everything works beautifully, but I'm sure there's a better way to do this via my neovim configuration without having to hack the installation. I think it's something a lot of people would probably find useful so it might be good to add it to the README.

kabouzeid commented 2 years ago

I don't know anything about csharp, so I don't know what the best defaults are here. Cleanest would be to just add your own custom installer that does what you need to your local config. You can either overwrite the csharp installer or give it a new name like csharp-arch.

See: https://github.com/kabouzeid/nvim-lspinstall#custom-installer

kabouzeid commented 2 years ago

In this case you could also only update the install script, if the other properties like cmd etc stay the same.

require'lspinstall/servers'.csharp.install_script = [[
  your bash install script copied and adjusted from lua/lspinstall/servers/csharp.lua here
]]
ELLIOTTCABLE commented 2 years ago

After much investigation, it's become pretty clear to me that nvim-lspinstall is actually the correct place to fix this problem (well, at least, in the sense that all the other providers have shifted the blame):

  1. Farthest-upstream, OmniSharp explicitly does not handle this, by design. Excerpt:

    However, I don't think this is an issue for OmniSharp itself as it already provides the necessary tools for achieving this - it's just the client must make the decisions, therefore I would recommend to move this issue to the client implementation(s).

  2. Directly above this, nvim-lspconfig talks about not setting the path ...

    By default, omnisharp-roslyn doesn't have a cmd set. This is because nvim-lspconfig does not make assumptions about your path.

    ... although this is actually talking about the path to the binary, not the path to the project.

Based on the above, the "correct" way to handle the strange requirement of omnisharp-osx (a requirement specific to that release, by the way), which is downloaded by nvim-lspinstall, seems to be … here in nvim-lspinstall. (At least, that's my takeaway from the above!)

Now, the advice from upstream on how to handle this is fairly straightforward, as the maintainer explains here:

Launching on global/local Mono should be controlled by the client integrating OmniSharp, e.g. that is what C# Extension for VS Code does. The run file is an integral part of the embedded Mono experience. In other words, the client should call mono OmniSharp.exe if it wants to use the global Mono or run if it wants to use embedded Mono.

In VScode (for Googling purposes — hi, other people who've spent literally forty-some hours Googling "vim omnisharp mono" and various other conversations! Hopefully I've helped you and saved you some of that time, here!), this feature is called omnisharp.useGlobalMono — the naming is somewhat misleading; this is not OmniSharp itself being configured, not in any way; it's only configuring what omnisharp-vscode does in regards to invoking Mono. (In the Vim ecosystem, nvim-lspinstall is currently playing the part played over there by omnisharp-vscode.)


Okay, hopefully I've convinced you to un-Close this Issue, with eight bajillion links from my research. Moving forward, here's what I suggest: we implement exactly what omnisharp-vscode does, here:

https://github.com/kabouzeid/nvim-lspinstall/blob/22975634102e30bcea72f9508dff9190900bcecf/lua/lspinstall/servers/csharp.lua#L1-L4

Instead of hard-coding in "./omnisharp/run", we should introduce a config-setting that is read in — preferably with the exact same name as the VScode setting, for user-friendliness and Googleability, something like g:omnisharp_useGlobalMono perhaps? — and then determines whether to use that run script shipped with OmniSharp, or instead use "mono", "./omnisharp/omnisharp/OmniSharp.exe".

The only reason I haven't already submitted this as a pull-request, is frankly that I'm not very familiar with the new NeoVim-lua-practices: is using a g: variable for configuration still a thing we do? Should we set that up somewhere else? I'm unsure.

kabouzeid commented 2 years ago

Is this going to be a problem: https://github.com/OmniSharp/omnisharp-vscode#note-about-using-net-5-sdks ?

They explicitly say to set useGlobalMono to never.

ELLIOTTCABLE commented 2 years ago

Possibly; but note that a lot of folks aren't actually using the latest tooling like that — and in fact, cannot. (I'm over here stuck on net472 for Unity, so, yikes!)

Er, elaboration: the set of people that notice affects has no intersection with the set of people who would want/need this fixed — Mono is pretty explicitly a dead product, at this point; it's mostly kept around to support certain older workflows; and the versions on which the errors relating to useGlobalMono occur are ".NET Framework", up to v4.7.2; released in 2018. Meanwhile, that notice is if you're using the very latest version, ".NET Core" v3.1, released this year in 2021 (yes the naming is a clusterfuck, don't worry, it's not just you.) CoreUnity's one of those.

A big part of what repeatedly fails-to-work across the ecosystem, and what useGlobalMono tries to address in VScode, is that OmniSharp doesn't ship with a Mono that includes references to some ancient versions of .NET — they explicitly opted to remove them. This includes the ancient-version-of-.NET that Unity still, in 2021, demands! So, a lot of Unity developers trying to do, well, anything at all, with C#, end up seeing a lot of errors relating to, say, System.Numerics.dll or whatever, as those have been stripped out of the version of Mono that their tooling (OmniSharp, though they don't know that) is using.

Thus, they're directed to install a full copy of Mono on their system, themselves (that then contains the stripped assemblies); and told to use omnisharp.useGlobalMono.

Hope that was all helpful; I've been learning this all myself over the last couple of days; so here's hoping you can avoid wasting your own time on it. 🤣

insidewhy commented 2 years ago

@ELLIOTTCABLE wow that's interesting.. I'm using this plugin because of Unity also and you're making me regret my choice of Unity even more (that and all the super basic bugs in Unity's AI that have been open for years).