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

Adding .net Core compatible PCL #8

Merged
merged 2 commits into from
May 30, 2016
Merged

Conversation

steviehailey
Copy link

This PR adds a new Portable Class Library project targetting net451 and portable5.4 which, for example, works with .net Core projects running on CoreCLR on Linux. Here is a summary of the changes I had to make to implement this followed by a few questions around stylistic preferences and how the existing test and packaging setup works.

Changes

  • There have been no functional changes to the existing projects.
  • The new Portable Class Library project is named Compare-NET-Objects-NewPCL and includes a new conditional compilation symbol "NEWPCL". This project imports/excludes the same classes as the existing Portable project so I have modified occurrences of "#if !PORTABLE" to also check for "&& !NEWPCL" (and vice-versa where we had "#if PORTABLE" anywhere). There are a couple of cases where the new PCL needs to make use of slightly different syntax from the pre-existing code which I why I didn't just assign "PORTABLE" to the new project and re-use all of the existing directives (more on that below).
  • The new PCL uses an xproj format project file which usually just picks up the code files from within the project's directory. VS doesn't offer an "Add as Link" option for these project types but I have managed to make the new PCL refer to the master code files under main project by using the "compile" section of the project.json file. Currently each .cs file is included explicitly by name but it is also possible to invert this to include everything via a wildcard and then explicitly exclude the files which it doesn't want.
  • There is a new static class to maintain backwards compatibility in the face of GetTypeInfo() replacing GetType() for reflection-related functions on Type in the new PCL. All projects now call the new GetTypeInfo() method and TypeBackwardsCompatibilityExtensions makes this available as extension method so that the pre-existing projects compile and behaves as before.
  • Type.UnderlyingSystemType is not available to the new PCL so I have used a compiler directive to maintain old behaviour for existing projects but fall back to direct use of the type name in the new PCL (see TypeHelper.IsByteArray(..)). The tests do all pass even if we were to make this change in the main project and remove compiler directives, however, there is the chance that this value could be different for some objects depending on how they are created so I erred on the side of caution in not making my change in the main project.
  • Similarly to Type.UnderlyingSystemType, Type.ReflectedType is not available to the new PCL so I have used a compiler directive let the new PCL fall back to Type.DeclaringType. Again, tests do all pass if we make this change in the main project and remove compiler directives, however, I have once again made the change only impact the new PCL (see TypeHelper.IsEnumerable(..), IndexerComparer.CompareIndexer(..) and CompareIndexer.IndexersHaveDifferentLength(..) and PropertyComparer.IsValidIndexer(..)).
  • Since there are now a few code branches which only execute when the new PCL is compiled I have add a new .net Core test project which just contains a few cases to smoke out any issues on these paths. This uses NUnitLite and a new .net Core console app wrapper around the tests so that they can be executed using "dnx Compare-NET-Objects-NewPcl-Tests". The tests make use of the "compile" directive in the project.json to import a couple of sample classes for use in the tests. We could theoretically import more tests from the main project to have those run but this would require a additional compiler directives to remove irrelevant tests and some NUnit v3 related code migration (e.g. replace [TestFixtureSetUp] with [OneTimeSetUp] etc).

Questions

  • I have been using the term "NewPcl" to distinguish this new-style PCL from the existing one in its project name and compiler directives. This assumes that we want to keep the existing portable library and cannot call it that. What is your preference on naming to distinguish the new-style PCL from the from the existing one e.g. NewPcl, Core, Platform54, Portable2, ModernPcl?
  • I have seen the pattern of excluding API functions entirely based on compiler directives but, for the cases mentioned above where I have strayed from this, what is your feeling updating the main code instead e.g. get rid of the compiler directives until such time as a test case proving the importance of the previous code arises?
  • What facility/appetite do you have for running the new .net Core test project in your current build setup and what are your thoughts on it only covering the differences from the main package?
  • I have locally produced nuget packages by using the "dnu pack Compare-Net-Objects-NewPcl" command but the project.json needs to be updated to specify a specific version number before running it. Can you clarify how you would see this fitting into your existing publishing process so that I know whether a version should be baked in?

Made required changes in existing projects to allow everything in new PCL to build using references to the same source files.
.net Core compatible test project only exercises branches of new PCL which veer off from behaviour of main and older Portable projects.
@GregFinzer
Copy link
Owner

Steve this is great. I have an internal tool that I have built to increment versions, perform builds using MSBuild and create NuGet packages on the fly using the NuGet API. It does a great many other things too like run unit tests and upload to NuGet. The build process would need to change for the .NET Core. Some questions:

  1. What is the folder distribution for .NET Core for NuGet packages?
  2. What is the folder distribution for PCL Profile 111?
  3. What MSBuild parameters would I use to build .NET Core?

Thanks,
Greg

@steviehailey
Copy link
Author

steviehailey commented Apr 19, 2016

