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

Windows: Resolves #11508: Allow installer to skip uninstallation step after repeated failures #11612

Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 70 additions & 3 deletions .yarn/patches/app-builder-lib-npm-24.13.3-86a66c0bf3.patch
Original file line number Diff line number Diff line change
@@ -1,15 +1,82 @@
# This patch prevents the installer from considering itself as a running instance of Joplin.
# This patch's goal is to work around an issue in the NSIS uninstaller on Windows:
# - For future uninstallers, this patch backports an upstream commit that changes how
# running copies of the app are found.
# - See https://github.com/electron-userland/electron-builder/pull/8133
# - If an existing uninstaller fails, gives an option to continue with the installation
# despite the failure.
# - Updates "uninstall failed" error messages to state that uninstallation failed (rather
# than incorrectly stating that the issue was with closing the app).
#
# See https://github.com/laurent22/joplin/pull/11541
diff --git a/templates/nsis/include/allowOnlyOneInstallerInstance.nsh b/templates/nsis/include/allowOnlyOneInstallerInstance.nsh
index fe5d45c730f36c9fe8d8cfea12e242e501b67139..af2ce5c90ac910b079e24992519bffe33d57668a 100644
index fe5d45c730f36c9fe8d8cfea12e242e501b67139..97b27fce6798e30e3e631221435f09b3579e77c3 100644
--- a/templates/nsis/include/allowOnlyOneInstallerInstance.nsh
+++ b/templates/nsis/include/allowOnlyOneInstallerInstance.nsh
@@ -42,7 +42,7 @@
${nsProcess::FindProcess} "${_FILE}" ${_ERR}
!else
# find process owned by current user
- nsExec::Exec `%SYSTEMROOT%\System32\cmd.exe /c tasklist /FI "USERNAME eq %USERNAME%" /FI "IMAGENAME eq ${_FILE}" /FO csv | %SYSTEMROOT%\System32\find.exe "${_FILE}"`
+ nsExec::Exec `%SYSTEMROOT%\System32\cmd.exe /c tasklist /FI "USERNAME eq %USERNAME%" /FI "PID ne $pid" /FI "IMAGENAME eq ${_FILE}" /FO csv | %SYSTEMROOT%\System32\find.exe "${_FILE}"`
+ nsExec::Exec `"$SYSDIR\cmd.exe" /c tasklist /FI "USERNAME eq %USERNAME%" /FI "IMAGENAME eq ${_FILE}" /FO csv | "$SYSDIR\find.exe" "${_FILE}"`
Pop ${_ERR}
!endif
!macroend
@@ -73,7 +73,7 @@
!ifdef INSTALL_MODE_PER_ALL_USERS
nsExec::Exec `taskkill /im "${APP_EXECUTABLE_FILENAME}" /fi "PID ne $pid"`
!else
- nsExec::Exec `%SYSTEMROOT%\System32\cmd.exe /c taskkill /im "${APP_EXECUTABLE_FILENAME}" /fi "PID ne $pid" /fi "USERNAME eq %USERNAME%"`
+ nsExec::Exec `"$SYSDIR\cmd.exe" /c taskkill /im "${APP_EXECUTABLE_FILENAME}" /fi "PID ne $pid" /fi "USERNAME eq %USERNAME%"`
!endif
# to ensure that files are not "in-use"
Sleep 300
@@ -91,7 +91,7 @@
!ifdef INSTALL_MODE_PER_ALL_USERS
nsExec::Exec `taskkill /f /im "${APP_EXECUTABLE_FILENAME}" /fi "PID ne $pid"`
!else
- nsExec::Exec `%SYSTEMROOT%\System32\cmd.exe /c taskkill /f /im "${APP_EXECUTABLE_FILENAME}" /fi "PID ne $pid" /fi "USERNAME eq %USERNAME%"`
+ nsExec::Exec `"$SYSDIR\cmd.exe" /c taskkill /f /im "${APP_EXECUTABLE_FILENAME}" /fi "PID ne $pid" /fi "USERNAME eq %USERNAME%"`
!endif
!insertmacro FIND_PROCESS "${APP_EXECUTABLE_FILENAME}" $R0
${If} $R0 == 0
diff --git a/templates/nsis/include/installUtil.nsh b/templates/nsis/include/installUtil.nsh
index 47367741632726ba0886ac516461dbe98b7aea58..daf71a18fd8112dd8980d71c2a82201ca132c21e 100644
--- a/templates/nsis/include/installUtil.nsh
+++ b/templates/nsis/include/installUtil.nsh
@@ -126,10 +126,11 @@ Function handleUninstallResult
Return

