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

Support for .NET core #90

Closed
kkozmic opened this issue May 4, 2015 · 123 comments
Closed

Support for .NET core #90

kkozmic opened this issue May 4, 2015 · 123 comments
Milestone

Comments

@kkozmic
Copy link
Contributor

kkozmic commented May 4, 2015

What is says on the tin.

This was triggered by devlooped/moq#168 but obviously there will me more demand for this than just from Moq.

So let's talk about it.

@smudge202
Copy link

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
Copy link
Contributor Author

kkozmic commented May 5, 2015

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
Copy link
Member

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

@kkozmic
Copy link
Contributor Author

kkozmic commented May 5, 2015

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
Copy link
Contributor Author

kkozmic commented May 5, 2015

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
Copy link
Member

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
Copy link
Contributor Author

kkozmic commented May 5, 2015

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
Copy link
Member

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

@kkozmic
Copy link
Contributor Author

kkozmic commented May 5, 2015

gotcha

@smudge202
Copy link

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
Copy link

MattGal commented May 6, 2015

@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
Copy link

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
Copy link

MattGal commented May 19, 2015

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
Copy link

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
Copy link

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 😰
  • 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
Copy link
Member

@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: nunit/nunit#575

@uhaciogullari
Copy link

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:

@MattGal
Copy link

MattGal commented Jun 16, 2015

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
Copy link

See #92.

@MattGal
Copy link

MattGal commented Jun 17, 2015

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
Copy link
Member

@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
Copy link

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

@jonorossi
Copy link
Member

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

@smudge202
Copy link

@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
Copy link
Member

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

@smudge202
Copy link

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

@jonorossi
Copy link
Member

@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.

@xied75
Copy link

xied75 commented Feb 16, 2016

@richlander Got a quick question as UWP is mentioned multiple times in the thread, I wonder how is that possible for DynamicProxy to be supported in UWP? Given the current .NET API allowed there, it doesn't look like you can emit new types? So when you say UWP support to follow, do you mean the .NET API surface could be extended?

Related but off topic to this issue is that who actually control what is available to UWP sandbox? Is that Windows team or .NET team?

@shanselman
Copy link

I agree with @adamralph a pre-release version would unblock a number of people @jonorossi

@hammett
Copy link
Contributor

hammett commented Feb 25, 2016

If this is a blocking issue, feel free (and blessed) to fork Castle.Core. @kkozmic, @jonorossi and myself seem more occupied by other activities. I have no interest in dealing with this codebase in my spare time, at least not for free. Please slightly change the name of the final assembly to indicate it's a fork and possibly not compatible.

Another route would be to make sure the PR(s) wont break Windsor, and they will be promptly accepted.

Thanks

@thecodejunkie
Copy link

@hammett @kkozmic @jonorossi is this the official stance from the Castle.Core team, i.e

"We won't have time / the desire to make Castle.Core compatible with the CoreCLR in the foreseeable future so we would rather see a community fork"

The reason I ask is because your comment does not come across as an official stance from the team and I think it is important that it is clear because, as @shanselman and @adamralph points out, there is a downstream community blocked.

Would the be any kind of timeframe when this would be done by the team? Forking is not going to be as clear cut (strong names and what not) and it would add an even larger burden on the community since that fork would have to keep pushing out mirror releases to be feature compatible and make sure bugs are removed if they have been reported and fixed in upstream.

@hammett
Copy link
Contributor

hammett commented Feb 25, 2016

It's unlikely that there will be much activity on Castle.Core that will cause added burden to keep the forks in sync. I dont want to speak for @jonorossi but seems that he -- as myself [who some might not know, originally wrote DynamicProxy way back when] -- doesnt have much time for this, So let's take the noble action of stepping out of the way.

I'd be willing to even give repo write access to whomever is sending the PRs if that would streamline the process and avoid the fork, as long as source code/behavior compatibility is preserved for Windsor.

I just dont want this to be a bottleneck as it has been for the past several months. I have no immediate interest in .net core or much of .net to be frank but there's no point in blocking this further.

@adamralph
Copy link

So let's take the noble action of stepping out of the way.

👏 that seems like the right thing to do. The other side of that is that some new people need to step up and take the baton as join as new maintainers.

I have no immediate interest in .net core or much of .net to be frank

Nothing personal, but that seems like strong grounds to step down as a maintainer of the project (which, perhaps, is what you are alluding to).

