search

MRtrix3 / mrtrix3

MRtrix3 provides a set of tools to perform various advanced diffusion MRI analyses, including constrained spherical deconvolution (CSD), probabilistic tractography, track-density imaging, and apparent fibre density
http://www.mrtrix.org
Mozilla Public License 2.0
294 stars 181 forks source link

New workflow for releases #2933

Closed daljit46 closed 4 months ago

daljit46 commented 5 months ago

This PR succeeds the work done in #2835. As mentioned there, instead of having a separate workflow for weekly releases, it's better to have a single CI workflow that manages both tagged and periodic releases. As a first step towards that, this PR introduces a new CI workflow which creates artefacts of release builds using GitHub Actions. The artefacts are named as mrtrix3-{os}-{branch}.{extension} (@MRtrix3/mrtrix3-devs let me know if we want to add further information like build dates). The workflow can be triggered manually, and the developer can specify the branch of MRtrix3 they want to build (the branch will need to be based on cmake). This will be useful firstly because it allows a developer to create a release for a particular feature branch of their choice. Secondly, it will allow us to test our build scripts for our upcoming releases without creating a tagged release.

Since GitHub only allows workflow_dispatch triggers if the workflow file is on the default branch, the merge needs to be against master. This is slightly unfortunate since master is still not on cmake. To make sure things behave as expected, I have already tested that the workflow succeeds on a separate fork of MRtrix3.

Lestropie commented 5 months ago

I would have thought that for anything that is not based on an immutable tag, you would want the build date (yyyymmdd) embedded in the file name.

daljit46 commented 5 months ago

I would have thought that for anything that is not based on an immutable tag, you would want the build date (yyyymmdd) embedded in the file name.

Ok, will make changes to address this. Would a commit hash be even more preferable?

Lestropie commented 5 months ago

Would a commit hash be even more preferable?

I think that if a user requires a version with greater specificity than a branch name and date to the nearest week, they're probably capable of compilation from source. I'd be content with naming by date rather than commit since it's intended for interpretation by users, though also wouldn't be against having both.

Maybe another case worth considering here is where a developer has made some change to a branch, and then wants to make that code version immediately available to a user to utilise / test. There, telling the user that they have to wait up to a week for GitHub to build and package it is a bit clumsy. Will developers be able to trigger regeneration of the package for a specific branch on demand?

jdtournier commented 5 months ago

I'd favour both commit & date in the filename, personally.

Will developers be able to trigger regeneration of the package for a specific branch on demand?

Yes, I think that's the idea - @daljit46 can correct me if I'm wrong here...

daljit46 commented 5 months ago

Yes, that's correct. In fact, this PR only implements manually triggered releases which don't create a tagged release, but GA artefacts that can be downloaded by anyone who has a GitHub account.

daljit46 commented 5 months ago

I've changed the workflow to produce artefacts as mrtrix3-${commit_sha}-${date}.${extension}. An example run can be seen here (it'd be great if you could test these by downloading and running them locally).

This is ready for review.

daljit46 commented 5 months ago

Actually, just realised that there is a mistake in the Windows artefacts. Let me check that. EDIT: never mind, false alarm. The Windows artefact commit is different because the cloning of source doesn't happen on the fork. But just in case, I will clone using the standard GitHub Action.

daljit46 commented 5 months ago

Another little thing I noticed is that Github by default packages artefact using .zip as an extension. Should I remove the extensions from the artefacts name?