ci: automate creation of GitHub releases and tags #1571
Conversation
|
Actually, I guess because we have https://github.com/filecoin-project/builtin-actors/blob/master/.github/workflows/release.yml, automated asset publishing is happening and merging this issue does fully handle #1566. (It's using its own script, and it isn't using the usual IPDX workflows). @rjan90 and @Stebalien: I know there are concerns about making sure consistent bundles get created when the same commit is used between tags. Do you see any issue here with this approach? (I don't know how the non-determinism was being avoided when things were being done manually.) |
|
I don't think we have much choice but to manually overwrite the uploaded assets when doing something like the v14.0.0 final where it didn't differ from the previous and we didn't want new bundles. I wonder though whether we'd better having an option to not upload the assets on publish so we're not in a place of having the wrong assets on the release page until the creator manages to replace them with the desired one. Maybe a label to put on the PR that skips the upload? |
When publishing the final v14.0.0 yesterday, I actually got the same assets generated in v14.0.0 as in v14.0.0-rc1. Which might be because the build was executed on the exact same machine/OS. There is a bit more historical context are reproducible build here: #171 (comment).
Agree that having this option would be nice though. |
|
To summarise, if we merge it, the release flow would be:
Is that the desired behaviour here? |
|
Idea: what if we do the flow like this:
This gives the PR reviewer a chance to check the CIDs before they actually merge the PR. If the PRs are off for some reason, they can manually upload assets. |
|
I haven't tested this yet, but this should do it - fc7fce7 Would that be the desired setup? |
| __release_target_asset=`echo $__release_response | jq -r ".assets | .[] | select(.name == \"$release_target\")"` | ||
|
|
||
| if [ -n "$__release_target_asset" ]; then | ||
| echo "deleting $release_target" |
There was a problem hiding this comment.
What happens if the delete fails? Will the following upload fail as well?
There was a problem hiding this comment.
A delete failure should fail the script.
|
I'd like to see a unification of the work in filecoin-project/ref-fvm#2027 and filecoin-project/actors-utils#235 into here too so we have roughly the same pattern across them all, including |
Co-authored-by: Steve Loeppky <stvn@loeppky.com>
Co-authored-by: Steve Loeppky <stvn@loeppky.com>
|
In this repo, we're using cargo to create a |
|
@rvagg : I believe this is ready for your re-review. |
| exit 1 | ||
| fi | ||
|
|
||
| __release_upload_url=`echo $__release_response | jq -r '.upload_url' | cut -d'{' -f1` | ||
|
|
||
| __release_target_asset=`echo $__release_response | jq -r ".assets | .[] | select(.name == \"$release_target\")"` |
There was a problem hiding this comment.
not a blocker, more of a preference thing so take it or leave it: I prefer subshell $() use rather than backticks for this kind of thing, better ergonomics and IMO better readability
There was a problem hiding this comment.
Same here, but it was a convention already used in the script and I didn't want to update all the occurrences at this time given all the other changes we're introducing.
There was a problem hiding this comment.
Sure, seems OK, although I do kind of agree with @Stebalien @ filecoin-project/actors-utils#235 (review) that auto-publish is the ultimate form of this. The disconnect between here and actually publishing means we could easily end up with tagged but unpublished (and unpublishable?) crate versions.
Do we do I now described the entire release process in this repo in 562eee7 and included a section on possible future improvements. |
There was a problem hiding this comment.
Thanks for the updates!
Do we do cargo publish in this repo? I couldn't find any mention of that.
Agreed. I don't believe we do a cargo publish for this repo. Per https://github.com/filecoin-project/lotus/blob/master/documentation/misc/Update_Dependencies_Lotus.md#updating-builtin-actors , I believe it gets pulled in via other means.
Looking at https://crates.io/search?q=filecoin%20actors&sort=recent-updates , everything there is coming from Chainsafe.
Co-authored-by: Steve Loeppky <stvn@loeppky.com>
|
In ffda880 I incorporated a couple of suggestions from me about where the git tag is created. I'm going to squash/merge given verbal during 2024-08-14 Lotus maintainers call where @Stebalien said these changes are good given we don't have to do crate publishing. |
|
Thank you for all the reviews, and help getting it merged 🙇 |
This automates the creation of GitHub releases and associated GitHub tags based on the changes to the version in the root Cargo.toml file.
The automation uses the same principles as the one you might know from Go projects where we control the current version via version.json file (https://github.com/ipdxco/unified-github-workflows?tab=readme-ov-file#versioning). This is the exact same workflow just updated to workflow with Cargo.toml.