[CPVM] Add support for optional pack the pinned transitive dependencies.

[cristinamanum]

Details about Problem

One of the features that CPVM supports is enforcing transitive dependencies to the central defined versions. As all the testing and development for an application is done using the specified dependency set, on pack the enforced/pinned transitive dependencies are packed as well.

However this may not be the desired experience for some of the scenarios.

Suggestion

Consider to optional pack the transitive dependencies .

Related with: openiddict/openiddict-core#1113

[tebeco]

Hello,

Good catch on that, the behavior seen in OpenIddict is effectivly a "NO GO" for library author. Top idea that comes to mind

• this is a HUGE breaking change from the current dotnet pack (from netcoreapp2.0)
• this look like we are going back to package.config that previously leaked all transitive to a file, but now it transform Transitive to Direct
• this look like a mix of a "lock file" but with version being open, that's weird

As a library author, I might not be whiling to apply this
As an application author, it's probably not important

I really think that as it look like, this will kill the adoption of it for all library author

Opportunity

This feature is not activated by default
This feature is still in preview, this means that it is subject to possible change.
A big thanks to @kevinchalet for seeing this before this is released, this gives everyone the opportunity to avoid an unwanted behavior that could be very hard to rollback in the next years

Side effects

Consumer madness

As a consumer I won't be able to understand anymore what the library I consume directly depends on.
This means I can't understand anymore the surface of it
Checking the Direct dependent libraries is a way to understand how it's being though
like for example the "desired" dependencies, and the desired version

Funny Scenario

• you depend on Foo
• Foo depends on A
• At that time Foo resolved package A 1.0.1
• Package A depends on package B
• Package A realize there's a bug and it should not reference B (and dont use it)
• Package A release a fix by removing it in 1.0.2
• Your lib will explicitly expose Foo / A / B

