Cmake: refactor separate shared / static build targets

libgme / game-music-emu

Blargg's video game music emulation library, which allows audio applications to easily add playback support for the music of many classic video game consoles.

GNU Lesser General Public License v2.1

59 stars 12 forks source link

Cmake: refactor separate shared / static build targets #95

Closed tadashibashi closed 1 month ago

tadashibashi commented 1 month ago

I attempted to refactor the CMakeLists to allow separate static / shared builds

Changes:

added an additional GME_BUILD_STATIC flag set to OFF by default
separated shared / static build targets
pulled out similar logic across both targets into variables to then set in both
tried to carefully handle logic so it is the same result as before

Also, I'm not sure if it's necessary, but I have it generating a .pc file for each.

sezero commented 1 month ago

Also, I'm not sure if it's necessary, but I have it generating a .pc file for each.

Haven't read any other parts (and you generated against your copy of libgme instead of mainstream master branch hence the conflicts), but: Generating a separate .pc file for shared and static builds is the wrong thing: the pc file should handle both.

Wohlstand commented 1 month ago

I'll take the look tomorrow as now I am very tired after the long trip and I going to sleep.

tadashibashi commented 1 month ago

you generated against your copy of libgme instead of mainstream master branch hence the conflicts

Yes, I noticed that - didn't realize continuing from the old branch from the last pull request would cause this issue. I'll close this one.

Generating a separate .pc file for shared and static builds is the wrong thing: the pc file should handle both.

Yes, since both probably should use the same flags there shouldn't be separate ones, thanks for the feedback.

Also, after attempting to link the demo to the dynamic lib for some reason all the symbols are undefined even though LIBGME_VISIBILITY and BLARGG_EXPORT appear to be defined correctly. I'll reopen a new pull request if I can figure this out.