@hammett
Copy link
Contributor

hammett commented Feb 25, 2016

I havent been a "maintainer" since 2012 - my lack of participation on this and other threads should have made this fact abundantly clear.

@jonorossi has been doing a fantastic job, but OSS is always second (or 3rd, or 4th) to whatever pays the bills. And you dont see a lot of people willing to step in. A PR is not a takeover.

@rjdevereux
Copy link

First off, all the interest in this thread shows how important this project is to the .Net community, directly and indirectly a lot of people are dependent on it. Thanks for all the work you guys have done building and maintaining it.

There have been offers to help from the MSFT guys, if you look back through the posts you can find @richlander offering to help organizing communication and pledging support from MSFT developers, I'm hoping those offers still stand. But they were also cautious and wanted to respect that they don't own the project.

Sorry if I'm putting words in other people's mouths, but I think there is a lot of excitement to keep this moving forward and what folks are looking for is a little direction on how to move forward from the maintainers, I think there are people ready to help get this working on Core, they just need direction or permission.

@bob-devereux-zocdoc
Copy link

Am I right at pull 125 is where this is blocked right now?

@hammett
Copy link
Contributor

hammett commented Feb 25, 2016

I'm aware.

Directions were given, but repeating: the PR as stands is not fully compatible, so either fork Castle.Core with our blessing or provide a PR that doesnt break Windsor. Nothing more, nothing less.

@jonorossi
Copy link
Member

Guys please relax, this is getting heated and a little disrespectful for no good reason.

I'll start by saying that @hammett doesn't speak for me (as he did mention), and I do not share the same opinions as he does (which he is definitely entitled to).

I've been involved with the Castle Project for the last decade and a maintainer for majority of that time and until recently I've never encountered so many demands (not just .NET Core related) and so little willingness to actually help. I've never been paid for a single hour of time or ever worked on an employer's time, it has been all out of love. If it wasn't obvious to those in the community the Castle Project has been suffering badly from having so few maintainers over the last few years, and ironically Microsoft has dealt the biggest blows to our community since our highs in 2007-2010 with each new project they released killing one of our own and shrinking our community.

Personally I've been mostly ignoring Castle stuff for the last few weeks because I have had a lot going on in my life, I've recently lost a non-immediate family member, also lost one of my best friends to suicide, been massively behind on consulting work and swamped, and just this week moving out of the office I sublease because it is closing with short notice.

I don't want to drone on but I've put in a couple hundred hours already to get Castle Core ported and supportable on .NET Core, and stated here in this issue (#90 (comment)) I was waiting for RC2 to be released which no one even commented to say had slipped from release early February to now having no date at all. If you don't think I was committed to get this out and don't think the work I was doing was worthwhile then I might as well give up and walk away from all of this, my efforts are obviously not appreciated.

+1s and more demands don't help things move along. #125 wasn't actually finished (but @hammett has now merged), I had asked for the PR to include all the other AssemblyInfo attributes which we'd still need to make a release (#119 (comment)), I don't know why everyone was jumping on one tiny PR.

Replies to comments since my last comment from Jan 5:

  • @thecodejunkie I don't like that you made your comment look like a quote from @hammett, but no @hammett's stance isn't the Castle Project's official stance, what's left of our PMC haven't ever discussed the topic, if it wasn't clear from @hammett's comment I'm really the only one keeping the lights on lately.
  • @adamralph no need for 👏 and disrespect, @hammett hasn't been standing in anyone's way to contributing.
  • @rjdevereux there has been help from Microsoft, I've been working with @jeremymeng over the past 6 months, no disrespect to him but I've had to hold his hand on each PR to ensure the changes keep the codebase maintainable. As I said above in this comment the PR wasn't finished, I just didn't explicitly state that because I was sidetracked.
  • @bob-devereux-zocdoc obviously not now since @hammett just merged it, I still don't know why everyone got hung up on Move [ClsCompliant(...)] to CommonAssemblyInfo.cs #125, maybe because I went dark, but there is still plenty more and I've been saying that for a while in all the different threads but I feel like the only one looking at the future of this project not the quickest way to hack everything to get a pre-release build out.

Where to from here? My opinion of DNX RC1 was that it was flaky and Microsoft was barely supporting it, but now that RC2 has been delayed until sometime in the future what do people want to do? Remember the Castle Project isn't a vendor, you don't pay anyone, you guys are the community, you have to help decide where we go, you have to get involved, I don't want to hear anymore demands, I want solutions. This issue has become long and its final purpose will be to create new issues of remaining things people can work on.

@hammett
Copy link
Contributor

hammett commented Feb 25, 2016

hey @jonorossi . I jumped in exactly coz you seem to have gone dark. Of course your involvement is appreciated. I merged that small PR exactly coz it's small. I just want this "blocking" status out of the way. Since I dont see any future release happening, IMHO the risk is very low.

@jonorossi
Copy link
Member

@hammett no problem, my fault. With every new Castle Project notification I dreaded coming back to any of this.

@hammett
Copy link
Contributor

hammett commented Feb 25, 2016

This is not an easy codebase to deal with. That may partially explain the lack of people stepping in and changing code.

I even remember as a MSFT, suggesting to the .net team to implement the same DP support -- as it's such valuable functionality that so many projects depend on -- with the welcome side effect of unburdening us, that back in 2009/10. I wish they had listened.

And from what it seems, they regret that too.

@jonorossi
Copy link
Member

@hammett I'd love all the runtime bugs to be Microsoft's problem, that way they'd probably get fixed, I know @kkozmic loved getting knocked back on MSConnect every time :). Remember the pain of .NET 3.5 SP1.

@thecodejunkie
Copy link

@jonorossi first of all let me pass on my condolences for the recent events that have taken place in your life, I truly mean that. I faced a chain of events last year which made me minimize my own commitments on thins that were not essential for my daily life.

I think there's been a chain of misunderstandings across the board here. Nobody is trying to drum up and drama not point any fingers. I never, intentionally, made anything look like a quote by @hammett I paraphrased and in fact, repeatedly, asked if I had interpreted his remarks correct, that is all. Nothing more, nothing less. No ill intentions.

+1s and more demands don't help things move along

My guess is that the +1 are not there to stress anyone (at least that's how I interpreted them), but rather highlight that this is something that is important for other people as well. Unfortunately GitHub has no upvote system in place, unlike GitLabs. But I digress. Castle.Core is a "close to the metal" component for many in our community (closed or open) and as suck when people start migrating their projects over to CoreCLR, there will be a ripple effect. I've seen it in the project I help maintain as well, which indirectly caused a ripple effect on Castle.Core (because of our usage of FakeItEasy).

Many of us here are open-source maintainers ourselves. I created Nancy 5+ years ago and have been part of the team that maintains it ever since. @adamralph have been maintaining FakeItEasy for many years now. Trust me, we really understand what it means when you say that is is a labor of love. I've invested thousands of hours on my project alone and our main "competitors" are the entire ASP.NET team and their range of frameworks and tooling teams :)

I don't want to hear anymore demands, I want solutions

The problem is that Castle.Core is a complicated project and you simply do not jump into a project like that and start pulling at the inner mechanics, there are simply too many moving parts to understand. What might be needed is if you guys could work with the community and what ever contributors you have been having in the past. This means identifying problems and create issues that are up-for-grabs to the community. That is what we did with Nancy. NancyFx/Nancy#2220 generally project maintains frowns upon someone sending massive "I changed a lot of stuff" pull-requests, so creating a list of known issues might aid contributions.

As good start could be to run the .NET Portability Analyzer on the project and extract issues from there. Our experience has also been that if you reach out to Microsoft they are quite willing to help with CoreCLR related questions and I've seen @jeremymeng do some work.

We, the team at Nancy, are more than happy to help answer questions and share our experience with our journey on being compatible with CoreCLR. I doubt anyone would feel comfortable with sending deep pull-request and we are also currently working on our own CoreCLR story

My opinion of DNX RC1 was that it was flaky and Microsoft was barely supporting it

We are currently targeting rc1 and have had very little issues with the DNX runtime, the APIs or the tooling. And this is after having worked with it for over 40 hours per week for the past 3 weeks. The API changed between rc1 and rc2 aren't that big (I think @jeremymeng can provide details here) and it will be a lot of runtime / tooling changes.

Hope this clears things up

Thanks

/Andreas

@adamralph
Copy link

@hammett

I havent been a "maintainer" since 2012 - my lack of participation on this and other threads should have made this fact abundantly clear.

Perhaps "maintainer" isn't the correct term, but you have commit rights and are exercising them (proven by your merge today of #125). This sends the message to the community that you are a person who is actively involved in the project but

I have no immediate interest in .net core or much of .net to be frank

suggests otherwise.

Again, there is nothing personal in this, my observation that the latter comment is strong grounds for stepping down as a maintainer ("committer" if you like), is purely about giving the community a sense of who is actively involved, has an interest in moving the project forward, can be polled for opinion, etc.

@adamralph
Copy link

Guys please relax, this is getting heated and a little disrespectful for no good reason.

@jonorossi really? Personally I've done my best to stay polite and approach this constructively. Can you point me to any offending comments?

@adamralph
Copy link

@adamralph no need for 👏 and disrespect, @hammett hasn't been standing in anyone's way to contributing.

@jonorossi again, this was not meant in a disrespectful way. Apologies if it was misconstrued, but I was genuinely applauding @hammett's stance on this. I have the greatest respect for it.

@adamralph
Copy link

👍 @thecodejunkie #90 (comment) largely echoes my thoughts

@jonorossi
Copy link
Member

@thecodejunkie thanks, that means a lot, 2016 has been a rollercoaster. Sorry I saw our names and the formatted quote syntax and then another set of quotes, it seemed intentional.

I really do appreciate your work, I've been using Nancy for a few years now (ever since MonoRail lost all its contributors) and you and others are running that project very well. Every time I've interacted with you guys or follow threads I've been envious how quickly you respond and get issues sorted. Since @kkozmic moved on to non-.NET work I've felt like I can't even keep up with new issues being created let alone actually fix issues myself, I'm not blaming anyone definitely not @kkozmic who has done heaps more than fair share on Castle.

Don't worry we went through that whole massive PR of hacky code that got the thing sort of compiling the middle of last year, which is why I spent hundreds of hours individually porting things across cleanly so we could continue maintaining the codebase on multiple runtimes with a whole pile of conditional compilation symbols that make sense (well to me at least).

The parts I wanted help with are all the bits around the sides (including the build), and getting log4net, NLog and Serilog integrations upgraded. It looks like we're actually still using beta8, not even rc1, I recall I couldn't get the VS unit test runner to work with rc1 but I assume that is fixed now. Someone mentioned earlier in this thread that NUnitLite got shipped quite a while after I wrote an NUnit shim over xUnit.net, but I didn't hear back from anyone to contribute anything other than mentioning it. Just logged a bunch of issues for these (tagged up-for-grabs), it is now 3:23am so I'll have to find another chance to see what else is left to do.

In terms of getting things fixed, most of the issues I logged against coreclr/corefx are assigned to rtm, but the main one I wanted fixed (and I made that clear) got set to Future without any explanation of why it couldn't be done now even though the code from the desktop CLR is still in there just compiled out.

@adamralph
Copy link

@jonorossi to echo @thecodejunkie, your efforts are hugely appreciated. As an owner of several OSS projects I know how difficult it can be to give the time you want to to these things and how often it can be a thankless task.

One good thing here is that this dialog has started and the community is being made aware of the challenges being faced. Clearly more muscle is needed to make .NET core support and other things happen so I hope that we as a community can step up and put in the effort required. @hammett has indicated a willingness to bring more committers on-board (granting write access) and for me, that is a good path forward for the project. I realise how difficult it can be to find willing and able people for this, but again, I'm hoping the community will step up here.

@jonorossi
Copy link
Member

@adamralph yep, I think I misread your emoji, one of the reasons I dislike them. It's late here (3:31am) after a crappy day. When I was referring to things getting heated, I was actually referring to the push to fork rather than just contribute here, sorry @hammett I don't want to see a fork.

We've always been willing to bring on new committers, we've just had no one stay around for long enough to get there for a long time.

For those that want to know about .NET Core stuff not our people problems, I've just created a bunch of issues to track further work, please participate in those issues, I am now closing this issue.

@shanselman
Copy link

Thanks folks, to all including @jonorossi @adamralph @hammett etc. Sorry this was a painful thread. Yes, things will get better after RC2. Do let me or @jeremymeng how we can help advocate and help. I'm working on other ways as well. @martinwoodward

@jonorossi
Copy link
Member

I know there are a lot of people subscribed to this issue so I'll post it here so as many people notice as possible. I've just released 4.0.0-alpha001 from our netcore branch:

Please log any problems you find with this version as separate issues, thanks.

@SergeySagan
Copy link

Any updates on this? Specifically for .NET Core RTM?

@castleproject castleproject locked and limited conversation to collaborators Jul 6, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests