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

New 6.12 Nuget algorithm leads to extra packages being downloaded. #13930

Closed
mikernet opened this issue Nov 15, 2024 · 35 comments
Closed

New 6.12 Nuget algorithm leads to extra packages being downloaded. #13930

mikernet opened this issue Nov 15, 2024 · 35 comments
Assignees
Labels
Area:NewDependencyResolver Issues related to the new dependency graph resolver Functionality:Restore Priority:1 High priority issues that must be resolved in the current sprint. RegressionFromPreviousRTM A regression from the last RTM. Example: worked in 6.2, doesn't work in 6.3 Type:Bug

Comments

@mikernet
Copy link

mikernet commented Nov 15, 2024

NuGet Product Used

dotnet.exe, MSBuild.exe, Visual Studio Package Management UI

Product Version

6.12

Worked before?

Yes

Impact

I'm unable to use this version

Repro Steps & Context

New nuget algorithm references very different packages and versions, some of which have vulnerabilities, i.e. System.Private.Uri version 4.3.0.

Steps to reproduce:

  • Clone https://github.com/Singulink/Singulink.UI
  • Open .sln in VS
  • Build once, notice it restores packages correctly and builds fine
  • Close and reopen VS
  • Build again, notice that the packages referenced are now completely different in both the .WinUI projects, the project.assets.json files are wildly different, and you get errors that the projects reference packages with vulnerabilities.

Note that putting this in directory.build.props or the project .csproj files does not fix the issue when building from VS:

    <RestoreUseLegacyDependencyResolver>true</RestoreUseLegacyDependencyResolver>

It only "fixes" the issue when passed as a command-line argument into msbuild.exe, which means I don't know how to fix the VS development experience issues this now causes. The only way I can get the project to build again is to clear nuget cache, reopen the solution, and then I get one build until it breaks again.

Verbose Logs

No response

@martinsuchan
Copy link

martinsuchan commented Nov 15, 2024

We're seeing issues with the new NuGet Dependency Resolver as well. Our repro:

  • we use our own mirror of nuget.org with only some packages being availble, some vulnerable packages were removed and some of our 1st party packages are there as well
  • we have .NET 4.8 project with these references:
<PackageReference Include="jQuery" Version="1.10.2" />
<PackageReference Include="jQuery.Validation" Version="1.11.1" />
  • both these package versions are available on our NuGet server
  • jQuery.Validation 1.11.1 has transitive dependency in nuspec to jQuery 1.4.4, but this version of jQuery is not available on our nuget server

Previous (expected) restore behavior with nuget 6.11:

  • nuget restore works, jQuery 1.10.2 and jQuery.Validation 1.11.1 are restored, with no warnings

Current broken behavior in nuget 6.12.1:

  • The nuget restore process fails every time with this message:
    NuGet.targets(180,6): error : The feed 'ourserver [url]' lists package 'jQuery.1.4.4' but multiple attempts to download the nupkg have failed. The feed is either invalid or required packages were removed while the current operation was in progress. Verify the package exists on the feed and try again.

Note when I enabled CPM for this sample project, this error disappeared in nuget 6.12.1.

@andrew-from-toronto
Copy link

andrew-from-toronto commented Nov 15, 2024

Seeing something similar to this.. we're using lock files and on some projects we build out to specific RIDs, in our case linux-x64 and windows-x64.. This causes references to some packages like runtime.any.System.Runtime which as of .NET 9 and this change I guess throws a build/restore error Error NU1101 : Unable to find package runtime.any.System.Runtime. No packages exist with this id in source(s): nuget.org

Tried the RestoreUseLegacyDependencyResolver prop but did not notice any change in behaviour.

@nkolev92
Copy link
Member

@martinsuchan

Does setting RestoreUseLegacyDependencyResolver change anything for you?
I'm not confident your issue and the one by Mike here are equivalent.

@andrew-from-toronto

Similar question as Martin.
Does RestoreUseLegacyDependencyResolver change the behavior for you?

@mikernet
Copy link
Author

mikernet commented Nov 15, 2024

The two .WinUI projects in the repro repo mentioned in OP also adds a bunch of runtime.any.System.XXX transitive dependencies on the second restore/build that are not present on the first build, which appears to be part of the issue here.

@andrew-from-toronto
Copy link

