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

refactor(nsis): use nsis's built-in com plugin instead of ApplicationID plugin #9527

Merged
merged 9 commits into from
Apr 24, 2024

Conversation

Legend-Master
Copy link
Contributor

@Legend-Master Legend-Master commented Apr 21, 2024

Use nsis's built-in com plugin instead of ApplicationID plugin, this reduces the installer size by 150-200 KB, and also fixes pinned shortcut not getting cleaned up on uninstall

Comment on lines 682 to 683
WinShell::UninstAppUserModelId "${BUNDLEID}"
WinShell::UninstShortcut "$DESKTOP\${MAINBINARYNAME}.lnk"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already remove the desktop shortcut on Line 691

We shouldn't remove the shortcut on uninstall, otherwise when updating it will be removed and never re-created again. We should detect if we are updating and skip removing the shortcut.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what they did in their example, I don't know if we should do it or not, I can take a look tomorrow

I don't think uninstall runs on update, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already remove the desktop shortcut on Line 691

Oh, was on mobile, forgot that this (UninstShortcut) is to unpin the shortcut

@amrbashir
Copy link
Member

amrbashir commented Apr 22, 2024

I believe this will require us to upload WinShell to our binary-releases repo but looking at https://nsis.sourceforge.io/WinShell_plug-in there seems to be an alternative way using NSIS built-in COM plugin, can we try using that instead?

@Legend-Master
Copy link
Contributor Author

I could take a look tomorrow, but I'm really getting tired of fighting nsi scripts 😂, this plugin is pretty small, and does the job, if we can take it I think we should

@amrbashir
Copy link
Member

I could take a look tomorrow, but I'm really getting tired of fighting nsi scripts 😂

No problem, just let me know and I can fight it for you

@Legend-Master Legend-Master changed the title Use WinShell instead of ApplicationID for nsis installer refactor(nsis): use nsis's built-in com plugin instead of ApplicationID plugin Apr 23, 2024
@Legend-Master Legend-Master marked this pull request as ready for review April 23, 2024 03:04
@Legend-Master Legend-Master requested a review from a team as a code owner April 23, 2024 03:04
@amrbashir amrbashir merged commit 68c39b8 into tauri-apps:dev Apr 24, 2024
14 checks passed
@Legend-Master Legend-Master deleted the nsis-winshell branch April 25, 2024 03:48
amrbashir pushed a commit that referenced this pull request Apr 29, 2024
…ID plugin (#9527)

* Use WinShell instead of ApplicationID

* Uninst shortcut before removing start menu one

* Use nsis's buit-in com plugin instead of WinShell

* Remove download ApplicationID code

* Add change file

* Clippy and format

* Allow dead code on extract_zip

* Qualify extract_zip path to make clippy happy

* Move macro up
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants