ROS build farm are broken for develop branch

[jlblancoc]

Description

See: https://build.ros2.org/job/Jdev__gtsam__ubuntu_noble_amd64/136/

Why?

Reason: the use of embedded Eigen version by default leads to not using the SYSTEM flag for those includes, so any warning there is treated as an error.

Solutions

To modify CMake scripts so..

1. Use Eigen from the system version by default,
2. Like (1), but only when it's detected we are building for the ROS build farm (kida hack, but have been there...)
3. Add the SYSTEM flag to embedded version of Eigen (if not done already, right? @Gold856 )

What do you think, @dellaert ?

I would like (1), but I know you like backwards compatible behavior, so the second choice would probably be (3), and if it's already done but is not working for some cmake limitation (cannot use SYSTEM for non-system directories? never checked that..), then use (2).

[dellaert]

You are correctly modeling my view: (3) if possible, (2) if not :-)

[jlblancoc]

SYSTEM was already there:
https://github.com/borglab/gtsam/blob/develop/cmake/HandleEigen.cmake#L45

so it will have to be (2)...

[Gold856]

What if we instead use a pragma in the problematic file to suppress that specific warning? That's probably a much cleaner solution and much more fine grained than using a pragma in an Eigen header and having it affect all downstream consumers.
Alternatively, if this is a big enough problem, we could put the pragma in DisableStupidWarnings.h like originally planned.

[jlblancoc]

Mmm... Do you mean the embedded Eigen3's DisableStupidWarnings.h file? If you agree with this, it would be another workaround.
It would imply remembering about this when/if someday the embedded version is upgraded.

Do you agree with this change, Frank @dellaert ?

[Gold856]

Yes, that's the header I was referencing. Although I still think using pragmas in the affected gtsam files is a better method, mostly because of the granularity of control we get and the fact that it means we don't need to mess with Eigen. Also, it's likely that system Eigen packages would also be affected, so this solution would ensure that the warnings don't trigger on system Eigen packages as well.

[jlblancoc]

I agree with you, @Gold856 .

If you have time for a small PR , please tag me so I can check it. If not I'll add it to my list...
Log with the error: https://build.ros2.org/job/Jdev__gtsam__ubuntu_noble_amd64/136/console

Key part: -Werror=maybe-uninitialized

In file included from /tmp/ws/src/gtsam/gtsam/3rdparty/Eigen/Eigen/Core:294,
03:39:15                  from /tmp/ws/src/gtsam/gtsam/3rdparty/Eigen/Eigen/Dense:1,
03:39:15                  from /tmp/ws/src/gtsam/gtsam/base/OptionalJacobian.h:24,
03:39:15                  from /tmp/ws/src/gtsam/gtsam/base/Matrix.h:27,
03:39:15                  from /tmp/ws/src/gtsam/gtsam/geometry/FundamentalMatrix.cpp:8:
03:39:15 In copy constructor ‘Eigen::PlainObjectBase<Derived>::PlainObjectBase(const Eigen::PlainObjectBase<Derived>&) [with Derived = Eigen::Matrix<double, 3, 1>]’,
03:39:15     inlined from ‘Eigen::Matrix<_Scalar, _Rows, _Cols, _Options, _MaxRows, _MaxCols>::Matrix(const Eigen::Matrix<_Scalar, _Rows, _Cols, _Options, _MaxRows, _MaxCols>&) [with _Scalar = double; int _Rows = 3; int _Cols = 1; int _Options = 0; int _MaxRows = 3; int _MaxCols = 1]’ at /tmp/ws/src/gtsam/gtsam/3rdparty/Eigen/Eigen/src/Core/Matrix.h:414:65,
03:39:15     inlined from ‘gtsam::FundamentalMatrix::FundamentalMatrix(const gtsam::Matrix3&)’ at /tmp/ws/src/gtsam/gtsam/geometry/FundamentalMatrix.cpp:43:47:
03:39:15 /tmp/ws/src/gtsam/gtsam/3rdparty/Eigen/Eigen/src/Core/PlainObjectBase.h:512:17: error: ‘*(double*)((char*)&svd + offsetof(Eigen::JacobiSVD<Eigen::Matrix<double, 3, 3, 0, 3, 3>, 2>,Eigen::JacobiSVD<Eigen::Matrix<double, 3, 3, 0, 3, 3>, 2>::<unnamed>.Eigen::SVDBase<Eigen::JacobiSVD<Eigen::Matrix<double, 3, 3, 0, 3, 3>, 2> >::m_singularValues.Eigen::Matrix<double, 3, 1, 0, 3, 1>::<unnamed>.Eigen::PlainObjectBase<Eigen::Matrix<double, 3, 1, 0, 3, 1> >::m_storage.Eigen::DenseStorage<double, 3, 3, 1, 0>::m_data.Eigen::internal::plain_array<double, 3, 0, 0>::array[2]))’ may be used uninitialized [-Werror=maybe-uninitialized]
03:39:15   512 |       : Base(), m_storage(other.m_storage) { }

PS: plus all other places including Eigen/Dense