Duplicate nupkg files are placed when install .NET Global Tool with .NET 8 SDK

filzrev commented 5 months ago

Describe the bug

When install .NET Global Tools with dotnet tool install command with .NET 8 SDK. Duplicated .nupkg file copy is created to tool installed directory.

Sometimes nupkg file has large size (e.g. docfx package takes about 250MB) It's desirable duplicated file is not created.

To Reproduce

Run dotnet tool install dotnet-counters -g command.
Check installed files (e.g. %USERPROFILE%\.dotnet\tools\.store\dotnet-counters\8.0.510501\dotnet-counters\8.0.510501)
Following 2-files are existing. And these file hash are same.
- dotnet-counters.8.0.510501.nupkg
- dotnet-counters.nupkg

Exceptions (if any)

This issue occurs when install .NET global tool by using .NET 8 SDK. When using .NET 6/.NET 7 SDK (by specifying specific sdk by using global.json)

This problem is not occurred. (duplicated dotnet-counters.nupkg file is not placed)

Further technical details

Tested .NET SDK versions
- 6.0.421
- 7.0.408
- 8.0.204
OS:
- Windows

`dotnet --info`

.NET SDK: Version: 8.0.204 Commit: c338c7548c Workload version: 8.0.200-manifests.7d36c14f Runtime Environment: OS Name: Windows OS Version: 10.0.20348 OS Platform: Windows RID: win-x64 Base Path: C:\Program Files\dotnet\sdk\8.0.204\ .NET workloads installed: [maui-windows] Installation Source: VS 17.9.34728.123 Manifest Version: 8.0.7/8.0.100 Manifest Path: C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.maui\8.0.7\WorkloadManifest.json Install Type: FileBased [maccatalyst] Installation Source: VS 17.9.34728.123 Manifest Version: 17.2.8004/8.0.100 Manifest Path: C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.maccatalyst\17.2.8004\WorkloadManifest.json Install Type: FileBased [ios] Installation Source: VS 17.9.34728.123 Manifest Version: 17.2.8004/8.0.100 Manifest Path: C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.ios\17.2.8004\WorkloadManifest.json Install Type: FileBased [android] Installation Source: VS 17.9.34728.123 Manifest Version: 34.0.52/8.0.100 Manifest Path: C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.android\34.0.52\WorkloadManifest.json Install Type: FileBased [wasm-tools] Installation Source: VS 17.9.34728.123 Manifest Version: 8.0.4/8.0.100 Manifest Path: C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.workload.mono.toolchain.current\8.0.4\WorkloadManifest.json Install Type: FileBased Host: Version: 8.0.4 Architecture: x64 Commit: 2d7eea2529 .NET SDKs installed: 6.0.129 [C:\Program Files\dotnet\sdk] 6.0.203 [C:\Program Files\dotnet\sdk] 6.0.321 [C:\Program Files\dotnet\sdk] 6.0.421 [C:\Program Files\dotnet\sdk] 7.0.118 [C:\Program Files\dotnet\sdk] 7.0.203 [C:\Program Files\dotnet\sdk] 7.0.315 [C:\Program Files\dotnet\sdk] 7.0.408 [C:\Program Files\dotnet\sdk] 8.0.204 [C:\Program Files\dotnet\sdk] .NET runtimes installed: Microsoft.AspNetCore.App 6.0.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 6.0.26 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 6.0.29 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 7.0.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 7.0.18 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 8.0.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.NETCore.App 6.0.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 6.0.26 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 6.0.29 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 7.0.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 7.0.18 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 8.0.4 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.WindowsDesktop.App 6.0.5 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 6.0.26 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 6.0.29 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 7.0.5 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 7.0.18 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 8.0.4 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Other architectures found: x86 [C:\Program Files (x86)\dotnet] registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\x86\InstallLocation] Environment variables: Not set global.json file: D:\a\sandbox\sandbox\global.json

filzrev commented 5 months ago

It seems behavior differences between SDK versions are caused by PR #33835.

As far as I have confirmed, NuGet package extraction is executed with following steps.

Step1. Download NuGet package file as dotnet-counters.8.0.510501.nupkg (ToolPackageDownloader.cs]

Step2. Extract NuGet package by NugetPackageDownloader::ExtractPackageAsync

At Step2. PackageSaveMode is set to Defaultv3. So additional nupkg is created(or overwritten) on package extractions.

And nupkg name is set to dotnet-counters.nupkg by NuGetPackagePathResolver So duplicated file copy are created.

Desired behaviors

For .NET Global Tools installation. Both nupkg (and nuspec file) are not needed. So set PackageSaveMode to Files only.

For .NET Local Tools installation. nupkg is used for NuGet global packages folder. So it need to preserve versioned .nupkg.

baronfel commented 5 months ago

Thank you for the analysis here - @nkolev92 does this sound correct to you and/or is there some documentation about the intent/usage of the PackageSaveMode values?

@filzrev would you be interested in sending a PR to correct this?

nkolev92 commented 5 months ago

There's 2 issues at hand:

Duplicate nupkgs. This is just how the APIs are being called currently. The playing with the nupkgs names leads to that: Here's how the NuGet side installation is done: https://github.com/NuGet/NuGet.Client/blob/12d5c661b77d4933f82a293008c0d56d1d6ce32b/src/NuGet.Core/NuGet.Commands/RestoreCommand/ProjectRestoreCommand.cs#L374-L407 Not sure why you need the package name without the version the version, but I'd say consider removing that.
Nupkg as part of the tools installation folder. The challenge with PackageSaveMode is that it changes the definition of what installed means. Once you use it in one place, you need to account for it in every location. It's been a source of bugs in the past. Unless size of the folder is extremely important, I'd say stick with the defaults of DefaultV3.

filzrev commented 5 months ago

would you be interested in sending a PR to correct this?

I'm note familiar with this SDK repository. So It would be helpful if anyone who have related knowledge could create PR if possible.

Unless size of the folder is extremely important, I'd say stick with the defaults of DefaultV3.

I thought it's reasonable to keep current behaviors. and fix duplicate nupkgs behavior issue only. And If the package size is a concern, manually remove .nupkg files.

It seems .NET docker image takes this approach for PowerShell tool installation. https://github.com/dotnet/dotnet-docker/blob/main/src/sdk/8.0/bookworm-slim/amd64/Dockerfile#L47-L48

dotnet / sdk