RequireExplicitUpgrade not requiring explicit upgrade

[Trenly]

Brief description of your issue

When installing Microsoft Teams, it should be pinned through RequireExplicitUpgrade as implemented in #1795 and released in v1.4.2161-preview

Installing teams and changing the version to trigger an upgrade reveals that the package is not requiring explicit upgrade.

https://github.com/microsoft/winget-pkgs/blob/master/manifests/m/Microsoft/Teams/1.5.00.21668/Microsoft.Teams.installer.yaml#L8

Steps to reproduce

• Uninstall Microsoft Teams
• Reinstall Microsoft Teams using winget install Microsoft.Teams --scope=machine --source=winget
• Modify the ARP entry of Teams Machine-Wide Installer to have DisplayVersion be a lower value, so that an upgrade would be triggered. Be sure to only change the DisplayVersion
• Run winget upgrade --all

Expected behavior

Teams to be pinned by default and not upgrade without explicit upgrade - as it was installed with a manifest that stated explicit upgrade was required

Actual behavior

PS D:\Git\winget-pkgs> winget uninstall Microsoft.Teams
Found Microsoft Teams [Microsoft.Teams]
Starting package uninstall...
Successfully uninstalled
PS D:\Git\winget-pkgs> winget install Microsoft.Teams --scope=machine -s=winget
Found Microsoft Teams [Microsoft.Teams] Version 1.5.00.21668
This application is licensed to you by its owner.
Microsoft is not responsible for, nor does it grant any licenses to, third-party packages.
Downloading https://statics.teams.cdn.office.net/production-windows-x64/1.5.00.21668/Teams_windows_x64.msi
  ██████████████████████████████   124 MB /  124 MB
Successfully verified installer hash
Starting package install...
Successfully installed
PS D:\Git\winget-pkgs> winget list teams
Name                         Id              Version
--------------------------------------------------------
Teams Machine-Wide Installer Microsoft.Teams 1.5.0.21668

// Registry Modified Manually

PS D:\Git\winget-pkgs> winget list teams
Name                         Id              Version     Available
---------------------------------------------------------------------
Teams Machine-Wide Installer Microsoft.Teams 1.5.0.21667 1.5.00.21668
PS D:\Git\winget-pkgs> winget upgrade --all
Name                                                               Id                                Version          Available
-----------------------------------------------------------------------------------------------------------------------------------
Microsoft .NET SDK 6.0.304 (x64)                                   Microsoft.DotNet.SDK.6            6.0.304          6.0.401
|
Teams Machine-Wide Installer                                       Microsoft.Teams                   1.5.0.21667      1.5.00.21668
Windows Software Development Kit - Windows 10.0.22000.832          Microsoft.WindowsSDK              10.0.22000.832   10.0.22621.1
Microsoft .NET SDK 6.0.203 (x64)                                   Microsoft.DotNet.SDK.6            6.0.203          6.0.401
9 upgrades available.
1 package(s) have version numbers that cannot be determined. Use "--include-unknown" to see all results.

(1/5) Found Microsoft .NET SDK 6.0 [Microsoft.DotNet.SDK.6] Version 6.0.401
|
(5/5) Found Microsoft Teams [Microsoft.Teams] Version 1.5.00.21668
This application is licensed to you by its owner.
Microsoft is not responsible for, nor does it grant any licenses to, third-party packages.
Downloading https://statics.teams.cdn.office.net/production-windows-x64/1.5.00.21668/Teams_windows_x64.msi
  ██████████████████████████████   124 MB /  124 MB
Successfully verified installer hash
Starting package install...
Successfully installed

1 package(s) have version numbers that cannot be determined. Use "--include-unknown" to see all results.

Note in the Teams Install.log that it shows the manifest has RequireExplicitUpgrade: True.

UpgradeLog.zip; Note- Had to zip the upgrade log since it was too big to upload raw

Environment

PS D:\Git\winget-pkgs> winget --info
Windows Package Manager (Preview) v1.4.2161-preview
Copyright (c) Microsoft Corporation. All rights reserved.

Windows: Windows.Desktop v10.0.19044.2006
System Architecture: X64
Package: Microsoft.DesktopAppInstaller v1.19.2161.0

Logs: %LOCALAPPDATA%\Packages\Microsoft.DesktopAppInstaller_8wekyb3d8bbwe\LocalState\DiagOutputDir

Links
---------------------------------------------------------------------------
Privacy Statement   https://aka.ms/winget-privacy
License Agreement   https://aka.ms/winget-license
Third Party Notices https://aka.ms/winget-3rdPartyNotice
Homepage            https://aka.ms/winget
Windows Store Terms https://www.microsoft.com/en-us/storedocs/terms-of-sale

[Trenly]

I think this is because changing the version to be lower causes this line to return no metadata, as Microsoft.Teams only keeps the latest version published, so older versions metadata aren't in the repo.

winget-cli/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp

Line 191 in a394ea8

auto installedMetadata = updateContext.Get<Execution::Data::InstalledPackageVersion>()->GetMetadata();

This means that the logic described in the comments is likely not sound.

winget-cli/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp

Lines 182 to 190 in a394ea8

	// Filter out packages that require explicit upgrades.
	// We require explicit upgrades only if the installed version is pinned,
	// either because it was manually pinned or because the manifest indicated
	// RequireExplicitUpgrade.
	// Note that this does not consider whether the update to be installed has
	// RequireExplicitUpgrade. While this has the downside of not working with
	// packages installed from another source, it ensures consistency with the
	// list of available updates (there we don't have the selected installer)
	// and at most we will update each package like this once.

If there are no older versions available in the source to get the pinning metadata from, it will always be upgraded, regardless of the pinning status. I would say the behavior should be changed so that if either the version being upgraded from, or the version being upgraded to, have RequireExplicitUpgrade: true then the package requires explicit upgrade.

There are a few reasons that checking the version being upgraded to for requiring explicit upgrade may be important. The upgrade could introduce breaking changes that weren't known before release, the package may be from a different source, older package versions may no longer be available. While this has the downside of not hiding the installer from the upgrade list, as long as a clear message is printed to the user (e.g <Package> was skipped as it requires explicit upgrade) I don't see much of an issue with it

[florelis]

In theory, the logic for requiring explicit upgrade is based on the tracking catalog, so it shouldn't matter if there isn't a matching version in the source. Unless there's a bug with that...
I think the original target of the feature were packages that would auto-update themselves. Thinking of that scenario, I believe it's better to not require explicit upgrade when moving from a version that didn't require it to one that does, because it means updating to a version that now can auto-update.

[Trenly]

I believe this is resolved with the latest changes to the pinning flow