-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Improve behavior of 'UpgradeBehavior: deny' #4300
Conversation
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 I've tested locally and this change does catch the use cases where the user installs a version that has Regardless - in the current flow, if the upgrade for the package is called explicitly, the selected installer is checked for Without adding details on the UpdateBehavior to the index, this seemed the most appropriate route to me |
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). |
I would argue that is the way it should be, especially for deny, since 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? |
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? |
@Trenly and I were discussing some of the behavior changes over at: |
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. |
upgrade --all
will execute update flow for denied packages #4298winget upgrade
list #4299This PR extends the behavior of
UpgradeBehavior: deny
to the PackageTrackingCatalog so that packages which cannot be upgraded via winget are not included withwinget 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.Microsoft Reviewers: Open in CodeFlow