Packaging enhancements #32
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,9 +29,6 @@ jobs: | |
| with: | ||
| submodules: true | ||
|
|
||
| - uses: actions/setup-node@v3 | ||
| with: | ||
| node-version: ${{ matrix.node }} | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,10 +23,9 @@ | |
| "electron:preview": "electron core/_build/ext/app/electron/main.js", | ||
| "electron:dir": "electron-builder build --linux --dir", | ||
| "electron:linux": "electron-builder build --linux", | ||
| "electron:ci": "./scripts/ci.sh", | ||
| "electron": "electron-builder build --publish never", | ||
| "test": "cd core && rm -f junk.db && node _build/app/server/companion.js sqlite gristify junk.db && ls junk.db" | ||
| }, | ||
| "resolutions": { | ||
| "jquery": "3.5.0", | ||
|
|
@@ -40,9 +39,23 @@ | |
| "includeSubNodeModules": true, | ||
| "icon": "core/static/icons/grist.png", | ||
| "win": { | ||
| "target": [ | ||
| { | ||
| "target": "nsis" | ||
| }, | ||
| { | ||
| "target": "portable" | ||
SleepyLeslie marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| ], | ||
| "icon": "core/static/icons/grist.ico" | ||
| }, | ||
| "portable": { | ||
| "artifactName": "${productName}-windows-portable-${version}-${arch}.${ext}" | ||
| }, | ||
| "nsis": { | ||
| "artifactName": "${productName}-windows-installer-${version}-${arch}.${ext}", | ||
| "perMachine": true | ||
| }, | ||
| "mac": { | ||
| "artifactName": "${productName}-mac-${version}-${arch}.${ext}", | ||
| "icon": "core/static/icons/grist.icns", | ||
|
|
@@ -108,9 +121,6 @@ | |
| "role": "Editor", | ||
| "icon": "core/static/icons/gristdoc" | ||
| } | ||
|
There was a problem hiding this comment. According to this documentation, this option does nothing when There was a problem hiding this comment. FYI otherwise this option tells NSIS to skip the installation mode page and always installs Grist Desktop system-wide. This might not be a good idea in restricted environments where user does not have admin privileges, but we now have the portable build so this shouldn't matter much? There was a problem hiding this comment. Pretty sure this was the hard-to-find solution to some gnarly problem, but we shouldn't keep it out of superstition. Someone will shout if it turns out to have some effect that was important, and then we can document the reason better next time :-). Cheers. |
||
| ] | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| #!/bin/bash | ||
|
|
||
| set -e | ||
|
|
||
| case $TARGET_ARCH in | ||
| "x86") | ||
| ARCHFLAG="--ia32" | ||
| ;; | ||
| "x64") | ||
| ARCHFLAG="--x64" | ||
| ;; | ||
| "arm64") | ||
| ARCHFLAG="--arm64" | ||
| ;; | ||
| *) | ||
| echo "Target architecture $TARGET_ARCH not supported" | ||
| exit 1 | ||
| ;; | ||
| esac | ||
|
|
||
| electron-builder build $ARCHFLAG --publish always |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2578,9 +2578,9 @@ camelcase@^6.0.0: | |
| integrity sha512-Gmy6FhYlCY7uOElZUSbxo2UCDH8owEk996gkbrpsgGtrJLM3J7jGxl9Ic7Qwwj4ivOE5AWZWRMecDdF7hqGjFA== | ||
|
|
||
| caniuse-lite@^1.0.30001587: | ||
| version "1.0.30001620" | ||
| resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001620.tgz#78bb6f35b8fe315b96b8590597094145d0b146b4" | ||
| integrity sha512-WJvYsOjd1/BYUY6SNGUosK9DUidBPDTnOARHp3fSmFO1ekdxaY6nKRttEVrfMmYi80ctS0kz1wiWmm14fVc3ew== | ||
|
|
||
| [email protected]: | ||
| version "7.1.1" | ||
|
|
@@ -3590,9 +3590,9 @@ [email protected]: | |
| mime "^2.5.2" | ||
|
|
||
| electron-to-chromium@^1.4.668: | ||
| version "1.4.774" | ||
| resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.4.774.tgz#1017d1758aaeeefe5423aa9d67b4b1e5d1d0a856" | ||
| integrity sha512-132O1XCd7zcTkzS3FgkAzKmnBuNJjK8WjcTtNuoylj7MYbqw5eXehjQ5OK91g0zm7OTKIPeaAG4CPoRfD9M1Mg== | ||
|
|
||
| electron-updater@^5.3.0: | ||
| version "5.3.0" | ||
|
|
@@ -3753,9 +3753,9 @@ es-errors@^1.3.0: | |
| integrity sha512-Zf5H2Kxt2xjTvbJvP2ZWLEICxA6j+hAmMzIlypy4xcBg1vKVnx89Wy0GbS+kf5cwCVFFzdCFh2XSCFNULS6csw== | ||
|
|
||
| es-module-lexer@^1.2.1: | ||
| version "1.5.3" | ||
| resolved "https://registry.yarnpkg.com/es-module-lexer/-/es-module-lexer-1.5.3.tgz#25969419de9c0b1fbe54279789023e8a9a788412" | ||
| integrity sha512-i1gCgmR9dCl6Vil6UKPI/trA69s08g/syhiDK9TG0Nf1RJjjFI+AzoWW7sPufzkgYAn861skuCwJa0pIIHYxvg== | ||
|
|
||
| es6-error@^4.1.1: | ||
| version "4.1.1" | ||
|
|
@@ -4219,9 +4219,9 @@ fast-text-encoding@^1.0.0: | |
| integrity sha512-VhXlQgj9ioXCqGstD37E/HBeqEGV/qOD/kmbVG8h5xKBYvM1L3lR1Zn4555cQ8GkYbJa8aJSipLPndE1k6zK2w== | ||
|
|
||
| fast-xml-parser@^4.2.2: | ||
| version "4.4.0" | ||
| resolved "https://registry.yarnpkg.com/fast-xml-parser/-/fast-xml-parser-4.4.0.tgz#341cc98de71e9ba9e651a67f41f1752d1441a501" | ||
| integrity sha512-kLY3jFlwIYwBNDojclKsNAC12sfD6NwW74QB2CoNGPvtVxjliYehVunB3HYyNi+n4Tt1dAcgwYvmKF/Z18flqg== | ||
| dependencies: | ||
| strnum "^1.0.5" | ||
|
|
||
|
|
@@ -7506,9 +7506,9 @@ schema-utils@^3.1.1, schema-utils@^3.2.0: | |
| ajv-keywords "^3.5.2" | ||
|
|
||
| selenium-webdriver@^4.20.0: | ||
| version "4.21.0" | ||
| resolved "https://registry.yarnpkg.com/selenium-webdriver/-/selenium-webdriver-4.21.0.tgz#d38aebfc34770421a880afcfdb7bd8fe85ce9174" | ||
| integrity sha512-WaEJHZjOWNth1QG5FEpxpREER0qptZBMonFU6GtAqdCNLJVxbtC3E7oS/I/+Q1sf1W032Wg0Ebk+m46lANOXyQ== | ||
| dependencies: | ||
| jszip "^3.10.1" | ||
| tmp "^0.2.3" | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this check removed? It was checking something pretty subtle and weird but I see there are no comments as to what exactly :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Not against removing, just checking why)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked at the code and couldn't understand why this check needs to be present. It was doing no useful work (basically
if true; then echo ok; fithen repeat for another time), and there was no comment to explain its significance. I thought it was legacy leftovers not cleaned up. I double checked my CI and packaging runs and this step never failed (I would be surprised if it did). My guess is it might be checking whether the Windows association step worked (script is run with bash rather than powershell), and was likely added for debugging purposes, so I removed it. If anything breaks in the future I can always come back to it.Please do share any extra context you have on what the problem was.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it was likely something like that, figuring out the association, and that a good version of bash was running. Doing bash-like stuff in windows was delicate, and I could easily imagine it breaking in future again. Still, since these checks have no comments, fine to remove them. Thanks for explaining.