Improve behavior of 'UpgradeBehavior: deny'

[Trenly]

• I have signed the Contributor License Agreement.
• This pull request is related to an issue.
  • Resolves upgrade --all will execute update flow for denied packages #4298
  • Resolves Denied packages should be hidden from winget upgrade list #4299

---

This PR extends the behavior of UpgradeBehavior: deny to the PackageTrackingCatalog so that packages which cannot be upgraded via winget are not included with winget upgrade --all. However, the package is still checked for whether or not an upgrade is available. If an upgrade is available, the user will be informed of the upgrade through a new table added at the end of the upgrade list. If the package is up to date, the table will not be shown.

PS D:\Git\winget-pkgs> wingetdev upgrade
Name           Id                   Version Available Source
------------------------------------------------------------
GitHub Desktop GitHub.GitHubDesktop 3.3.11  3.3.12    winget
1 upgrades available.

1 package(s) cannot be upgraded using winget. Please use the method provided by the publisher for upgrading these packages.
Name    Id              Version  Available Source
-------------------------------------------------
Discord Discord.Discord 1.0.9034 1.0.9035  winget

6 package(s) have pins that prevent upgrade. Use the 'winget pin' command to view and edit pins. Using the --include-pinned argument may show more results.

Microsoft Reviewers: Open in CodeFlow

[yao-msft]

I believe UpgradeBehavior is the behavior when an older version is upgrading to this version. It's not meant for upgrading from this version. We'll only know upgrade is denied by the manifest from the new version. We may need another way to fix this issue.

[Trenly]

I believe UpgradeBehavior is the behavior when an older version is upgrading to this version. It's not meant for upgrading from this version. We'll only know upgrade is denied by the manifest from the new version. We may need another way to fix this issue.

While it is true that UpgradeBehavior is when an older version is upgrading (same with RequireExplicitUpgrade), the deny behavior is more similar to RequireExplicitUpgrade in that when a user installs a package with the behavior of deny, IMO it shouldn't be upgraded even when a new version is available. This is because (typically) upgrade behavior doesn’t change between versions, and that the deny behavior indicates publishers have a special way the program should be updated.

I've tested locally and this change does catch the use cases where the user installs a version that has deny, it then excludes it from future upgrades.

Regardless - in the current flow, if the upgrade for the package is called explicitly, the selected installer is checked for UpgradeBehavior: deny before it is installed, which should cover most edge cases this would miss.

Without adding details on the UpdateBehavior to the index, this seemed the most appropriate route to me

[yao-msft]

I understand this approach is the easiest at the moment and it probably works for most of the cases. But this approach is misusing the UpgradeBehavior deny concept. Once it's written in the tracking catalog, those packages will not be able to upgrade automatically if a new version removes the UpgradeBehavior deny in the future, unless user deletes the tracking catalog.

I think for improving user experience, we can show the package as skipped instead of failure when performing upgrading all. For code logic correctness, we have to check the latest version for UpgradeBehavior.

For winget upgrade list, we should filter out by latest selected installer Upgradebehavior (which may be heavy).
Or
A new manifest field like RequiresExplicitUpgrade for indicating deny all upgrading in the future (but it's still weird, we can not predict in the future that a new version which supports upgrade will never happen).
Or
As a workaround fix, for those packages that have UpgradeBehavior deny in the latest version, set RequiresExplicitUpgrade on all current manifests. This solves winget upgrade --all too,

[Trenly]

Once it's written in the tracking catalog, those packages will not be able to upgrade automatically if a new version removes the UpgradeBehavior deny in the future, unless user deletes the tracking catalog

I would argue that is the way it should be, especially for deny, since Deny is set when there are known issues with upgrading the package through WinGet.

It's the same theory as for RequireExplicitUpgrade - if a user installs a version that requires explicit upgrade, won't it require explicit upgrade forever, even if a newer version doesn't require explicit upgrade?

[yao-msft]

Yes, RequireExplicitUpgrade will require explicit upgrade forever. I think it's an attribute for describing the installed version (the installed version attribute will not change regardless of future available version, it's meant for this installed version only). The UpgradeBehavior is an attribute for describing how to operate on incoming available version (which may change in future versions). Our tracking catalog is for recording installed version.

Implementation wise, I don't feel we'd mix the installed version attribute with available version attribute in the manifest. So I suggested a new manifest attribute just to describe "block upgrade from this installed version" if we want to. Unless if the team agrees with this mixed usage, writing to the tracking catalog would be the way to go.

Btw, this is a side question. RequireExplicitUpgrade is like pinning, it is soft. Deny is blocking, it's strong. Do we want manifest author to control blocking upgrade from an installed version and we write to index without user input?

[denelon]

@Trenly and I were discussing some of the behavior changes over at:

• Make InstallationNotes flow-dependent #3483

[denelon]

Maybe it makes sense to look at adding the additional notes fields to the manifest and then we can consider altering some of the behaviors in the client.