kmaragon / Konscious.Security.Cryptography

MIT License
212 stars 20 forks source link

Nuget package brings in unnecessary dependencies when used with .NET Framework #28

Closed bradgentry closed 5 years ago

bradgentry commented 5 years ago

Starting with 1.0.6, the nuget package does not have .NET Framework dependencies specified, so when it is added to a .NET Framework project, it brings in 45 additional Microsoft packages. Since we don't want to do that, we cannot upgrade from 1.0.5. (Same issue exists in the Argon2 package)

kmaragon commented 5 years ago

I was just writing a whole thing before I realized that we're talking really old versions.

Ok - this was back when Microsoft ditched project.json and brought back csproj files. The migration obviously didn't quite migrate for .NET Framework. Unfortunately, I have no windows machine to test the non .NET Core case. But I can see in the nuget website what the dependencies looked like in 1.0.5 and then in 1.0.6. So I can try to replicate that in the csproj file at least (with newer versions for NETCore).

kmaragon commented 5 years ago

And in an impeccable moment of timing @alenas just opened a PR today to take care of this. The caveat is that the PR switched what was properly set up as .NET Framework 4.5.1 in the project.json days (<= 1.0.5) to .NET Framework 4.6.

Will you be ok with targeting 4.6 rather than the 4.5.1 that 1.0.5 had? If so, I'll go ahead and push this package (potentially with a required version bump)

alenas commented 5 years ago

I added 4.6, as that is compatible with net standard 1.3 - I tried with net45 - but then I get error on build, that Array does not have a definition of Empty.

bradgentry commented 5 years ago

4.6 works for us. We are targeting 4.7.2. Just to confirm, will you also be updating the Argon2 package?

bradgentry commented 5 years ago

Sorry, I see that this repo also includes Argon2. I thought it was just for Blake2. As long as both Nuget packages have the fix, we should be good to go.

kmaragon commented 5 years ago

I published the packages yesterday. Let me know if they look better.

bradgentry commented 5 years ago

They appear to be working as expected. Thanks!

kmaragon commented 5 years ago

I forward your thanks to @alenas who actually fixed this. Thanks for the update!

alenas commented 5 years ago

;) my pleasure