search

adplug / adplay-dos

AdPlug's DOS frontend
8 stars 1 forks source link

Add GitHub actions for cross compilation, add Adplug 2.3.3 support #3

Closed AranVink closed 11 months ago

AranVink commented 1 year ago

Almost 18 years since the last version of Adplay for Dos. This PR hopes to change that :smile:

Goal of this PR

New build system The build system uses DJGPP cross compilation, and uses binaries from https://github.com/andrewwutw/build-djgpp. It creates a Docker image with these binaries installed, which is then used to cross compile the different dependencies. Docker was chosen because I wanted to have reproducible builds, not relying on a manual setup of DJGPP, which is a lot of work and adds a lot of complexity. Dependencies are retrieved from Git (see corresponding build scripts for used Git branches/tags). The Adplay sources are retrieved from the currently checked out working directory and mounted as a volume. By doing make binarydist a zip file is created, which can then be retrieved once the compilation is complete.

Verification I was able to run the new binaries on a real hardware: P3 1Ghz, Sound Blaster AWE64 in DOS. It can now playback formats that have been added since Adplug 1.6, for example DRO v2 files :musical_note::smile:

Some notes/room for improvements Currently some separate patches have been made for Adplug 2.3.3, I hope to have these ready in a PR to Adplug soon, so they can be merged upstream. I would suggest to wait with merging this PR until they are ready. I will add a minimum required version of Adplug to the build pipeline.

Even though GCC 12 is used, I have not updated any other code. There are a lot of warnings, probably related to newer C versions/standards and changes in the GNU toolchain. I haven't had the time/knowledge to address these.

I took the liberty to already update the version number of Adplay from 1.6 to 1.7, given the amount of changes/new Adplug version.

Let me know what you think, any feedback is highly appreciated. I was new to cross compilation, DJGPP, and the GNU toolchain before starting this, so there will be improvements possible in the make/configure area that I'm unaware of.

mywave82 commented 1 year ago

This PR probably needs to be split up a bit making reviewing a more clean process. Most easy to start with is probably .cvsignore / .gitignore.

AranVink commented 1 year ago

Agreed with splitting up this PR into multiple separate ones. I also realized Adplug compatibility needed a bit more work, and it would make sense to add DJGPP support to Adplug's GitHub actions with this as well, to verify it can compile using that toolchain, just like Adplug is configured now with multiple architectures/toolchains. Once that's done I'll revisit this PR.

Marked this PR as draft for now.

AranVink commented 11 months ago

I've decided to abandon this PR, and start with a fresh rebase and split it up into multiple PR's. Any comments raised here have been addressed in the new PR already.