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

fix(bundler/nsis): use uninstallstring to find wix installation #7326

Merged
merged 3 commits into from
Jul 8, 2023

Conversation

luoffei
Copy link
Contributor

@luoffei luoffei commented Jun 30, 2023

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Docs
  • New Binding issue #___
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change?

  • Yes, and the changes were approved in issue #___
  • No

Checklist

  • When resolving issues, they are referenced in the PR's title (e.g fix: remove a typo, closes #___, #___)
  • A change file is added if any packages will require a version bump due to this PR per the instructions in the readme.
  • I have added a convincing reason for adding this feature, if necessary

Other information

@luoffei luoffei requested a review from a team as a code owner June 30, 2023 08:22
@amrbashir
Copy link
Member

amrbashir commented Jul 3, 2023

could you please rebase your PR and target 1.x branch?

@luoffei luoffei changed the base branch from dev to 1.x July 5, 2023 13:54
@luoffei luoffei changed the base branch from 1.x to dev July 5, 2023 13:54
@luoffei luoffei changed the base branch from dev to 1.x July 5, 2023 14:03
@luoffei
Copy link
Contributor Author

luoffei commented Jul 5, 2023

could you please rebase your PR and target 1.x branch?

done

Copy link
Member

@amrbashir amrbashir left a comment

Choose a reason for hiding this comment

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

You will also need to sign your commits, otherwise I won't be able to merge the PR

Comment on lines 138 to 146
ClearErrors
ReadRegStr $2 HKLM "SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall\$1" "UninstallString"
Push $2
Call GetFirstStrPart
Pop "$R0"
StrCmp $R0 "MsiExec.exe" 0 wix_done
StrCpy $R7 "wix"
Copy link
Member

Choose a reason for hiding this comment

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

Instead of a custom function, there is built-in functions https://nsis.sourceforge.io/StrFunc to manipulate strings

So add at the top

!include "StrFunc.nsh"
${StrCase}
${StrLoc}

and then

ReadRegStr $R0 HKLM "SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall\$1" "UninstallString"
${StrCase} $R1 $R0 "L"
${StrLoc} $R0 $R1 "msiexec" ">"
StrCmp $R0 0 0 wix_done
StrCpy $5 "wix"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unicode true
SetCompressor /SOLID lzma

I need to move these two lines to the head of the file.

Copy link
Member

Choose a reason for hiding this comment

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

why so?

tooling/bundler/src/bundle/windows/templates/installer.nsi Outdated Show resolved Hide resolved
Signed-off-by: luofei <luoffei@outlook.com>
amrbashir
amrbashir previously approved these changes Jul 8, 2023
@amrbashir amrbashir changed the title fix(bundler/nsis): Find the wix installation package fix(bundler/nsis): use uninstallstring to find wix installation Jul 8, 2023
@amrbashir amrbashir merged commit 32218a6 into tauri-apps:1.x Jul 8, 2023
@amrbashir
Copy link
Member

Thank you

@luoffei luoffei deleted the uninstallwix branch July 8, 2023 01:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🔎 In audit
Development

Successfully merging this pull request may close these issues.

2 participants