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

Removed net462 from test project #65

Merged
merged 1 commit into from
Dec 28, 2018
Merged

Removed net462 from test project #65

merged 1 commit into from
Dec 28, 2018

Conversation

abergs
Copy link
Collaborator

@abergs abergs commented Dec 22, 2018

Running ```dotnet test`` generates a bunch of errors if net462 is targeted.

@MeikTranel
Copy link
Contributor

What kind of errors?

@abergs
Copy link
Collaborator Author

abergs commented Dec 22, 2018

System.TypeLoadException : Could not load type 'System.Security.Cryptography.ECPoint' from assembly 'netstandard, Version=2.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51'.
Stack Trace:

@MeikTranel
Copy link
Contributor

That's the error i told you about here: MeikTranel/MigrationSupport#1 (comment)

The issue is that .NET Standard 2.0 has APIs that .NET v4.6.2 doesn't have, but the .NET team still marked 4.6.2 as .NET Standard 2.0 compatible. One of the 43 APIs missing is the one you have here (https://github.com/dotnet/standard/blob/master/docs/comparisons/netstandard2.0_vs_net461/System.Security.Cryptography.md); which is why your test fails on .NET462. Because you produce .NET Standard packages this means that there can be consumers of your package using .NET 4.6.2, that will eventually hit this bug. That's why i added 4.6.2 in the first place.

@MeikTranel
Copy link
Contributor

I just checked. There's a nuget package that can help you avoid this scenario.

Copy link
Contributor

@MeikTranel MeikTranel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's try to avoid this. I will test things locally to fix this problem differently.

@abergs
Copy link
Collaborator Author

abergs commented Dec 22, 2018

Hmm. I see. Well. We either take on a dependency (maybe Source Include this as well?) or add a disclaimer that net.462 is not supported due to this issue (not very nice)

@MeikTranel
Copy link
Contributor

Well the dependency would only exist for the net46X depedency group, not for the main netstandard2.0 dependencygroup.

@abergs
Copy link
Collaborator Author

abergs commented Dec 22, 2018

I believe the failing test is due to #32

@aseigler
Copy link
Collaborator

I will be fixing #32 soon. Just been considering the best way to do it.

@aseigler
Copy link
Collaborator

This should be clean now.

@abergs
Copy link
Collaborator Author

abergs commented Dec 23, 2018

@MeikTranel Do you think we should avoid removing net462?

@aseigler It does passes in the CI now. Good work.

@MeikTranel
Copy link
Contributor

Sorry, christmas chores came first 😄
We should at least try to make it sane. I was thinking we put a net461/net462 depedency group in there which has some msbuild targets erroring when you're trying to build it with net461 or net462.
Because the additional nuget dependency might not be such an easy task.

@MeikTranel
Copy link
Contributor

I just cooked something together locally. This appends a task to BeforeBuild that checks if the current targetframework is one of the offending frameworks and puts a big ol' error out.

We can put a help url and better text in there of course.

pcxp90kjez

@abergs
Copy link
Collaborator Author

abergs commented Dec 25, 2018

@MeikTranel Looks really good! Send a PR? :) And good idea to link to an issue at github (or a net462.md file) in the repo.

@MeikTranel
Copy link
Contributor

Will do but tomorrow tho. Already back into Christmas Dinner Madness V2 😂

@abergs abergs merged commit e613950 into master Dec 28, 2018
@abergs abergs deleted the no462 branch March 24, 2020 16:01
@abergs abergs added the enhancement Enhancements or general improvements label May 28, 2020
@abergs abergs mentioned this pull request May 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancements or general improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants