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

Should Moq stop targeting .NET Standard in favor of .NET Core? #630

Closed
stakx opened this issue Jun 13, 2018 · 14 comments
Closed

Should Moq stop targeting .NET Standard in favor of .NET Core? #630

stakx opened this issue Jun 13, 2018 · 14 comments
Milestone

Comments

@stakx
Copy link
Contributor

stakx commented Jun 13, 2018

There's a discussion over at castleproject/Core#374 (triggered by https://github.com/dotnet/corefx/issues/29365#issuecomment-385102487) whether Castle.Core should stop targeting .NET Standard in favor of targeting .NET Core directly. Since Moq relies on Castle.Core, we'd have to do the same as long as we want to keep using the latest versions of DynamicProxy.

This discussion is a good one to have independently from Castle.Core also, since Moq too has a direct dependency on the unlisted(*) System.Reflection.Emit package (in order to be able to mock delegates). I am personally in favor of abandoning .NET Standard for the reasons given in castleproject/Core#374 (comment). (Also, we had some reports in the past asking why Moq doesn't work on some platforms, even though they supposedly implement .NET Standard. That confusion could have been avoided by not targeting .NET Standard in the first place.)

What are some significant disadvantages of targeting netcoreappX.Y instead of netstandard1.3, apart from possibly disgruntling our user base?

/cc @kzu


(*) The package has been relisted—citing from https://github.com/dotnet/corefx/issues/29365#issuecomment-398130364 (with emphasis added by me):

[We] have relisted the latest version of the existing packages:

https://www.nuget.org/packages/System.Reflection.Emit/4.3.0
https://www.nuget.org/packages/System.Reflection.Emit.Lightweight/4.3.0

However as mentioned earlier in this thread these packages claim to be netstandard1.1 compatible but that is a lie. They will only work on .NET Core and .NET Framework. So if you are consuming them from a netstandard library expect that library to fail if it is ran on any other .NET Standard implementation.

@michal-ciechan
Copy link

Just a thought

If any users have installed Moq, and NuGet decided to add references to the .NET Standard dlls, then they may have to re-install the package with -force.

I just checked our projects, and they reference the correct net40/net45 assemblies.

According to NuGet Docs - Matching assembly versions and the target framework in a project

When NuGet installs a package that has multiple assembly versions, it tries to match the framework name of the assembly with the target framework of the project.

Hopefully everyone is already referencing the correct net4x libs

I remember having an issue with EF, when they did some target changes which forced us to reinstall the NuGet package using -force to ensure correct version was referenced.

@stakx
Copy link
Contributor Author

stakx commented Jul 8, 2018

@michal-ciechan - I'm not sure I understand. You seem to be implying that no longer targeting .NET Standard would have the nice side effect of making it impossible for NuGet to sometimes pick inappropriate DLLs... is that what you were trying to say or am I misinterpreting you?

@stakx stakx added this to the 4.10.0 milestone Jul 14, 2018
@tomchavakis
Copy link

I guess that .NET Standard is more abstract/portable implementation and can be used in all .NET implementations rather than .NET Core Library which is more specific.

https://docs.microsoft.com/en-us/dotnet/standard/net-standard

@stakx
Copy link
Contributor Author

stakx commented Jul 19, 2018

@tomchavakis - please see the quote in the (*) footnote in the first post above, and the linked issue on dotnet/corefx. The thing is that System.Reflection.Emit (which Moq depends on) made it into .NET Standard by accident... it is only functional on .NET Framework & Mono & .NET Core so it would be more honest and potentially less misleading to target these platforms directly.

@kzu
Copy link
Member

kzu commented Jul 24, 2018

rather than targeting .netcore, I think the path forward is to multi-target .netcore AND net461.

@stakx
Copy link
Contributor Author

stakx commented Jul 24, 2018

