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

Desktop: Resolves #8258: Upgrade to Electron 25 #8451

Merged
merged 7 commits into from
Jul 12, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
10 changes: 8 additions & 2 deletions .github/scripts/run_ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,12 @@ if [[ $GIT_TAG_NAME = v* ]]; then
# It can be removed once we upgrade to electron-builder@23, however we
# cannot currently do this due to this error:
# https://github.com/laurent22/joplin/issues/8149
PYTHON_PATH=$(which python) USE_HARD_LINKS=false yarn run dist
#
# electron-builder@24, however, still expects the python binary to be named
# "python" and seems to no longer respect the PYTHON_PATH environment variable.
# We work around this by aliasing python.
alias python=$(which python3)
laurent22 marked this conversation as resolved.
Show resolved Hide resolved
USE_HARD_LINKS=false yarn run dist
else
USE_HARD_LINKS=false yarn run dist
fi
Expand All @@ -207,7 +212,8 @@ else

if [ "$IS_MACOS" == "1" ]; then
# See above why we need to specify Python
PYTHON_PATH=$(which python) USE_HARD_LINKS=false yarn run dist --publish=never
alias python=$(which python3)
USE_HARD_LINKS=false yarn run dist --publish=never
else
USE_HARD_LINKS=false yarn run dist --publish=never
fi
Expand Down
13 changes: 13 additions & 0 deletions .yarn/patches/app-builder-lib-npm-24.4.0-05322ff057.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
diff --git a/templates/nsis/include/allowOnlyOneInstallerInstance.nsh b/templates/nsis/include/allowOnlyOneInstallerInstance.nsh
index a1fd1875d852ff69c087a3103eff827c20d37ca5..5222614ddad3276876857e7a9dde4017a6b9fc85 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 `cmd /c tasklist /FI "USERNAME eq %USERNAME%" /FI "IMAGENAME eq ${_FILE}" | %SYSTEMROOT%\System32\find.exe "${_FILE}"`
+ nsExec::Exec `cmd /c tasklist /FI "USERNAME eq %USERNAME%" /FI "PID ne $pid" /FI "IMAGENAME eq ${_FILE}" | %SYSTEMROOT%\System32\find.exe "${_FILE}"`
Comment on lines +9 to +10
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

See the explanation for this change here: electron-userland/electron-builder#6865 (comment)

This seems to fix #8149. To test it, I

  1. Installed a copy of Joplin with the updated installer and opened Joplin.
  2. Opened a new copy of the installer while Joplin was running and had the installer close the exiting Joplin application (then proceeded with the re-installation).
  3. Re-installed Joplin using the installer with no running copies of Joplin.

All of the above tests used the "only for me" option in the dialog (the above logic should only affect the "only for me" option).

I then installed with the "anyone who uses this computer" option, then uninstalled.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've tested with the following user accounts:
image
image
and it still seems to work...

However, the unpatched version of the installer also seems to work (though the unpatched installer does seem to close itself and not install if the app is already running in some cases, while the patched installer does not).

Pop ${_ERR}
!endif
!macroend
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@
"[email protected]": "patch:react-native-camera@npm%3A4.2.1#./.yarn/patches/react-native-camera-npm-4.2.1-24b2600a7e.patch",
"[email protected]": "patch:react-native-vosk@npm%3A0.1.12#./.yarn/patches/react-native-vosk-npm-0.1.12-76b1caaae8.patch",
"[email protected]": "patch:eslint@npm%3A8.39.0#./.yarn/patches/eslint-npm-8.39.0-d92bace04d.patch",
"eslint@^8.13.0": "patch:eslint@npm%3A8.39.0#./.yarn/patches/eslint-npm-8.39.0-d92bace04d.patch"
"eslint@^8.13.0": "patch:eslint@npm%3A8.39.0#./.yarn/patches/eslint-npm-8.39.0-d92bace04d.patch",
"[email protected]": "patch:app-builder-lib@npm%3A24.4.0#./.yarn/patches/app-builder-lib-npm-24.4.0-05322ff057.patch"
}
}
6 changes: 3 additions & 3 deletions packages/app-desktop/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -107,16 +107,16 @@
},
"homepage": "https://github.com/laurent22/joplin#readme",
"devDependencies": {
"@electron/rebuild": "3.2.13",
"@joplin/tools": "~2.12",
"@testing-library/react-hooks": "8.0.1",
"@types/jest": "29.5.1",
"@types/node": "18.15.13",
"@types/react": "16.14.41",
"@types/react-redux": "7.1.25",
"@types/styled-components": "5.1.26",
"electron": "19.1.4",
"electron-builder": "22.11.7",
"electron-rebuild": "3.2.9",
"electron": "25.2.0",
"electron-builder": "24.4.0",
"glob": "10.2.7",
"gulp": "4.0.2",
"jest": "29.5.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/app-desktop/tools/electronRebuild.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ async function main() {
// wrong one. However it means it will have to be manually upgraded for each
// new Electron release. Some ABI map there:
// https://github.com/electron/node-abi/tree/master/test
const forceAbiArgs = '--force-abi 89';
const forceAbiArgs = '--force-abi 116';

if (isWindows()) {
// Cannot run this in parallel, or the 64-bit version might end up
Expand Down
Loading