[Feature]: Add option to allow versions of transitive dependencies to be overridden

[marcin-krystianc]

Recently we've noticed a PR that disables the transitive pinning behaviour for CPVM. It worries us because we started to rely on that feature (or rather on the unintentional side effect of that feature).

So the problem we are struggling with is the slowness of the restore operation. For large solutions that contain many projects and reference many packages the time needed for restore operation becomes unacceptable (see https://github.com/NuGet/Home/issues/10030 for details). As it is explained in the linked issue, the slowness is caused by the exponential nature of the graph walking algorithm.

We've discovered that enabling the CPVM for such solutions makes it to restore much quicker. It turns out that transitive pinning helps to reduce the impact of exponential nature of the graph walking algorithm. Because of the transitive pinning the graph walking needs to traverse the dependency graph of each centrally managed package only once. It happens only when a package node is visited from the top level node and it is skipped when visited from all other nodes (due to the "Nearest wins" rule).

I know that transitive pinning is a controversial feature. Some users want it but others really don't. But do you consider bringing it back and making it an opt-in feature as it was suggested in the comments of https://github.com/NuGet/Home/issues/10115 ? It is really important for us, so I'm happy to work on making this change if you are willing to accept it.

[rconard]

@marcin-krystianc, this is our current plan. The actual implementation may be a little rough as we get the pieces in place, but we're working on it. 👍

[jimmylewis]

Throwing my vote on this too, hoping to see the priority bumped up. Honestly, I don't know where else to file it, but PackageReference has been completely unusable in my team's repo for the last several years due to the complex (and disorganized) nature of our dependencies. CPVM with transitive dependencies was our hope to finally make the move back to standardized tooling for our dependency management in a way that would not be grossly inconvenient for our developers.

For context, my team depends on several components coming from several other teams each in their own repos which all publish to a variety of feeds (our upstream list is impressive). This results in a lot of complex dependency conflicts. The options to resolve those are to: (a) add any conflicting dependencies as primary to each project in order to force resolution of the package version (which really sucks, because it comes up all the time in new ways, and really devaules PackageReference in general), (b) try to brute-force the package versions to get to a coherent graph (which really sucks, because it's basically a trial and error process, we have literally had developers spend multiple days trying to update a single package and resolve the ensuing conflicts, sometimes without ever finding a successful combination), or (c) build a custom mechanism other than PackageReference to manage our binary dependencies (which really sucks, because it doesn't handle any other asset types, unless we basically reproduce a system to replace all of NuGet's functionality).

We've been living with (c) for a very long time, but it's a recurring tax to maintain and doesn't work with any .NET tooling features. I've already spent over a week of time trying to move to PackageReference with CPVM on the basis that it would solve our transitive version problems (because that's how the CPVM spec reads). Having transitive pinning removed explains a lot of the difficulties I've been encountering.

[tompazourek]

I have updated from Visual Studio 2019 16.8 to 16.9 (released this week) and transitive pinning doesn't seem to work anymore and lot of stuff broke just by updating VS on developers' machines (because the version of the MSBuild, which contains NuGet changes).

@cristinamanum Was the transitive pinning disable rolled out as part of Visual Studio 16.9?

The main issue for us is that the projects in solution that reference a package directly get one version, and projects that reference a package indirectly get different version, which results in package downgrades and we're back in the dependency hell... And that is even though the version is fixed in Directory.Packages.props.

Also note that the documentation still doesn't reflect the changes, the docs still say that the transitive dependencies are pinned: https://github.com/NuGet/Home/wiki/Centrally-managing-NuGet-package-versions If you have some, thanks for any information or links on the topic, it's a bit difficult to find up-to-date info...

Anyway, transitive dependency pinning is a great feature, and one of the main reasons why we adopted CPVM. So it would be awesome to have it opt-in.

[marcin-krystianc]

Hi @rconard,

are you actively working on bringing back the transitive pinning and making it opt-in feature ? If not, I'll start working on it next week. Any guidance on the implementation details will be very appreciated.

[rconard]

I will defer to @nkolev92 and @zivkan for implementation details.

I believe that we laid the groundwork with #3719 to bring this functionality back as we implemented a feature flag that could evolve into the opt-in feature that you proposed.

[zivkan]

@marcin-krystianc The NuGet team currently has other work as a higher priority, so we're not actively working on CPVM at this time.

As for guidance, I assume that the opt-in will be at a per-project level. Our msbuild property gathering and reading is spread across NuGet.targets and MSBuildStaticGraphRestore. Once the work here is done, we'll also need to create a PR on the dotnet/project-system to tell it to pass through any new properties in Visual Studio. But only do that when the PR for NuGet is done, or very, very close to done and we're certain we're not going to rename the msbuild property.

The PR you linked shows where the transitive dependencies code was #if def-ed out. Then use the debugger to see the call-stack/program flow to get from nuget.exe restore, RestoreTask.cs (for msbuild/dotnet restore), MSBuildStaticGraphRestore.cs, and VsSolutionRestoreService.cs, through to the dependency resolver code. Avoid public API changes, where you can, and introduce overloads to avoid breaking changes where you can't. As you see, additions to restore are a pain, because there are at least 4 entry-points where new msbuild properties need to be read and flowed through to RestoreCommand. We can provide assistance as the implementation progresses, but ultimately these 4 entry points will need to be tested.

Otherwise, I don't have any special insights/guidance.

If anyone thinks that maybe there should be a nuget.config setting, rather than a per-project setting, then we probably need a design spec to discuss the various options and decide before wasting too much time on an implementation that gets partially thrown away if we decide to go down another route. A nuget.config setting would make the implementaton significantly easier (and avoid needing a PR on project-system). But I'm not sure if transitive pinning makes sense as a per-project setting, or a nuget.config setting, or both. Maybe this needs customer development and therefore PM work, and therefore shouldn't have any implementation started until we've decided how the opt-in should work. Maybe we do need a design spec first.

[batzen]

@zivkan Why were the changes made in the first place? Whats the purpose of CPVM when we can't manage versions centrally anymore? If some "internal customers" don't want transitive version pinning, why are they using CPVM in the first place?

[zivkan]

The discussion is in https://github.com/NuGet/Home/issues/10115. As you can see, the people who dislike the functionality are not Microsoft employees.

[tompazourek]

@batzen I think that without transitive dependency pinning, CPVM essentially becomes just a very simple cosmetic feature, just like syntactic sugar. It's just to save you repeating the version number in many places. It's still a little bit useful, but not very much.

It's the transitive dependency pinning that actually solved a difficult problem, and that is to get the versions of the dependencies under control and get out of the dependency hell in complex many-project solutions. Without transitive dependency pinning, you do actually use different versions of packages in different projects. For example, your test projects might end up using a different version of a library, because some direct dependency is missing somewhere in the chain (basically you can be testing something different than what you're running)... Transitive dependency pinning simplifies dependency management greatly for large complex solutions. It makes it much less fragile. With it, when someone adds a single <PackageReference> to a single project, you have more confidence that it doesn't change the dependency trees and resolved versions as much in unexpected places.

But some library authors didn't like that it changed the dependency surface of their libraries, and so it was disabled entirely without a way to opt-in, and now no one can have the nice things. 😞 I believe CPVM is the most useful for really large solutions with many projects, and that it's much more useful for applications than for libraries. And that's because applications need to have the runtime binding redirects set up correctly, and you actually have full control over the dependencies. For libraries, it's still the consumer of the library who has the full control, so the transitive dependency pinning isn't that useful. You can set it up in some way, but depending on the consumer of the library, the actual dependency graphs and assemblies that end up in runtime might look completely different.

I believe the decision to disable transitive dependency pinning entirely without providing an opt-in was a rushed decision and also a wrong decision to make. Just because transitive dependency pinning isn't a perfect fit for library authors, doesn't mean that it's not useful anywhere. That's why I'd love to see a comeback of that feature. And from what I read in these threads about CPVM, many other people actually do find it useful as well.

[jimmylewis]

Whole-heartedly agree with @tompazourek. Transitive pinning is the hugely appealing for managing dependencies; without it, trying to micro-manage dependency versions eventually leads to adding layers of transitive dependencies as primary references, which is little better than packages.config.

If it's just pinning primary reference versions, that problem was already solved a number of ways (maybe less aesthetically pleasing than the Directory.Packages.props file, but solved nonetheless).

[marcin-krystianc]

I've finally made the change to bring the transitive dependency feature back, go to https://github.com/NuGet/NuGet.Client/pull/4025 for more details

[KirillOsenkov]

Adding my voice in support of bringing back transitive pinning. The vast majority of consumers want this behavior, and we should either turn it back on or at least make it optional (and I strongly suggest defaulting it back to on, so that in the rare case where you need it off people can turn it off).

Optimize for the vast majority (not to mention that turning it off was a breaking change which should have just added an option instead of a hard break).

[avivanoff]

Please make this happen. https://github.com/NuGet/NuGet.Client/pull/4025 has been there for four months now.

[lazy8]

Hello everyone, what's the status on this issue?

The "NeedsTriageDiscussion" tag was added and then removed again, but I don't see any kind of discussion or outcome, or any kind of activity at all, aside from people who just need the feature CPVM to be working again.

There is a PR that fixes this issue (NuGet/NuGet.Client#4025), and it's assigned to @nkolev92, but he was unassigned here. What does that mean?

Also, it has been tagged as a "NewIssue" when it was argued here quite convincingly that this is rather a defect, because the feature "CPVM" is currently broken or unusable for most consumers, by a behavior change introduced in Nuget 5.9 with no workaround.

[JonDouglas]

Hey everyone,

First off, thank you to everyone involved on this thread. I'm hopefully going to help get everyone up to speed as I just returned from parental leave and will be helping out here in the future.

I want to express some gratitude to @marcin-krystianc for the PR that brings this feature back as an opt-in property. This PR is significant enough to need a further technical pass and we are actively reviewing its implications. We’ll reply shortly once we’ve gone through the PR with an update.

We are working on a long-term plan that we will communicate to everyone with regards to the CPVM feature-set as a whole and its on-going development.

If you’d like to chat with us about this feature in the short-term, please book a call with us here:

aka.ms/talktonuget

We apologize in advance for all the confusion & delay in getting a response out to everyone. Thanks for your patience as we try to make this right.

[JonDouglas]

Hi everyone,

It's been two weeks since I last posted, I wanted to bring everyone up to speed. We have been busy interviewing many of you on this thread & other channels about this feature and understanding the pain, workarounds, and perspectives. A big thank you to so many of you who booked a call already as your feedback is extremely helpful!

We will be writing up a document to help share our learnings, challenges, and more very shortly. Be on the look-out for that sometime next week where you can help us out by providing your feedback & comments on.

There is one major question we have with regards to this feature that a quick reaction to this comment can help solve. Although we have got a good idea of the direction from interviews & comments made already in various threads, let's try a quick poll below.

Should transitive pinning be made an opt-in or opt-out feature?

• 👍 - Is a vote for opt-in. Transitive pinning will be off by default and you would be expected to opt-in to the feature for the desired behavior.
• 👎 - Is a vote for opt-out. Transitive pinning will be on by default and you would be expected to opt-out of the feature for any undesired behavior.

Thanks and have a great weekend.

[chrise9]

Hi @JonDouglas - do you have any updates on this? It's been a few weeks since your last post and it would be good to get an idea as to when progress might be made. Thanks.

[JonDouglas]

I posted on the PR this morning. I will have a larger update shortly. Sorry for the wait. I owe an update here.

[JonDouglas]

Hi all,

We recently published a small document/proposal that includes a few thoughts around this behavior & some questions that would further help us move this feature forward. If would mean a lot if you can provide your feedback in the PR itself:

https://github.com/NuGet/Home/pull/11391

Thanks!

[tebeco]

As long as it's optional that's fine. A platform can only grow better when customer can choose what they want to opt-in / opt-out

[JonDouglas]

Hi friends,

We're getting closer to finalized PRs for both version overrides and optional transitive pinning. We'd really appreciate any last comments on the following proposal before we merge it as it has some limitations you all may be aware of for transitive pinning scenarios.

https://github.com/NuGet/Home/pull/11569

[batzen]

optional transitive pinning

Does that wording mean transitive pinning is off by default? That would be very surprising as the community voted for on by default.

[tebeco]

it means that people that voted against as part of the community will be able to have a choice.

This is a win/win. You want it ? it's on by default you don't want it ? there a flag to disable it

What would be the concern that it serves more people ?

[forki]

2022 and pinning deps is still controversial in .NET. 😂

[lazy8]

@batzen

optional transitive pinning

Does that wording mean transitive pinning is off by default?

That was also my first knee-jerk reaction and had me scratching my head a bit ... but if you read the proposal, it talks about an opt-in to override the central version (via VersionOverride). I read that as: Transitive pinning will actually be the default, but you can opt-in to override that at project-level if you need/want. And, in addition, you can centrally disallow the use of that option. This would be perfectly in line with the results of @JonDouglas's poll taken above, that you were referring to. But please correct me if I misunderstood it.

[Smartisa]

There's another question that bothers me for a long time. Is there a way to know the top-level dependencies from "packages. config"? Thanks!