Ingest updated packages from dotnet/runtime #20300
Conversation
Pilchie
added
the
area-infrastructure
JunTaoLuo
force-pushed
the
johluo/ingest-runtime
branch
from
a8bf675 to
2ef5d0a
Compare
| @@ -28,6 +28,9 @@ Microsoft.Extensions.Diagnostics.HealthChecks.IHealthChecksBuilder | |||
| <Reference Include="Microsoft.Extensions.Diagnostics.HealthChecks.Abstractions" /> | |||
| <Reference Include="Microsoft.Extensions.Hosting.Abstractions" /> | |||
| <Reference Include="Microsoft.Extensions.Options" /> | |||
|
|
|||
| <!-- TODO: Remove this workaround --> | |||
| <Reference Include="Microsoft.Extensions.Logging.Abstractions" /> | |||
There was a problem hiding this comment.
@ericstj @maryamariyan Here's an interesting change. The package M.E.Hosting.Abstractions built in Extensions has a dependency on M.E.Logging.Abstractions:
<group targetFramework=".NETCoreApp5.0">
<dependency id="Microsoft.Extensions.Configuration.Abstractions" version="5.0.0-preview.4.20201.2" exclude="Build,Analyzers" />
<dependency id="Microsoft.Extensions.DependencyInjection.Abstractions" version="5.0.0-preview.4.20201.2" exclude="Build,Analyzers" />
<dependency id="Microsoft.Extensions.FileProviders.Abstractions" version="5.0.0-preview.4.20201.2" exclude="Build,Analyzers" />
<dependency id="Microsoft.Extensions.Logging.Abstractions" version="5.0.0-preview.4.20201.2" exclude="Build,Analyzers" />
</group>however, the one built from runtimes does not:
<group targetFramework=".NETCoreApp5.0">
<dependency id="Microsoft.Extensions.Configuration.Abstractions" version="5.0.0-preview.4-runtime.20201.16" />
<dependency id="Microsoft.Extensions.DependencyInjection.Abstractions" version="5.0.0-preview.4-runtime.20201.16" />
<dependency id="Microsoft.Extensions.FileProviders.Abstractions" version="5.0.0-preview.4-runtime.20201.16" />
</group>I'm able to work around this by adding an explicit reference in projects like this but I'm curious why this is happening. Is dotnet/runtime doing some smart trimming of unused references? I took a look at Hosting.Abstractions and it seems like it doesn't actually use anything from Logging.Abstractions. Is that why that reference is removed?
There was a problem hiding this comment.
The packaging system in dotnet/runtime only includes PackageReferences based on assembly references in the final assembly. So if a reference is never used it won't end up in the package.
There was a problem hiding this comment.
Wouldn't this be a breaking change? If we are okay with making the breaking change, we should post an announcement.
There was a problem hiding this comment.
We typically never make this type of breaking change as our packages only ever expose types which are part of the reference surface area and we never remove types, so it's not something we really do. ASP.NET must make changes to package dependencies all the time: do you document any time you remove a PackageReference from your implementation? The fix is for people to simply add a reference to the package they are already using.
There was a problem hiding this comment.
I think we would need to document it but I can't find an example of when we did it in the past. @anurse do we need to announce that we're removing a package dependency from a package? For example, M.E.Hosting.Abstractions no longer depends on M.E.Logging.Abstractions so the user will need to add an explicit reference to it in their csproj.
There was a problem hiding this comment.
@JunTaoLuo if you've added all "lost" Microsoft.Extensions.Logging.Abstractions references, this dependency removal is not a breaking change in the shared framework nor in any package shipped from dotnat/aspnetcore. Have you confirmed that?
There was a problem hiding this comment.
No, I didn't address the other packages/assemblies that are affected, for example https://github.com/dotnet/aspnetcore/blob/master/src/HealthChecks/HealthChecks/src/Microsoft.Extensions.Diagnostics.HealthChecks.csproj#L30 is now missing a transitive dependency on System.ComponentModel.Annotations. However, I want to address this separately. After another look at the runtime/ref packs, I'd like to get this in.
There was a problem hiding this comment.
I would say we can make an announcement in aspnet/Announcments and call it good. It's a pretty minor breaking change in a major version.
|
@dotnet/aspnet-build @dotnet/extensions-migration This PR is ready for another look. I've fixed up all the remaining failing tests. |
There was a problem hiding this comment.
but we may need to react to resolution of "breaking change" conversation
| @@ -28,6 +28,9 @@ Microsoft.Extensions.Diagnostics.HealthChecks.IHealthChecksBuilder | |||
| <Reference Include="Microsoft.Extensions.Diagnostics.HealthChecks.Abstractions" /> | |||
| <Reference Include="Microsoft.Extensions.Hosting.Abstractions" /> | |||
| <Reference Include="Microsoft.Extensions.Options" /> | |||
|
|
|||
| <!-- TODO: Remove this workaround --> | |||
| <Reference Include="Microsoft.Extensions.Logging.Abstractions" /> | |||
There was a problem hiding this comment.
@JunTaoLuo if you've added all "lost" Microsoft.Extensions.Logging.Abstractions references, this dependency removal is not a breaking change in the shared framework nor in any package shipped from dotnat/aspnetcore. Have you confirmed that?
|
I checked the ref pack and the runtime packs for Windows-x64 and LInux-x64. All seems correct. |
Uh oh!
There was an error while loading. Please reload this page.