NET 4.0 and .NET 4.5. versions need to be signed with different keys

[forki]

As discussed in fsprojects/Paket.VisualStudio#38 (comment) we have a very weird issue with using Rx in a Visual Studio extension. It seems the Xamarin addin uses also Rx and this breaks Paket.VisualStudio and AFAIK also "GitHub for Visual Studio 2015" (which ships with VS).

Quoting @shana from the other issue:

  Microsoft is shipping two separate builds of system.reactive, one for .NET 4.0 and another for .NET 4.5. Both with the same signing key snd version. As a result, the runtime doesn't distinguish them (runtime version is not a part of the assembly identification) and will only load one of them, whichever comes first. If the first is the one for .net 4.0, any 4.5 methods will not be a part of it.

  Result: MethodNotFoundException

  The only way to fix this is to either load your extension before they do (if you can, it's a race), or ship your own copy of system.reactive with a different signing key so it can be loaded side-by-side with the one Xamarin is loading, which is what I'm doing. I have a fork at https://github.com/shana with a different key, you can also use the ones that ship with the GitHub Extension for VS2015.

So @shana has already "fixed it" for "GitHub for Visual Studio" and uses a custom build from https://github.com/shana/Rx.NET/tree/custom-key

/cc @shana please upload your nuget package to your releases page so that we can reuse it as a temporary fix.

[clairernovotny]

For things like VS-addins using shared components, it seems like you'll always be subject to this issue if you use a common library. If you can't load into a separate appdomain (and the VS model may not allow it), you may be better-off using ILMerge to internalize your references. That way you'll always get the dependencies you compiled and tested against. That approach would shield you against more than just Rx version differences.

[shana]

Ooops, looks like I missed this discussion 😞

It's probably not relevant that I comment on this now, but for future reference:

@forki The nuget packages are at https://github.com/github/VisualStudio/tree/master/lib.

@onovotny Two DLLs with the same name, same version and same key mean that they are ABI-compatible. Having breaking ABI changes without changing anything in the identification of the library is a critical compatibility-breaking bug. ILMerge or custom signing are workarounds, consequences to the problem, not the actual solution to the underlying problem.

[clairernovotny]

@shana That's not necessarily true in both directions or "bait and switch" packages won't work right. What is true is that the newer/wider platforms should include the surface area of the older/smaller platform. It's impossible to say that a .NET 4.5 version cannot add a new method that older platforms do not support.

This is really a case where the VS add-in system is stepping on itself w.r.t versioning.

[shana]

@onovotny There is a large difference between a profile and a DLL version. The DLL version (it's name, version and signing key) represents a set of APIs. Shipping two copies of the same DLL with the same exact version but different APIs is an ABI breaking change, period. If you have a public API and you want to rewrite how it does things because a newer .NET profile supports newer stuff, you can do that just fine, as long as the public API doesn't change. If the public API changes, a version change is required.

This is not a case of the VS addin model going wrong. The .NET runtime will not distinguish between two DLLs which have the same name, version and signing key, so you have to guarantee that a DLL with identity X has API x.

[kzu]

I disagree with all that's been said ;-)

There is a trivial way to solve this in the VS extension ecosystem: ProvideBindingRedirectionAttribute. It's even documented on MSDN: https://msdn.microsoft.com/en-us/library/microsoft.visualstudio.shell.providebindingredirectionattribute.aspx

This is not something that Rx needs to solve. Extension authors must. We've done exactly this thing to solve the Rx issue for Xamarin.

So I'd close this issue since there's nothing to do IMHO.

[shana]

@kzu That attribute is only useful if your extension is requesting an older version of some assembly and the ecosystem is able to provide a new one. In the Xamarin extension case, that's precisely how you broke us - you compiled against an older version of Rx.Net. Our extension is compiled against the latest one, there is no redirection we can request that can protect us from any random extension loading before us with an older Rx.Net.

The redirection attribute is for extension authors to provide easy upgrade paths for themselves, not to prevent other people from breaking them.

[OmerRaviv]

@kzu I think any discussion of how a Visual Studio extension might handle this does not speak to the root cause of the issue, which is that it will break any .NET application that allows 3rd parties to develop plugins for it. Visual Studio is just one example (albeit probably the best one – since several popular VS extensions were broken by it, and I’d probably be underestimating if I said that thousands of users were affected).

It seems to me that you’re suggesting a .NET extensibility model that’s broken by default. If this issue is closed, every single plugin author will slowly become aware of the problem through receiving reports from angry users, then either ILMerge or fork and compile their own private version of Rx with a different key, then test and deploy it. Surely we can do better than that, even if it requires some heavy-duty version bookkeeping.

[kzu]

This is the feature you need to fix it for real: #209 😉

[clairernovotny]

@OmerRaviv Issue #205 should address it as far as the generic case, wouldn't it?

[OmerRaviv]

@kzu That's a really cool solution technically, but it's really a workaround and does not address the root cause, and assumes we can expect every plugin author to do quite a bit of work just to implement a workaround to get un-broken (again, because the extensibility model is broken by default).

@onovotny I believe so! I was just making the argument for why #205 is really an absolute necessity IMHO.

[kzu]

Not disagreeing on the challenges for VS extension authors ;)

[clairernovotny]

Fixed with #212