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

[Package Issue]: Discord.Discord doesn't update DisplayVersion -> remove Discord.Discord #172175

Closed
2 tasks done
treysis opened this issue Sep 7, 2024 · 14 comments
Closed
2 tasks done
Labels
Area-External Issue-Bug It either shouldn't be doing this or needs an investigation.
Milestone

Comments

@treysis
Copy link
Contributor

treysis commented Sep 7, 2024

Please confirm these before moving forward

  • I have searched for my issue and not found a work-in-progress/duplicate/resolved issue.
  • I have not been informed if the issue is resolved in a preview version of the winget client.

Category of the issue

Installation issue.

Brief description of your issue

Unfortunately, the Discord uses Squirrel.Windows for its installer, which in its current version doesn't update the DisplayVersion information in the registry: Squirrel/Squirrel.Windows#1187

Therefor, until this is resolved, I suggest to remove Discord from the winget repository in the meantime. It doesn't make sense to me to keep a non-working package in the repository.

Steps to reproduce

  • install older version of Discord
  • update Discord via winget
  • DisplayVersion under ARP is not updated

Actual behavior

DisplayVersion information in the registry is not updated by the package's installer.

Expected behavior

DisplayVersion information should be updated by the installer.

Environment

Windows Package Manager v1.8.1911
Windows: Windows.Desktop v10.0.19045.4842
System Architecture: X64
Package: Microsoft.DesktopAppInstaller v1.23.1911.0

Screenshots and Logs

No response

@treysis treysis added the Issue-Bug It either shouldn't be doing this or needs an investigation. label Sep 7, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Triage This work item needs to be triaged by a member of the core team. label Sep 7, 2024
treysis added a commit to treysis/winget-pkgs that referenced this issue Sep 7, 2024
treysis added a commit to treysis/winget-pkgs that referenced this issue Sep 7, 2024
treysis added a commit to treysis/winget-pkgs that referenced this issue Sep 7, 2024
treysis added a commit to treysis/winget-pkgs that referenced this issue Sep 7, 2024
treysis added a commit to treysis/winget-pkgs that referenced this issue Sep 7, 2024
treysis added a commit to treysis/winget-pkgs that referenced this issue Sep 7, 2024
treysis added a commit to treysis/winget-pkgs that referenced this issue Sep 7, 2024
treysis added a commit to treysis/winget-pkgs that referenced this issue Sep 7, 2024
treysis added a commit to treysis/winget-pkgs that referenced this issue Sep 7, 2024
treysis added a commit to treysis/winget-pkgs that referenced this issue Sep 7, 2024
treysis added a commit to treysis/winget-pkgs that referenced this issue Sep 7, 2024
treysis added a commit to treysis/winget-pkgs that referenced this issue Sep 7, 2024
treysis added a commit to treysis/winget-pkgs that referenced this issue Sep 7, 2024
treysis added a commit to treysis/winget-pkgs that referenced this issue Sep 7, 2024
@mdanish-kh
Copy link
Contributor

I don't think removing is the right call. Keeping the manifest still adds value for install/uninstall scenarios. There may be users that rely on winget install Discord.Discord (a command that works correctly) on a fresh machine.

update Discord via winget

Curious, how are you able to do this right now? You shouldn't be able to as we added UpgradeBehavior: deny in the manifest to block upgrades for this package.

 ~ winget upgrade Discord.Discord
The package cannot be upgraded using winget. Please use the method provided by the publisher for upgrading this package.

While it is unfortunate that package does not work well for upgrade scenarios, removing it altogether seems a bit excessive and more importantly a breaking move

@microsoft-github-policy-service microsoft-github-policy-service bot added Area-External and removed Needs-Triage This work item needs to be triaged by a member of the core team. labels Sep 7, 2024
@treysis
Copy link
Contributor Author

treysis commented Sep 7, 2024

@stevenlele please don't push new versions of Discord.Discord until Squirrel removes the bug in their installer script.

@treysis
Copy link
Contributor Author

treysis commented Sep 7, 2024

You shouldn't be able to as we added UpgradeBehavior: deny in the manifest to block upgrades for this package.

