roboticslab-uc3m / questions-and-answers

A place for general debate and question&answer
https://robots.uc3m.es/developer-manual/appendix/repository-index.html
2 stars 0 forks source link

Add C++ examples to main build #92

Closed PeterBowman closed 3 years ago

PeterBowman commented 3 years ago

Our vision repo has recently gained Python bindings (ref) that get compiled along with everything else, that is, they generate targets in the main build (although disabled by default). So far we excluded them from make -C build/ all and forced users to generate a bindings/build/ directory, configure CMake and build separately (of course, the main project ought to be compiled and installed at this point).

Why not applying this very same logic to C++ examples? I have given it a try at https://github.com/roboticslab-uc3m/vision/commit/182286aeae2f96501954754b243ad5cb11f7622f with the following changes:

I found it works out-of-the-box for whole-project builds with ENABLE_examples=ON as well as builds that target nothing more than one specific example.

Drawbacks, i.e. what we actually have to or should alter in the CMakeLists.txt for each example:

PeterBowman commented 3 years ago

This can be easily solved with a QUIET option, ideally set from parent list file:

I got this solved with a similar solution already adopted for find_package(ROBOTICSLAB_VISION REQUIRED): https://github.com/roboticslab-uc3m/vision/commit/70af1cdef64643e008b108a385e08403d10793f5. It is important to remark, that...

...this is mandatory since calling find_package on the project currently being built causes trouble:

if(NOT TARGET ROBOTICSLAB::YarpCloudUtils)
    find_package(ROBOTICSLAB_VISION REQUIRED)
endif()

....and this is not, but prevents from issuing duplicate find_package calls, and therefore extra CMake log:

if(NOT YARP_FOUND)
    find_package(YARP 3.3 REQUIRED COMPONENTS os sig)
endif()

Regarding the main build, all dependencies (including 3rd-party ones) should be appended (as usual) to the "Soft dependencies" list at root CMakeLists.txt, for clarity and uniformity. Then, more conditionals could be added to examples/cpp/CMakeLists.txt:

option(ENABLE_examples "Enable/disable C++ examples" OFF)

if(ENABLE_examples)
    if(TARGET ROBOTICSLAB::VisionInterfaces)
        add_subdirectory(exampleColorRegionDetector)
        add_subdirectory(exampleHaarDetector)
    endif()

    if(TARGET ROBOTICSLAB::YarpCloudUtils)
        add_subdirectory(exampleMeshFromCloud)
        add_subdirectory(exampleMeshFromLiveRGBD)
        add_subdirectory(exampleProcessCloud)
        add_subdirectory(exampleSceneReconstructionClient)
    endif()

    add_subdirectory(exampleRemoteGrabber)
    add_subdirectory(exampleRemoteRGBDSensor)
endif()

I mean: adding a if(MY_DEPENDENCY_FOUND) clause around the offending example subdirectory.

PeterBowman commented 3 years ago

Proposal: whenever a library target alias is created, use it (https://github.com/roboticslab-uc3m/vision/commit/247265b7e6c0620833d51b328325f98da788b3b1). For consistency, I'd create such aliases for library created by the project, even if those are not linked by any internal component (example, unit test or application).

PeterBowman commented 3 years ago

Once added to the main build, should C++ be enabled by default (currently OFF)?

jgvictores commented 3 years ago

Once added to the main build, should C++ be enabled by default (currently OFF)?

I'd say yep. ^^

jgvictores commented 3 years ago

Once added to the main build, should C++ be enabled by default (currently OFF)?

But would be okay with nay.

PeterBowman commented 3 years ago

I believe we can stay with default OFF for now. TODO:

jgvictores commented 3 years ago

Yay! @PeterBowman thanks!!