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

(#508) Fix nuspec casing on non-Windows #2957

Merged
merged 3 commits into from
Jan 6, 2023

Conversation

TheCakeIsNaOH
Copy link
Member

@TheCakeIsNaOH TheCakeIsNaOH commented Jan 4, 2023

Description Of Changes

The NuGet.Client libraries will install nuspec files with a lowercase filename, even if the package ID contains uppercase character(s) and the nuspec in the nupkg is also uppercase. This behavior means that on non-Windows platforms, the nuspec is not able to be found, because it has incorrect casing in the filename.

This corrects that behavior on non-Windows platforms by renaming the nuspec to the correct cased filename. On Windows, there is no need to adjust the casing, because the filesystem is case-insensitive.

The to_lower() calls when setting the PackageResult.Name are removed, so the output of the name of the package will match the actual casing of the name.

Also, integration tests are added to ensure that packages that have IDs
with uppercase character(s) can be installed, upgraded, listed,
and uninstalled.

Motivation and Context

Not being able to install/upgrade packages with uppercase IDs would be a breaking change.
This also adds tests to ensure that #2409 is fixed.

Testing

On a Linux system run:

mono ./choco.exe install wget
mono ./choco.exe list --local-only
mono ./choco.exe uninstall wget
mono ./choco.exe install wget --version=1.21.2
mono ./choco.exe upgrade wget

And ensure all operations succeed

Operating Systems Testing

  • Windows 10 22H2

Change Types Made

  • Bug fix (non-breaking change).
  • Feature / Enhancement (non-breaking change).
  • Breaking change (fix or feature that could cause existing functionality to change).
  • Documentation changes.
  • PowerShell code changes.

Change Checklist

  • Requires a change to the documentation.
  • Documentation has been updated.
  • Tests to cover my changes, have been added.
  • All new and existing tests passed?
  • PowerShell code changes: PowerShell v2 compatibility checked?

Related Issue

Fixes #2409
Part of #508
https://app.clickup.com/t/20540031/PROJ-451

@choco-bot
Copy link

@TheCakeIsNaOH TheCakeIsNaOH marked this pull request as ready for review January 5, 2023 00:14
@TheCakeIsNaOH TheCakeIsNaOH marked this pull request as draft January 5, 2023 00:31
@TheCakeIsNaOH TheCakeIsNaOH marked this pull request as ready for review January 5, 2023 17:08
The NuGet.Client libraries will install nuspec files with a lowercase
filename, even if the package ID contains uppercase character(s) and
the nuspec in the nupkg is also uppercase. This behavior means that on
non-Windows platforms, the nuspec is not able to be found, because it
has incorrect casing in the filename.

This corrects that behavior on non-Windows platforms by renaming the
nuspec to the correct cased filename. On Windows, there is no need
to adjust the casing, because the filesystem is case insensitive.
This remove to_lower() calls when setting the PackageResult.Name
so the output of the name of the package will match the actual
casing of the name.
This adds integration tests to ensure that packages that have IDs
with uppercase character(s) can be installed, upgraded, listed,
and uninstalled.
@gep13 gep13 force-pushed the uppercase-id-linux branch from 9533572 to 153516d Compare January 6, 2023 09:45
Copy link
Member

@gep13 gep13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@gep13 gep13 merged commit 88e6ab4 into chocolatey:develop Jan 6, 2023
@gep13
Copy link
Member

gep13 commented Jan 6, 2023

@TheCakeIsNaOH thanks for getting this fixed up!

@TheCakeIsNaOH TheCakeIsNaOH deleted the uppercase-id-linux branch January 6, 2023 15:14
@TheCakeIsNaOH TheCakeIsNaOH mentioned this pull request Jan 9, 2023
46 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot uninstall packages with uppercase package IDs on Mono
3 participants