Skip to content
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

[Bug]: When using the CentralPackageTransitivePinningEnabled feature please avoid redundant references #11818

Open
ericstj opened this issue May 13, 2022 · 3 comments
Labels
Area:RestoreCPM Central package management Functionality:Restore Priority:2 Issues for the current backlog. Type:DCR Design Change Request

Comments

@ericstj
Copy link

ericstj commented May 13, 2022

NuGet Product Used

dotnet.exe

Product Version

7.0.100-preview.4.22227.3

Worked before?

No

Impact

It bothers me. A fix would be nice

Repro Steps & Context

  1. Grab the project from this gist: https://gist.github.com/ericstj/badca81e76de6875b1bad0958e70b0dc
  2. dotnet pack with a 7.0 preview4 SDK.
  3. Examine the nuspec.

Expect:
No dependency listed for `System.Text.Encodings.Web, since the version specified centrally is equivalent to the transitive dependency.

    <dependencies>
      <group targetFramework=".NETStandard2.0">
        <dependency id="System.Text.Json" version="6.0.0" exclude="Build,Analyzers" />
      </group>
    </dependencies>

Actual:
A redundant dependency on System.Text.Encodings.Web is listed.

    <dependencies>
      <group targetFramework=".NETStandard2.0">
        <dependency id="System.Text.Json" version="6.0.0" exclude="Build,Analyzers" />
        <dependency id="System.Text.Encodings.Web" version="6.0.0" exclude="Build,Analyzers" />
      </group>
    </dependencies>

The dependency is largely just a cosmetic nuisance, but I expected it to go away when the dependency didn't need to be lifted.

Verbose Logs

No response

@ViktorHofer
Copy link

cc @jeffkl

@ericstj
Copy link
Author

ericstj commented Oct 27, 2022

@jeffkl can this be closed now or did it not make it in?

@jeffkl
Copy link
Contributor

jeffkl commented Oct 27, 2022

Unfortunately this turned out to be a lot harder than it sounded. Basically NuGet's graph resolution algorithm takes in all of the inputs and walks the package sources to get the entire tree. Then the tree is then trimmed and eclipsed versions are removed. The versions of packages you specify are only passed to the code that walks the dependencies and the code that trims the tree is not currently aware of all the inputs so it is unable to then leave a dependency alone if the same version that was resolved is what was pinned.

So for now I'm leaving this on our backlog and we'll have to revisit it if we get enough upvotes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area:RestoreCPM Central package management Functionality:Restore Priority:2 Issues for the current backlog. Type:DCR Design Change Request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants