Support for .NET core

[kkozmic]

What is says on the tin.

This was triggered by https://github.com/Moq/moq4/issues/168 but obviously there will me more demand for this than just from Moq.

So let's talk about it.

[smudge202]

Happy to help on this. Looks like an awful lot of work (at a glance). If we can come up with a game plan, can try to rope in more people to give us a hand...

//CC @mattridgway, @herecydev, @sblackler

[kkozmic]

thanks @smudge202 for your kind offer.

Perhaps let's start by getting the lay of the land. What makes you think it's an awful lot of work? Do you have a report from the Portability Analyser you can post?

I don't have a working Windows VM at the moment which makes it somewhat tricky for me to do this.

[jonorossi]

The report output was linked in the moq issue: http://dotnet.github.io/port-to-core/Moq4_ApiPortabilityAnalysis.htm

[kkozmic]

Oh right. I see. At a glance it looks like a fair bit of those are actually coming from DictionaryAdapter. We had merged it into Castle.Core for simplicity in pre-Nuget era, but I wouldn't be too fussed to split it out if it helps us get some wins here.

[kkozmic]

any chance we can get that report grouped by usage, so we can see where the problematic APIs are being used across our codebase?

[jonorossi]

And much of the rest is CAS/security transparency, serialization/remoting, smtpclient facade, and the simplified reflection API.

For things like GetTypeInfo() (which is only in .NET 4.5/4.6 only) I think we'd have to implement our own extension method which would mirror the .NET 4.5 one to reduce conditional compilation. We'll have to conditionally compile DP strong name support out, as well as other things.

[kkozmic]

given we're most likely going to make this a v4.0 release, we have some room for breaking changes I suppose.

Dropping Silverlight support is something I wanted to do for a long while.

What's the issue with strong naming @jonorossi ?

[jonorossi]

.NET Core doesn't support strong naming, so the Reflection.Emit API doesn't support it, no biggie.

[kkozmic]

gotcha

[smudge202]

So glad that didn't turn into a strong naming debate... :)

Not sure if the report can be changed to group by usage like kkozmic asked. Personally, I'd create a new vNext library targetting only dnxcore50, dump all existing cs files into the new project's directory, and start working through the error list.

In my opinion, best approach is to get the code to run against dnxcore50, then bring in net451/net40, make any changes required using ifdefs, then bring in other profiles you want to support, rinse lather repeat.

Take the TypeInfo and dnxcore code as a baseline, from which all other profiles are ifdef'd in.

[MattGal]

@smudge202 , it's actually not too bad to include existing C# files using an ASP.NET VNext project in a sub-directory of the solution (as it will pick up files adjacent / below it automatically...) and use the "code" tag to include the original C# files in parent directories, setting the define constant for Silverlight (which works in many case) and defining an additional one such as CORECLR or DNXCORE50 for where the implementation has to differ from SL5.

Duplicating all the existing sources is not fun for bug fixes / updating features.

[smudge202]

Yeh, I took the same approach with the FluentAssertions port I've been working on. Just use compile in project.json to pull in code.

[MattGal]

I've going to start working on this this week, so if anyone already has please let me know so I don't duplicate effort. If all goes well, I should be done in a week or two.

[uhaciogullari]

Do you have any plans about migrating the unit tests to xUnit.net? Only it has a working test runner in DNX (or CoreCLR or whatever they are calling it now).

I think Castle Project is one of the best things about .NET. Let me know if I can do anything to help it work.

[uhaciogullari]

I have attemped to compile the project on DNX. I have only added a project file and referenced some NuGet packages. Castle.Core classes remain unmodified.

https://github.com/uhaciogullari/Core/tree/core

• I had VS 2015 RC and VS Code on my machine.
• Installed DNX (or DNVM) like documented here. Don't forget to run "dnvm upgrade" or you don't get dnu.
• Created an ASP.NET class library with yo.
• Copied the generated file to Castle.Core folder
• Executed dnu restore in that folder
• Executed dnu build and got 3290 compile errors :cold_sweat:
• At this point I started referencing system libraries but they are split into many NuGet packages.
• VS Code autocomplete doesn't help much. It probably shows the packages for full CLR.
• Opened up [NuGet Package Explorer] and start digging the feed at https://www.myget.org/F/aspnetvnext/api/v2
• Started referencing popular & sensible packages, see if they decrease the compile errors by running dnu restore & dnu build.
• Managed to get down to 1342 errors. There are a lot of Xml errors that could not be get rid of by referencing the packages. I think I tried all of them, they may have changed the namespaces. This requires code modification in cs files so I left them.
• dnu build command was not working on Mono when I checked it a few weeks ago. There were a few workarounds but I didn't have the time to try it yet. Maybe some folks can try to build it from my branch and compare the output.

