mapbox / node-pre-gyp

Node.js tool for easy binary deployment of C++ addons
BSD 3-Clause "New" or "Revised" License
1.11k stars 259 forks source link

Release workflow #744

Open acalcutt opened 1 week ago

acalcutt commented 1 week ago

Please note, this is untested right now and I am giving it for review and/or ideas for https://github.com/mapbox/node-pre-gyp/issues/739.

This is a release workflow for node-pre-gyp. It is based on the node release workflow for maplibre-native at https://github.com/maplibre/maplibre-native/blob/main/.github/workflows/node-release.yml which I worked on getting working.

The idea behind this was, someone needs to approve and add changelog for each release, which should be a PR. When the version in package.json changes, this release workflow will check if that version has been published. If it has not published it, it will do that and also create a github release. the github release will also contain the text of the CHANGELOG.md for the version being published.

The workflow is also made to deal with pre-releases, so if the version is a pre-release, it will publish it as the next npm release

Whoever creates the PR for a new release would follow the instructions in RELEASE.md

Needs 'NPM_TOKEN' Repository secret. A secret that contains a user npm token with permission to publish

cclauss commented 1 week ago

@mapsam Given all the progress that has been made in the past 24 hours, I believe that we need a provisional release of node-pre-gyp to https://www.npmjs.com/package/@mapbox/node-pre-gyp. This pre-release would allow multiple parties to kick the tires (for a week or two?) before we make a general release.

You or one of your colleagues would need to work with @acalcutt to add the required NPM_TOKEN as a repository secret.

If you have a different workflow for solving #739 then we would follow your lead.

acalcutt commented 1 week ago

Just an FYI, if you did follow my instructions, when running the first command, any 'pre' version will make a pre-release/next release when published. So with a version incremted like npm version preminor --preid pre --no-git-tag-version would bring this package to 1.1.0-pre.0 from the current 1.0.11 and the workflow would publish it as a pre-release.

acalcutt commented 1 week ago

I gave this a test on my fork, and fixed a few issues and make it more similar to the ci. it should now be working correctly. I used it to make this test release

https://www.npmjs.com/package/@acalcutt/node-pre-gyp/v/2.0.0-pre.1 https://github.com/acalcutt/node-pre-gyp/releases/tag/v2.0.0-pre.1

it was made in this workflow run https://github.com/acalcutt/node-pre-gyp/actions/runs/9735220985

mapsam commented 1 week ago

Nice work y'all πŸ™Œ happy to get NPM_TOKEN added as a repo secret for the @mapbox/node-pre-gyp module. I'll report back when that has been added.

Update: see my comment in https://github.com/mapbox/node-pre-gyp/issues/739#issuecomment-2200796285

mapsam commented 1 week ago

@acalcutt @cclauss unsurprisingly, it's a little hard for me to get a static NPM token for our @mapbox/ namespace packages. Internally we're working on a better solution for public repos + npm + outside maintainers that will resolve this. For now I've set a reminder to refresh the token periodically before it is set to expire.

A couple questions:

mapsam commented 1 week ago

I've added an NPM_TOKEN secret to the repo πŸ‘ I take it back - the tokens I have access to only have 1 hour lifecycles. Working on getting a longer term, static token.

acalcutt commented 1 week ago

do you need me to modify any of the branch protections for the master branch? Currently no one is allowed to manually push to that branch This does not require any changes to branch protections.

are there protections to enforce only repo maintainers can trigger a workflow? Just want to be extra cautious outside contributors and forks cannot run the workflow themselves

The only way to trigger this would be to push a version change to the master branch on this repo into package.json. So either by a PR, which a maintainer would have to approve, or by a commit by someone who has admin rights to bypass branch protection.

If someone forks this repo, the workflow will not be able to publish because a valid 'NPM_TOKEN' will not exist in the new repository (it would only exist in this one). They could add their own NPM_TOKEN into their repo, but they would have to have permissions to update the package or it fails.

mapsam commented 1 week ago

Thanks for the quick response @acalcutt!

acalcutt commented 1 week ago

For example, to publish my test version, I had to do 3 things

1.) change my package.json package name https://github.com/acalcutt/node-pre-gyp/blob/master/package.json#L2

2.) change the workflow to check my version of the package is published https://github.com/acalcutt/node-pre-gyp/blob/master/.github/workflows/release.yml#L26

3.) Provide my own NPM_TOKEN repository secret with my own npm key, that has permission to @acalcutt/node-pre-gyp

cclauss commented 1 week ago

I agree that the branch protections should not be changed.

@acalcutt Would you be interested in becoming a maintainer of this repo?

acalcutt commented 1 week ago

I don't mind submitting PRs once and a while or helping out where I can, but I'm not sure how much of a maintainer I want to be. The things I have submitted so far are mainly stuff I know from trying to keep this limping along for my own projects.

I've mainly submitted these changes in hopes of seeing some up to date releases so I no longer need to maintain @acalcutt/node-pre-gyp. However in my current maplibre-native release I still publish node 16, so to move to this I would have to officially drop it (which i plan to do soon anyway), so using this package would be a breaking change once it is published.

cclauss commented 1 week ago

@mapsam Should this be merged or should something else be done in its place?

mapsam commented 5 days ago

@cclauss if the workflow works for you, it works for me!

My initial questions were just focused on blast-radius security because I wasn't able to get a well-scoped NPM token. I've since been able to generate an NPM_TOKEN only scoped to just the @mapbox/node-pre-gyp module, so my concerns there are settled. That token has been updated in the repo and should be available to test with now πŸ‘

acalcutt commented 3 days ago

My feeling is it has been tested and works and can always be modified more in the future if another approach is found. It seems worth giving it a try.

On Fri, Jul 5, 2024 at 11:57β€―AM Sam Matthews @.***> wrote:

@cclauss https://github.com/cclauss if the workflow works for you, it works for me!

My initial questions were just focused on blast-radius security because I wasn't able to get a well-scoped NPM token. I've since been able to generate an NPM_TOKEN only scoped to just the @mapbox/node-pre-gyp module, so my concerns there are settled. That token has been updated in the repo and should be available to test with now πŸ‘

β€” Reply to this email directly, view it on GitHub https://github.com/mapbox/node-pre-gyp/pull/744#issuecomment-2211107573, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA454GC724A62KEYZHMEMNDZK267NAVCNFSM6AAAAABKDJPV2OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMJRGEYDONJXGM . You are receiving this because you were mentioned.Message ID: @.***>