Hey,

1. What is the folder distribution for .NET Core for NuGet packages?
I have examined the package produced when I run "dnu pack Compare-NET-Objects-NewPcl" and the folder we're interested in is named "dotnet5.4". This is a screenshot of the package for reference (I am not allowed to attach the actual package).
newpclpackage

Manually tweaking the latest CompareNetObjects package using NuGetPackageExplorer and adding this folder with a copy of the new PCL binaries results in a package which works on Linux with .net Core. For background, it seems that with NuGet v3.3 we use "dotnet54" and this becomes "netstandard14" with NuGet v3.4 (from here: https://docs.nuget.org/create/targetframeworks).
One gotcha to watch out for is that Linux seems to be more picky about case than Windows and seems to take issue to "CompareNETObjects" being referenced in a project.json file when the package is cased "comparenetobjects.nupkg" - naming the package file as-per the package title fixed that when I was trying this out though.

2. What is the folder distribution for PCL Profile 111?
I have not actually worked with the numbered profiles before, but, from what I can see on the targetframeworks page linked above "Profile111" means ".NETFramework 4.5+Windows 8.0+WindowsPhone 8.1" which I believe you already support. There does appear to be a typo on this framework listing from the issue, where it says "Profile111"="portable-net451+netcore451+wpa81" at the same time as saying ".NET Framework 4.5.1+Windows 8.1+Windows Phone 8.1".
If by "supporting Profile111" we mean "will work on .net Core" then I believe we've ticked that box, but, my lack of understanding in this area may be a source of confusion... :)

3. What MSBuild parameters would I use to build .NET Core?
I have been using the new DNX CLI tools rather than MSBuild. For example:

To build the NuGet package (from root of solution)

dnu pack Compare-NET-Objects-NewPcl

Points to note:

  • By default this outputs package to /Compare-NET-Objects-NewPcl/bin/Debug/ but a param allows a different folder to be selected
  • A Release build can be produced using the configuration switch
  • Package version is taken from project.json (and can optionally replace an asterisk with the value of environment variable DNX_BUILD_VERSION)
  • Package name is based on the name of the folder containing the xproj file

To run tests using CoreCLR runtime (from root of solution)

dnvm use 1.0.0-rc1-update1 -r coreclr
dnu restore
cd Compare-NET-Objects-NewPcl-Tests
dnx Compare-NET-Objects-NewPcl-Tests

People do talk about using MSBuild to compile an xproj but it sounds like they advise using the DNU commands directly e.g. I can see TeamCity is offering a plugin which also goes in this direction.

What do you think about incorporating use of the DNX CLI tools?

Cheers,
Stephen

@GregFinzer
Copy link
Owner

Thanks for all the great information. It will be a while until I can get to this. We just started a Hackathon at work at night, plus I have my normal day job, my side business, and three kids.

@steviehailey
Copy link
Author

Hey Greg, thanks for the update.
It would be nice to avoid my branch being too long-lived without merging to master. I appreciate that full integration with the existing NuGet packaging process would be a fair amount of work, but, is it worth having the new PCL merged in even without that integration? That way consumers can at least build their own package and we could fix-forward in the face of any changes required when we come to integrate it with the pipeline?
Cheers,
Stevie

@GregFinzer GregFinzer merged commit a9948f6 into GregFinzer:master May 30, 2016
@bestekov
Copy link

@steviehailey , @GregFinzer , I'm trying to get this to work in a new .net core 1.0.0 class library and I'm getting the error "The type 'List<>' is defined in an assembly that is not referenced. You must add a reference to assembly 'mscorlib, Version=2.0.5.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e, Retargetable=Yes'"

@steviehailey , you said you got this working with .netcore somehow. What's the secret? I tried even just importing the whole project into my solution and messing around with different options int he project.json but I can't get it to properly recognize dependencies.

@bestekov
Copy link

Ah wait, no, this was user error. I was still actually targeting the nuget version instead of the local version. Of note though, you'll need to update your project.json for the changes in the RTM version.

@GregFinzer
Copy link
Owner

@steviehailey or @bestekov how do I update the project.json to the RTM version?

@bestekov
Copy link

I used these two guides:
https://docs.asp.net/en/latest/migration/rc1-to-rtm.html
https://docs.asp.net/en/latest/migration/rc2-to-rtm.html

I ran into a bunch of compatibility issues with the main Compare-NET-Objects library when I tried to target netstandard1.6 (the current class library target for core), so I ended up just using net451 for the framework (I'm using this for a one-off utility on windows so being cross-platform wasn't vital to me).

Here's the project.json I ended up with for the NewPcl project:
project.txt

Honestly, I'm pretty new to c# and programming in general, so my understanding of how all the new pieces fit together is a bit murky, but by doing this I was able to reference the project from a .net core class library.

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

Successfully merging this pull request may close these issues.

4 participants