Here's the last build log by the way: https://gist.github.com/uhaciogullari/ce214c172f12eb1fca0c

[jonorossi]

@uhaciogullari There are no plans to migrate from NUnit to xUnit.net. With just the Castle Core and Windsor projects we've got nearly 3000 unit tests, I don't think migrating them is a good use of time and it is likely to introduce bugs into the unit tests.

The NUnit team is aware of this: https://github.com/nunit/nunit/issues/575

[uhaciogullari]

I have managed to get down to 578 errors. Here's the last error log : https://gist.github.com/uhaciogullari/9b1762008ad36adbd071

Summary of the problems I could find:

• Binary serialization is not supported
• Remoting is not supported
• System.Net.Mail is not supported yet
• System.Configuration is not supported, I guess this assembly will be replaced by Microsoft.Framework.Configuration.* packages.
• There are some problems with Xml namespaces. I referenced every Xml package but it still can't find some XSLT types.
• Reflection API may have changed. Many properties on Type class cause errors.

[MattGal]

This sounds about right. I have been sidetracked for a couple weeks and not paying attention to GitHub, but I have the project fully building for Core 5.0 here: https://github.com/mattgal/Core and intend to make a tentative PR (tests are still up in the air on this... figuring out the XUnit/Nunit thing has been a challenge) tonight so folks can look at it.

[richlander]

See #92.

[MattGal]

Pinging for any comments? It will probably be another week til I work on this some more, but I'd love to get some direction on any changes folks think are needed to take in the PR. As it currently stands, it should not affect building the existing projects at all (if it does, that's an unintentional bug...)

[jonorossi]

@MattGal thanks for the WIP PR, however could you update it to not include a copy of all the unit tests. GItHub won't let me review it because the diff is too big:

Showing 556 changed files with 29,258 additions and 193 deletions.

Sorry, we could not display the entire diff because too many files (556) changed.

[smudge202]

@jonorossi Wouldn't it be better to clone the originating branch and review locally than to remove the tests?

[jonorossi]

@smudge202 then what was the point of the pull request if I can't comment on the code inline?

[smudge202]

@jonorossi https://msdn.microsoft.com/en-gb/library/bb385979.aspx - article explains code annotations related to TFS, but same applies to git as far as I know.

[jonorossi]

@smudge202 that is for TFVC, but yes Git has support for "blaming". I'm confused, why are you pointing out this feature?

[smudge202]

I wonder if code annotations will allow you to "comment on the code inline"?

[jonorossi]

@smudge202 the annotate/blame/praise feature in version control systems allow you to view a file's contents with the last commit displayed next to each line.

[richlander]

That's certainly unfortunate:

@MattGal thanks for the WIP PR, however could you update it to not include a copy of all the unit tests. GItHub won't let me review it because the diff is too big

How about @MattGal rebase his PR into two, one for the code and one for the tests, but treat it as one meta PR? If they are clearly split by directory than filter-branch can do the trick.

[jonorossi]

No need for a second PR, I don't understand the hesitation of just excluding the WIP copied unit tests. You guys don't actually expect us to merge a PR that contains a copy of all the unit tests do you?

[richlander]

Help me understand. Were the unit tests changed or why are they included?

[jonorossi]

@richlander did you read @MattGal's PR description?

A tentative set has been put in using XUnit.

If NUnit isn't going to happen for .NET Core any time soon (I see no one has engaged with Charlie, Rob and folk, https://github.com/nunit/nunit/issues/575) then moving to xUnit.net is a different discussion we'd have to entertain with that knowledge.

[MattGal]

@jonorossi I have updated the PR to not include the test projects any more, leaving them in a branch on my fork if anyone wants to use them as a basis. I don't mind; as is, the regular versions of Castle still build as before. I'd like to get this in so others who use Castle more can find issues with the CoreCLR version and I can start building CoreCLR Moq4 on top of this, can you suggest what more I should do to get to that point?

[richlander]

@jonorossi I talked to @MattGal about this. @MattGal mostly clarifies it in his comment above, but it is worth capturing our conversation.

• It's a bad form to make a big shift (e.g. testing framework) w/o consultation/approval from the project maintainers.
• Currently XUnit is the only testing framework supported for CoreCLR, so the tests were/are valuable.
• We should get NUnit to support CoreCLR, too. After we finish we Moq, we can consider helping there.
• For the immediate term, the tests are valuable and should be persisted (for anyone to use), but should not be part of the PR.

[kkozmic]

Very interesting conversation guys ☺