Your consumer will never be able to get rid of B until YOU update everything and "potentially" Foo too
If I apply the same to Security fix and rather than B, I say that A containes a security vulnerability, this means that the consumer will potentially depends on vulnerable package ? (even if the code might not be called, but you can't really do anything)

I am aware that

• consumer could force pin the dependency
• Code would be possible not loaded / used
  But
• This is very un-intuitive to do so when you try to update, you don't expect to pin doing transitive
• This might raise false alert on tool that scan for dependency issue

Network

This also means that when the dependencies tree will change, the nuget client will do requests for package that should not be resolved directly, but rather transitively.
Does this means Extra Hoop ?
Or forcefully downloading binary that might never be used ?

[rconard]

As you all know, we are working very hard to use your user feedback to make our decisions for Central Package Version Management.

@kevinchalet and @tebeco have noticed a change in the way that transitive dependency pinning works with CPVM. For web services, applications, and scripts, it should heave a smaller impact. For library authors, it could lead to transitive dependencies leaking across packages.

We changed the transitive behavior for CPVM based on feedback from some of our internal teams, and we have tried to design the feature to maximize the chances that a dependency will be available if it is referenced.

We want to understand from you, our users, what behavior you want to see in the .NET 5 SDK GA release.

Example Directory.Packages.props

<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
    <ItemGroup>
        <!-- foo has a dependency on bar v2 -->
        <PackageVersion Include="foo" Version="1" />
        <PackageVersion Include="bar" Version="3" />
    </ItemGroup>
</Project>

Example.csproj

<Project>
  <ItemGroup>
    <PackageReference Include="foo"/>
  </ItemGroup>
</Project>

Example Outputs

	Pack	Restore
No Transitive Pinning (Old CPVM Behavior)	foo v1	foo v1 bar v2
Transitive Pinning (New CPVM Behavior)	foo v1 bar v3	foo v1 bar v3

Questions

1. Which behavior do you need for your applications?
2. Which behavior do you need for your libraries?
3. If you prefer the "Old CPVM Behavior" that does not use Transitive Pinning (how NuGet handles transitive packing before CPVM), would you want/need the option to enable Transitive Pinning? How would you want to enable it?

[kevinchalet]

The No Transitive Pinning behavior (used in 3.1.x) offers the most consistent experience: it works exactly like how <PackageReference /> has worked for years. I believe it should be the default behavior, as it's the least surprising one.

The Transitive Pinning behavior differs from what people are used to when it comes to NuGet packages and <PackageReference />. As mentioned in the mail thread, it's a behavior that makes sense and having an <EnableTransitivePinning>true</EnableTransitivePinning> option would be a good idea for those who are aware of how it works and deliberately want to use it.

[kevinchalet]

BTW, the behavior of Transitive Pinning could be largely improved by not adding an explicit package reference when the version configured via CPVM is already referenced transitively. It would at least greatly reduce the noise, which is one of the main concerns.

E.g:

Example Directory.Packages.props

<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
    <ItemGroup>
        <!-- foo has a dependency on bar v2 -->
        <PackageVersion Include="foo" Version="1" />
        <!-- same version as the one referenced by foo -->
        <PackageVersion Include="bar" Version="2" />
    </ItemGroup>
</Project>

Example.csproj

<Project>
  <ItemGroup>
    <PackageReference Include="foo"/>
  </ItemGroup>
</Project>

	Restore	Restore
No Transitive Pinning (Old CPVM Behavior)	foo v1	foo v1 bar v2
Transitive Pinning (New CPVM Behavior)	foo v1	foo v1 bar v2

[tebeco]

I think having the choice is the key here
"Do you want that ?" as an MsBuild Props
"Do you want Oldest compliant transitive" as an MsBuild Props
"Do you want Latest compliant transitive" as an MsBuild Props
Let us choose, even if it breaks us (but not by default though)

Questions

1. Which behavior do you need for your applications?
2. Which behavior do you need for your libraries?

Same for both, I try to always have the latest bits but having a lockfile. This allows on the consumer side to quickly pin what I depend on so I keep being in control
The restore is always done in --locked-mode, This allows to be consistent over time.
Then when you decide to update, you can and you benefits from the lockfile diff to see what happened

So I guess it's the "old behavior"

3. If you prefer the "Old CPVM Behavior" that does not use Transitive Pinning (how NuGet handles transitive packing before CPVM), would you want/need the option to enable Transitive Pinning? How would you want to enable it?
   I'm not sure I full understand the question and the implication of it.
   If there's a place I would enable pinning, it's only in the context of "having the choice to do it or not", which is almost always when you are the consumer :
   But in an Application I expect this to be a lockfile as an application does not have to "expose transitive to someone"

If my lib depends on another lib foo, I could want to pin foo to a specific version in the PackageReference, and I would want this SAME constraint to reflect in the dotnet pack result
I expect that because I made an explicit statement when doing this:
<PackageReference Include="Foo" Version="[1.2.3]" /> ( or <PackageVersion Foo Version="[1.2.3]" />

Now what the pinning intend to make sure that when you resolves the dependency tree, you could resolve VERY OLD version
The thing is that nuget tries really hard "not" to break thing, which could be understood, and being explicit about that tree would enforce the minimal version about the "when it was build", so that looks like a great idea too ;)

Though, what I'm aiming at (and this is a very personal opinion, is to have a boolean to tell nuget to resolve the latest even if it breaks me.
This would be my personal choice to "enable resolve highest for transitive", and I would just create a PackageReference and "pin" the one that cannot be resolved for the "latest version"
The goal would be:

• always have a lockfile
• always works dotnet restore --locked-mode
• always wildcard on major for direct dependency
• OPT_IN (switch) to resolve latest transitive

Update are "on demands" and can be easily automated on CI local branch:

• remove lock file
• dotnet restore --force-evaluate
• build / test (E2E if applicable)
• push / create PR (or even merge if all flags are green)
• if build is red ==> "A human need to take a look at it"

This reminds me of this issue a bit where the idea is a Property to choose how the transitive are resolved
#1192

=> give back the choice to the owner of the developper that will run dotnet restore

Aside nuget

We've used paket at work from 2016 to 2018, and it has the best overall system about "expressing what you want", and "we will get it done + a lock file"
We moved back to dotnet + MsBuild since .netcore ~2.0 2.1 as paket was not handling cross targeting and Metapackage at that time
I'm really happy with the way dotnet <PackageReference works today and I do not plan to go back to paket with the current evolution being made by nuget since then

The previous "OPT_IN" mentioned earlier is what I would love to have:
https://fsprojects.github.io/Paket/dependencies-file.html#Resolver-strategy-for-transitive-dependencies

[marcin-krystianc]

The No Transitive Pinning behavior (used in 3.1.x) offers the most consistent experience: it works exactly like how <PackageReference /> has worked for years. I believe it should be the default behavior, as it's the least surprising one.

The Transitive Pinning behavior differs from what people are used to when it comes to NuGet packages and <PackageReference />. As mentioned in the mail thread, it's a behavior that makes sense and having an <EnableTransitivePinning>true</EnableTransitivePinning> option would be a good idea for those who are aware of how it works and deliberately want to use it.

I also think that it is better to make the "No Transitive Pinning" behaviour to be the default one. It is exactly as @kevinchalet said, the default behaviour should be consistent with how PackageReference has worked for years. The idea for implementing the the opt-in for "Transitive Pinning" with a property like <EnableTransitivePinning>true</EnableTransitivePinning> also sounds good to me.

[AArnott]

I think transitive pinning would make the most sense for the reason given in the description. But I have no objection to it being an optional behavior. My preference would be to make transitive pinning be on by default.

[joeltankam]

#10578 seems to be an example of undesired pinned transitive dependencies.

[zivkan]

There are more upvotes and engagement in #10389 than this issue (plus a PR linking to that issue), so closing this one as a duplicate.

[JonDouglas]

Hi everyone,

There's some differing opinions on whether or not an opt-in or opt-out experience for transitive pinning is desired. To learn more from people on this thread, I'd love to invite you all to talk directly to the NuGet team about your thoughts on transitive pinning & central package version management as a whole. You can schedule a call with us here:

https://aka.ms/talktonuget

Your feedback will really help us out! Otherwise, if you'd like to leave some thoughts/feedback on #10389, it would be greatly appreciated.