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

Upgrade OmniSharp #1656

Open
RA-Kooi opened this issue Aug 30, 2022 · 10 comments
Open

Upgrade OmniSharp #1656

RA-Kooi opened this issue Aug 30, 2022 · 10 comments

Comments

@RA-Kooi
Copy link

RA-Kooi commented Aug 30, 2022

Please upgrade OmniSharp to the latest version for .NET6 support.

I have tried simply upgrading to the latest OmniSharp, but that ended up giving me a whole lot of exceptions whenever I actually tried to write code. I think some of the API changed in how you communicate with the server, I haven't really bothered to investigate that deep.
Looking at some old issues it seems that the LSP implementation of OmniSharp was rather shoddy, but perhaps that could be revisited as manually adding the latest STDIO version using g:ycm_language_server seems to work at a cursory glance.

There is also a dotnet core version of it now, which may perhaps be a better choice for those who want to use the latest .NET versions and experiment with preview releases.

@puremourning
Copy link
Member

We can't. #1631

Omnisharp is broken and they have no interest in fixing it as far as we can tell.

@puremourning
Copy link
Member

But basically, unless you or another community member send a PR that updates this and all the tests pass, I feel like we're stuck. I don't use c# so I have no impetus to do it. @mispencer has in the past contributed a lot, and we're very thankful for their contributions.

If using the LSP interface works then great you should be able to do that. Last time we checked it lacked features vs the API we currently use. But I haven't bothered looking at it either, nor do I plan to.

So in summary:

  • upstream is broken, for well over a year
  • alternatives require a lot of work
  • there's little chance I am likely to have any time or impetus for that
  • strong well tested PR welcome.

@mispencer
Copy link
Contributor

I created a fork of Omnisharp that supports .NET 6 earlier that I personally use, but it is a ugly hack.

However, it doesn't appear that the Omnisharp maintainers will fix this issue. I will attempt to fix it properly myself when I have time.

@RA-Kooi
Copy link
Author

RA-Kooi commented Aug 30, 2022

If using the LSP interface works then great you should be able to do that. Last time we checked it lacked features vs the API we currently use. But I haven't bothered looking at it either, nor do I plan to.

If you remember, what features did it lack?
I might be able to do a quick and dirty check to see if I can get it to work using the LSP interface.

@mispencer
Copy link
Contributor

mispencer commented Sep 4, 2022

I created a PR on the Omnisharp repo to resolve the blocking issue: OmniSharp/omnisharp-roslyn#2446

@RA-Kooi
Copy link
Author

RA-Kooi commented Sep 4, 2022

Is this supposed to fix the issue entirely? I pulled your omnisharp version and built it, however I still get exceptions thrown when trying to use it in a C# project.

@mispencer
Copy link
Contributor

It works in my testing. Are you using the new executable in ymcd?

Steps to using this Omnisharp fork:

  1. Clone my fork somewhere
  2. Build it: dotnet build .\src\OmniSharp.Http.Driver\OmniSharp.Http.Driver.csproj (adjust as needed for your OS)
  3. Go to your ycmd folder and open ycmd\completers\cs\cs_completer.py
  4. Comment out all lines of PATH_TO_ROSLYN_OMNISHARP
  5. Add a new line below (adjust as needed for your OS):
    PATH_TO_ROSLYN_OMNISHARP = '__ASBOLUTE_PATH_TO_OMNISHARP_FOLDER__\\bin\\Debug\\OmniSharp.Http.Driver\\net6.0\\'

@RA-Kooi
Copy link
Author

RA-Kooi commented Sep 4, 2022

What I did was the following:

  1. Cloned your fork
  2. Built it using ./build.sh --target Quick --configuration Release
  3. Went to ~/.vim/bundle/YouCompleteMe/third_party/ycmd
  4. Applied the following patch:
diff --git a/ycmd/completers/cs/cs_completer.py b/ycmd/completers/cs/cs_completer.py
index bb754498..b1a1db21 100644
--- a/ycmd/completers/cs/cs_completer.py
+++ b/ycmd/completers/cs/cs_completer.py
@@ -349,7 +349,7 @@ class CsharpCompleter( Completer ):
       omnisharp_server = responses.DebugInfoServer(
         name = 'OmniSharp',
         handle = completer._omnisharp_phandle,
-        executable = PATH_TO_ROSLYN_OMNISHARP,
+        executable = self._roslyn_path,
         address = 'localhost',
         port = completer._omnisharp_port,
         logfiles = [ completer._filename_stdout, completer._filename_stderr ],
@@ -431,7 +431,7 @@ class CsharpSolutionCompleter( object ):

     self._ChooseOmnisharpPort()

-    command = [ PATH_TO_OMNISHARP_ROSLYN_BINARY,
+    command = [ self._roslyn_path,
                 '-p',
                 str( self._omnisharp_port ),
                 '-s',
  1. Set the ycm_roslyn_binary_path to let g:ycm_roslyn_binary_path= expand(baseRuntimePath . '/../Projects/C#/omnisharp-roslyn/artifacts/publish/OmniSharp.Http.Driver/mono/OmniSharp.exe')

I will try using dotnet build directly, as this is a mono build which I think might be the culprit here, and report back.

@RA-Kooi
Copy link
Author

RA-Kooi commented Sep 4, 2022

Building a dotnet version from your fork makes it load, but fails with a different exception:

System.IO.FileLoadException: Could not load file or assembly 'System.Configuration.ConfigurationManager, Version=6.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51'. Could not find or load a specific file. (0x80131621)
File name: 'System.Configuration.ConfigurationManager, Version=6.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51'

If it matters, I changed the .NET version to 6.0.400 as that's what I have installed.

Edit: This can be reproduced on my machine simply by creating a console project and adding it to a new sln. Opening Program.cs and writing Console. gives no completion options.

@RA-Kooi
Copy link
Author

RA-Kooi commented Sep 4, 2022

Ok, deleting System.Configuration.ConfigurationManager.dll as per OmniSharp/omnisharp-roslyn@0f2ddb7 seems to make it work. That will not be an issue for if/when this gets merged into upstream.

If they won't merge it, I think we should look into using the LSP frontend of OmniSharp.

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

No branches or pull requests

3 participants