microsoft / winget-cli

WinGet is the Windows Package Manager. This project includes a CLI (Command Line Interface), PowerShell modules, and a COM (Component Object Model) API (Application Programming Interface).
https://learn.microsoft.com/windows/package-manager/
MIT License
23.35k stars 1.45k forks source link

"Pad" shorter versions with empty parts when comparing #5001

Closed Trenly closed 1 day ago

Trenly commented 2 days ago

When versions are created, any empty parts are trimmed off. This means that 22.0.0 is stored as [22] in memory. This causes an issue when comparing to a version that had additional parts, but should be sorted lower than the version which is trimmed, such as 22.0.0-rc. This is due to the comparator seeing that [22] has no more parts to compare after the first iteration, while [22, 0, 0-rc] does, causing it to hit the condition where whichever version has more parts is considered to be greater.

This PR updates the logic so that the version comparison effectively contains as many empty parts as are needed to fully complete the comparison. This retains the logic that if a part is not equal to the part it is being compared against, the result of that comparison will be returned but eliminates the need for checking if the versions have the same number of parts since, effectively, they do; it's just that the extra parts needed to complete the comparison are all empty.

Additional tests have been added to cover this corner case.

If possible, I would ask that this also be cherry-picked into any future 1.9 releases (assuming this PR passes review and merges)

Microsoft Reviewers: Open in CodeFlow
florelis commented 2 days ago

/azp run

azure-pipelines[bot] commented 2 days ago
Azure Pipelines successfully started running 1 pipeline(s).
florelis commented 2 days ago

/azp run

azure-pipelines[bot] commented 2 days ago
Azure Pipelines successfully started running 1 pipeline(s).
Trenly commented 2 days ago

I'll get it right one of these times!

Trenly commented 1 day ago

We should update the comment explaining the sorting

Agreed and updated (and apologies for all the re-reviews @florelis!)

florelis commented 1 day ago

/azp run

azure-pipelines[bot] commented 1 day ago
Azure Pipelines successfully started running 1 pipeline(s).
Trenly commented 1 day ago

@florelis - Should I raise a cherry pick into 1.9, or has that next patch already been built?