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

Specify whether a package is upgradable if already installed #3482

Closed
o-l-a-v opened this issue Jul 28, 2023 · 12 comments
Closed

Specify whether a package is upgradable if already installed #3482

o-l-a-v opened this issue Jul 28, 2023 · 12 comments
Labels
Issue-Feature This is a feature request for the Windows Package Manager client.
Milestone

Comments

@o-l-a-v
Copy link

o-l-a-v commented Jul 28, 2023

Description of the new feature / enhancement

Some packages do not support being upgraded by a newer installer, like Chocolatey (MSI, Wix).

Related issue over at choco:

Chocolatey in winget-pkgs repo:

Some context about Chocolatey

The Chocolatey MSI installer can be used for initial install only, upgrading should be done using choco itself.

  • If you install a newer version of the MSI installer over an older version, Chocolatey breaks.
  • If you uninstall Chocolatey MSI, Chocolatey itself gets left behind.

AFAIK there is no way Winget can successfully upgrade Chocolatey right now.

Proposed technical implementation details

  • New manifest property to tell whether a new version can be installed over an old version? Remember that mentioned edge case won't work if specifying UpgradeBehavior to uninstallPrevious either.
  • Add option to existing manifest property UpgradeBehavior: no, false or similar?
  • (Probably not) Add optional custom upgrade command? choco upgrade chocolatey. Wait and check exit code for success.
@o-l-a-v o-l-a-v added the Issue-Feature This is a feature request for the Windows Package Manager client. label Jul 28, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Triage Issue need to be triaged label Jul 28, 2023
@denelon denelon removed the Needs-Triage Issue need to be triaged label Jul 28, 2023
@denelon
Copy link
Contributor

denelon commented Jul 28, 2023

@pauby Is this the intended behavior for the MSI/(wix) package?

@denelon
Copy link
Contributor

denelon commented Jul 28, 2023

We do have an installation notes field that could be used to let customers know they should perform upgrades via Chocolatey directly, and it would likely be best to figure out how to handle users who might have installed older versions of the MSI so WinGet doesn't end up in a state where the user is confused.

@denelon
Copy link
Contributor

denelon commented Jul 28, 2023

I don't know how chocolatey is versioned, so it might also make sense to modify the name to indicate the package is the "Chocolatey Installer" or something like that. When users look in Windows Apps & Features they might be confused by thinking the version reported matches the version of Chocolatey itself.

@pauby
Copy link

pauby commented Jul 28, 2023

The Chocolatey CLI MSI is intended only for the installation of Chocolatey CLI.

image

But the intention is not to break the installation for people who do upgrade via the MSI. It should either do the upgrade or not do the upgrade, and exit gracefully. At the moment it is broken.

That's what we are going to look at for the next release (which is imminent). Until that time, we'll pick up any issues in the choco repository.

We do have an installation notes field

We'll look at updating that and I'll ask the team to reach out if we need more information on that.

@denelon
Copy link
Contributor

denelon commented Jul 28, 2023

Awesome! If you want some additional options, you could look at "expected return codes". Then you could exit with a specific code and have a message reported to the user regarding the reason the install didn't "succeed", but that it is a known expected result.

@corbob
Copy link

corbob commented Aug 2, 2023

@denelon Paul asked me to take a look at the Chocolatey MSI. We've gone ahead and fixed it for upcoming versions, but we would like to update the InstallationNotes for the existing packages. Would the process for those be to open a single PR to update all of the manifests? Or would we need 3 PRs for the 3 different versions?

Assuming of course that we're allowed to update the existing manifests.

@mdanish-kh
Copy link
Contributor

@corbob You would need 3 separate PRs as the CI pipelines can only validate a change in single manifest per PR.

Assuming of course that we're allowed to update the existing manifests.

Certainly so!

@R-Adrian
Copy link

R-Adrian commented Sep 17, 2023

just posting a note to add my experiences about the list of broken-on-update / misidentified packages:

Discord.Discord - does self-updates and does not update the version number in the registry when doing so, applying an update via winget will break/override the user preferences and forcefully enable auto-start on system boot microsoft/winget-pkgs#90642

Mozilla.Firefox.DeveloperEdition - does self-updates and shows a "stable" version number in the registry even though actually the self-update installs a developer/beta version

TeamViewer.TeamViewer - does self-updates and any upgrade applied (even via the self-update integrated one) will forcefully enable a system service with Automatic startup type, service that runs as Local System account (NT AUTHORITY\SYSTEM user) and it forcefully enables that service even if the user disables the "Start TeamViewer with Windows" checkbox in the program options and any other start-on-system-boot functions.
yes, TeamViewer behaves a bit like malware here - i have flagged this malware-like behaviour on their forums multiple times and they do it on purpose. The system account service running all the time even if the user does not want it is intended by design.

@corbob
Copy link

corbob commented Oct 13, 2023

Is this actually solved by RequireExplicitUpgrade: true? The schema docs seem to indicate that this is for applications that upgrade themselves.

Related(?) PR: microsoft/winget-pkgs#122575

@R-Adrian
Copy link

the explicit upgrade flag is not really helpful when the installer is badly designed in the first place, it only excludes the package from upgrade --all but does not prevent actually trying to upgrade it by upgrade <package name here>.

In the sample cases that i mentioned above (Discord, TeamViewer) even if the installer might behave as if it's doing an upgrade it actually deletes and/or overrides some user settings and resets them to defaults instead of doing an upgrade that preserves user preferences.

In this matter the TeamViewer installer is actually a bit malicious on purpose - instead of a proper upgrade that preserves user preferences and service settings it wipes the service registered to run as NT AUTHORITY\SYSTEM and registers a fresh service in its place, with the same service name and file path, sets it to Automatic start and starts it... and it does this even if the previous entry for the TV service was set to Disabled startup type, so that the service does not run.

@R-Adrian
Copy link

i was looking through the Winget v1.6 changes and i see that in v1.6 we can now have UpgradeBehavior: deny in manifests, which seems to be exactly what we need to solve this issue...
#3512

The problem is that the validation pipelines are not prepared to parse this because there is currently no v1.6 manifest schema
https://github.com/microsoft/winget-pkgs/tree/master/doc/manifest/schema
so this issue depends on microsoft/winget-pkgs#121127

@denelon
Copy link
Contributor

denelon commented Mar 6, 2024

Fixed 😊

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

6 participants