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

Error comparing versions #4998

Closed Trenly closed 1 day ago

Trenly commented 2 days ago

Brief description of your issue

See the discussion -

TL;DR

    RequireLessThan("1-rc", "1"); // Test case pases
    RequireLessThan("1.2-rc", "1.2"); // Test case passes
    RequireLessThan("1.0-rc", "1.0"); // Test case fails
Trenly commented 2 days ago

[Policy] Issue Bug

Trenly commented 2 days ago

I've linked my branch that I've used for testing. The only test that is currently failing is the one mentioned in the initial comment.

Test results ``` PS E:\winget-cli\src\x64\Debug\AppInstallerCLITests> .\AppInstallerCLITests.exe version* Filters: version* ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ AppInstallerCLITests.exe is a Catch v2.11.0 host application. Run with -? for options ------------------------------------------------------------------------------- VersionCompare ------------------------------------------------------------------------------- E:\winget-cli\src\AppInstallerCLITests\Versions.cpp(187) ............................................................................... E:\winget-cli\src\AppInstallerCLITests\Versions.cpp(145): FAILED: REQUIRE( vA < vB ) with expansion: {?} < {?} =============================================================================== test cases: 13 | 12 passed | 1 failed assertions: 271 | 270 passed | 1 failed ```

Edit:

When a version is created, all trailing zeroes are trimmed off. This means that 22.0.0, when broken into parts, is actually stored as [22] and not as [22, 0, 0]. This means that it's hitting the condition where i >= other.m_parts.size() and is just breaking the for loop instead of actually trying to pad with zeroes when comparing

Edit: Confirmed by removing the trimming, which caused the sorting to be correct. This isn’t a real fix though. . .