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

VersionRange causes issues when packages have multiple DisplayVersion #4125

Open
Trenly opened this issue Feb 1, 2024 · 8 comments
Open
Labels
Issue-Feature This is a feature request for the Windows Package Manager client.

Comments

@Trenly
Copy link
Contributor

Trenly commented Feb 1, 2024

Brief description of your issue

In a manifest, different installers can have different DisplayVersion. Because of the way the matching logic works, a range is created encompassing the highest and lowest version within the manifest. Between two manifests, the version ranges are not allowed to overlap.

This causes issues when the display versions are vastly different between installers. Take Zoom.Zoom for example - The exe installer writes 5.17.2 (29988) while the msi writes 5.17.29988. This means that the version range is calculated as 5.17.2 - 5.17.29988. When version 5.17.3 is released, it is seen as a version range overlap because it is greater than 5.17.2 and less than 5.17.29988. This makes it impossible to achieve correct version mapping for Zoom.

Steps to reproduce

microsoft/winget-pkgs#137363

Expected behavior

I expect that when a DisplayVersion is in the manifest, the ARP entry must exactly match that version. If different installers have different DisplayVersion, then an exact match would be required with a DisplayVersion corresponding to an installer to determine the manifest version

Actual behavior

microsoft/winget-pkgs#137363 (comment)

Environment

Winget 1.5 through 1.7-preview
@stephengillie
Copy link

It seems like a goal is to solve the version matching issue without invoking local storage on a PC, but honestly having something like a local winget.db, with a managed list of what's installed, could vastly improve the user experience in certain situations.

@Trenly
Copy link
Contributor Author

Trenly commented Feb 1, 2024

It seems like a goal is to solve the version matching issue without invoking local storage on a PC, but honestly having something like a local winget.db, with a managed list of what's installed, could vastly improve the user experience in certain situations.

I disagree, since applications can change underneath that database - In fact, installed.db already exists today for packages the user installed with WinGet.

@stephengillie stephengillie added Issue-Feature This is a feature request for the Windows Package Manager client. and removed Needs-Triage Issue need to be triaged labels Feb 1, 2024
@JohnMcPMS
Copy link
Member

The goal of the DisplayVersion version mapping feature is two-fold (at least to my memory, which I admit is not infallible...):

  1. To allow for creating strong ordering when inherently unordered versions are used by packages.
    • For instance, using a commit hash as part of a version does not indicate which version is newer.
    • This does require that every version be known though, since there is no way to place an unknown version as that was the original problem.
  2. To allow for "marketing" versions, similar to the way that Visual Studio uses years as the well known version number, but internally the actual version is more "normal". This makes it easier for a consumer to map to what they are told, "VS 2022", rather than "17".
    • Because some of the target packages for this portion also had some "drift" amongst various installers (ex. x86 is .1, x64 is .2, etc.), we support a range of versions mapping to the same package version value.
    • It is an error for multiple package versions to have overlapping DisplayVersion ranges.

With that history out of the way, we can discuss the issue at hand: How could we support the case where different installers use wildly different version numbers, leading to overlapping ranges.

Forcing exact matching only would be an option, although I don't like that it means that we are making an ordered version into an unordered version if we don't know about it. It would also mean adding support for multiple ranges since the easiest method would be to simply create N "ranges" for the N unique versions in the manifest.

I'm not sure that we can automatically detect different modes amongst the versions to create multiple intended ranges. The potential for error seems high. It would probably be better to have it indicated in the manifest in some fashion. Multiple ranges also inherently become unordered if they need this, since we no longer can place an unknown version.

Because the versions differ between installer types for this instance, one solution would be splitting the package into EXE and MSI variants. That might not work very well, and generally I'm not a fan of it as a solution. But if coupled with the feature for "virtual packages", it is possible that the user experience could be maintained. If Zoom.Zoom was a virtual package collection of Zoom.Zoom.MSI and Zoom.Zoom.EXE, I feel like almost everything would be the same except you would see that one of the installer specifics was actually installed via list/upgrade

@Trenly
Copy link
Contributor Author

Trenly commented Feb 1, 2024

To allow for "marketing" versions, similar to the way that Visual Studio uses years as the well known version number, but internally the actual version is more "normal". This makes it easier for a consumer to map to what they are told, "VS 2022", rather than "17".

  • Because some of the target packages for this portion also had some "drift" amongst various installers (ex. x86 is .1, x64 is .2, etc.), we support a range of versions mapping to the same package version value.
  • It is an error for multiple package versions to have overlapping DisplayVersion ranges.

This is partially accurate to the way I remember it. The concept of "Marketing Version" as I remember it had more to do with exact versions published on an ISV's website. For example - major versions like "VS 2022" and "VS 2017" are generally separate packages; however - the .NET team publishes on the website that their version is 3.1.32 but the actual version is 3.1.32.31915. This makes the "Marketing Version" 3.1.32, which we use for the PackageVersion, since that is what users see.

When it comes to packages having "drift", the range still would need to be in the manifest and specified within the DisplayVersion for each installer. If there is only one DisplayVersion, the range for the package has the same minimum and maximum and is, in effect, an exact match?

Forcing exact matching only would be an option, although I don't like that it means that we are making an ordered version into an unordered version if we don't know about it. It would also mean adding support for multiple ranges since the easiest method would be to simply create N "ranges" for the N unique versions in the manifest.

I guess my question is really around what you mean by if we don't know about it ? As I alluded to above, in order to even create a version range, doesn't there have to be multiple DisplayVersion specified? If I'm understanding correctly - your concern is that a user may have a package version installed which does not match exactly to a DisplayVersion specified in the repository? If that is the case, then would that not be the same as the case as it is today, where if the version does not fall into a range that Winget is aware about, then the cli attempts to reason about whether it is greater than or less than the latest available version?

@JohnMcPMS
Copy link
Member

If there is only one DisplayVersion, the range for the package has the same minimum and maximum and is, in effect, an exact match?

Yes, this is the case.

If I'm understanding correctly - your concern is that a user may have a package version installed which does not match exactly to a DisplayVersion specified in the repository?

Correct.

If that is the case, then would that not be the same as the case as it is today, where if the version does not fall into a range that Winget is aware about, then the cli attempts to reason about whether it is greater than or less than the latest available version?

We attempt to map the display version from the installed entry. If it falls into a range (which may be a single value), then it is that exact package version. Otherwise it is considered greater or less than a version. It must be mapped so that we can order it. For the commit hash type of version, we end up just doing string sorts which is probably not helpful. Forcing exact matching will result in this for any package type.

@Trenly
Copy link
Contributor Author

Trenly commented Feb 1, 2024

We attempt to map the display version from the installed entry. If it falls into a range (which may be a single value), then it is that exact package version. Otherwise it is considered greater or less than a version. It must be mapped so that we can order it. For the commit hash type of version, we end up just doing string sorts which is probably not helpful. Forcing exact matching will result in this for any package type.

I guess it's the entire concept of the ranges which confuses me, then, because for any given Package Version the exact DisplayVersion of each installer should be known. In fact, in order to create a range which is more than a single version, there must be multiple DisplayVersion which are known. Even taking the example of "drift" where different architectures have different versions written to registry - the versions for the architectures must be known in order to create the range.

Out of curiosity, I took a look at the number of packages that had multiple DisplayVersion defined. Of all the packages in the repo, only 31 manifests have multiple DisplayVersion defined, and those manifests correspond to only 10 packages. Of those 10 packages, only 6 of them truly have multiple display versions defined - the others are errors with whitespace or comments because I was lazy when writing the PowerShell to check. Of those packages, 4 of them only differ by x86 or x64 (or similar) being appended after the actual display version. This leaves just 2 packages that truly have version ranges - Zoom.Zoom (which we know has issues) and VideoLAN.VLC (Which is due to an installer issue where the msi version is written incorrectly).

This means that 99% of the time, the "range" is actually a single version and exact mapping wouldn't impact these packages. The other 1% of the time, the exact versions are known anyways.

PowerShell and output of files with multiple DisplayVersion
PS D:\Git\winget-pkgs> $installerManifests = gci -Recurse -Filter '*.installer.yaml'    
PS D:\Git\winget-pkgs> $displayVersionManifests = $installerManifests|? {$null -ne $($_|Get-Content -Encoding UTF8|sls 'DisplayVersion')}
PS D:\Git\winget-pkgs> $multipleVersionManifests = $displayVersionManifests | ? {  $($_ |Get-Content -Encoding UTF8|sls 'DisplayVersion'|Select-Object -Unique).count -gt 1 }
PS D:\Git\winget-pkgs> $displayVersionManifests.Count
1791
PS D:\Git\winget-pkgs> $multipleVersionManifests.Count
31
PS D:\Git\winget-pkgs> $multipleVersionManifests.FullName
D:\Git\winget-pkgs\manifests\k\KDE\Krita\4.4.3\KDE.Krita.installer.yaml
D:\Git\winget-pkgs\manifests\l\L0phtHoldings\L0phtCrack\7.2.0\L0phtHoldings.L0phtCrack.installer.yaml
D:\Git\winget-pkgs\manifests\m\Maltego\Maltego\4.5.0\Maltego.Maltego.installer.yaml
D:\Git\winget-pkgs\manifests\o\OliverBetz\ExifTool\12.41\OliverBetz.ExifTool.installer.yaml
D:\Git\winget-pkgs\manifests\o\OliverBetz\ExifTool\12.44\OliverBetz.ExifTool.installer.yaml
D:\Git\winget-pkgs\manifests\o\OliverBetz\ExifTool\12.60\OliverBetz.ExifTool.installer.yaml
D:\Git\winget-pkgs\manifests\o\OliverBetz\ExifTool\12.70\OliverBetz.ExifTool.installer.yaml
D:\Git\winget-pkgs\manifests\o\OliverBetz\ExifTool\Development\12.62\OliverBetz.ExifTool.Development.installer.yaml    
D:\Git\winget-pkgs\manifests\o\OliverBetz\ExifTool\Development\12.65\OliverBetz.ExifTool.Development.installer.yaml    
D:\Git\winget-pkgs\manifests\o\opentrack\opentrack\2022.3.2\opentrack.opentrack.installer.yaml
D:\Git\winget-pkgs\manifests\r\RomanKubiak\Ctrlr\5.6.0\RomanKubiak.Ctrlr.installer.yaml
D:\Git\winget-pkgs\manifests\v\VideoLAN\VLC\3.0.17.4\VideoLAN.VLC.installer.yaml
D:\Git\winget-pkgs\manifests\w\WidelandsDevelopmentTeam\Widelands\1.0\WidelandsDevelopmentTeam.Widelands.installer.yaml
D:\Git\winget-pkgs\manifests\z\Zoom\Zoom\5.15.10.20823\Zoom.Zoom.installer.yaml
D:\Git\winget-pkgs\manifests\z\Zoom\Zoom\5.15.11.21032\Zoom.Zoom.installer.yaml
D:\Git\winget-pkgs\manifests\z\Zoom\Zoom\5.15.12.21574\Zoom.Zoom.installer.yaml
D:\Git\winget-pkgs\manifests\z\Zoom\Zoom\5.15.2.18096\Zoom.Zoom.installer.yaml
D:\Git\winget-pkgs\manifests\z\Zoom\Zoom\5.15.3.18551\Zoom.Zoom.installer.yaml
D:\Git\winget-pkgs\manifests\z\Zoom\Zoom\5.15.6.19959\Zoom.Zoom.installer.yaml
D:\Git\winget-pkgs\manifests\z\Zoom\Zoom\5.15.7.20303\Zoom.Zoom.installer.yaml
D:\Git\winget-pkgs\manifests\z\Zoom\Zoom\5.16.0.22201\Zoom.Zoom.installer.yaml
D:\Git\winget-pkgs\manifests\z\Zoom\Zoom\5.16.1.22523\Zoom.Zoom.installer.yaml
D:\Git\winget-pkgs\manifests\z\Zoom\Zoom\5.16.10.26186\Zoom.Zoom.installer.yaml
D:\Git\winget-pkgs\manifests\z\Zoom\Zoom\5.16.2.22807\Zoom.Zoom.installer.yaml
D:\Git\winget-pkgs\manifests\z\Zoom\Zoom\5.16.5.24296\Zoom.Zoom.installer.yaml
D:\Git\winget-pkgs\manifests\z\Zoom\Zoom\5.16.6.24712\Zoom.Zoom.installer.yaml
D:\Git\winget-pkgs\manifests\z\Zoom\Zoom\5.17.0.28375\Zoom.Zoom.installer.yaml
D:\Git\winget-pkgs\manifests\z\Zoom\Zoom\5.17.1.28914\Zoom.Zoom.installer.yaml
D:\Git\winget-pkgs\manifests\z\Zoom\Zoom\5.17.2.29988\Zoom.Zoom.installer.yaml
D:\Git\winget-pkgs\manifests\z\Zoom\Zoom\5.17.5.31030\Zoom.Zoom.installer.yaml
D:\Git\winget-pkgs\manifests\z\Zoom\Zoom\5.17.7.31859\Zoom.Zoom.installer.yaml

Edit: I do realize that this is just the winget-pkgs repo, and that others could have private sources set up, but I feel it's a representative enough sample to draw some assumptions

@JohnMcPMS
Copy link
Member

The range part isn't actually that important; it is just data that is fed into the mapping function. The mapping is the part that breaks down if there are overlaps in the version space (avoiding the term range here) that package versions occupy.

What we need to do in these cases is translate the display version into the package version space. We only make decisions about upgrades in the package version space. And we cannot assume that we have complete knowledge of the set of released installers.

So we have a function (explanatory names here) mapToPackageVersion(displayVersion). It needs to do the mapping. If the displayVersion falls into one of the package version DisplayVersion ranges (which maybe be a single value, meaning they are equal), then the result is that package version. But if it doesn't fall into a range, we return a version that indicates its relation to a known package version (the "greater than" versions that we will show for example).

Now imagine that we allow only exact values, and there is a case where the exact values for two versions is: version 1 { A, D } and version 2 { B, E }. I am using letters for the DisplayVersion here to make them obviously distinct while still providing a strong ordering. What is the result of mapToPackageVersion(C)? Is an upgrade available?

Note: All of this breaks down for the inherently unordered version cases, but in those cases the goal is only to enable the addition of ordering data.

@Trenly
Copy link
Contributor Author

Trenly commented Feb 2, 2024

Now imagine that we allow only exact values, and there is a case where the exact values for two versions is: version 1 { A, D } and version 2 { B, E }. I am using letters for the DisplayVersion here to make them obviously distinct while still providing a strong ordering. What is the result of mapToPackageVersion(C)? Is an upgrade available?

Would it not be the same as it is today, where version "C" doesn’t fit anywhere nicely, and an upgrade is always available.

In fact, I think this behavior can be seen today if a new manifest version doesn’t contain DisplayVersion and older manifests do

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Feature This is a feature request for the Windows Package Manager client.
Projects
None yet
Development

No branches or pull requests

3 participants