search

mity / md4c

C Markdown parser. Fast. SAX-like interface. Compliant to CommonMark specification.
MIT License
756 stars 138 forks source link

linking with the imported cmake targets does not work #145

Closed niclasr closed 3 years ago

niclasr commented 3 years ago

When linking with cmake using the exported imported targets in a cmake file like this:

cmake_minimum_required(VERSION 3.18)
project(md4c-test LANGUAGES C)

find_package(md4c REQUIRED)
add_executable(md4ctest md4ctest.c)
target_link_libraries(md4ctest md4c)

the include directory is not added automatically to the compile command md4ctest. This can be fixed by adding INCLUDES DESTINATION ${CMAKE_INSTALL_INCLUDEDIR} to the INSTALL(TARGETS command of the md4c and md4c-html targets.

Secondly md4cHtml or md4c-html package is not found at all. The reason for this is that the directory and the name of the config are different. Pick one such as md4c-html and have the config name changed to md4c-htmlConfig and it works or change the install directory to md4cHtml and it also works, they must be the same as can be seen in the cmake docs on search order. I suggest using the first since it is consistent with the target name. I have made this an issue instead of a pull request since both ways work and I want to know which one you think is the best.

When the above works md4c html complains about not finding the target md4c that is wants to link to. This can be fixed by adding PRIVATE to its target_link_libraries command but that might have implications on static linking, I guess.

I can make a pull request with the above changes if you are interested.

mity commented 3 years ago

Oh yes, CMake. It's sometimes hard to make it work indepenently on the generator used or on the platform. And I'm the 1st to admit it's not my strong side: I use copy&paste there.

So yes, if you can cook a PR, it would be highly appreciated.

niclasr commented 3 years ago

I have now made a PR #146 . It includes a fix for the last problem as well. I don't know what you think of the namespace addition since it might break backward compatibility.