Remove dnx451 and dnxcore50 TFM's #518
Conversation
|
Hi @henkmollema, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution! The agreement was validated by .NET Foundation and real humans are currently evaluating your PR. TTYL, DNFBOT; |
|
Meh, apparently it's not that simple.. |
|
Looks like it might be the |
|
Did you check the code for any |
|
@CesarBS yeah there are some. I was trying to get rid of them but I got some vague errors in |
|
@davidfowl / @CesarBS what to do with this guy: var isDarwinMono =
#if DNX451
IsWindows ? false : PlatformApis.IsDarwin;
#else
false;
#endifIt's in |
|
Maybe this is the way to go? http://stackoverflow.com/a/721194/719967 Are you able to test if everything works on Mac OS X w/ Mono after this change? |
|
Use |
|
It builds after removing the I'm not really sure how to test this correctly. Perhaps it's better if one of you guys take over? |
|
That sounds about right. |
| #else | ||
| false; | ||
| #endif | ||
| var isDarwinMono = !IsWindows && PlatformApis.IsDarwin; |
There was a problem hiding this comment.
This check isn't right though.
There was a problem hiding this comment.
I figured.. Not sure how to replace the #if DNX451.
There was a problem hiding this comment.
Microsoft.Extensions.PlatformAbstractions:
RuntimeType
There was a problem hiding this comment.
@henkmollema Do you plan on making the change to use RuntimeType?
There was a problem hiding this comment.
Whoops, forgot about this one. Yes, I'll look into it right not.
/cc @davidfowl @halter73
Just a test - this seems to fix the build
|
@halter73 @davidfowl updated it with |
|
And how about the |
|
Thanks. It looks good, but I'm not sure about removing I'm going to look into it and get back to you. |
|
It's going to have to be removed as part of the move to the CLI. |
| @@ -24,12 +25,16 @@ static PlatformApis() | |||
| IsDarwin = string.Equals(GetUname(), "Darwin", StringComparison.Ordinal); | |||
| } | |||
| #endif | |||
|
|
|||
| IsMono = PlatformServices.Default.Runtime.RuntimeType == "Mono"; | |||
There was a problem hiding this comment.
I think we will need/be able to get rid of this check and the entire [DllImport("__Internal" hack once we move away from dnx. Users will have to install libuv on Mac when using Mono.
|
Do you guys want me to do anything for this PR? /cc @davidfowl / @halter73 |
| @@ -1,6 +0,0 @@ | |||
| namespace Microsoft.AspNet.Server.Kestrel | |||
| { | |||
| public class StandardsPoliceCompileModule : Microsoft.StandardsPolice.StandardsPoliceCompileModule | |||
There was a problem hiding this comment.
So we are no longer getting any warning from standards police after this is removed, but I see the issue you are running into. Having any .cs file in the compiler/preprocess folder is causing dnu build to fail with a null ref violation.
@davidfowl Any idea why this is happening now?
There was a problem hiding this comment.
Because of the target framework changes. It can't find the right dependencies in the lock file. Null ref is bad but DNX is gone anyways.
We can turn this into an analyzer once we get support in the cli for that
|
Thanks! |
|
😄 |
As per aspnet/Hosting#531.