search

NuGet / Home

Repo for NuGet Client issues
Other
1.5k stars 252 forks source link

`dotnet list package` does not work if project is using central package management system, after upgrading to `.NET 8.0` #13632

Closed parase03 closed 1 month ago

parase03 commented 3 months ago

NuGet Product Used

dotnet.exe

Product Version

8.0.x

Worked before?

7.0.x

Impact

I'm unable to use this version

Repro Steps & Context

  1. Create a project that is managing package versions centrally.
  2. Run dotnet restore
  3. Run dotnet build
  4. Run dotnet list package --vulnerable using any .NET 8.0 SDK (in the example, 8.0.303 was used).

Expected behavior: The command must complete and list any vulnerable package. Actual behavior: The command fails as it cannot read the package references.

Since upgrading to .NET 8.0, the NuGet command to list packages (with any combination, but particularly with --vulnerable), is failing to complete when ran against a project which is using Directory.Package.props for managing its dependencies centrally.

The command mentions that it is unable to read the package references from the project, like the picture below:

net8

Quite literally the same project and command, when ran using the .NET 7.0 SDK, is completing without any issues:

net7 0

This does not have to do with the .NET the project targets, as in both examples above the project has been built for .NET 7.0, but the command to view the vulnerable packages has been ran using a different SDK vesion (7.0.316 and 8.0.303).

Also, it seems that the issue is not re-creatable when the central package management is disabled and versions are mentioned in the .csproj files.

Verbose Logs

Unable to read a package reference from the project `PATH_TO_CSPROJ_FILE`. Please make sure that your project file and project.assets.json file are in sync by running restore.
jgonz120 commented 3 months ago

Hello! There is a current issue with CPM, https://github.com/NuGet/NuGet.Client/pull/5866, which we're working on fixing. In your setup are you enabling CPM on individual projects or within the Directory.Package.props file?

parase03 commented 3 months ago

Hello @jgonz120! In my setup, the CPM is enabled for the entire solution in a central Directory.Package.props file, located in the root of the solution (where the .sln file lies).

jgonz120 commented 3 months ago

I haven't been able to reproduce this using https://github.com/nkolev92/CentralPackageManagementDemo as an example repo.

After cloning I added a global.json file so I could specify the dotnet version and updated the projects to use net7.0 so I could test it in both versions.

Are there any differences with that example project and yours that you think could help us get a repro?

parase03 commented 3 months ago

Hello again! I managed to find the culprit but don't know if this is an expected behavior or a bug.

Indeed with the project you mentioned I wasn't able to reproduce the issue, so I compared with my local repository. I found out that the only difference in terms of structure and files, was the existence of Directory.Build.props, in addition to the CPM. Apologies for not mentioning this initially, I thought it was irrelevant.

In this file, there were also global package references mentioned. So I copied the same file in the the example repo and was able to reproduce this using .NET 8.0 SDK. When I moved these references to the Directory.Packages.props (while leaving the file with the rest of its contents), the problem was resolved and dotnet list package --vulnerable was able to work again.

Interestingly enough, when I switched .NET SDK to version 7.0 using global.json, and restored Directory.Build.Props to its original contents, the command wasn't failing.

This begs the question: was this behavior with v7.0 a pre-existing bug that got fixed with v8.0, or is the current behavior a bug? It looks like .NET 7.0 is parsing both Directory.* files for package references when a list package command is issued, whereas .NET 8.0 looks only in the CPM-specfic one.

Can (or should) Directory.Build.props contain global package references?

To re-create this, I followed these steps:

  1. Cloned https://github.com/nkolev92/CentralPackageManagementDemo project
  2. Created a Directory.Build.Props using this sample.
  3. I moved the below section: 
    <ItemGroup>
<GlobalPackageReference Include="Microsoft.SourceLink.GitHub" Version="1.1.1" />
</ItemGroup>

    from Directory.Package.Props into the Directory.Build.Props.

  4. I opened a terminal and run dotnet restore, followed by dotnet list package --vulnerable.
nkolev92 commented 2 months ago

I can repro it: https://github.com/nkolev92/CentralPackageManagementDemo/tree/listPackageBug. The assets file property is set, so it's not that.

Nigusu-Allehu commented 2 months ago

Here is what I was able to find out:

The asset file generated during restore contains GlobalPackageReferences defined in Directory.Build.Props as one of the items in the list of targets for the project.

However, as described in the docs, we expect GlobalPackageReference to be defined in Direcotry.Package.Props only. As a result, when we try to resolve the version of the GlobalPackageReference package from Directory.Package.Props, it results in an exception because the package reference only exists in Directroy.Build.Props.

We could address this in two ways

  1. Make sure restore does not read GlobalPackageReference from Directory.Build.Props
  2. Do version resolution for CPM using both Directory.Build.Props and Directory.Package.Props
nkolev92 commented 2 months ago

I'm not sure if the docs mean to imply that only Directory.Packages.props is supported.

There's implied support for PackageVersion and Directory.packages.props for what our tooling can do (dotnet add package and VS updates), but in the end, it's MSBuild, so I'm not sure if that's supposed we're explicitly disallowing.

Restore will use items as long as they're defined with the msbuild evaluated project and does not really focus on where that's coming from.

Idea #1 would conflict with the MSBuild integration, so probably not a possible solution. Similar thing applies to idea #2, when we take the items at restore time, we don't differentiate.

I think this is a list package limitation, not something that should drive restore behavior.

Why does list package raise an error suggesting that the project assets file is not available? What's the first bug there?

Nigusu-Allehu commented 2 months ago

I agree, 1 would conflict with the MSBuild integeration. For 2, when I said version resolution, I was refering to https://github.com/NuGet/NuGet.Client/blob/15991039d603b8da5d2603315fc9c5c2cfb91a07/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs#L800 where we try to retrieve the version for installed GlobalPackageReference packages. We only try to get the version from Directory.Package.Props, adding Direcotry.Build.Props would allow us not to run into this conflict where a package is in the asset targets and not in Directory.Package.Props.

nkolev92 commented 2 months ago

Ah, makes sense.

Yeah, I think list package needs to just consider the full project for GlobalPackageReference, similarly to how PackageReference is done.