Closed arthurbiancarelli closed 5 years ago
Update following the previous discussion,
I integrated your suggestion (but using only GLOB
instead of GLOB_RECURSE
then REMOVE_ITEM
). It has the exact same behaviour but maybe more concise.
Personally, I'll still go with the explicit solution, but hey you're the boss :-)
In a second time I would do the same for the sources files, compiling only the relevant sources depending on the platform/options. I'll maybe make another contribution in a future time if you're interested.
For some context I use your library in a bigger project, and I'll even maybe make more contributions on the display
part, especially to get more informations about the displays in use. But first I needed to have the CMake ready for use
Hmm yeah, the globbing is much clearer now, thanks!
As for source file compilation selection – thank you for the offer, but I'd really rather prefer it stay as-is.
Yes, the PR is looking great, except for one last thing: the public headers seem to go into $PREFIX/include/*.hpp
, and the canonical way to include infoware is via #include <infoware/subsystem.hpp>
. Changing this line to PUBLIC_HEADER DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/infoware
has installed them correctly on my machine, but I'm not sure if CMake will handle this correctly and I don't really have a good way to check?
Ok yes it's better, I just checked and it works fine. ( I have an external project that uses the installed files and I just had to change the include path #include <infoware/system.hpp>
instead of #include <system.hpp>
)
As for source file compilation selection, as you want, it's just better with IDE's (you just have the necessary sources for your project) but it's just cosmetic (The linker will get rid of the empty objects). A better argument for this feature would maybe be some cases you could have mixing platforms and options. For instance, I'm working on making the display functions working on macOs. So I have display_apple.cpp
sources guarded by #ifdef __APPLE__
but thing is, you could also use Xlib
on this platform so to enable this you would have to guard this source file by #if defined(__APPLE__) && !defined(INFOWARE_USE_X11)
. For this use case it's Ok you only have 2 options but it would become problematic in the future if you want to have multiple options available per platform
Thank you for your contribution!
Released in v0.2.0
!
Making a second PR for the installation target as requested
With these additions, when running the installation target (
make install
), the installation directory will be set with:_Note: The installation directory can be set with
CMAKE_INSTALL_PREFIX
, otherwise it will be the default GNU directories on UNIX platforms (/usr/include
,/usr/lib
)_To choose the headers that will be exported, they have to be labelled as
PUBLIC_HEADERS
, hence the variable I added and theset_target_properties
The
share
folder is useful for external projects built with CMake too using the library. Indeed in another project, to use this library, all you have to do then isfind_package(infoware)
andtarget_link_libraries(${PROJECT_NAME} PRIVATE infoware)
. CMake is then handling everything else (Include directories, compile definitions, dependencies).CMake is creating
infowareConfig.cmake
that contains the common infos for you project and is creating a config file by build type (infowareConfig-debug.cmake
for instance) for specific rules. TheinfowareConfigVersion.cmake
is just here to handle the versions, for instance an external project can dofind_package(infoware 1.0)
to have a specific versionWhen doing
find_package(infoware)
in my project, CMake will take automatically the corresponding library (debug or release), depending on my project'sCMAKE_BUILD_TYPE
.The debug postfix I added (notice the d at the end libinfowared) is not mandatory but useful just to know in which mode the library was built (when configured with CMake with
CMAKE_BUILD_TYPE
).