-
Notifications
You must be signed in to change notification settings - Fork 1
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 section for YCM/cmake common practices #18
Comments
A few remarks on YCM based on my experience:
For the record, most of this was discovered at roboticslab-uc3m/color-debug#6. |
Updated issue title to "Add section for YCM/cmake common practice" (added cmake). Added link to roboticslab-uc3m/questions-and-answers#18 in description. |
I'll give some pointers on this, based on two real use cases. Header-only repos + include paths propagated via
Related issues: roboticslab-uc3m/color-debug#6, roboticslab-uc3m/color-debug#8. Header-only repos + include paths propagated via
Related issues: roboticslab-uc3m/questions-and-answers#44, asrob-uc3m/robotDevastation#122. |
Regarding this setup, I've observed the following behavior on two different machines: no gtests -> no tests -> sudo apt install libgtest-dev -> only way to enable tests is by completely erasing the cmake cache. |
I noticed that both |
@jgvictores fixed at roboticslab-uc3m/kinematics-dynamics@6063082, I'll update the |
roboticslab-uc3m/kinematics-dynamics@6063082 works! Thank you so much!! |
Regarding CMake common practices, I'd draw a few conclusions from roboticslab-uc3m/questions-and-answers#44 and sum them up in the manual. |
Thanks to https://travis-ci.org/roboticslab-uc3m/amor-main/jobs/356811908 I found out that superbuild-like repos should never enforce a lower YCM version (via Remember that pulling specific releases of YCM originated at roboticslab-uc3m/questions-and-answers#24. Anyway, I'd recommend pulling YCM master for superbuild-like repos. If any upstream problem arises, we'll learn of that thanks to daily CI cron jobs. We only have to make sure that current YCM is compatible with the version number set in |
Another CMake guideline proposal: set |
In spite of that, we can assume that most repos just need YARP for whatever purpose they exist for. When working on roboticslab-uc3m/questions-and-answers#45, we observed that certain components inside |
We haven't spoken about CPack, yet, but here is another guideline for superbuild repos. Per asrob-uc3m/robotDevastation@cfac906, this: install(DIRECTORY ${CMAKE_BINARY_DIR}/install/
DESTINATION "./") is better than this: install(DIRECTORY ${CMAKE_BINARY_DIR}/install/
DESTINATION ${CMAKE_INSTALL_PREFIX}) |
Regarding CMake options, care must be taken to ensure that:
That is, path traversal matters. It's advisable to respect the following sequence at root CMakeLists.txt: add_subdirectory(cmake) # regular, find- and ycm-modules
add_subdirectory(libraries) # libraries to link against
add_subdirectory(programs) # apps that may depend on one or more libraries
add_subdirectory(tests) # should come last, but...
add_subdirectory(share) # ...we may define dirs here that depend on ENABLE_tests, too In relation to this last line, see roboticslab-uc3m/yarp-devices/share/CMakeLists.txt. |
Another one: in Travis, set
Tracing this variable back to YCM sources:
In fact, we ended up copying around the following lines in several #-- Configure Git (needed by YCM)
- if [ ! `git config --get user.email` ]; then `git config --global user.email '[email protected]'`; fi
- if [ ! `git config --get user.name` ]; then `git config --global user.name 'Travis CI'`; fi Edit: (from robotology/ycm/cmake-next/proposed/ExternalProject.cmake, that is, set # If ``SCM_DISCONNECTED`` is set, the update step is not executed
# automatically when building the main target. The update step can still
# be added as a step target and called manually. This is useful if you
# want to allow to build the project when you are disconnected from the
# network (you might still need the network for the download step).
# This is disabled by default.
# The directory property ``EP_SCM_DISCONNECTED`` can be used to change
# the default value for all the external projects in the current
# directory and its subdirectories. Edit2: see robotology/ycm-cmake-modules#225. |
I reverted roboticslab-uc3m/kinematics-dynamics@ebfe41e. Not sure why it worked without |
As spoken with @jgvictores, we strive to keep all declarations as close as possible to the Also, there are cases in which CMake variables defined by Config.cmake or Find.cmake must be accessible in distinct folders. In order to avoid redundant Edit: scope vs order in which options are declared/defined. The former improves readability (one may expect that options defined in subfolders are not visible to sibling folders), but the latter is what actually happens (variables are stored in a global cache). Edit 2: roboticslab-uc3m/yarp-devices#183 arised because of non-centralized CMake options (and my lack of care, of course!). |
Old style: # root/components/
if(ENABLE_mycomponent)
add_subdirectory(mycomponent)
endif() New preferred style: # root/components/
add_subdirectory(mycomponent) and # root/components/mycomponent/
if(ENABLE_mycomponent)
# do something
endif() |
Most of the YCM hacks laid out in previous comments (and mainly #18 (comment)) are now outdated due to the outcome of roboticslab-uc3m/questions-and-answers#55: in non-superbuild repos, YCM should be consumed as a hard dependency, thus treating all dynamically pulled repos as hard deps, too (e.g. color-debug). |
Just found CGold: The Hitchhiker’s Guide to the CMake, still WIP. @jgvictores you might consider adding this to asrob-uc3m/tutoriales. |
I'm going to fiddle with this again on superbuild repos at roboticslab-uc3m/questions-and-answers#78. We don't care about that anymore on non-superbuild projects s since YCM is regarded as a hard dependency by them (roboticslab-uc3m/questions-and-answers#66). |
Gather a few examples of usual pitfalls, recommended usage and hints like roboticslab-uc3m/questions-and-answers#39 (comment).
Also: roboticslab-uc3m/questions-and-answers#18
The text was updated successfully, but these errors were encountered: