robotology / yarp

YARP - Yet Another Robot Platform
http://www.yarp.it
Other
527 stars 195 forks source link

Don't build YARP_catch2 unless explicitly requesting tests #3050

Closed PeterBowman closed 1 year ago

PeterBowman commented 1 year ago

Is your feature request related to a problem? Please describe. If I'm correct, YARP_catch2 is a required component and there is no way to disable it AToW. It lives in extern/catch2/ and is directly referenced via CMake from only a bunch of files related to testing.

Describe the solution you'd like Don't build YARP_catch2 unless testing is explicitly requested in CMake configuration (via -DYARP_COMPILE_TESTS=ON).

Additional context As a result of introducing C++20 in https://github.com/robotology/yarp/pull/3039, the combo YARP master (at https://github.com/robotology/yarp/commit/b3ea6329d456447e327643e244274b7f65c984ee) + Ubuntu 22.04 + Clang 14.0.0 + C++20 leads to the following error:

Scanning dependencies of target YARP_catch2
[  0%] Building CXX object extern/catch2/CMakeFiles/YARP_catch2.dir/catch2/catch_amalgamated.cpp.o
In file included from /home/runner/work/kinematics-dynamics/kinematics-dynamics/.deps/yarp/extern/catch2/catch2/catch_amalgamated.cpp:15:
In file included from /home/runner/work/kinematics-dynamics/kinematics-dynamics/.deps/yarp/extern/catch2/catch2/catch_amalgamated.hpp:666:
/usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/chrono:2320:48: error: call to consteval function 'std::chrono::hh_mm_ss::_S_fractional_width' is not a constant expression
        static constexpr unsigned fractional_width = {_S_fractional_width()};
                                                      ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/chrono:2320:48: note: undefined function '_S_fractional_width' cannot be used in a constant expression
/usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/chrono:2275:2: note: declared here
        _S_fractional_width()
        ^
1 error generated.

Logs: clang.txt.

The root cause is not YARP itself, but dependency management on GitHub runners; see https://github.com/actions/runner-images/issues/8659. Nevertheless, I think this could be addressed on YARP's side if Catch2 should be skipped unless actually required.

randaz81 commented 1 year ago

Thank you @PeterBowman, I integrated your suggestion in 9cdf77e2b40dba9280dd20b707c4bae6f655f0b3

Unfortunately, it is not possible to notice any improvement in the CI, since also the builds which do not compile tests fail for similar reasons elsewhere (e..g in qt inclusions, see https://github.com/robotology/yarp/actions/runs/6903696174/job/18782969646?pr=3048, build number 8)

For this reason, I am migrating the CI to use clang17, obtained from LLVM instead of the official Ubuntu22.04 packages (which includes clang15). I think using an updated clang version will beneficial for different other reasons too.

randaz81 commented 1 year ago

done