Skip to content

Add DJGPP support to Adplug#193

Merged
mywave82 merged 5 commits into
adplug:masterfrom
AranVink:master
Oct 5, 2023
Merged

Add DJGPP support to Adplug#193
mywave82 merged 5 commits into
adplug:masterfrom
AranVink:master

Conversation

@AranVink

@AranVink AranVink commented Sep 2, 2023

Copy link
Copy Markdown
Contributor

Add a few #ifdef preprocessor commands to check for DJGPP being used a compiler for DOS.
Prerequisite PR for PR on Adplay-DOS: adplug/adplay-dos#3

Comment thread src/rix.cpp Outdated
Comment thread src/player.h Outdated
Comment thread adplugdb/getopt.c
@AranVink AranVink marked this pull request as draft September 3, 2023 11:05
@AranVink

AranVink commented Sep 6, 2023

Copy link
Copy Markdown
Contributor Author

After some fiddling with GitHub actions I managed to add DJGPP compilation support.
It can now use DJGPP - GCC 12.2.0 and DJGPP - GCC 4.8.5. To get GCC 4.8.5 to work I had to include stdint.h and remove some older specific redeclarations of uint*.
Running tests is really hard to do AFAIK when cross-compiling like this, so it only checks if it can run make all. Maybe when I've updated Adplay for DOS I could look into some way of adding tests there. (Also see adplug/adplay-dos#3)

Build results are available in my fork: https://github.com/AranVink/adplug/actions/runs/6091183567

Please review and let me know what you think. I'm very sorry for the commit spam, I thought I was only pushing to my own repository while doing these tests, but unfortunately this PR and CI still got triggered through my own repo/branch. 😅

@AranVink AranVink marked this pull request as ready for review September 6, 2023 22:37
@mywave82

mywave82 commented Sep 8, 2023

Copy link
Copy Markdown
Contributor

I am quite busy the next two weeks, but I will try to grab some time every now and and probably rebuild to commit history (doing some git rebase magic)

@AranVink

AranVink commented Sep 8, 2023

Copy link
Copy Markdown
Contributor Author

Much appreciated, if you want me to make a new PR with all commits squashed let me know (I made this mess 😉)

@mywave82

Copy link
Copy Markdown
Contributor

I rebased your tree, so unless you are very familier with git, just do a fresh clone of your own git repository on your computer.

I believe a lot of the fiddling with setting CXXFLAGS etc. to include -I/foo/djgpp/include can be avoided if djgpp extracted into /usr/local/djgpp, as that is probably the PREFIX is was compiled for.

@binarymaster

Copy link
Copy Markdown
Member

I think some fixes from here could be already pushed to master, like header and type fixes. They are trivial.

@AranVink

AranVink commented Oct 3, 2023

Copy link
Copy Markdown
Contributor Author

Thank you for revisiting this. I'll take another look this weekend on the CXXFLAGS/paths.

Are there any other concerns or attention points that would prevent this from being merged in the near future? I'd like this PR to be as complete as possible, so I can continue with upgrading Adplay for DOS.

@mywave82

mywave82 commented Oct 3, 2023

Copy link
Copy Markdown
Contributor

We are getting close to something now (git rebase is needed again to clean up the history)

@mywave82

mywave82 commented Oct 5, 2023

Copy link
Copy Markdown
Contributor

Sorry for the extra spam lately, trying to make the github workflow be a smooth as possible.

@mywave82 mywave82 merged commit db2cb13 into adplug:master Oct 5, 2023
@mywave82

mywave82 commented Oct 5, 2023

Copy link
Copy Markdown
Contributor

Thank you for your patience

@AranVink

AranVink commented Oct 6, 2023

Copy link
Copy Markdown
Contributor Author

Thank you for following up and making the tweaks so it can be merged, again much appreciated. I'll continue working on Adplay for DOS, building a CD pipeline, and maybe even a CI pipeline (when tooling allows).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants