Add OmniSharp ExternalAccess

[333fred]

We had a discussion some months ago about adding an ExternalAccess layer for OmniSharp, as they use a decent amount of private reflection for various services. I've gone through the O# codebase and looked for everything that is using reflection to work. This is the first time I've set up one of these layers, so please let me know what I need to change.

[CyrusNajmabadi]

Was this modeled after the f# ea impl? Thanks!

[333fred]

Was this modeled after the f# ea impl? Thanks!

Yes. I can't say for certain it was modelled correctly, but the intent was to mirror how F# does it. From what I could see though, F# mostly plugs into existing Roslyn infrastructure (which is then used by VS), whereas O# does a bit of that, and a bit of calling infrastructure itself, serving as the VS side here.

[CyrusNajmabadi]

I think a big part of the f# layer here (if I understand it correctly) is that it's actually a separate dll for the ea layer. This differs from ts, where the ea layer is mixed into the main code for that layer.

[333fred]

I think a big part of the f# layer here (if I understand it correctly) is that it's actually a separate dll for the ea layer. This differs from ts, where the ea layer is mixed into the main code for that layer.

The intention with my change here is that we have a dedicated MS.CA.EA.OS dll.

[CyrusNajmabadi]

Great. Thanks for the clarification. I can review tomorrow once @sharwell Looks. Thanks!

[333fred]

@sharwell addressed feedback. I also created a separate .CSharp dll for OmniSharp, as they also have a layering system like we do where their .Roslyn project only references the common base, and .Roslyn.CSharp is where they add in the .CSharp roslyn dlls.

[333fred]

I rebased this to squash the fixup commits. The two commits at the end, adding a unit test project to assert we're keeping enums in sync, are new.

[333fred]

I think the main thing this is waiting for is a strong-name key for omnisharp. Their assemblies are unsigned today, which doesn't work with IVT as roslyn is signed. I'll work on getting them signed in the next couple of days and add the public key info here when I do.

[333fred]

OmniSharp/omnisharp-roslyn#2143 is now merged, and this PR is updated to add OmniSharp's signing key directly.

[333fred]

@jasonmalinowski @JoeRobich you had a couple of questions I requested more info on: is there anything I need to do? Or is this ready for merge?

[jasonmalinowski]

So generally OK with this but want two things:

1. Would like a signoff from @JoeRobich or @dibarbet that the NuGet upload handling is correct, or at least as correct as this PR can have it if it's still under investigation.
2. We get something written up in the docs folder explaining what the ownership process is for this.

For that second item specifically, what the breaking change policy is. For some of our existing external access projects, we can't make any breaking change until we've checked with the team to make sure it's not being used, or we do a complicated dance; the IDE team is well aware of that process since by now I think everybody has personally failed to do it at least once and learned the hard way. Since Omnisharp still chooses to upgrade to newer Roslyn versions (right?) I presume the process here is something more where if we're making a change we'd want to notify them (and we should clarify who "them" is and what communication medium is chosen) so at least they have a heads up that it's coming so they can adapt on next upgrade.

And the docs here can be a single sentence of "ping on GitHub, wait until somebody gives you a thumbs up, and you're good". Just want something written down here.

[jasonmalinowski]

How was Omnisharp using this? We have a PR that will ultimately remove this (#47579) and mostly curious.

[333fred]

https://github.com/OmniSharp/omnisharp-roslyn/blob/ff21ac475c/src/OmniSharp.Roslyn/WorkspaceServices/OmniSharpSymbolRenamedCodeActionOperationFactoryWorkspaceService.cs

[JoeRobich]

Specify access modifier?

[333fred]

@jasonmalinowski added a doc.

[JoeRobich]

Specify access modifier?

[jasonmalinowski]

@333fred looks great, thanks!