@kzu, understood, I wasn't about to abandon the .NET Framework. Is net461 a safe target, though? It seemingly caused a lot of binding redirect problems when it was decided that .NET 4.6.1 should implement .NET Standard 2.0 retrospectively. Or would it be safer to target both (e.g.) net45 (what we have now) and net471?

@kzu
Copy link
Member

kzu commented Jul 24, 2018 via email

@stakx
Copy link
Contributor Author

stakx commented Jul 24, 2018

@kzu - No, .NET 4.5 is no longer supported. But regarding my question, that isn't actually relevant. Take net451 then (or any other .NET Framework version before 4.6.1).

What I am really worried about is that targeting net461 might be a bad choice, because when it was made to implement .NET Standard 2.0, it turned out that at least some System.* packages (e.g. System.Net.Http) were versioned incorrectly (think assembly versions). This wouldn't have been too noticeable if NuGet tooling hadn't appeared to prefer netstandardX.Y binaries over netXY ones, even when your own code targeted netXY. Meaning MSBuild would copy all those netstandardX.Y shim DLLs to the output directory, which in turn would cause assembly version binding conflicts between the framework's own assemblies and the .NET Standard shim DLLs, as your code might end up tryling to load both of these. (See #566, esp. #566 (comment).)

I have no idea whether this problem still persists or whether it has been fixed in the meantime, but I am not anxious to repeat this ordeal. Also, not everyone is running the latest version of everything (though with the way how .NET has "progressed" over the last couple of years, that has become more or less a requirement if one wants to stay sane. 😜)

Because of the above, I was wondering whether it might be safer to keep targeting an earlier version of the .NET Framework (e.g. net451) and net471, because that comes with real .NET Standard 2.0 support ("batteries included") where the shim DLLs are no longer necessary (IIRC).

@kzu
Copy link
Member

kzu commented Jul 26, 2018

Well, to make matters worse, on the next version of VS, I'm getting an error saying even net471 isn't safe to use with NS2 assemblies, and that I should target net472 instead :/. So, ugh... let's not kick the hornet's nest again until more is known for sure :)

@eloekset
Copy link

eloekset commented Aug 8, 2018

According to this Twitter thread, the documentation isn't updated, but all libraries should target netstandard2.0 in addition to lower standards (if supported) to minimize dependency hell for users of the library: https://twitter.com/davidfowl/status/1025434242832945154

If Moq had a target for .NET Standard 2.0, I probably wouldn't get this long update list by installing Moq into an XUnit test project targeting netcoreapp2.1:
image

@stakx
Copy link
Contributor Author

stakx commented Aug 8, 2018

@eloekset: Yes, targeting .NET Core or Standard 2.0 should result in much fewer dependencies.

Apart from that, see #630 (comment)

@stakx
Copy link
Contributor Author

stakx commented Dec 6, 2018

Based on these references:

I assume the way forward is to keep a .NET Standard target, after all. netstandard2.1 will include System.Reflection.Emit APIs (where we'll need to add checks for the relevant feature flags), and .NET Standard 1.x now has the APIs via the relisted NuGet package (even though it might not actually work on all .NET Standard 1.x-supporting platforms).

Given the information in the cross-platform targeting guide linked above, Moq should probably keep its current targets (net45 and netstandard1.3) and get two additional new ones: net461 and netstandard2.0.

@stakx
Copy link
Contributor Author

stakx commented Dec 6, 2018

Closing this, since it's pretty clear from the reactions to the posts above, as well as from the new information that's become available recently, that .NET Standard support in Moq is a wanted feature.

@stakx
Copy link
Contributor Author

stakx commented Mar 10, 2019

.NET Standard 2.0 is in, however I've decided to drop .NET Standard 1.3. There doesn't appear to be much point in keeping it:

  • Both .NET Core 1.0 and 1.1 will reach their End of Life in June.
  • .NET Core 2.x are covered by the new netstandard2.0 target.
  • .NET Framework 4.5+ (and Mono) are covered by the existing net45 target.
  • Moq 4 doesn't run on any other platform, due to its dependency on System.Reflection.Emit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants