Release script didn't create signature files

[MylesBorins]

the release seemed to go off without a hitch but SHASUMS256.txt.asc was not created or uploaded to the server and failed silently. I had to manually create the file and upload by hand.

Refs: nodejs/nodejs.org#2795
Refs: https://twitter.com/hichaelmart/status/1197641843896442881

[rvagg]

I think what's probably happening is that the set -e in release.sh is doing its thing but its non-obvious to a user running the script. This is where signing is supposed to happen: https://github.com/nodejs/node/blob/b8f8f0500213e7347d92465dd5120f632c05e253/tools/release.sh#L203-L207

So it's checking for a non-zero exit code before signing. But it's probably not even getting there, if there's an upload failure then the script probably bails with a non-zero exit code and that's not clear to the person running the script - although you should have seen an error from that ssh promote command I would think.

Maybe what we need to try in there is replace the set -e with something like

onerror() {
  echo "*** ERROR occurred at line $(caller) ***" >&2
}

trap onerror ERR

Would someone mind tackling that?

It doesn't answer the question of what failed in the first place with that ssh ... promote, I hope nothing else is missing from the release!

Re "manually" redoing it, the script has a -s option that lets you sign an existing release, see note at the top of the file. I don't remember why that was needed, I think it's supposed to be redundant now and signing should just happen--you'll see a path down lower in the script where if signversion is present it'll just sign, otherwise it'll do everything else and then sign.

I don't do releases anymore so it's difficult for me to even test this workflow. I'd like for @nodejs/releasers to take more ownership of release.sh and make it do what they need and work like they want. There's obviously stuff on the server that it needs to interact with so there needs to be Build interaction and expertise on that, but it can all be found at:

• dist-promotable, dist-promote and dist-sign, called by release.sh on the server, are all in https://github.com/nodejs/build/tree/master/ansible/www-standalone/tools/dist
• release promote scripts that dist-promote and dist-sign uses: https://github.com/nodejs/build/tree/master/ansible/www-standalone/tools/promote which also contains the fairly new check_assets.js that checks you're promoting what you need.
• dist-indexer https://github.com/nodejs/nodejs-dist-indexer which also includes ls-types.js which is called as nodejs-ls-types by dist-promotable

Perhaps we need to start a workflow doc somewhere to document what's doing what so y'all can start fixing and changing what you need?

[BridgeAR]

We definitely have to address this soon. Doing this manually is pretty bad.

[rvagg]

Doing this manually is pretty bad.

again, why are you doing it manually? what's not working in the process? The thing my comment is addressing is dealing with a failure in the process but it doesn't answer what's failing.

@nodejs/releasers when you're releasing next time, please be on the lookout for anything that looks abnormal, a warning or an error or anything out of the ordinary.

In the meantime, you're welcome to fix the script @BridgeAR.

[BridgeAR]

@rvagg I guess the only issue is in the release post script, not with the signature being uploaded. At least that's how it seems to me.

The file has been created successfully automatically, it was just not taken into account while creating the blog post.

[mhdawson]

@BethGriggs maybe you can add to the Release team agenda to discuss how the releasers might be able to help out on this front. I think getting to the point where the releasers can help maintain the script makes sense as they are both the most familiar/able to test it out and at the same time the most impacted by problems.

[MylesBorins]

The problem with maintaining the script is testing. With the current issue it is hard to reproduce and thus hard to debug / fix. This is a hard problem no matter who is trying to fix it. Would be happy to pair with someone in the new year to hack on this, but definitely could use some help on figuring out how to test without affecting published builds

…

On Thu, Dec 5, 2019, 2:32 PM Michael Dawson ***@***.***> wrote: @BethGriggs <https://github.com/BethGriggs> maybe you can add to the Release team agenda to discuss how the releasers might be able to help out on this front. I think getting to the point where the releasers can help maintain the script makes sense as they are both the most familiar/able to test it out and at the same time the most impacted by problems. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2059?email_source=notifications&email_token=AADZYV2HAZTKBNJ5UAOMVMDQXF6N7A5CNFSM4JQJFVLKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGCMCJQ#issuecomment-562348326>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADZYV36K2BSBSD4N5JA37TQXF6N7ANCNFSM4JQJFVLA> .

[mhart]

Looks like this happened again with v13.8.0. https://nodejs.org/dist/v13.8.0/