Fix tagged release creation and package upload
fedorov opened this issue · 10 comments
Those have been broken since we migrated to GitHub actions :-( Summary below is what I communicated to Vamsi by Discord, but it is better to continue the discussion here for transparency and coordination, so I replicate what I said below.
Some time ago I migrated dcmqi to GA from Appveyor/TravisCI/CircleCI. That was my one and only major foray into GA. I think I got most of the things figured out, with one exception. We used to have the process where tagged git commits that follow the versioning convention would be uploaded to GitHub and a new release would be created, but that got broken, and I feel it would take me too much time to figure out how to fix it.
The original CI was set up by Jean-Christophe @jcfr from Kitware (whom you will hopefully meet at the project week!), and he used a dedicated python package to handle this, see https://github.com/QIICR/dcmqi/blob/master/.github/workflows/cmake-linux.yml#L60-L65. I think the motivation for that package was to deal with the differences across CI platforms.
Packaging and upload of the "latest" release works fine, but not the tagged releases, which I believe would be handled by this clause in the command: --release-packages "build/dcmqi-build/dcmqi-*-linux.zip"
This is the commit that I tagged, which would be expected to generate and upload a release package: 697ff25
but it only uploaded latest: https://github.com/QIICR/dcmqi/actions/runs/6671898376/job/18134744092
It may be as simple as the package not assigned the expected file name. As you can see, the package file is named dcmqi-1.2.6-linux-20231027-697ff25.tar.gz, and the release pattern is dcmqi-*-linux.zip. Which as I just now realize can't be right - it should have tar.gz extension. But it also failed on all other platforms to upload the tagged release package.
The reason I would really like to fix it is because yesterday we merged the dcmqi improvement that speeds up DICOM SEG read by orders of magnitude, and I would want to have a release to make this capability easily accessible ("latest" packages are not persistent, so it makes no sense pointing users to include them into notebooks etc).
Few more bits of information that might be useful. Here are the CI platform specific scripts that were working to create and upload release packages:
- Windows: https://github.com/QIICR/dcmqi/blob/880bb8268a9636f4242448523db7fa7f69c2f07e/appveyor.yml
- Linux: https://github.com/QIICR/dcmqi/blob/880bb8268a9636f4242448523db7fa7f69c2f07e/.circleci/config.yml
- Mac: https://github.com/QIICR/dcmqi/blob/81768b58ab1ba0380c9bef504e37c360fbd40886/.travis.yml
It could also be that I did not replicate the steps properly in GitHub actions!
I should also correct myself - it was not just me who did the migration to GitHub actions - Michael Onken @michaelonken contributed a lot to this as well, helping fixing bugs and addressing many of the related issues as well! 🙏
Happy to work on this! I'll first try to troubleshoot with scikit itself. And if you are okay with using github api, you may consider the following.
In a recent workflow, we tried using GitHub API to handle releases with help of a python script get_latest_index.py running inside GitHub actions update_release_attachment.yml. I also think with the Matrix approach in GitHub actions, we should be able to build and publish for all three platforms parallelly with a single workflow. Might be worth considering as it may reduce maintenance.
Switching to github API in place of scikit would be completely fine, if fixing the scikit approach is not easy. Using matrix approach is also interesting. The current organization of the workflow is due to the need to support multiple CI platforms before GitHub actions, it was just the most natural first step in migration.
reading scikit documentation carefully suggests that the reason why the commit 697ff25 did not trigger release is because the commit was tagged with v1.2.6 where as scikit expects just 1.2.6 I think. which is why the GA logs also says it did not find a release tag
@fedorov I did not test on my fork yet, will test and report back.
I would be very surprised if that's the culprit. The version tagging has been following the same pattern since the first releases, you can see the tags in https://github.com/QIICR/dcmqi/releases.
@fedorov you were right.. tags starting with 'v' were also recognized as released tags.. I found the issue is related to fetching tags in github actions while checking out code. actions/checkout#206
I submitted a PR, as with this PR, it should recognized the tags, but I couldn't check if release attachments are created properly for mac and linux.
update: I made some minor changes to expected release file names and everything should now work as expected.
Will close once we confirm it works with a release tag - I will make one soon!
It worked! 🎉
