Release script didn't create signature files  #2059

Activity

1. rvagg commented on Nov 22, 2019

   

   rvagg

   on Nov 22, 2019

   Member

   More actions

   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?
2. BridgeAR commented on Dec 3, 2019

   

   BridgeAR

   on Dec 3, 2019

   Member

   More actions

   We definitely have to address this soon. Doing this manually is pretty bad.
3. rvagg commented on Dec 4, 2019

   

   rvagg

   on Dec 4, 2019

   Member

   More actions

   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.
4. BridgeAR commented on Dec 4, 2019

   

   BridgeAR

   on Dec 4, 2019

   Member

   More actions

   @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.
5. mhdawson commented on Dec 5, 2019

   

   mhdawson

   on Dec 5, 2019

   Member

   More actions

   @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.
6. MylesBorins commented on Dec 6, 2019

   

   MylesBorins

   on Dec 6, 2019 via email

   ContributorAuthor

   More actions

   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> .
7. mhart commented on Feb 6, 2020

   

   mhart

   on Feb 6, 2020

   More actions

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

   closed this as completedon Jun 21, 2020