In that case it shouldn't be listed in the upgrade list, right? I do understand that this would be an issue for winget-cli. EDIT: I saw you reported this in microsoft/winget-cli#4299
EDIT 2: Although it's not true, that updates don't work (yes, if they're denied, winget blocks it). The update itself works, but just the version number is not updated.

I don't see how this is a breaking move, Discord can still be installed using the traditional methods. It's also not a given that and when the most recent version will land in winget's repo.

Furthermore, removing ShiningLight's OpenSSL also finally made their owner aware of the gravity of the issue and to change the installer script to reflect the correct version information. If we just leave it the way it is, Squirrel will never implement that urgently needed fix. So yeah, I think it's important to go this way. It's change, but change for the better!

@stevenlele
Copy link
Contributor

In that case it shouldn't be listed in the upgrade list, right? I do understand that this would be an issue for winget-cli.

It used to be listed separately because of RequireExplicitUpgrade: true, but somehow the combination with the new UpgradeBehavior: deny (or just a new version of WinGet) broke that behavior - Discord goes back to the main list. I'm not sure if it's a bug.

@mdanish-kh
Copy link
Contributor

mdanish-kh commented Sep 7, 2024

I don't see how this is a breaking move

Breaking move for anyone that uses winget install Discord.Discord or winget uninstall Discord.Discord (as an example, think of someone using this command to install / uninstall the application on a fleet of devices). These commands won't work after the manifests are gone

Discord can still be installed using the traditional methods.

WinGet exists because developers want a one-command solution instead of traditional methods. Why take that ability?

Furthermore, removing ShiningLight's OpenSSL also finally made their owner aware of the gravity of the issue and to change the installer script to reflect the correct version information.

That's subjective, there's no guaranteeing that there will be any movement on the issue even if the package is removed (not factoring if that even is the right way to invoke change). We should not break things for existing WinGet users for something that may not have an effect

@R-Adrian
Copy link

R-Adrian commented Sep 8, 2024

The update itself works, but just the version number is not updated.

that "update" also has another user consent problem: when launched by WinGet the Discord installer ignores user preferences and forcefully enables autostart with Windows on next logon even if this is autostart is disabled in the Discord app settings itself and in the windows registry / start menu.
The installer ignores all that and forcefully sets Discord to start at logon.
This behavior and the version number not being updated in registry are also in #90642 from December 2022

When Discord is let to handle updates itself then the autostart at logon problem does not happen, but it still does not update the version number in the registry (even if the version is displayed correctly in the app).

TL;DR version: because of Discord's relatively good self-update mechanism i'm against removing it from WinGet manifests, it should be kept only for initial installation and then left alone to handle updates itself. (i.e. as it is now with UpgradeBehavior: deny RequireExplicitUpgrade: true )

@treysis
Copy link
Contributor Author

treysis commented Sep 8, 2024

that "update" also has another user consent problem: when launched by WinGet the Discord installer ignores user preferences and forcefully enables autostart with Windows on next logon even if this is autostart is disabled in the Discord app settings itself and in the windows registry / start menu.

But that's common with other apps as well? E.g. Opera if updated through winget will set itself as the default browser. Or Spotify always overwrites its shortcut in the start menu when updated through winget compared to updating from inside the app.

@R-Adrian
Copy link

R-Adrian commented Sep 8, 2024

But that's common with other apps as well?

then if updating those apps causes forced changes of already-expressed user consent refusal then probably those apps should also have UpgradeBehavior: deny RequireExplicitUpgrade: true

i think i could probably add AnyDesk and TeamViewer to that list of badly behaving apps - each update of these forcefully enables a service that runs with SYSTEM level permissions even if the user has manually disabled that service and any start-with-OS setting in the app.
Also, PowerShell 7.4 forcefully enables telemetry on each update, in violation of the GDPR, even if the user has epressly denied consent for telemetry - the PowerShell installer wipes that environment variable preference and thus enables telemetry.

(i have long since disabled any sort of auto-start and also pinned Discord/AnyDesk/Teamviewer/PowerShell v7 in WinGet settings to tell it to stop trying to auto-update them - it is still notifying me but no longer tries to update them)

@Trenly
Copy link
Contributor

Trenly commented Sep 8, 2024

In that case it shouldn't be listed in the upgrade list, right? I do understand that this would be an issue for winget-cli.

It used to be listed separately because of RequireExplicitUpgrade: true, but somehow the combination with the new UpgradeBehavior: deny (or just a new version of WinGet) broke that behavior - Discord goes back to the main list. I'm not sure if it's a bug.

The UpgradeBehavior has no effect on if it is listed or not. RequreExplicitUpgrade only works if Discord was initially installed through WinGet. If Discord was installed outside of WinGet, then it will show in the list for all upgrades and not require explocit targetting

@Trenly
Copy link
Contributor

Trenly commented Sep 8, 2024

After reading through the rest of this thread, I agree with @R-Adrian that removing this application could break several user workflows and that it should remain in the repo for that reason.


There are many applications who's self-upgrade doesn’t update the display version, but where using the installer does. WinGet handles this just fine by downloading and re-installing using the installer to ensure everything is up to date. The reason Discord is specifically marked the way it is was due to #90642.

Regarding wherher other applications should be marked as requiring explicit upgrade or denying upgrade entirely - that should be taken on a case by case basis with the behavior of each application that is thought to be badly behaving verified not only by the PR submitter but also by others in the community.


It is also important to remember that you can exclude applications from being updated on your own systems without removing them from the community repo. Just run winget pin add <package>.

If you don’t like the way WinGet handles updates for an application, you can pin it and then WinGet won’t try to upgrade it when you use winget upgrade --all.

Close with reason: Known issue; Package should remain in repo

@dlong500
Copy link

WinGet exists because developers want a one-command solution instead of traditional methods. Why take that ability?

@mdanish-kh for what it's worth, the value of winget diminishes if there is a bunch of clutter from apps that don't conform to standards. If apps can't populate a simple version string in the right place it's not outlandish to consider whether they should be allowed to participate in the ecosystem. At the very least there should be some real pressure to get an app as popular as Discord to fix an issue on their end (which in turn might end up fixing the issue for other Squirrel-based apps).

@tute123456
Copy link

tute123456 commented Nov 27, 2024

this command works for me
pws(admin right)
choco upgrade discord.install

@treysis
Copy link
Contributor Author

treysis commented Nov 27, 2024

@tute123456
We are talking about winget.

@denelon denelon added this to the 1.10 Packages milestone Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-External Issue-Bug It either shouldn't be doing this or needs an investigation.
Projects
None yet
Development

No branches or pull requests

9 participants