aspnet / KestrelHttpServer

[Archived] A cross platform web server for ASP.NET Core. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
2.63k stars 528 forks source link

Remove Microsoft.AspNetCore.Server.Kestrel.Https package #3101

Closed Tratcher closed 5 years ago

Tratcher commented 5 years ago

The contents were merged into Kestrel.Core in 2.1 and this assembly now only contains type forwarders.

davidfowl commented 5 years ago

What does this mean exactly? Remove just the package or the DLL?

Tratcher commented 5 years ago

Both.

davidfowl commented 5 years ago

I'm not sure we should do that but maybe it will break very little people.

Tratcher commented 5 years ago

Worst case someone has a dead package reference they need to remove, but not many people would have had that package reference to begin with, it would have been transitive.

davidfowl commented 5 years ago

No, worst cast is that they get an assembly load failure because they referenced a type in that assembly that is no longer able to forward to the main assembly.

Tratcher commented 5 years ago

Except moving to 3.0 requires a recompile that would prevent that.

Eilon commented 5 years ago

Legacy 3rd party packages could be referencing the old types, no?

Tratcher commented 5 years ago

Unlikely, look at the type list: https://github.com/aspnet/KestrelHttpServer/blob/5c1fcd664d39db8fe5c8e38052a3cc29f90322f6/src/Kestrel.Https/Properties/AssemblyInfo.cs#L9-L12

These types are used almost exclusively in Program.cs.

davidfowl commented 5 years ago

You can't know that, it's unlikely but possible for a library to use one of these types.

Tratcher commented 5 years ago

So remove it in preview1 and collect feedback.

muratg commented 5 years ago

@davidfowl Do you really want to carry forward forever? We can do some breaking changes.

davidfowl commented 5 years ago

@muratg yes, it's a judgement call. While I agree the risk here is lower than the average assembly I wouldn't remove it unless we had some data that libraries didn't depend on it (3rd parties, not our own).

Tratcher commented 5 years ago

@shirhatti, @glennc you had some nuget dependencies data that might answer @davidfowl's question, right?

Tratcher commented 5 years ago

I scraped some nuget data, here are a few hits:

Date: 10/1/2018 12:00:00 AM - 10/31/2018 12:00:00 AM Kaze 1.1.8 - Uses 2.1, types already moved Microsoft.AspNetCore.App 2.1.5 Microsoft.AspNetCore 2.1.4 Microsoft.AspNetCore.All 2.1.5 Microsoft.AspNetCore.Server.Kestrel 2.1.2 OPCFoundation.NetStandard.Opc.Ua.Symbols 1.4.354.19-preview OPCFoundation.NetStandard.Opc.Ua 1.4.354.19-preview - Uses 2.0, could break Wlniao.XCore 2.1.8 - No code references, only packages ZoroChain 0.1.0 - Uses 2.1, types already moved Neo 2.9.1 - Uses 2.1, types already moved Prometheus.Client.MetricServer 2.0.0 - Uses 2.0, could break Wlniao.XCore 2.0.4 - Uses 2.1, types already moved WireMock.Net 1.0.4.18 - Multi-target netstandard 1.3 and 2.0, would not break.

Summary: Most libraries have moved to 2.1 and would not be affected. There are a few still using 2.0 that could break.

davidfowl commented 5 years ago

Prometheus.Client.MetricServer 2.0.0 - Uses 2.0, could break

That one seems important. I don't know what the other ones are

OPCFoundation.NetStandard.Opc.Ua 1.4.354.19-preview - Uses 2.0, could break

Not could, will break.

Tratcher commented 5 years ago

Understood, but that doesn't mean the dll should stay indefinitely. There's a difference between compat and paralysis, we need a depreciation procedure for stuff like this.

The package is already going away in 3.0, only the assembly is left. We've done the first step of type forwarding for two minor releases. Are there any other mechanics for obsoleting an assembly? Removal in the next major version is the standard for obsolete components, especially with plenty of notice/previews. We can even contact some of these projects to proactively warn them.

davidfowl commented 5 years ago

I think it’s fine to remove things once we have gone through the process of obsoleting things in prior releases. It’s no good to break in the next major without deprecating in the versions before it. It’s on us to make sure we’re a bit more rigorous when we do these things so that more people are aware.

Tratcher commented 5 years ago

This assembly has been empty except forwarders for two minor releases. What more could have been done? I'm not aware of any additional mechanics for obsoleting an assembly.

davidfowl commented 5 years ago

What more could have been done? I'm not aware of any additional mechanics for obsoleting an assembly.

Writing an announcement?

Tratcher commented 5 years ago

We can do that now. They'll have a full release cycle to react.

Tratcher commented 5 years ago

https://github.com/aspnet/Announcements/issues/329

davidfowl commented 5 years ago

Can you also file issues on those projects that we know will break linking them to the announcement?

Tratcher commented 5 years ago

Removed in Preview2 (looks like this missed the Preview1 deadline by 1 day). I'll followup with any packages targeting 2.0.x, though the message there is a more broad "2.0 is out of support, please move to 2.1".

Tratcher commented 5 years ago

90% of the libraries targeting kestrel.https 2.0 were blockchain libraries, and most of those were copies of eachother that had only ever uploaded one or two versions.

davidfowl commented 5 years ago

Sure still reach out by filing issues

Tratcher commented 5 years ago

Already done.

davidfowl commented 5 years ago

You da man