tools: add custom private key option for release.sh

[krydos]

Hi,
I just noticed issue here - #14378 and tried to solve it.
So I've added -i option for release.sh that allows user to specify non-default private key for ssh and scp commands.

I've tested script manually with some debug echo statements since I'm not sure we have any tests for our tools (or idk about them).

One more thing is shift $((OPTIND-1)) right after while loop. It's important to save variable such as $1, $2 etc, that are used in the script. So these variables should not be broken.

If anyone could help me to test this script it would be great :)

Checklist

• commit message follows commit guidelines

Affected core subsystem(s)

tools

[Fishrock123]

Oh nice! Thanks!

As I said I'm not this familiar with bash but I can certainly try using this for 8.2.1 (#14399).

[Fishrock123]

I think the main thing would be to ensure this works with -s, which uses a much more rudimentary flag detection mechanism.

[krydos]

ah, didn't notice -s there... I hope it works because of shift $((OPTIND-1)) but I'm not sure.
If no one will against getopts I guess it would be nice to rewrite -s flag to the way how -i works

[Fishrock123]

So, I used this for 8.2.1 and it worked just fine! 😄

(I did not need to use -s though.)

[rvagg]

Nice work @krydos! It'd be nice to do -s in the same way but that's not a big deal. I'll review and test shortly and be back with feedback.

[rvagg]

-s doesn't work anymore with this, it gets chopped off completely. I guess because it's technically an option and getopts includes it in OPTIND even though it's not handled by the case

so could you try pulling the -s up into the getopts handling? it always needs a version number argument.

[krydos]

Sure, I'll try to convert -s option to getopts as soon as possible

[krydos]

I've changed how -s is parsed. It took too much time to find a way how to show custom message when option is passed but argument for this option is not... And didn't find solution yet :(

Previously in case nothing is passed to -s option you could see message Please supply a version string to sign. Now you will see this one ./release.sh: option requires an argument -- s (default one produced by getopts). If it's ok then great, if not I will try to come up with something else.

And again, I tried to debug it with multiple echo statements but I was unable to debug if script in a whole works correctly or not.
If someone could test it, it would sooo great.
I hope I didn't break anything 😟

[Fishrock123]

Hmm, the script doesn't actually seem to run now? All variants with valid options just seem to return from the script without doing anything. :/

[krydos]

ah, excuse me, I didn't really get the purpose of $OPTERR and used it wrongly.
Should be working from now.

Will re-review

[Fishrock123]

I promoted the armv6 binaries for 8.2.1 with the latest version of this.

Both -i and -s worked just fine. I wasn't able to test without -i though.

[Fishrock123]

@rvagg @jasnell could ya'll re-review?

[Fishrock123]

@nodejs/build & @jasnell can someone with bash knowledge please review / sign off on this?

[gibfahn]

If you want a nicer error message you can do this:

diff --git a/tools/release.sh b/tools/release.sh
index 4225be4cc7..fcb5ef1242 100755
--- a/tools/release.sh
+++ b/tools/release.sh
@@ -16,7 +16,7 @@ signcmd=dist-sign
 customsshkey="" # let ssh and scp use default key
 signversion=""
 
-while getopts "i:s:" option; do
+while getopts ":i:s:" option; do
     case "${option}" in
         i)
             customsshkey="-i ${OPTARG}"
@@ -25,6 +25,11 @@ while getopts "i:s:" option; do
             signversion="${OPTARG}"
             ;;
         \?)
+            echo "Invalid option -$OPTARG."
+            exit 1
+            ;;
+        :)
+            echo "Option -$OPTARG takes a parameter."
             exit 1
             ;;
     esac

up to you whether it's worth it though.

[gibfahn]

Both -i and -s worked just fine. I wasn't able to test without -i though.

@Fishrock123 FWIW you can add the IdentityFile option to the ~/.ssh/config entry for direct.nodejs.org, something like this should work (then you could test without the -i):

Host direct.nodejs.org
     User dist
     IdentityFile ~/.ssh/custom_id_rsa

[gibfahn]

Looks fine with or without the proposed fix.

[krydos]

@gibfahn thank you for the help. I didn't know that colon at the beginning of the arguments definition switch getopts to silent error reporting mode. Very cool!

[BridgeAR]

Landed in 237a42d