${if} $R0 != 0
- MessageBox MB_OK|MB_ICONEXCLAMATION "$(uninstallFailed): $R0"
+ # MessageBox MB_OK|MB_ICONEXCLAMATION "$(uninstallFailed): $R0"
DetailPrint `Uninstall was not successful. Uninstaller error code: $R0.`
- SetErrorLevel 2
- Quit
+ DetailPrint `Continuing anyway. See https://github.com/laurent22/joplin/pull/11612.`
+ # SetErrorLevel 2
+ # Quit
${endif}
FunctionEnd

@@ -216,7 +217,7 @@ Function uninstallOldVersion
IntOp $R5 $R5 + 1

${if} $R5 > 5
- MessageBox MB_RETRYCANCEL|MB_ICONEXCLAMATION "$(appCannotBeClosed)" /SD IDCANCEL IDRETRY OneMoreAttempt
+ MessageBox MB_RETRYCANCEL|MB_ICONEXCLAMATION "$(appCannotBeUninstalled)" /SD IDCANCEL IDRETRY OneMoreAttempt
Return
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If simple, adjust this so that Retry skips the uninstall check and Cancel calls Abort.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This has been updated (see the screen recording in the pull request description):
screenshot: The old version of Joplin could not be removed. Click retry to skip this step.

${endIf}

diff --git a/templates/nsis/messages.yml b/templates/nsis/messages.yml
index a1c2847fa48d79f835b30b48e999ccaf3c818657..0f92e0d4f87248a6df8a7fc1adef4c918539299e 100644
--- a/templates/nsis/messages.yml
+++ b/templates/nsis/messages.yml
@@ -235,3 +235,8 @@ uninstallFailed:
sv: Det gick inte att avinstallera gamla programfiler. Försök att köra installationsprogrammet igen.
uk: Не вдалось видалити старі файли застосунку. Будь ласка, спробуйте запустити встановлювач знов.
zh_TW: 無法俺安裝舊的應用程式檔案。 請嘗試再次執行安裝程式。
+
+
+appCannotBeUninstalled:
+ en: "The old version of ${PRODUCT_NAME} could not be removed. \nClick Retry to try again or Cancel to continue with the installation."
+
4 changes: 2 additions & 2 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -15614,7 +15614,7 @@ __metadata:

"app-builder-lib@patch:app-builder-lib@npm%3A24.13.3#./.yarn/patches/app-builder-lib-npm-24.13.3-86a66c0bf3.patch::locator=root%40workspace%3A.":
version: 24.13.3
resolution: "app-builder-lib@patch:app-builder-lib@npm%3A24.13.3#./.yarn/patches/app-builder-lib-npm-24.13.3-86a66c0bf3.patch::version=24.13.3&hash=e2b05a&locator=root%40workspace%3A."
resolution: "app-builder-lib@patch:app-builder-lib@npm%3A24.13.3#./.yarn/patches/app-builder-lib-npm-24.13.3-86a66c0bf3.patch::version=24.13.3&hash=426355&locator=root%40workspace%3A."
dependencies:
"@develar/schema-utils": ~2.6.5
"@electron/notarize": 2.2.1
Expand Down Expand Up @@ -15646,7 +15646,7 @@ __metadata:
peerDependencies:
dmg-builder: 24.13.3
electron-builder-squirrel-windows: 24.13.3
checksum: e023282fbf7381b0b504ae0835dabb26ad0092c32fad89116aa259f790bba912805338bc895ed205b70a193e39559b3939e816c0afeacaca49f3b55c3446a807
checksum: 798be23013c87f4acf94b10c2f874d0c0d7198625b8e36b1c61a2f5fd11b28be9684cb9f1bd0a26897e46d3fe1af1e0b1d860d996ba1197f4d4eea9b20605880
languageName: node
linkType: hard

Expand Down
Loading