CMakeLists fixes - Githubissues

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

70 stars 13 forks source link

CMakeLists fixes #92

Closed tadashibashi closed 4 months ago

tadashibashi commented 4 months ago

Fix static / shared library bug: static libraries were produced even when the shared flag was on
Bump version of CMake to 3.5: a later version in my IDE reported that prior versions are soon to be deprecated
Rename generic option names with GME_ prefixed before it, using the old names as defaults
Add GME_BUILD_EXAMPLES option to control adding example projects (added by default if GME is the root CMake project)

sezero commented 4 months ago

Why should we not build both a shared and a static library again ???

Wohlstand commented 4 months ago

Better to make that explicitly: have TWO arguments that tells build static or build shared. Ye, I think that should be done. But as a separated fix. I'm out of home, so, can't right now.

sezero commented 4 months ago

Better to make that explicitly: have TWO arguments that tells build static or build shared. Ye, I think that should be done. But as a separated fix. I'm out of home, so, can't right now.

The cmake'ry in libxmp does it right, it should be a good guide for it.

Wohlstand commented 4 months ago

My libADLMIDI and libOPNMIDI too.

tadashibashi commented 4 months ago

How about making an object library add_library(gme-obj OBJECT) and then linking the target objects to gme for shared and gme-static for static targets? Then the files only need to compile once.

I'm looking at gme/CMakeLists.txt, and some concerns are making sure that gme links to zlib lstatically on static builds according to the comments, and generating the .pc file. I'm not familiar with pkg-config. Because the .pc contains build flags, would it be necessary to generate one per library type?

sezero commented 4 months ago

How about making an object library add_library(gme-obj OBJECT) and then linking the target objects to gme for shared and gme-static for static targets? Then the files only need to compile once.

Then you'll possibly export symbols from the static library which we don't want.

tadashibashi commented 4 months ago

I see, OBJECT libraries export all symbols, even if unnecessary. Then it's best to just add the sources both times.

Wohlstand commented 4 months ago

As a note: Shared and Static libraries sometimes gets compiled with different compilation flags (primarily to specify export/visibility attributes for the shared library or don't specify them for the static). So, can't just by "OBJECT".

Wohlstand commented 4 months ago

Then it's best to just add the sources both times.

Ye, I usually do:

I have the INTERFACE thing, where I attach all general things like dependencies and general flags, and sources too.
Two targets of shared and static libraries, where I link this INTERFACE thing and specify all specific flags for each of static and the shared thing.
Shared list of sources is just easier, and lists of definitions flags and link libraries too.

tadashibashi commented 4 months ago

Thanks for sharing your method - I also use INTERFACE too, and think it could be added once the common flags get sorted out. The current CMakeLists are a little confusing because some flags appear unnecessary for either dynamic or static and are set both times, but appear to be sensitive.

But in my closed pull request there's a problem where symbols are not getting exported during dynamic library builds (I'm building on an M1 Mac). If you have time, could you possibly review it? The file to check would be gme/CMakeLists.txt

Wohlstand commented 2 months ago

Failed to generate code suggestions for PR

What do you mean?