@andrew-from-toronto

Similar question as Martin. Does RestoreUseLegacyDependencyResolver change the behavior for you?

No, there was no change in behaviour with that property. I tried in both our Directory.Build.props and in the affected csproj directly

@mikernet
Copy link
Author

Also dotnet nuget why System.Private.Uri says that the project does not have any dependencies on it 🫤

@mikernet
Copy link
Author

@andrew-from-toronto From what I can tell it doesn't work from csproj and .props, as I mentioned - try passing it to msbuild.exe as documented here: https://learn.microsoft.com/en-us/nuget/consume-packages/package-references-in-project-files#nuget-dependency-resolver

@andrew-from-toronto
Copy link

@andrew-from-toronto From what I can tell it doesn't work from csproj and .props, as I mentioned - try passing it to msbuild.exe as documented here: https://learn.microsoft.com/en-us/nuget/consume-packages/package-references-in-project-files#nuget-dependency-resolver

I did see that, but realistically even if that works it's not an acceptable solution. I can't ask our dev team to restore packages like that. Just opening the projects in our IDEs throws the errors as it tries to restore packages.

@mikernet
Copy link
Author

Yeah, obviously 😄 - I would love a solution that fixes it for now, if anyone has one. Can I overwrite the new nuget with 6.11 or something? I just need this stuff to work at the moment but I need VS 17.12.

@mikernet
Copy link
Author

I think the request was to try it for purposes of determining whether that is indeed the root cause, not as an acceptable solution, but I don't want to speak for Nik so maybe he can clarify.

@nkolev92
Copy link
Member

Yes, we're just triaging and making sure we know how many separate issues we're tracking. After all, we want to be able to fix them all.
Seems like @andrew-from-toronto's issue is different from this one.

@andrew-from-toronto
Copy link

andrew-from-toronto commented Nov 15, 2024

I did some more testing with msbuild (including the RestoreUseLegacyDependencyResolver prop) directly, and in VS, Rider.. Seems like my issue is actually a bug in the latest Rider release with .NET 9 support (2024.3). Sorry to add to the confusion here, I'll drop out.

@nkolev92
Copy link
Member

Also dotnet nuget why System.Private.Uri says that the project does not have any dependencies on it 🫤

Tracking issue: #13718
We're actively working on that.

@martinsuchan
Copy link

martinsuchan commented Nov 15, 2024

@martinsuchan

Does setting RestoreUseLegacyDependencyResolver change anything for you? I'm not confident your issue and the one by Mike here are equivalent.

Update to my findings:

@nkolev92 nkolev92 added Functionality:Restore Area:NewDependencyResolver Issues related to the new dependency graph resolver and removed Triage:Untriaged labels Nov 15, 2024
@nkolev92
Copy link
Member

6.12.1 and 9.0.100 are running the same code, so it's a bit unusual that they're reporting different results.

@martinsuchan

If you compare the project.assets.json between 6.12.1 of NuGet.exe and 6.11 (if you still have both) are they the same or different?

We'll need to recreate the repro on our end to confirm that we have a fix.

@mikernet
Copy link
Author

mikernet commented Nov 15, 2024

@martinsuchan Are you sure it isn't actually problematic for both, but you just tried nuget restore first? First restore seems to work fine for me, it's the second restore that seems to break things.

@kasperk81
Copy link

9.0.100 has 6.12.0-rc.127+19756345139c45de23bd196e9b4be01d48e84fdd.19756345139c45de23bd196e9b4be01d48e84fdd. i copied these find dotnet10/ -name 'NuGet.*.dll' | grep -vi resource from dotnet10 daily build to 9.0.100 system installation, so far looking good.

@nkolev92 nkolev92 self-assigned this Nov 15, 2024
@nkolev92 nkolev92 added RegressionFromPreviousRTM A regression from the last RTM. Example: worked in 6.2, doesn't work in 6.3 Priority:1 High priority issues that must be resolved in the current sprint. labels Nov 15, 2024
@martinsuchan
Copy link

Sorry, both nuget 6.12.1 and dotnet 9.0.100 fails the restore. 🤦‍♂️
I have already replaced my global nuget instance with nuget 6.11.1 and this version works. When I tried again nuget 6.12.1 saved in a different folder, then it fails on the same issue.

@nkolev92
Copy link
Member

