-
Notifications
You must be signed in to change notification settings - Fork 562
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
Add netcoreapp2.0 solution and project files #81
Conversation
This duplicates functionality in #68 as that version supports .NET Standard, which works with .NET Core 2.0. |
@onovotny |
I guess I'm wondering why you need this at all though. Once my PR is merged in, it'll build there too... What does this add over the other PR? I'm happy to take PR's to my fork as well. |
@onovotny You change 337 files, its a lot of work to check this changes. I suggest to add a solution file, two project files, and insert a conditional compilation in AssemblyInfo.cs |
The files I change are all Unit tests to rename the Main method because of compiler things. The rest are globed in. |
@onovotny In my opinion, I propose a more elegant solution. |
@doomkin I respectfully disagree. Please look at my csproj compared to what's here. One csproj for all platforms and almost nothing in it: |
@onovotny It looks compact. I inserted the framework
|
@doomkin That's a temporary limitation until 1.8.2 is shipped. The auto-versioning library I'm using, The alternative is to use |
@doomkin Also, there should be no need to build for |
@onovotny This does not work. I hope you will attach your version of the |
@doomkin I've pushed a branch that can do this That version has to drop the |
@onovotny Thank you, but this does not work:
|
A couple things.
I've removed those two dependencies in that branch: But I would not expect that to be ultimately merged in. The end result may require Windows to build in order to fully support the older frameworks. What targets are supported ultimately is not my call. If the library is NETStandard only, and older things can be dropped, then great. Otherwise, I'm afraid you wouldn't be able to build it there. Note that that doesn't affect actually consuming the library however it is built. |
@onovotny This does not work too: 5019 Errors.
My crypto.netcore2.csproj works fine. Perhaps it can be simplified. Please note that I do not change anything except AssembyInfo.cs
Information about assembly contains in
Therefore, my PR causes more trusted than yours when it comes to cryptography. Do you agree? |
No, your PR is not more trusted. I don't need to change anything important in the code itself. The PR here adds an additional project to build and then package. That's hard to maintain. My PR leverages the multi-targeting capabilities to generate targets for all supported platforms within a single project. The only real things that were changed are this: In the tests: In the code, a few places where No significant changes were made to the code. |
@onovotny Ok, men 🥇 Trust is the same |
That's not true. libgit2sharp does indeed build on linux, including with NB.GV applied. |
I guess I'm a little lost here. I don't see NB.GV being added here, nor linux being added to any CI server. So where does this error repro, and why with this PR only? |
@AArnott It was an earlier commit in this branch https://github.com/onovotny/bc-csharp/blob/dotnet-build/crypto/src/crypto.csproj, that I was trying to help @doomkin build this on his preferred OS. I removed it from that branch to try and unblock him.... |
Any news when this might hit the nuget package? |
It's already in Portable.bouncycastle. You can see it in my pr and fork. |
@onovotny it would be nice if it was included in the official nuget package, but thanks for the information. |
FWIW, my fork is essentially the official portable version. Once the current set of changes in master are released as stable, I believe the intent is to merge my pr. Always happy for code reviews to verify authenticity, and my libraries are all authenticode signed so you can be sure they came from me. |
|
@onovotny Just thought to notify you of the conflict in AssemblyInfo.cs as I was reading the comments in this PR, if it is still valid (it's still Open). |
@onovotny My apologies. I had both PR's open in tabs and have mixed them up. |
Obsoleted by the impending release of v2.0 NuGet package with support for net461, netstandard2.0, net6.0. |
I suggest to add a solution file, two project files, two build task files for VSCode and insert a conditional compilation in AssemblyInfo.cs to avoid build error. Tested on Arch Linux on
netcoreapp2.0
bydotnet build BouncyCastle.netcore2.sln
.