search

sestoft / C5

C5 generic collection library for C#/.NET
http://www.itu.dk/research/c5/
MIT License
1.04k stars 179 forks source link

Add support for Profile259 and .NET Standard 1.0 #56

Closed AArnott closed 7 years ago

AArnott commented 7 years ago

This gets C5 working virtually everywhere. It already ran on .NET 3.5 and the broadest of net40 PCLs. This PR adds the broadest of net45 PCLs and .NET Standard 1.0

This also consolidates all product projects and test projects into just one shipping project and one test project. Using the power of the multi-targeting support in VS 2017, we no longer need several projects.

I also added an appveyor.yml script, which gets the full build and test run executed, and the .nupkg file stored as an artifact as you can see here. I would encourage the project maintainer to do as I have done in adding an AppVeyor build definition of their own for this project after accepting this PR so that they can get official builds and PR validation for free.

The previous attempts to get support for .NET Standard and .NET Core before (as is currently in the master branch) is somewhat flawed: it has a disabled MemorySafeEnumerator. I fixed that. No, it doesn't require targeting .NET Standard 2.0.

I slightly modified a few interfaces so that they don't vary across target frameworks. This will avoid TypeLoadException being thrown when a library is built against (for example) the .NET 3.5 version of this assembly and then run in a .NET 4.5 application. As changing an interface is a breaking change, I've bumped the version of the built package to 3.0-beta.

Closes #55 Closes #50 Closes #51 Closes #47

ondfisk commented 7 years ago

Great work. Cannot get package restore to work, though. I can build if I do "dotnet restore" from CLI first. In VS 2017 I get "Cannot infer TargetFrameworkIdentifier..." and "The target "GetWinFXPath does not exist in the project" errors. Also assemblies cannot be resolved in C5 (yellow triangle) on Dependencies/Assemblies/System+System.Data+System.Drawing+System.Xml and Dependencies/NuGet/MSBuild.Sdk.Extras. Can you help?

AArnott commented 7 years ago

I did most of my development on VS 2017 Preview (15.3) and VS Code. Are you on 15.2? I'll try to repro.

AArnott commented 7 years ago

I tried on a new machine with 15.2 installed and I saw the same errors. After restoring from the command line, the errors went away and I could build successfully. Then if I closed and reopened the solution, all the yellow triangles went away.

After you used dotnet restore, did you try reloading the solution in VS?

Supposing a manual command line restore is required to bootstrap this (on account of the PCL profiles that the solution still supports, BTW), given the lack of active development on this repo, I suspect the impact will be very minimal relative to the positive impact of consuming the new target platforms via nuget. Do you agree, @ondfisk?

ondfisk commented 7 years ago

Agree, merged. Will push to NuGet ASAP

AArnott commented 7 years ago

Thanks, @ondfisk. Before you publish, I have two more (small) PRs for you to consider.