ValleyBell / libvgm

A more modular rewrite of most components from VGMPlay. will include sub-libraries for audio output, sound emulation and VGM playback
irc://irc.digibase.ca/#vgmrips
136 stars 33 forks source link

Split CI for integration in vgmplay-libvgm #86

Closed OPNA2608 closed 2 years ago

OPNA2608 commented 2 years ago

These are changes required to add CI testing for vgmplay-libvgm in a way that reuses existing CI code from libvgm. A followup PR on vgmplay-libvgm will happen once this has been merged.

Most of the logic has been moved into a local action. We can execute this action from this repo's CI, with the correct arguments, to test building libvgm as normal. But we can also execute it when the whole repo has been checked out as a submodule in vgmplay-libvgm, as a dependency step. This way there won't be any cross-repository code duplication, we don't need to worry about uploading & fetching any artifacts across repos for the CI to work and this hard dependency of vgmplay-libvgm on the libvgm submodule only exists on the CI system. As long as the submodule is only ever bumped to a known-good revision of libvgm (and neither the runners nor homebrew experience breaking changes), libvgm should build fine in vgmplay-libvgm.

A downside of this is that I had a really hard time converting some things into a functioning action. For example, it seems to be impossible to specify a shell based on another parameter in an action? I kept getting parsing errors no matter what I tried, as such I had to resort to duplicating some steps, check the platform & compiler situation and hardcode a specific shell as needed. To cut down abit on the code duplication I moved some code into script files, only set up the necessary environment in the steps and call these scripts. If this hurts readability too much in your opinion then I can revert this. Another problem was path encoding of ${{ github.workspace }} vs ${GITHUB_WORKSPACE} so I had to write extra code to convert windows paths into a compatible format.

ValleyBell commented 2 years ago

The readability is fine enough, IMO, and using script file to reduce code duplication is a good idea.

Thanks for all the effort!