@martinsuchan

note is it possible that this my error has source in this known issue? Project and package in the same graph with the same name but different dependencies may lead to incorrect versions of the dependencies of that id #13888

We can confirm whether it's related.

Can you use the dotnet install scripts and install 9.0.200 preview?
Alternatively, I can get a NuGet.exe version if that's easier for you.

@mikernet
Copy link
Author

@nkolev92 Should I try that as well? Where are these preview install scripts?

@martinsuchan
Copy link

martinsuchan commented Nov 15, 2024

Note our proxy NuGet server works in a way that it "offers" all nugets available on nuget.org, but if you try to download some disabled packages, it fails. Maybe this has changed in dotnet 9.0.100 - it tries to download jQuery 1.44 and fails even though jQuery 1.10.,2 is defined at the top level?

@kasperk81
Copy link

kasperk81 commented Nov 15, 2024

download from middle column https://github.com/dotnet/sdk/blob/main/documentation/package-table.md and unzip in somewhere like c:\temp\dotnet9.0.200, then c:\temp\dotnet9.0.200\dotnet.exe build

@mikernet
Copy link
Author

As far as I can tell, it doesn't look like it solves the issue.

@martinsuchan
Copy link

martinsuchan commented Nov 15, 2024

download from middle column https://github.com/dotnet/sdk/blob/main/documentation/package-table.md and unzip in somewhere like c:\temp\dotnet9.0.200, then c:\temp\dotnet9.0.200\dotnet.exe build

Ok, I tested dotnet restore with dotnet 9.0.200-0.24565.9 with the same error.

@nkolev92
Copy link
Member

nkolev92 commented Nov 15, 2024

Hey @mikernet,

I'm narrowing down the root cause for your issue. I'll list out my findings, let me know if you're running into anything different.

  • The issue in your case is that there are additional packages such as System.Private.Uri that are vulnerable and you're getting more warnings now.
  • You are only experiencing issues after the 17.12 upgrade. You did not have issues in 17.11 (the 2 big changes are transitive vulnerabilities and algorithm)
  • You are only experiencing issues in Visual Studio, there are no issues on the commandline.
  • RestoreUseLegacyDependencyResolver doesn't seem to make a difference. I was able to confirm this. There doesn't seem to be differences between the algorithms, it's something else.
  • CLI & VS are where the discrepancy seems to be. VS has RuntimeIdentifier set which leads to more packages getting restored in VS. This is likely a thing happening in the WinUI targets.

Next steps:

  • Compare 17.11 (or any earlier version) and 17.12 in VS.

It is possible that the only difference you're seeing between VS versions is audit defaulting to all (which includes transitives) vs previous reporting only direct.
All that being said, it is unusual that VS & non-VS have different RuntimeIdentifiers set. I'll validate that NuGet is not having issues here and if I'm correct, we can report that to someone who can help with that discrepancy.

@mikernet
Copy link
Author

mikernet commented Nov 15, 2024

Interestingly, it looks like adding:

    <EnableMsixTooling>true</EnableMsixTooling>

To the WinUI csproj files fixes this issue and it now works in VS 17.12. Maybe this is an issue for WinAppSdk instead? Or perhaps VS team?

@nkolev92
Copy link
Member

Back with more info.

17.8 and 17.11 behave the same, so for some reason, for WinUI projects, in VisualStudio, the RuntimeIdentifiers property are set, but not on the CLI. This leads to the different behavior for both.

Summary:

In @mikernet's case, the different is not due to the resolver, but rather due to 2 other factors.

Thank you for all the quick responses and clear repro steps @mikernet. It helped us get to the bottom of this quickly.

@martinsuchan I want to try to understand the issues you're experiencing next. If you have any repro you can provide for me, that'd be ideal.
If not, I'll try to piece together what I can from your descriptions here and see how much progress I can make from that.

@mikernet
Copy link
Author

@nkolev92 As I mentioned above, EnableMsixTooling fixes the issue with the packages being resolved differently in VS so I went with that for now - I don't get any warnings/errors in VS anymore. I prefer that instead of messing with the package auditing.

@nkolev92
Copy link
Member

@martinsuchan

I'm digging into your issue, and I was able to narrow it down.