To simplify the move how about we do it in chunks. Start off with dynamic proxy as it is the most commonly used part and leave dictionary adapter, email sender to make the move easier. Divide and conquer.

Also, risking hijacking the conversation, we should soon start looking at which frameworks versions we want to support and how many binary versions we need to produce. Perhaps a separate albeit related discussion

[richlander]

@kkozmic Are you suggesting that @MattGal should split his PR into pieces instead of taking it all (modulo the tests) at once?

I assume that your second point has more to do with how this change will affect your NuGet package. Yes?

[uhaciogullari]

Breaking changes in Reflection API leads to many changes. Many properties on Type class are now moved to TypeInfo class which can be created by calling GetTypeInfo extension method.

Should we go on with preprocessor directives or GetTypeInfo method? It's only available on .NET 4.5 and later though. Creating the same extension method for older builds should work if you plan on supporting them. Here's a detailed post about this change. It's written for WinRT but it's the same for .NET Core I think.

[jonorossi]

I've merged the mono-support branch (#95) into master because there was quite a lot of work there which was stable, and I wanted it in before we start getting any of this work in. We also need that better Mono support since DNX is currently using Mono on non-Windows.

Today I've also introduced two new conditional compilation symbols that will ease the migration to .NET Core. The .NET Framework builds will now define FEATURE_SERIALIZATION (#96) and FEATURE_LEGACY_REFLECTION_API (#98) (see the README for the configuration matrix which shows what builds with what). These symbols won't be defined in a .NET Core build and the intention is to continue pulling out whatever isn't supported into symbols we can enable on the full .NET Framework.

This work should help to reduce the churn of code that #92 will introduce. I don't speak for @kkozmic, but personally I think with the amount of code changed in #92 and the number of new TODOs added it is pretty hard to code review and ensure we get the changes in without causing defects.

As per the PR descriptions, the serialization symbol should be complete, however I'm only part way through the reflection API migration. I'll continue on that and continue working on getting these changes in safely.

[uhaciogullari]

@jonorossi Nice work, it fixed many compile errors on .NET Core. Without any code change it's down to 176 errors (leaving DictionaryAdapter and MailSender out). Most of them are related to Reflection API, others are related to Remoting, Configuration and Security mainly.

Here's the build log: https://gist.github.com/uhaciogullari/d6fa16be781a905ee6d6

[kkozmic]

@richlander re: splitting the PR - yes, just floating it out there as an option if we find it's too much to do it all in one go.

re packages - it's partially about nuget but also, about effort. If nobody uses Silverlight 4 anymore, why spend time/effort keeping Castle running on sl4?

[MattGal]

Hi all, I now have bandwidth to basically devote full time to this for a while, but I don't mean to step on anyone's toes or duplicate effort. It sounds like @uhaciogullari is working on the same thing as I am. Can I get some clarity on whether I should keep going on my efforts to make a .NET Core version of Castle? I have a list of issues from the PR that I will start addressing although some of them have no clear answer (for instance, what to do with respect to unit tests, given NUnit is not supported on Core)

Thanks, Matt

[davidfowl]

We're (ASP.NET team) are also pretty interested in getting Moq (and therefore Castle) up and running on CoreCLR. I just did a scan of @MattGal 's changes and they are pretty tiny, it's mostly ifdefs. Is there anything we can do to push this forward? We want to help because we actually want to use this stuff!

[uhaciogullari]

@MattGal I haven't done anything really, just created the project.json file with the correct packages and tried to compile it in DNX. I wanted to see the breaking changes in .NET API.

@davidfowl I'm no contributer but I think you can work on your fork and publish it as an unofficial port since it's an open source project. You got your own NuGet feed for .NET Core, you also ported the tests to xUnit. This was how it went with xUnit port as far as I remember. Contributers have to make sure that the PR meets their quality standarts and you're in a hurry.

I can't understand why the Reflection API gets broken in every .NET subset that comes out of Microsoft. Mono and Xamarin have their own problems but at least they try to offer the same API. Did anything significant get ported to WinRT or portable libraries? The best is Silverlight and I have no idea how much work went into porting something like Castle Windsor.

It's all the same this time. CoreCLR even has Reflection.Emit and many reflection assemblies but a basic tool like NUnit cannot be ported with ease. I really want to understand why we need the changes, I can't imagine the type system working differently in CoreCLR. You probably have very good reasons but I never saw a post about them, neither a way to deal with those changes. We are only left with broken tools.

I also wonder how developers are expected to port their applications to .NET Core. I think preprocessor directives cause a mess and porting a decent sized application will be practically impossible. Maybe we are not supposed to port them, just write new apps in .NET Core? What about our libraries? We will miss lot of them.

