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

Linux: Installing packages with uppercase versions #13844

Closed
edvilme opened this issue Oct 8, 2024 · 7 comments
Closed

Linux: Installing packages with uppercase versions #13844

edvilme opened this issue Oct 8, 2024 · 7 comments
Labels
Resolution:External This issue appears to be External to nuget Type:Bug

Comments

@edvilme
Copy link

edvilme commented Oct 8, 2024

NuGet Product Used

dotnet (linux)

Product Version

.NET 10 (8 and 9)

Worked before?

No response

Impact

It's more difficult to complete my work

Repro Steps & Context

On Linux systems, it is not possible to install packages with uppercase characters in their version. The .NET SDKcalls NugetV3LocalRepository to find the package files, however its PathResolver member is set to enforce lowercase by default.
This means it will always search for the file with lowercase versions (even if it is stored with uppercase characters). This is not an issue on Windows because of the case agnostic fs, but it impacts Linux customers.

Here are some related issues: #13275, dotnet/sdk#24419, dotnet/sdk#39105, dotnet/sdk#41844

One of the issues mentions that NuGet expects all versions to be normalized. However this is currently not enforced by dotnet pack -p:Version command.

Some proposed solutions are:

  1. Remove default lowercase normalization for NugetV3LocalRepository::PathResolver
  2. Have overloading of NugetV3LocalRespository::FindPackage(String id, NugetVersion version, bool lowercase) so that consumers of that method are able to specify whether they want to check for lower/uppercase versions
  3. Start enforcing lowercase versions on dotnet pack (having the generated package filenames be lowercased)

cc: @nkolev92

Verbose Logs

No response

@nagilson
Copy link

nagilson commented Oct 8, 2024

From #13275

It seems like the code is not accounting for the fact that nupkgs always get normalized versions (lowercase))

@nkolev92 Does NuGet always lowercase the versions on the file when it downloads the nupkg? This might be true, but it doesn't appear to be true when you do dotnet pack. It seems like a bug that NugetV3LocalRepository.FindPackage() (https://github.com/dotnet/dotnet/blob/f00f52d70a4adb29a184dd72ee8f4b54f59d14c9/src/nuget-client/src/NuGet.Core/NuGet.Protocol/PackagesFolder/NuGetv3LocalRepository.cs#L72) always uses the default constructor of PathResolver, since that always assumes the package version in the file on disk will be lowercase.

This is causing a problem for us here: https://github.com/dotnet/sdk/blob/65d4a80668e0a52226d917ab4e6336b2864d1162/src/Cli/dotnet/ToolPackage/ToolPackageDownloader.cs#L338

@nkolev92
Copy link
Member

nkolev92 commented Oct 8, 2024

NuGet lowercases the packages when it writes them to global packages folder.
The lowercasing is done through VersionFolderPathResolver when it's passed to PackageExtractor.InstallFromSourceAsync.

As long as you pass a correct VersionFolderPathResolver there it should work.

The lowercase normalization should happen at package install time.

@nkolev92 nkolev92 closed this as completed Oct 8, 2024
@nkolev92 nkolev92 reopened this Oct 8, 2024
@edvilme
Copy link
Author

edvilme commented Oct 9, 2024

NuGet lowercases the packages when it writes them to global packages folder. The lowercasing is done through VersionFolderPathResolver when it's passed to PackageExtractor.InstallFromSourceAsync.

As long as you pass a correct VersionFolderPathResolver there it should work.

The lowercase normalization should happen at package install time.

Ok, so you're saying Nuget expects all packages' versions to be lowercased then?

@nkolev92
Copy link
Member

nkolev92 commented Oct 9, 2024

Packages named "MyPackage" and "mypackage" are seen as identical by the Client and servers: https://learn.microsoft.com/en-us/nuget/reference/nuspec#id.

In order to avoid potential issues such as casing changes of ids between versions, or casing issues because of how the package was specified in the PackageReference vs what it's in the nuspec, NuGet normalizes the global packages folder to lowercase: NuGet/NuGet.Client#557.

From a package author perspective, you can use any casing you want, as long as you're aware that id/version are case insensitive and they'd be deduplicated

@jebriede jebriede added WaitingForCustomer Applied when a NuGet triage person needs more info from the OP and removed Triage:Untriaged labels Oct 9, 2024
@zivkan
Copy link
Member

zivkan commented Oct 10, 2024

We should probably improve the NuGet Client SDK to make it easier to fall into the pit of success (case insensitive comparisons & extraction), rather than the pit of failure (case sensitive comparisons and/or extraction).

From my read of this issue, it sounds like the .NET tool downloader is extracting without lowercasing the version string? I'm curious what APIs the .NET SDK is using to do that, and how that differs from the APIs NuGet uses during restore. Although I've got too many other things I should be working on, so I'm not going to investigate it myself right now 😞

@nkolev92
Copy link
Member

The tools implementation explicitly overrides the path resolver: https://github.com/dotnet/sdk/blob/a4e5d121bf4d32de6c8c79432b2d3283b68f7a67/src/Cli/dotnet/NugetPackageDownloader/NuGetPackagePathResolver.cs#L10-L25, which doesn't account for casing.
In restore, we always call the InstallFromSourceAsync methods of PackageExtractor which forces you to pass a VersionFolderPathResolver, which by default handles casing correctly.

Effectively the tools call the V2 folder codepaths (packages.config, and Windows only), while restore with PR and GPF installation calls the V3 codepaths.

We can probably start by documenting those PackageExtractor APIs better.

@jeffkl
Copy link
Contributor

jeffkl commented Oct 17, 2024

This will be addressed in the .NET SDK by dotnet/sdk#43988

@jeffkl jeffkl closed this as not planned Won't fix, can't repro, duplicate, stale Oct 17, 2024
@jeffkl jeffkl added Resolution:External This issue appears to be External to nuget and removed WaitingForCustomer Applied when a NuGet triage person needs more info from the OP labels Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution:External This issue appears to be External to nuget Type:Bug
Projects
None yet
Development

No branches or pull requests

6 participants