search

robotology / whole-body-estimators

YARP devices that implement estimators for humanoid robots.
27 stars 12 forks source link

Switch to generate YARP idl files at every build to fix compatibility with YARP master #131

Closed traversaro closed 2 years ago

traversaro commented 2 years ago

While working on https://github.com/robotology/robotology-superbuild/issues/900, I had problems with the idl generated files, as the one generated with YARP 3.5 (I guess) had problems to be compiled with YARP master. To avoid the problem at the root, I switched to generate the idl files at each build (instead of committing them manually) and I removed the autogenerated files that at the moment were committed in the repo. This was done by just generating the idl files in the build directory instead of the source directory.

traversaro commented 2 years ago

Example of failure with latest YARP master without this change (from https://github.com/robotology/robotology-superbuild/runs/4297622640):

2021-11-23T10:22:43.2450517Z [210/248] Performing build step for 'whole-body-estimators'
2021-11-23T10:22:43.2452638Z FAILED: src/whole-body-estimators/CMakeFiles/YCMStamp/whole-body-estimators-build /__w/robotology-superbuild/robotology-superbuild/build/src/whole-body-estimators/CMakeFiles/YCMStamp/whole-body-estimators-build 
2021-11-23T10:22:43.2455777Z cd /__w/robotology-superbuild/robotology-superbuild/build/src/whole-body-estimators && /usr/bin/cmake --build . && /usr/bin/cmake -E touch /__w/robotology-superbuild/robotology-superbuild/build/src/whole-body-estimators/CMakeFiles/YCMStamp/whole-body-estimators-build
2021-11-23T10:22:43.2458229Z [1/37] Building CXX object idl/wholeBodyDynamicsSettings/CMakeFiles/wholeBodyDynamicsSettings.dir/autogenerated/src/KinematicSourceType.cpp.o
2021-11-23T10:22:43.2460315Z FAILED: idl/wholeBodyDynamicsSettings/CMakeFiles/wholeBodyDynamicsSettings.dir/autogenerated/src/KinematicSourceType.cpp.o 
2021-11-23T10:22:43.2467020Z /usr/bin/c++ -D_USE_MATH_DEFINES -I/__w/robotology-superbuild/robotology-superbuild/src/whole-body-estimators/idl/wholeBodyDynamicsSettings/autogenerated/include -isystem /__w/robotology-superbuild/robotology-superbuild/build/install/include -g -fPIC -std=gnu++1z -MD -MT idl/wholeBodyDynamicsSettings/CMakeFiles/wholeBodyDynamicsSettings.dir/autogenerated/src/KinematicSourceType.cpp.o -MF idl/wholeBodyDynamicsSettings/CMakeFiles/wholeBodyDynamicsSettings.dir/autogenerated/src/KinematicSourceType.cpp.o.d -o idl/wholeBodyDynamicsSettings/CMakeFiles/wholeBodyDynamicsSettings.dir/autogenerated/src/KinematicSourceType.cpp.o -c /__w/robotology-superbuild/robotology-superbuild/src/whole-body-estimators/idl/wholeBodyDynamicsSettings/autogenerated/src/KinematicSourceType.cpp
2021-11-23T10:22:43.2473066Z In file included from /__w/robotology-superbuild/robotology-superbuild/src/whole-body-estimators/idl/wholeBodyDynamicsSettings/autogenerated/src/KinematicSourceType.cpp:13:0:
2021-11-23T10:22:43.2475629Z /__w/robotology-superbuild/robotology-superbuild/src/whole-body-estimators/idl/wholeBodyDynamicsSettings/autogenerated/include/KinematicSourceType.h:25:1: error: expected class-name before '{' token
2021-11-23T10:22:43.2476969Z  {
2021-11-23T10:22:43.2477250Z  ^
2021-11-23T10:22:43.2479534Z /__w/robotology-superbuild/robotology-superbuild/src/whole-body-estimators/idl/wholeBodyDynamicsSettings/autogenerated/include/KinematicSourceType.h:27:9: error: 'int KinematicSourceTypeVocab::fromString(const string&)' marked 'override', but does not override
2021-11-23T10:22:43.2481322Z      int fromString(const std::string& input) override;
2021-11-23T10:22:43.2481780Z          ^~~~~~~~~~
2021-11-23T10:22:43.2483985Z /__w/robotology-superbuild/robotology-superbuild/src/whole-body-estimators/idl/wholeBodyDynamicsSettings/autogenerated/include/KinematicSourceType.h:28:17: error: 'std::__cxx11::string KinematicSourceTypeVocab::toString(int) const' marked 'override', but does not override
2021-11-23T10:22:43.2485761Z      std::string toString(int input) const override;
2021-11-23T10:22:43.2486207Z                  ^~~~~~~~
2021-11-23T10:22:43.2487222Z [2/37] Building CXX object idl/wholeBodyDynamicsSettings/CMakeFiles/wholeBodyDynamicsSettings.dir/autogenerated/src/Gravity.cpp.o
2021-11-23T10:22:43.2488719Z [3/37] Building CXX object libraries/ctrlLibRT/CMakeFiles/ctrlLibRT.dir/src/filters.cpp.o
HosameldinMohamed commented 2 years ago

To understand the issue, before, the auto-generated files are pulled from the remote repo. So CMake with the variable CMAKE_CURRENT_SOURCE_DIR would find them then doesn't generate them if they are the same.

The problem could appear when the compiled YARP version and the YARP version in which the auto-generated files are different.

So the solution is to add the auto-generated files in the build directory to avoid such mismatch?

I guess it might happen also if the user compiles with one YARP version and then switches YARP version and tries to compile again? but at least it won't happen out-of-the-box!

I know I just rephrased what you said above! I am just making sure I understood it correctly!

traversaro commented 2 years ago

I guess it might happen also if the user compiles with one YARP version and then switches YARP version and tries to compile again? but at least it won't happen out-of-the-box!

Yes, that is correct. However, if you switch some version of the dependencies and you do not cleanup the build, problems are kind of expected, so this should not be a big problem.

I know I just rephrased what you said above! I am just making sure I understood it correctly!

Yes, this is a good idea. I think your recap is correct.

traversaro commented 2 years ago

Thanks! I'll update the CHANGELOG with the fix and then merge it!

Good point, I forgot it.

traversaro commented 2 years ago

@HosameldinMohamed if it is ok for you, we can merge to unblock https://github.com/robotology/robotology-superbuild/pull/926, thanks!

HosameldinMohamed commented 2 years ago

Sorry I waited few minutes for the CI after committing then I forgot to merge it!

HosameldinMohamed commented 2 years ago

@HosameldinMohamed if it is ok for you, we can merge to unblock robotology/robotology-superbuild#926, thanks!

you mean robotology/robotology-superbuild#900?

traversaro commented 2 years ago

Sorry I waited few minutes for the CI after committing then I forgot to merge it!

Ah ok, no problem!

traversaro commented 2 years ago

@HosameldinMohamed if it is ok for you, we can merge to unblock robotology/robotology-superbuild#926, thanks!

you mean robotology/robotology-superbuild#900?

https://github.com/robotology/robotology-superbuild/pull/926 is the PR that should fix https://github.com/robotology/robotology-superbuild/issues/900 .