chore: implement electron-configure-package-win32.sh
#962
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
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,6 +23,7 @@ | |
| "author": "Juan Cruz Viotti <[email protected]>", | ||
| "license": "Apache-2.0", | ||
| "copyright": "Copyright 2016 Resinio Ltd", | ||
| "companyName": "Resinio Ltd", | ||
| "packageIgnore": [ | ||
| "LICENSE", | ||
| "Makefile", | ||
|
|
||
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,126 @@ | ||
| #!/bin/bash | ||
|
|
||
| ### | ||
| # Copyright 2016 resin.io | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| ### | ||
|
|
||
| set -u | ||
| set -e | ||
|
|
||
| OS=$(uname -o 2>/dev/null || true) | ||
| if [[ "$OS" != "Msys" ]]; then | ||
| echo "This script is only meant to be run in Windows" 1>&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
|
There was a problem hiding this comment. As this script runs a win32 executable (rcedit.exe), it ought to include the only-running-on-windows check There was a problem hiding this comment. Indeed. In theory, this script could run in the other operating systems on top of Wine, but lets ignore that for now :) |
||
| ./scripts/build/check-dependency.sh upx | ||
| ./scripts/build/check-dependency.sh unzip | ||
|
|
||
| function usage() { | ||
| echo "Usage: $0" | ||
| echo "" | ||
| echo "Options" | ||
| echo "" | ||
| echo " -p <electron package>" | ||
| echo " -n <application name>" | ||
| echo " -d <application description>" | ||
| echo " -v <application version>" | ||
| echo " -c <application copyright>" | ||
| echo " -l <application license file>" | ||
| echo " -m <company name>" | ||
| echo " -a <application asar (.asar)>" | ||
| echo " -i <application icon (.ico)>" | ||
| echo " -w <download directory>" | ||
|
There was a problem hiding this comment. This should probably do the same TMPDIR= trick that you use in There was a problem hiding this comment. I think I prefer to be explicit about it and ask it as an option rather than relying on the environment at all. |
||
| echo " -o <output directory>" | ||
| exit 1 | ||
| } | ||
|
|
||
| ARGV_ELECTRON_PACKAGE="" | ||
| ARGV_APPLICATION_NAME="" | ||
| ARGV_APPLICATION_DESCRIPTION="" | ||
| ARGV_VERSION="" | ||
| ARGV_COPYRIGHT="" | ||
| ARGV_LICENSE="" | ||
| ARGV_COMPANY_NAME="" | ||
| ARGV_ASAR="" | ||
| ARGV_ICON="" | ||
| ARGV_DOWNLOAD_DIRECTORY="" | ||
| ARGV_OUTPUT="" | ||
|
|
||
| while getopts ":p:n:d:v:c:l:m:a:i:w:o:" option; do | ||
| case $option in | ||
| p) ARGV_ELECTRON_PACKAGE="$OPTARG" ;; | ||
| n) ARGV_APPLICATION_NAME="$OPTARG" ;; | ||
| d) ARGV_APPLICATION_DESCRIPTION="$OPTARG" ;; | ||
| v) ARGV_VERSION="$OPTARG" ;; | ||
| c) ARGV_COPYRIGHT="$OPTARG" ;; | ||
| l) ARGV_LICENSE="$OPTARG" ;; | ||
| m) ARGV_COMPANY_NAME="$OPTARG" ;; | ||
| a) ARGV_ASAR="$OPTARG" ;; | ||
| i) ARGV_ICON="$OPTARG" ;; | ||
| w) ARGV_DOWNLOAD_DIRECTORY="$OPTARG" ;; | ||
| o) ARGV_OUTPUT="$OPTARG" ;; | ||
| *) usage ;; | ||
| esac | ||
| done | ||
|
|
||
| if [ -z "$ARGV_ELECTRON_PACKAGE" ] \ | ||
| || [ -z "$ARGV_APPLICATION_NAME" ] \ | ||
| || [ -z "$ARGV_APPLICATION_DESCRIPTION" ] \ | ||
| || [ -z "$ARGV_VERSION" ] \ | ||
| || [ -z "$ARGV_COPYRIGHT" ] \ | ||
| || [ -z "$ARGV_LICENSE" ] \ | ||
| || [ -z "$ARGV_COMPANY_NAME" ] \ | ||
| || [ -z "$ARGV_ASAR" ] \ | ||
| || [ -z "$ARGV_ICON" ] \ | ||
| || [ -z "$ARGV_DOWNLOAD_DIRECTORY" ] \ | ||
| || [ -z "$ARGV_OUTPUT" ] | ||
| then | ||
| usage | ||
| fi | ||
|
|
||
| unzip "$ARGV_ELECTRON_PACKAGE" -d "$ARGV_OUTPUT" | ||
|
|
||
| mv "$ARGV_OUTPUT/electron.exe" "$ARGV_OUTPUT/$ARGV_APPLICATION_NAME.exe" | ||
| cp "$ARGV_LICENSE" "$ARGV_OUTPUT/LICENSE" | ||
| echo "$ARGV_VERSION" > "$ARGV_OUTPUT/version" | ||
| rm -f "$ARGV_OUTPUT/resources/default_app.asar" | ||
|
|
||
| RCEDIT_VERSION=v0.7.0 | ||
| RCEDIT="$ARGV_DOWNLOAD_DIRECTORY/rcedit.exe" | ||
|
|
||
| ./scripts/build/download-tool.sh -x \ | ||
| -u "https://github.com/electron/node-rcedit/raw/$RCEDIT_VERSION/bin/rcedit.exe" \ | ||
|
There was a problem hiding this comment. Curiosity finally got the better of me... ;-) electron/node-rcedit#23 |
||
| -c "42649d92e1bbb3af1186fb0ad063df9fcdc18e7b5f2ea82191ecc8fdfaffb0d8" \ | ||
| -o "$RCEDIT" | ||
|
|
||
| "$RCEDIT" "$ARGV_OUTPUT/$ARGV_APPLICATION_NAME.exe" \ | ||
| --set-version-string "FileDescription" "$ARGV_APPLICATION_NAME" \ | ||
| --set-version-string "InternalName" "$ARGV_APPLICATION_NAME" \ | ||
| --set-version-string "OriginalFilename" "$(basename "$ARGV_OUTPUT")" \ | ||
| --set-version-string "ProductName" "$ARGV_APPLICATION_NAME - $ARGV_APPLICATION_DESCRIPTION" \ | ||
| --set-version-string "CompanyName" "$ARGV_COMPANY_NAME" \ | ||
| --set-version-string "LegalCopyright" "$ARGV_COPYRIGHT" \ | ||
| --set-file-version "$ARGV_VERSION" \ | ||
| --set-product-version "$ARGV_VERSION" \ | ||
| --set-icon "$ARGV_ICON" | ||
|
|
||
| upx -9 "$ARGV_OUTPUT/*.dll" | ||
|
|
||
| cp "$ARGV_ASAR" "$ARGV_OUTPUT/resources/app.asar" | ||
|
|
||
| if [ -d "$ARGV_ASAR.unpacked" ]; then | ||
| cp -rf "$ARGV_ASAR.unpacked" "$ARGV_OUTPUT/resources/app.asar.unpacked" | ||
| fi | ||
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.
Probably makes sense to only show these warnings when you're actually about to do the code-signing step, rather than showing them all the time to all users.
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.
And it probably makes sense to nest the
ifndef CODE_SIGN_CERTIFICATE_PASSWORDinside theifndef CODE_SIGN_CERTIFICATEtooThere 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.
Ahhh, I now see that the code-signing step is simply skipped if these env-vars aren't set.
Still it feels wrong to always display them though, even for users who are just building locally and will never be able to code-sign? Can't think of a good solution off the top of my head though.
Oooh! What if we instead had the code-signing as a precursor to the publishing step, so that only users who attempt to run the publishing targets will get warned about missing CODE_SIGN env-vars?
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.
The thing is that code-signing right before publishing might mean re-doing the whole packaging step. In OS X for example, we create a temporary read-write DMG, and we have to code-sign inside the DMG and then convert to a read-only DMG.
How would you handle that right before publishing? I'm not saying that it can't be done, but it means re-doing a lot of work that took us to the publishing step in the first place.
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.
Done!
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 guess one option would be to have a non-signed version as the "standard" DMG, and then have a special code-signed DMG required by the publish step? That's probably a bit too much of a step though just to silence a couple of warnings ;-)
Or a simple solution could just be a
SILENCE_CODE_SIGN_WARNINGS ?= 0in the Makefile, so the user could set aSILENCE_CODE_SIGN_WARNINGS=1env-var if they wanted to.OTOH, perhaps it's inconsistent behaviour to have the Makefile generate two different versions of the same file, based on the value of an env-var? (makes it hard to tell them apart?)
Consider if:
What if instead, there was e.g. a
release/out/signeddirectory, andmake publishdepended onrelease/out/signed/Etcher-1.0.0-beta.17-darwin-x64.dmg, which in turn depended on the temporary read-write DMG (could it be given a different name and made non-temporary?). And thenrelease/out/Etcher-1.0.0-beta.17-darwin-x64.dmgwould also depend on the same 'temporary' read-write DMG, and would always be the non-signed package.Would that work? ;)
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.
And for those scripts where you have the option to verify the signature, maybe it would make sense to sign the package / exe in the temporary directory, and only move it to
release/out/signed/after it's been verified.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 like the
signed/subdirectory idea. I propose to do it in another PR so we can do it for all the operating systems. Regarding the warning, what about simply not printing the warning, but report that the binary is not going to be signed, inmake info?If you agree on that, lets merge the PR as it is now, and I'll send a new one right away to make that change, so we do it for all operating systems at once.
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.
As I said here I think it'd make sense for the "standard build" e.g.
make electron-installer-dmgto produce unsigned images (and have them also be snapshot versions by default), and then have a special e.g.signed-electron-installer-dmgtarget to create (production-version?) files in thesigned/subdirectory, and only do the check-for-certificate-vars step as part of thesigned-electron-installer-dmgrules, and upgrade it from a warning to an error.(If that all makes sense? 😉 )
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.
Sounds good to me :) Let me know if I can help with any OS X testing (if you don't have access to OS X yet, you might want to ping the operations team to ship you a Mac Mini or something like that anyway)!