xR3b0rn / dbcppp

C/C++ DBC file parser/tool
MIT License
232 stars 76 forks source link

Improve cmake package installation by exporting libdbcpp include directories #96

Closed agalbachicar closed 2 years ago

agalbachicar commented 2 years ago

This issue can be seen as a follow up of #92

I think package configuration can be improved by using target_include_directories with libdbcppp and make it point to the base header file path in the install space.

In particular, when taking a look at this header file inclusion, it allows to build the library and other targets because of similar usage for the targets under that CMakeLists.txt file but fails to properly export them. Consequently, when doing find_package(libdbcppp REQUIRED) it fails to find the path the header files if instead of using global include_directories() calls, one uses:

target_link_libraries(my_target
  libdbcppp
)

It is worth noting that I am working in a colcon workspace (see here) and using its tools because this is part of a larger robotics project.

Find below the key difference between the exported cmake configuration files:

[...]
# Create imported target libdbcppp
add_library(libdbcppp SHARED IMPORTED)

set_target_properties(libdbcppp PROPERTIES
  INTERFACE_LINK_LIBRARIES "libxmlmm"
)
[...]

set_target_properties(libdbcppp PROPERTIES INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include/" INTERFACE_LINK_LIBRARIES "libxmlmm" ) [...]



 -----------------

If you are accepting PRs, I will be more than happy to send a proposal for further discussion. I've already built it in a workspace and try to use the basic example from another package. I am not an expert in the project, so please let me know if there is anything that I missed.

- [dbcppp fork](https://github.com/agalbachicar/dbcppp/) and [branch](https://github.com/agalbachicar/dbcppp/tree/agalbachicar/cmake_fixes_to_improve_package_discovery) with the fix.
- Sample [demo](https://github.com/agalbachicar/demo_dbcppp) that explains the repro steps and usage.
xR3b0rn commented 2 years ago

I enjoy seeing people improving the project. So yes, I highly appreciate PRs.

But since you put some work in preparing this issue, I want look deeper in what you did in order to be able to give a more valueable feedback. I will try to provide the feedback until the end of the week. However, since I am aware of the poor design of my CMake-scripts, which already got improved in https://github.com/xR3b0rn/dbcppp/pull/93 , the odds are very good that I will accept your PR.

xR3b0rn commented 2 years ago

Not much to say. I will accept your PR.

agalbachicar commented 2 years ago

Thanks!

xR3b0rn commented 2 years ago

Accepted and merged in #97