-
Notifications
You must be signed in to change notification settings - Fork 0
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
Decouple external build configuration (Travis CI/YCM) from local settings #18
Comments
In order to alleviate configuration efforts (particularly with installation guides in mind), we could avoid passing several |
Another ridiculous example (scroll right): roboticslab-uc3m/kinematics-dynamics@d5a9599. |
I did some CMake refactorization work in a bunch of repos. Namely:
Setting all options to
|
@jgvictores suggests to keep all/most options enabled and warn the user if it cannot get built for whatever reason (missing package, unsatisfied dependencies, etc.). |
Yes, essentially all enabled, and automatically disabling (without breaking, just messages/warnings) if requirements are not met. |
Warnings may be annoying, but help to keep users aware of which components could not be successfully configured. Setting everything to Idea: try the FeatureSummary CMake module. It was adopted some time ago in amor-api, then in project-generator. Devs may use it to register all available CMake options and provide short descriptions. Then, CMake generates a report after the configuration step. Example output of amor-api:
Starting from The following features have been enabled, we see the result of invoking |
YARP devices already provide some sort of summarization mechanism, check this
|
Perhaps we should move most |
Advantage: Faster and cleaner output of cmake. Seeing the disadvantage... Maybe only in the case a dep is detected to be used on more than one component, then move up one level? I have no idea.... |
Idea: place yarp_prepare_plugin(AmorCartesianControl
CATEGORY device
TYPE roboticslab::AmorCartesianControl
INCLUDE AmorCartesianControl.hpp
DEFAULT ON
DEPENDS ENABLE_KinematicRepresentationLib)
if(NOT SKIP_AmorCartesianControl)
find_package(AMOR_API REQUIRED)
# ... Obviously, the |
YARP is assumed to be a hard dependency. * #138 * roboticslab-uc3m/questions-and-answers#18
Working example at roboticslab-uc3m/kinematics-dynamics@14cf9cb. Template: find_package(3rdparty_pkg QUIET)
if(NOT 3rdparty_pkg_FOUND AND (NOT DEFINED ENABLE_MyComponent OR ENABLE_MyComponent))
message(WARNING "3rdparty_pkg package not found, disabling MyComponent")
endif()
# same with yarp_prepare_plugin()
cmake_dependent_option(ENABLE_MyComponent "Enable/disable MyComponent" ON
3rdparty_pkg_FOUND OFF)
if(ENABLE_MyComponent) # 'NOT SKIP_MyComponent' for YARP devices
# ...
else()
set(ENABLE_MyComponent OFF CACHE BOOL "Enable/disable MyComponent" FORCE)
endif() Travis YAML files may be updated so that local repository options are now omitted (see Regarding non-local CMake options (as described in the first comment in this issue, i.e. remote repos pulled via |
Expanding on my previous comment, such refactorization is only intended for devices/programs/libraries that depend on |
Proposed roadmap (I'll replicate and expand this in the issue description if nobody opposes):
TODO: I need to check this ideas on a superbuild-like repo, especially regarding CMake options and |
Another TODO: if I place (I was thinking about certain YARP devices in openrave-yarp-plugins) |
I think I've found an issue with the current proposal. Looking at these lines, cmake always goes through |
More examples:
|
This mechanism (#18 (comment)) is not aware of 3rd-party deps being available upon successive configure runs. Example:
|
Example: roboticslab-uc3m/yarp-devices@2fdc802. |
I think it's already supported in custom YARP devices (ref, available since 2.3.68.1), that is, add a |
Another example (test two conditions): roboticslab-uc3m/vision@f5c89fb. |
Note: new YARP components may require additional attention when working with RL projects depending upon each other, see roboticslab-uc3m/kinematics-dynamics@273ac97 and #65. |
TODO: reflect this new dependency+warning mechanism in project-generator and repositories derived from this one (mainly tools and asibot-hmi). |
More ideas: robotology/yarp@758d23c (Plugins not enabled due to missing dependencies are now shown in |
Somewhat a follow-up to #48 motivated by roboticslab-uc3m/openrave-yarp-plugins#91: if we just need a header file (usually, a YARP device interface) or a CMake module (rare) from a dependency, then clone/download, run |
For future reference, this was done at #54. |
YARP v3.0.x suffers from this same problem, too, after robotology/yarp@758d23c was introduced. Looks like the final part of my proposed solution (#18 (comment)), which also resembles what YARP aims to achieve, is colliding with set(ENABLE_MyComponent OFF CACHE BOOL "Enable/disable MyComponent" FORCE) |
Note for travelers: if(${var} MATCHES "^${var}$") is a legacy way of saying: if(NOT DEFINED ${var}) Found in CMakeDependentOption.cmake. |
CMake 3.13 introduces policy CMP0077. Not sure if we want this new behavior, though. |
This problem has been approached in some valid ways that are close enough to the intended result. I'm going to close it at last. A few remarks regarding what was discussed here:
Therefore, the original issue was already addressed quite a while ago. We ended up spreading boilerplate as detailed in previous comments (e.g. roboticslab-uc3m/vision@f5c89fb). Some people choose to see the ugliness in this code. The disarray. I choose to see the practicality. To believe there is an order to our CMake list files, a purpose. |
CMake 3.19 introduced presets which mostly behave like a JSON-formatted initial cache. |
I've set up CI builds for asibot-hmi with this configuration; note that this project depends on kinematics-dynamics, hence an additional
git clone
step. To avoid building KDL, which is a dependency for kinematics-dynamics itself, I had to configure it appropriately:First remark: any options irrelevant to my project could've been set to
OFF
/FALSE
in kinematics-dynamics' rootCMakeLists.txt
file. Therefore, I'd just concentrate on the funcionalities I'm really interested in, not the other way around.Then, I configured periodic cron jobs targeted at discovering issues linked with the development of YARP. However, my latest build errored because of recent changes to kinematics-dynamics (CI logs), which happened to introduce a new dependency on KDL. This forced me to take a look upstream and update my
.travis.yml
accordingly: roboticslab-uc3m/asibot-hmi@9b1959d.As a second remark, one must always keep an eye on each upstream repo because of new dependencies or changes in CMake configuration. This and the previous remark could be easily extended to YCM-like repos (e.g. teo-main) for the same reasons.
Proposal:
OFF
(FALSE
)ccmake
,cmake-gui
) in install guides for ease of initial CMake configuration phaseCMakeLists.txt
files, which are easily subject to changes, to provide you the desired configurationRegarding the last point and YCM projects, the
CONFIGURE_COMMAND
parameter toycm_ep_helper
should come in handy (docs).The text was updated successfully, but these errors were encountered: