Skip to content

Stop cascading version changes in servicing updates #3316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
natemcmaster opened this issue Jul 11, 2018 · 8 comments
Closed

Stop cascading version changes in servicing updates #3316

natemcmaster opened this issue Jul 11, 2018 · 8 comments
Assignees
Labels
Done This issue has been fixed Servicing-approved Shiproom has approved the issue task
Milestone

Comments

@natemcmaster
Copy link
Contributor

natemcmaster commented Jul 11, 2018

Our policy since 1.0.0 has been to always cascade version updates in the packages we own. e.g. if Logging has a product change in 2.1.x, then Kestrel, EF Core, Mvc, etc also re-ship with the updated Logging dependency. This has been done for a variety of reasons:

  • NuGet does not show updates for transitive dependencies, only direct ones
  • NuGet does resolves the lowest compatible transitive dependencies
  • ASP.NET Core ships to both .NET Framework (where transitive dependency version matters) and .NET Core (where it matters less if you use the shared framework)

While transitive dependencies is still an important scenario, this practice of always patching has led to bigger issues.

Proposal

Change 1

Stop cascading versions for every servicing patch.

For example, after 2.2.0 is finished, patching Logging to 2.2.1 would not necessarily mean Kestrel, Mvc, and SignalR need a 2.2.1 release.

Exceptions
This is the general rule, but there may arise important scenarios when the transitive version matters. When these occur, we will make manual updates. Examples may include:

  • security issues
  • high-impact functional changes in patches
  • (unlikely to happen -- in theory) API additions or changes in a patch

Implementation plan
We'll start by implementing this within ASP.NET Core on a repo by repo basis. Doing this from package to package within a repo is currently prohibitively expensive due to the way we use ProjectReference and nuspec generation.

Change 2

Work with NuGet to provide better tooling for finding and updating out of date transitive references. Some proposals currently in the incubation: NuGet/Home#5762, NuGet/Home#2867

Not changing

Metapackages, such as Microsoft.AspNetCore, AspNetCore.App, and AspNetCore.All will continue to cascade versions. These provide the best way for users to get the latest set of things, without hoisting dozens of transitive dependencies to be direct dependencies.

cc @DamianEdwards @davidfowl @Eilon @muratg

@natemcmaster natemcmaster added 1 - Ready task Servicing-consider Shiproom approval is required for the issue labels Jul 11, 2018
@natemcmaster natemcmaster added this to the 2.1.3 milestone Jul 11, 2018
@natemcmaster natemcmaster self-assigned this Jul 11, 2018
@muratg muratg modified the milestones: 2.1.3, 2.1.4 Jul 11, 2018
@muratg muratg removed the Servicing-consider Shiproom approval is required for the issue label Jul 11, 2018
@muratg muratg modified the milestones: 2.1.4, 2.1.3 Jul 11, 2018
@muratg muratg added the Servicing-consider Shiproom approval is required for the issue label Jul 11, 2018
@Eilon
Copy link
Contributor

Eilon commented Jul 11, 2018

I dig this.

@natemcmaster
Copy link
Contributor Author

There are a lot of product changes going into 2.1.3 so there are still a decent number of packages in the next update, but this change in strategy reduces the packages shipping from 192 to 87. I sent mail examining the effects of this change.

@cwe1ss
Copy link
Contributor

cwe1ss commented Jul 12, 2018

You've put in quite some effort to make versioning easier/consistent in the .NET Core ecosystem so this would be a step back in my opinion. It makes updating dependencies much harder for projects that do not depend on a meta package. Right now, I can just easily replace all "2.1.1" packages with "2.1.2".

Will you "skip" version numbers when you patch a library that hasn't been patched in a previous servicing release?

Example: In ASP.NET Core 2.1.3, Microsoft.Extensions.Configuration will not be patched and stays on "2.1.2". However, in ASP.NET Core 2.1.4 it needs a patch - will it get "2.1.3" or "2.1.4" (and therefore skip a version)?

If you don't skip versions, it will result in a mess where nobody knows which assemblies belong to a certain servicing release. (Microsoft.Extensions.Configuration 2.1.3 belongs to ASP.NET Core 2.1.4 and not ASP.NET Core 2.1.3)

@natemcmaster
Copy link
Contributor Author

While I agree the versioning consistency is maddening when not using a metapackage, I don't think this isn't going to make that problem better or worse. The current servicing strategy still leaves a jagged edge. For example, if you look at the dependencies of Microsoft.AspNetCore.App v2.1.2, you'll see that the latest versions of our packages are a mix of 2.1.0, 2.1.1, and 2.1.2.

Versioning consistency is something I don't think we can address in the 2.x timeframe, but it's one of the things I'd like to reconsider for 3.0.

@cwe1ss
Copy link
Contributor

cwe1ss commented Jul 12, 2018

thanks for your response @natemcmaster ! What do you think about my comment regarding "skipping" versions to at least ensure that any changed package has the same version as the meta package?

@natemcmaster
Copy link
Contributor Author

I think it introduces another problem -- how do I explain why 2.1.0 and 2.1.3 exist, but why there is no version in between? These tradeoffs are hard to make, but where we have currently landed is that packages version unto themselves.

@cwe1ss
Copy link
Contributor

cwe1ss commented Jul 12, 2018

Sure, that's much less of an issue though than having to explain why Some.Package 2.1.3 does NOT belong to ASP.NET Core 2.1.3 but to e.g. 2.1.4

Anyway - just wanted to post my opinion. I can probably live with whatever you guys decide 😄.

This was referenced Jul 12, 2018
@natemcmaster
Copy link
Contributor Author

This work was done in aspnet/Universe#1257 and will be part of 2.1.3

@natemcmaster natemcmaster added Done This issue has been fixed and removed 2 - Working labels Jul 13, 2018
@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Done This issue has been fixed Servicing-approved Shiproom has approved the issue task
Projects
None yet
Development

No branches or pull requests

4 participants