-
Notifications
You must be signed in to change notification settings - Fork 10
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
Add example source code for client-side detection of CiftiLib #15
Comments
I added the first two lines of that. I'm not that familiar with the imported target syntax of cmake. |
👍
It has got a lot of benefits and is worth investing some time on. The ideal usage from a client standpoint would be something like: find_package(CiftiLib REQUIRED)
add_executable(myexecutable myexecutable.cxx)
target_link_libraries(myexecutable CiftiLib::Cifti) whereby FYI, I should be submitting |
I looked for how QT5 delivers its cmake module, and it has this unnerving quote:
http://doc.qt.io/qt-5/cmake-manual.html Are they doing it wrong, or is that aimed at custom installs from source? Where do other libraries deliver their cmake modules when installed via debian?
Cool! |
Quoting the first snippet of your link: # Find the QtWidgets library
find_package(Qt5Widgets)
# Tell CMake to create the helloworld executable
add_executable(helloworld WIN32 main.cpp)
# Use the Widgets module from Qt 5.
target_link_libraries(helloworld Qt5::Widgets) This is the syntax for using imported target.
If you look at the file listing of the So in your case, you could generate a |
I read that part, what I was worried about is that the later part sounded like cmake may not search other directories for such config files, and you would have to run cmake like this to get it to work that way:
However, I tested this on an ubuntu 14.04 VM, and it seems as though qt5 is found without setting CMAKE_PREFIX_PATH (there is no qt5 cmake file in /usr/share/cmake-2.8/Modules). After some searching in the cmake man page, it appears to look in Note, however, that I wrote my own Findglib.cmake and similar for when it is compiled without using QT. Those packages deliver pkg-config files, but not cmake config files. It could get messy to handle this with a single cmake config file, perhaps it simply shouldn't install a cmake config file when it is not compiled against QT? I suppose I could have it add |
As stated in the
Not if you use imported targets.
Don't. The solution here is for both the The exported |
Looking at https://cmake.org/Wiki/CMake/Tutorials/Exporting_and_Importing_Targets , it has this syntax:
How magical is this syntax, will it actually take care of turning custom FindBlah.cmake scripts into statements inside that single exports file? If the resulting export will contain a Shorter question: do you have an example of how to deal with a library that doesn't provide a cmake file in this way? If not, I'd currently prefer not installing a cmake file when built against libxml++, because I currently don't see how to do it cleanly (when built against QT, every dependency already provides its own cmake find_package files, so it just works). |
I did a little more investigation, and started trying to make an imported target for libxml++, and then found that cmake 2.8.7 doesn't understand "INTERFACE" in add_library(). It also doesn't seem to understand the INTERFACE_LINK_LIBRARIES property when using "IMPORTED" in ADD_LIBRARY, and instead insists on using IMPORTED_LOCATION (and writes a "not found" into a makefile without giving a configuration error when it isn't set). Finally, the INSTALL(TARGET ... EXPORT ...) syntax doesn't appear to accept imported targets, and I am under the impression that that is required in order for the INSTALL(EXPORT ...) command to know where to find the dependencies. So, this isn't going to work nicely on cmake 2.8.7 (boost requires either 2 or 0 libraries, depending on whether QT is in use, both of which are problematic to a single imported target). I'm not sure about requiring a newer cmake (for instance, neurodebian builds connectome-workbench on ubuntu 12.04 and debian wheezy, which package cmake 2.8.7 and 2.8.9). I made a branch, export-experiment, that has my initial attempts in it, for future reference, or if you know some workarounds, or look for places where I went off the rails. |
Say you have the Your export should only contain an import statement on the target file, period. On the client-side, there will be no more detection of
I believe the benefits brought by imported targets from a client-side perspective are worth bumping the requirements on the minimum CMake version a little bit. You mentioned Ubuntu 12.04 and Debian Wheezy as concerns, but both will see their support window end soon. There is also very little chance that someone would deploy any of these 2 today, considering both Ubuntu 14.04 and 16.04 are available, and Debian Jessie has been around for almost 2 years. Finally, both have backports of
That's actually where the flexibility of CMake targets shines. You can have multiple dependency paths (like for xml support and boost in your case) and have them handled automatically within the definition of the target without having to tweak anything. You just link the |
So I came back to this, basically started over from master (the export-experiment2 branch), leaving the boost, etc code mostly as-is, but changing things to target properties of Cifti, and got down to this for what should be needed in the
But, besides the fact that it shouldn't require manually setting
Adding Strangely, doing |
So, something I found online indicated that when you have dependencies that are imported targets, yes, your I'm not currently using a namespace on the imported target, though. There's really only one library this project provides, so I'm not sure what the best strategy here is. Let me know what you think. |
Perhaps something that the description in
README
orUSAGE
is currently lacking is a code example for discovering and using the library from a client project.I have used the following snippet in my integration tests:
Feel free to copy or adapt this code if you wish.
The text was updated successfully, but these errors were encountered: