Skip to content

Conversation

@JunTaoLuo
Copy link

@JunTaoLuo JunTaoLuo commented Apr 14, 2020

Addresses item in dotnet/aspnetcore#19254

As we discussed previously, we wanted to understand the changes in out package graph and make appropriate announcements. I made a quick analyzer that found the following.

I've noticed that dotnet/runtime does a few things differently:

  1. If no code from a package dependency is actually used, it is not included as a dependency in the package produced. This can lead to breaking changes.
  2. If code from a transitive dependency is used, it will be included as a direct dependency. Though this results in a different set of dependencies listed in the package, I believe it does not cause a breaking change, especially given that we service all of those dependencies.

I've opened this as a PR so we can review each change if necessary. However, I don't plan to check this in.

@JunTaoLuo JunTaoLuo requested review from a team and analogrelay April 14, 2020 22:26
Package: Microsoft.Extensions.Configuration.Binder
TFM: .NETStandard2.0
Removed:
Microsoft.Extensions.Configuration
Copy link
Author

Choose a reason for hiding this comment

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

In this case, it turns out only code from M.E.Configuration.Abstraction is used. So the package removed the M.E.Configuration dependency and added a M.E.Configuration.Abstractions dependency.

Package: Microsoft.Extensions.Hosting.Abstractions
TFM: .NETCoreApp5.0
Removed:
Microsoft.Extensions.Logging.Abstractions
Copy link
Author

@JunTaoLuo JunTaoLuo Apr 14, 2020

Choose a reason for hiding this comment

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

This is a change we noticed during our ingestion. Along with the change in M.E.Options

Package: Microsoft.Extensions.DependencyInjection
TFM: .NETFramework4.6.1
Added:
System.Threading.Tasks.Extensions
Copy link
Author

Choose a reason for hiding this comment

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

I just checked, these were in the previous closure as well since Microsoft.Bcl.AsyncInterfaces included a reference to it so this is a false positive. I didn't include that package in the set of packages I was analyzing. The same goes for M.E.Hosting.Abstraction.

Copy link
Member

@ericstj ericstj left a comment

Choose a reason for hiding this comment

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

These all look reasonable. Thank you for analyzing this.

Copy link

@analogrelay analogrelay left a comment

Choose a reason for hiding this comment

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

Assuming none of the assemblies in these packages were actually used, this looks fine to me.

Package: Microsoft.Extensions.Logging.EventLog
TFM: .NETFramework4.6.1
Removed:
System.Diagnostics.EventLog

Choose a reason for hiding this comment

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

Is this because the type is just part of .NET Framework 4.6.1? (@ericstj ?)

Copy link
Member

Choose a reason for hiding this comment

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

Yes

@JunTaoLuo
Copy link
Author

Announcement made in aspnet/Announcements#415

@JunTaoLuo JunTaoLuo closed this Apr 20, 2020
@mmitche mmitche deleted the johluo/packages branch January 4, 2021 21:32
@ghost ghost locked as resolved and limited conversation to collaborators May 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants