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 2 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..25cb4690e945ff5c77d6aa0845480a6dca318031 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_YESNO|MB_ICONEXCLAMATION "$(uninstallFailedIgnore) (error code $R0)" IDYES ignoreError
DetailPrint `Uninstall was not successful. Uninstaller error code: $R0.`
personalizedrefrigerator marked this conversation as resolved.
Show resolved Hide resolved
SetErrorLevel 2
Quit
+ ignoreError:
${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..ebb3181568f0ed79d3490138d913326b3455e59e 100644
--- a/templates/nsis/messages.yml
+++ b/templates/nsis/messages.yml
@@ -235,3 +235,10 @@ 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 for more options."
+
+uninstallFailedIgnore:
+ en: Failed to remove old application files. Continue without removing old application files?
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=6e3b1c&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: e6ef8408c13887924825c5e97a26e61a9e6d7f7eedbb8623e50d2de003c8676971d8ca8cc0ec6ead8d7b6b461b908ec1979caccc60b074eb6623e8ed1fc3a8cf
languageName: node
linkType: hard

Expand Down
Loading