Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing BUILD/INSTALL_${TARGET}_INCLUDEDIR in CMakeLists.txt #6

Closed
claudiofantacci opened this issue Jan 16, 2017 · 12 comments
Closed

Comments

@claudiofantacci
Copy link
Collaborator

Some variable are missing before invoking install_basic_package_files() in the main CMakeLists.txt (line #70).

In particular, just before line #70, a

get_property(${PROJECT_NAME}_TARGETS GLOBAL PROPERTY ${PROJECT_NAME}_TARGETS)

is missing to properly use the associated global variable.

Furthermore, for install_basic_package_files() to work properly, the following 2 variables need to be declared:

BUILD_${PROJECT_NAME}_INCLUDEDIR
INSTALL_${PROJECT_NAME}_INCLUDEDIR

I have an implementation that seems to work fine, even though it is not a clean solution in the case a user uses EXTRA_PATH_VARS_SUFFIX.

We should briefly discuss a solution when we have some time :-)

@claudiofantacci
Copy link
Collaborator Author

cc @drdanz forse puoi aiutarci a venirne a capo!

@drdanz
Copy link
Member

drdanz commented Jan 17, 2017

@claudiofantacci This might help...

@claudiofantacci
Copy link
Collaborator Author

Indeed, you have to (for example)

set_property(GLOBAL APPEND PROPERTY ${VARS_PREFIX}_TARGETS template-lib)

and then

get_property(${VARS_PREFIX}_TARGETS GLOBAL PROPERTY ${VARS_PREFIX}_TARGETS)

before invoking install_basic_package_files(), which seems to be missing in our main CMakeLists.txt at line #70.

But even doing that, the generated *Config.cmake file reports

...
set_and_check(ExportedLib_INCLUDEDIR "")
...

instead of

...
set_and_check(AutoCanny_INCLUDEDIR "${PACKAGE_PREFIX_DIR}/include")
...

To fix this, I also have to

...
set_property(GLOBAL PROPERTY BUILD_${LIBRARY_TARGET_NAME}_INCLUDEDIR   ${CMAKE_CURRENT_SOURCE_DIR}/include)
set_property(GLOBAL PROPERTY INSTALL_${LIBRARY_TARGET_NAME}_INCLUDEDIR ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_INCLUDEDIR})
...

in the corresponding LIBRARY_TARGET_NAME CMakeLists.txt and lastly, just before invoking install_basic_package_files() call

get_property(BUILD_${LIBRARY_TARGET_NAME}_INCLUDEDIR   GLOBAL PROPERTY BUILD_${LIBRARY_TARGET_NAME}_INCLUDEDIR)
get_property(INSTALL_${LIBRARY_TARGET_NAME}_INCLUDEDIR GLOBAL PROPERTY INSTALL_${LIBRARY_TARGET_NAME}_INCLUDEDIR)

Have a look here (main CMakeLists.txt) and here (Lib CMakeLists.txt).

@Tobias-Fischer
Copy link
Member

Hi @claudiofantacci @drdanz,
I still have this issue. I am using the how-to-export-cpp-library example, and in the ./how-to-export-cpp-library/build/LibTemplateCMakeConfig.cmake, the following line appears:

set(LibTemplateCMake_INCLUDEDIR "")

So I assume this issue is not yet fixed. You can replicate this issue directly on master, I haven't done any modifications.

It would be great if you could have a look into this.

Thanks, Tobi

@claudiofantacci
Copy link
Collaborator Author

claudiofantacci commented Apr 10, 2017

We actually made a "fix" that also preserved some backward compatibility and that line is the price to pay, @traversaro probably knows best.

The template should work fine, you just need to link your target against LibTemplateCMake::LibTemplateCMake. For example, say you have mytarget target, then:

target_link_libraries(mytarget LibTemplateCMake::LibTemplateCMake)

without doing anything else. The other config files take care of the rest.

Let us know if you encounter any other problem or you have suggestions to improve usability and user experience.

@Tobias-Fischer
Copy link
Member

Hmmm .. sure, I can link to it, but how can I use the header files provided by the library? I.e. how can I #include "LibTemplateCMake.h" from an external project?

@claudiofantacci
Copy link
Collaborator Author

Includes are part of the LibTemplateCMake::LibTemplateCMake target properties.

You can find details in LibTemplateCMakeTargets.cmake and in particular in

set_target_properties(LibTemplateCMake::LibTemplateCMake PROPERTIES
  INTERFACE_INCLUDE_DIRECTORIES "<yout_path>/how-to-export-cpp-library/src/LibTemplateCMake/include"
)

So when you find the package such includes are already added.

@Tobias-Fischer
Copy link
Member

Great, get it, working fine, thanks a lot :)!

@claudiofantacci
Copy link
Collaborator Author

No worries @Tobias-Fischer, you are welcome :-)

@Tobias-Fischer
Copy link
Member

I just fully understood how to use this in a dependent project .. for other CMake newbies, this line will do the trick:
get_target_property(libtemplate_includes LibTemplateCMake::LibTemplateCMake INTERFACE_INCLUDE_DIRECTORIES)
Then, you can use it as usually:
include_directories(${CMAKE_CURRENT_SOURCE_DIR} ${libtemplate_includes})

@claudiofantacci
Copy link
Collaborator Author

@Tobias-Fischer that will work, but the pro of using target is to avoid code verbosity and reduce mistakes 😄
If you use targets, that line can be avoided.

@Tobias-Fischer
Copy link
Member

Tobias-Fischer commented Apr 26, 2017

Yes, yes .. in that specific code, I needed it for SWIG which otherwise does not find the include files. Sorry, should have specified that ;). Otherwise it's not necessary, very true.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants