Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Déjà vu #2392

Merged
merged 2 commits into from
Mar 14, 2018
Merged

Déjà vu #2392

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f5a1bb1
Default to the managed socket transport
halter73 Mar 14, 2018
d870bca
Add libuv project reference to sample apps
halter73 Mar 14, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions samples/SampleApp/SampleApp.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
<ItemGroup>
<ProjectReference Include="..\..\src\Kestrel\Kestrel.csproj" />
<ProjectReference Include="..\..\src\Kestrel.Https\Kestrel.Https.csproj" />
<ProjectReference Include="..\..\src\Kestrel.Transport.Libuv\Kestrel.Transport.Libuv.csproj" />
</ItemGroup>

<ItemGroup>
Expand Down
1 change: 1 addition & 0 deletions samples/SystemdTestApp/SystemdTestApp.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

<ItemGroup>
<ProjectReference Include="..\..\src\Kestrel\Kestrel.csproj" />
<ProjectReference Include="..\..\src\Kestrel.Transport.Libuv\Kestrel.Transport.Libuv.csproj" />
</ItemGroup>

<ItemGroup>
Expand Down
3 changes: 0 additions & 3 deletions src/Kestrel/Kestrel.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@
<ItemGroup>
<ProjectReference Include="..\Kestrel.Core\Kestrel.Core.csproj" />
<ProjectReference Include="..\Kestrel.Https\Kestrel.Https.csproj" />

<!-- Even though the Libuv transport is no longer used by default, it remains for back-compat -->
<ProjectReference Include="..\Kestrel.Transport.Libuv\Kestrel.Transport.Libuv.csproj" />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@natemcmaster I left this comment the first time I switched the default transport to managed sockets. Doesn't removing this dependency break back-compat?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the break only we're willing to take for 2.1.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be specific, I think this change will break any existing app that references just the main Kestrel package and calls UseLibuv.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is an intentional breaking change that we decided is worth taking. It should only affect a small group of people since we are keeping Transport.Libuv in the .All metapackage. It only breaks for people who depend directly on the .Kestrel package, and they can fix this break by adding a package reference to K.T.Libuv when they upgrade to 2.1. IMO we should post this to aspnet/announcements as a heads up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that's known. This will be documented as a breaking change in 2.1.

/cc @DamianEdwards @Eilon

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aspnet/Announcements#296

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you breaking semantic versioning by introducing breaking change in a minor version increment?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of the reasons listed here #2360. A few things are changing in 2.1 as well. We’re introducing a shared framework for ASP.NET Core and we’d prefer it if libuv wasn’t part of that. Also we feel like it will break few enough people that it shouldn’t matter that much. The workaround for people is also trivial and if you’re upgrading from ASP.NET Core 2.0 to 2.1 and you’re using the .All package everything still works.

Where things will break is if you were using .net Framework with libuv and were configuring the API explicitly (to say tweak the number of threads)

<ProjectReference Include="..\Kestrel.Transport.Sockets\Kestrel.Transport.Sockets.csproj" />
</ItemGroup>

Expand Down
4 changes: 2 additions & 2 deletions src/Kestrel/WebHostBuilderKestrelExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
using Microsoft.AspNetCore.Server.Kestrel.Core;
using Microsoft.AspNetCore.Server.Kestrel.Core.Internal;
using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal;
using Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv;
using Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.DependencyInjection.Extensions;
using Microsoft.Extensions.Options;
Expand All @@ -29,7 +29,7 @@ public static IWebHostBuilder UseKestrel(this IWebHostBuilder hostBuilder)
return hostBuilder.ConfigureServices(services =>
{
// Don't override an already-configured transport
services.TryAddSingleton<ITransportFactory, LibuvTransportFactory>();
services.TryAddSingleton<ITransportFactory, SocketTransportFactory>();

services.AddTransient<IConfigureOptions<KestrelServerOptions>, KestrelServerOptionsSetup>();
services.AddSingleton<IServer, KestrelServer>();
Expand Down
1 change: 1 addition & 0 deletions test/Kestrel.Tests/Kestrel.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

<ItemGroup>
<ProjectReference Include="..\..\src\Kestrel\Kestrel.csproj" />
<ProjectReference Include="..\..\src\Kestrel.Transport.Libuv\Kestrel.Transport.Libuv.csproj" />
<PackageReference Include="Microsoft.AspNetCore.Testing" Version="$(MicrosoftAspNetCoreTestingPackageVersion)" />
</ItemGroup>

Expand Down
4 changes: 2 additions & 2 deletions test/Kestrel.Tests/WebHostBuilderKestrelExtensionsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,13 @@ public void ApplicationServicesNotNullDuringUseKestrelWithOptions()
}

[Fact]
public void LibuvIsTheDefaultTransport()
public void SocketTransportIsTheDefault()
{
var hostBuilder = new WebHostBuilder()
.UseKestrel()
.Configure(app => { });

Assert.IsType<LibuvTransportFactory>(hostBuilder.Build().Services.GetService<ITransportFactory>());
Assert.IsType<SocketTransportFactory>(hostBuilder.Build().Services.GetService<ITransportFactory>());
}

[Fact]
Expand Down