Per your repro, the following project was enough:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net48</TargetFramework>
    <RootNamespace>_13930</RootNamespace>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="jQuery" Version="1.10.2" />
    <PackageReference Include="jQuery.Validation" Version="1.11.1" />
  </ItemGroup>
</Project>
  1. The resolved graph between new and old algorithm is the same. The end result is the same.
  • The old resolver ends up download Jquery 1.10.2 and JQuery.Validation 1.11.1
  • The new resolver ends up downloading JQuery 1.10.2, JQuery 1.4.4 and JQuery.Validation 1.11.1. The rewrite simplified the calculations but changed when the downloading happens and that's root cause of your issues here.

The unusual part here is:

NuGet.targets(180,6): error : The feed 'ourserver [url]' lists package 'jQuery.1.4.4' but multiple attempts to download the nupkg have failed. The feed is either invalid or required packages were removed while the current operation was in progress. Verify the package exists on the feed and try again.

This seems to suggest that JQuery 1.4.4 is being listed in the content resource, but it's not downloadable.
Can you validate that for me?

Here are some details:
The resource is question is https://learn.microsoft.com/en-us/nuget/api/package-base-address-resource.
For example, on nuget.org, the service index is https://api.nuget.org/v3/index.json, and the address for something like jquery would be: https://api.nuget.org/v3-flatcontainer/jquery/index.json.

Can you tell me if you see 1.4.4 on your server there?

@nkolev92 nkolev92 changed the title New 6.12 Nuget algorithm results in very different packages being referenced. New 6.12 Nuget algorithm leads to extra packages being downloaded. Nov 18, 2024
@martinsuchan
Copy link

@nkolev92 Hi, you summed it accurately. As far as I know our Nuget Proxy server lists all nuget versions from nuget.org as available, but if I try to download some old and/or vulnerable packages, then I'm not able to and get this error.
Looks like the new resolver cannot gracefuly handle such scenarios comparing to the old one?
I can check the index file tomorrow when I get back to my other PC.

@nkolev92
Copy link
Member

nkolev92 commented Nov 18, 2024

Just a quick clarification.
The downloading between the 2 algorithm is the same.

The new algorithm, tries to download that extra package, and it just so happens that it's not available.
There's a chance this could happen for in a different package graph as well. For example, transitive packages being considered etc.

You said you're using a proxy correct?
For the long-term success of your setup, I think the proxy needs to sanitize the package content list of versions as well, similar to how private upstreams work in Azure Artifacts for example.

More explicitly clarifying what available means: NuGet/docs.microsoft.com-nuget#3360

@mikernet
Copy link
Author

@nkolev92 They closed the issue I filed on developercommunity as a duplicate of this issue lol, which I assume is not what we want.

@martinsuchan
Copy link

@nkolev92 We use Nexus Repository 2.15 if that helps. As I understand it right currently it returns as "available" all packages on nuget.org even though some of them are blocked and cannot be downloaded. The question is if this is the default behavior or it can be configured to not return these blocked/vulnerable packages?

@nkolev92
Copy link
Member

@mikernet Sorry about that. Reopened now.

@martinsuchan
Today I merged some improvements to our protocol documentation to more clearly indicate that all packages listed should be available for download. NuGet/docs.microsoft.com-nuget@09a979d

I think that's something reaching out to the folks at Sonatype with some feedback.
Unfortunately, I don't know details of their implementation.

That being said, a quick summary for this issue.

I'm sorry for all the issues you're running into. We focus on building features for the feature areas we get most feedback about and security and scaling for large repositories were things that we've been hearing a lot of feedback about.
Unfortunately, with any change comes the potential for disruption, sometimes expected, like in the vulnerability case, sometimes unintentional like in the resolver case.

I can't tell you how much I appreciate all of your interactions, issue details, validations done etc.
It's truly helped us to get to the bottom of the issues.

At this point, I'll close this issue as a duplicate of #13943.
#13943 is newer, but it has fewer comments, and it's something that we believe other customers will be able to more easily find the workarounds in, and it's something we'll add to our release notes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area:NewDependencyResolver Issues related to the new dependency graph resolver Functionality:Restore Priority:1 High priority issues that must be resolved in the current sprint. RegressionFromPreviousRTM A regression from the last RTM. Example: worked in 6.2, doesn't work in 6.3 Type:Bug
Projects
None yet
Development

No branches or pull requests

5 participants