search

jketterl / digiham

tools for decoding digital ham communication
GNU General Public License v3.0
46 stars 14 forks source link

cmake library linking is wrong #2

Closed kolbma closed 4 years ago

kolbma commented 4 years ago

You can't use https://github.com/jketterl/digiham/blob/696209c87a9bbac66896d27fd9346034e68f6e51/CMakeLists.txt#L53

This stuff generates shared object files and links against. You can't have libversion.so for each program. It is a file conflict.

What you want to do is linking your object files to executables...

add_executable(rrc_filter rrc_filter.c version.o)

I think this is true for all target_link_libraries appearances. Or else you also miss the installation of library files.

kolbma commented 4 years ago

This should be the way...

CMakeLists.txt

jketterl commented 4 years ago

I'm sorry, but I fail to see the problem. I'm no cmake expert, but when I build it, it doesn't generate any shared libraries. What it does is, it generates static libraries, which will then be statically linked into the resulting binaries, i.e. their objects and symbols get copied into the binary. There is no libversion.so, nor is there any libversion.* being installed onto the system.

kolbma commented 4 years ago

You can read on https://cmake.org/cmake/help/v3.0/command/add_library.html that it depends on the platform what type of libraries are built if this is not specified.

So if you depend on static libraries you have to insert STATIC.

This would also make the build fail if building static libs is disabled. No shared libs are built on any platform. The dependencies for the executables are missed and can not be built. Result: build failure with a concrete message. And no executable with missing shared libs.

jketterl commented 4 years ago

OK, thanks for the documentation, it helped understand the issue.

I am tempted to use the STATIC declaration, simply because that's what I intended to do. I am currently trying to evaluate what limitations that would bring, the documentation doesn't go into that.