Self host project should also target netstandard2.0

NancyFx / Nancy

Lightweight, low-ceremony, framework for building HTTP based services on .Net and Mono

http://nancyfx.org

MIT License

7.15k stars 1.47k forks source link

Self host project should also target netstandard2.0 #2957

Closed Yitzchok closed 5 years ago

Yitzchok commented 5 years ago

Prerequisites

[x] I have written a descriptive pull-request title
[x] I have verified that there are no overlapping pull-requests open
[x] I have verified that I am following the Nancy code style guidelines
[x] I have provided test coverage for my change (where applicable)

Description

Support for self hosting in a netstandard2.0 project.

dnfclas commented 5 years ago

All CLA requirements met.

davidindra commented 5 years ago

I am also interested in this PR, when will it be merged?

grumpydev commented 5 years ago

Not sure about how the username is removed, would probably make more sense to remove the automatic reservation stuff if there's no way to get the username that's required to do it?

Yitzchok commented 5 years ago

Let me see if it will work with https://www.nuget.org/packages/System.Security.Principal.Windows/

cocowalla commented 5 years ago

AFAIK, URL reservations are only required if self hosting with http.sys (which you'd only use over Kestrel if you needed Windows authentication)?

And of course http.sys is Windows only, so perhaps the conditional checks here should be for the presence of Windows, rather than .NET Core?

grumpydev commented 5 years ago

@cocowalla is there a simple way to do that at runtime?

khellang commented 5 years ago

This package won't work on non-Windows OSes anyway, right? I'm surprised it even compiles for netstandard2.0...

grumpydev commented 5 years ago

@khellang not checked the code, but I assumed it would just be a noop on non-windows platforms?

khellang commented 5 years ago

So apparently it was ported to Core with 2.0 and subsequently brought into the standard. Still seems to be based on HTTP.SYS though; https://github.com/dotnet/platform-compat/issues/88

khellang commented 5 years ago

Anyway, you can use RuntimeInformation.IsOSPlatform to check OS at runtime: https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.runtimeinformation.isosplatform?view=netframework-4.8

grumpydev commented 5 years ago

That would mean changing the .net target to be 471 though i guess? As long as it runs as is on linux/osx, and doesn't blow up because that package is there it should be fine, that auto registration code is only a fallback if it doesnt have permission to bind (which should only be the case on windows)

khellang commented 5 years ago

That would mean changing the .net target to be 471 though i guess?

I think it's fine as it is now, using the System.Security.Principal.Windows package. Not sure what it does to Mono support, though.