I don't want to sound harsh, you are the good guys who are making it better and I love your work. It's Microsoft's fault that they did nothing about cross platform issue until containers and mobile platforms forced their hand. After all these years they could have called it .NET Absolution :smile:.

[davidfowl]

@davidfowl I'm no contributer but I think you can work on your fork and publish it as an unofficial port since it's an open source project. You got your own NuGet feed for .NET Core, you also ported the tests to xUnit. This was how it went with xUnit port as far as I remember. Contributers have to make sure that the PR meets their quality standarts and you're in a hurry.

That's a pretty lame thing to do IMO. We'd rather not create unofficial forks of popular open source projects. So far we've been trying to help popular library authors port their libraries to .NET Core with varying levels of success :smile: . This issue is also "up for grabs" which means that the contributors are ok with the community taking over and helping out. We're mostly looking for guidance on how to continue, we're not trying to force a PR down your throat. If you don't like how the change was made then we'll change it, after all we run OSS projects too and understand what it means to take on changes made by community members.

[jonorossi]

Sorry for the delay in replying, I've been unwell with the flu for the last few days. I'll try to reply to your comments tomorrow.

[MattGal]

@jonorossi, some updates: Changes:

• Modified the .NET Core project to now be a Portable Class Library targeting .NET 4.6 + .NET Core 5.0. This allows things like being able to see all the files in the solution (despite them being links), and building from the regular command line via MSBuild instead of needing DNX to build. Overall the experience is improved with respect to building the library like the others are.
• Brought back unit tests as XUnit (only way they’ll work on core) and got all but 22 passing.
• Renamed CORECLR to NETCORE To do:
• Integrate the new project into existing build scripts, document this in readme
• Drive to 100% unit test pass rate. (currently 22 failing, 658 passing in the XUnit version of the tests. Open questions / issues:
• For AssemblyBundleResource / AssemblyResourceFactory, the APIs that allow for explicit load of Assemblies are gone. There is load via AssemblyName, and when hosted in DNX there are DI interfaces for getting the same effect. Should I cut APIs that use .Load(string), take a dependency on DNX interfaces, or other?
• Still need guidance about what to do with respect to unit tests, as NUnit doesn’t look to be available on Core any time soon.

[MattGal]

Sorry to hear you've been ill, I am updating the PR, please give feedback when you have a moment.

[jonorossi]

Sorry I didn't get back to you on Sunday, I overestimated my recovery and the amount of other things that piled up.

@MattGal it is nice to know you've got more time to help work on this now. Your short bursts then silence for a week or more made things difficult to communicate, which was why I went off and tried making the port much easier to review than trying to get you to make those changes.

@davidfowl the changes definitely weren't small like they are now until I introduced the FEATURE_SERIALIZATION and FEATURE_LEGACY_REFLECTION_API conditional compilation symbols into master. I've hinted at it a couple of times, but my push is really to avoid another platform conditional compilation symbol and so I'm trying not to just rush a merge that'll just give us more technical debt.

[jonorossi]

@MattGal thanks for getting those bad merge changes fixed up and getting a PCL set up. The new set up looks like it should be easier to get into our build scripts.

Looking at your branch am I to assume that the Castle.Core-CoreCLR directory is no longer needed and that the PCL is in Castle.Core.NetCore? Regarding your question in the PR about whether it is fine for this to build with VS2015 only, yep that is fine.

Looking at your .NET Core copy of the unit tests I see they are quite out of date already. I'm working on a spike to see if we can use both testing frameworks, this will then be the precursor to us moving to xUnit.net completely or upgrading to a new NUnit with support for .NET Core in the future. I'll report back within a day or two with the results, and hopefully you'll be able to again remove your copy of the unit tests from the PR, we can't do much until we sort them out. I'd then like to get a build set up on our build server for the .NET Core target.

[MattGal]

I've squashed previous commits, deleted the CoreCLR folder, and updated the PR text. I am committed to fixing any bugs/issues that fall out of this, but I'd like to get some more feedback (ideally with some folks trying this on VS 2015) first.

[jonorossi]

@MattGal I've made some good progress on the unit tests, looking at getting them pushed soon. I've just tried opening your branch in VS2015 after getting it installed the other day and I get the following error. I can't find where to download the PCL reference assemblies for v5.0, it looks like it hasn't been released yet and the link from VS2015 to create a new project indicates the same.

The imported project "C:\Program Files (x86)\MSBuild\Microsoft\Portable\v5.0\Microsoft.Portable.CSharp.targets" was not found. Confirm that the path in the declaration is correct, and the fhe file exists on disk.