-
Notifications
You must be signed in to change notification settings - Fork 416
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
Move OmniSharp to target net46 only #915
Conversation
@DustinCampbell will you not support .NET Core in the future? Is it Full Framework or nothing? |
@mholo65:
There two aspects here:
This PR is only about the first point above. OmniSharp will no longer provide stand-alone builds that run on .NET Core. Primarily, this is because those builds are limited in what sorts of projects they can process. (i.e. MSBuild tasks targeting Full Framework will likely not run on CoreCLR). For OSX/Linux, we're going to be bundling a local Mono runtime so that OmniSharp will still run as a standalone application, without requiring Mono to be installed on the system (see #666 for details). The second point above is not changing at all. OmniSharp will continue to support processing both .NET Core and Full Framework projects. Does that answer your question? |
Note that this will just be formalizing what we already do in C# for VS Code to run OmniSharp. |
@DustinCampbell yes! Thanks, this is good news for us in Cake team, now we only need to target full framework for the Cake-plugin! |
812aec7
to
6ebf09f
Compare
OK. Builds are passing. Tests are still somewhat problematic but they're working at the moment. Next up: packaging. |
@david-driscoll & @filipw: I added you two in case you want to start looking at this or would like to try the branch out. |
Note: This change does not move our libraries over to netstandard2.0. I think that's possibly the next thing, but it's not needed yet. |
2683270
to
250cc24
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm all for it!
I'm very much eager to try this too.
For example it will help tackle some edge cases in scripting due to how we need to add references to runtime assemblies rather than (as you'd normally expect) to compile assemblies, which can be problematic when running OmniSharp on .NET Core.
250cc24
to
db8bdbf
Compare
OK. I'm starting over a bit. I've rebased onto master with the new updates to the build. |
ba59590
to
b8e28ed
Compare
3d1af90
to
fde0984
Compare
@DustinCampbell any plans on updating target framework for |
No plans yet. Keeping the target framework it at 4.6 means that it will work on more Windows versions without the user needing to do anything. |
@DustinCampbell does this mean we won't be able to debug omnisharp on macOS/Linux (via vscode)? |
I would expect the Mono Debugger extension to work, though I've never tried it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Do we have a plan to handle new mono versions, some repo or script we can setup to manage that?
Not yet. We're just using the bits that I regularly cobble together for C# for VS Code. I have a script that I use to copy the bits out of an installation of Mono on OSX. However, for Linux x86/x64, I have VMs where I sudo apt-get the latest mono and copy the runtimes out. |
Are you guys OK if I merge this? It's a big change, so I want to be sure. |
absolutely, I can't wait to get rid of .NET Core 😃🏆🍻 |
OK. Here it goes. 😄 |
Fixes #666
Tagging @david-driscoll and @filipw specifically for a review since this is a significant change. It'd be great if anyone looking at this could build this branch and try it out.
This PR moves OmniSharp to target .NET Framework 4.6 exclusively and removes all of the netcoreapp1.1 builds. With this change, there are now only six flavors of OmniSharp that will be released:
--assembly-loader=strict
flag must be specified when launch this build with Mono).Important note This change does not affect the project types that OmniSharp supports. OmnISharp still supports .NET Core and .NET Framework, and Xamarin, Unity and other Mono-based projects.
For discussion: I chose to not include
libuv.dll
in the *Nix builds, but left them in the Windows builds. Allow me to explain my reasoning:brew install libuv
orsudo apt-get install libuv1
). It is not as easy to acquire on Windows however.I'm open to discussion on this decision.