MRPT / mrpt

:zap: The Mobile Robot Programming Toolkit (MRPT)
https://docs.mrpt.org/reference/latest/
BSD 3-Clause "New" or "Revised" License
1.89k stars 627 forks source link

mrpt fails to build on 32bit architectures with 64bit time_t type #1305

Closed doko42 closed 2 months ago

doko42 commented 3 months ago

seen with 2.11.10, the code didn't change in 2.11.12.

mrpt fails to build on 32bit architectures with 64bit time_t type, as seen with the current Debian/Ubuntu time_t 64 transitions:

In file included from /<>/python/all_mrpt_system.cpp:7: /<>/python/src/mrpt/system/datetime.cpp: In function ‘void bind_mrpt_systemdatetime(std::function<pybind11::module&(const std::__cxx11::basic_string&)>&)’: /<>/python/src/mrpt/system/datetime.cpp:60:96: error: address of overloaded function with no contextual type information 60 | M("mrpt::system").def("time_tToTimestamp", (mrpt::Clock::time_point (*)(const long &)) &mrpt::system::time_tToTimestamp, "Transform from standard \"time_t\" to TTimeStamp.\n \n\n timestampTotime_t\n\nC++: mrpt::system::time_tToTimestamp(const long &) --> mrpt::Clock::time_point", pybind11::arg("t")); | ^~~~~~~~

jlblancoc commented 3 months ago

Thanks @doko42 ! I just worked today in the new release v2.12.0 upstream (here) + the corresponding one on gbp that fix that bug.

doko42 commented 3 months ago

did you identify the relevant commit, that could be backported to 2.11.10?

jlblancoc commented 3 months ago

It was a relatively-large PR: https://github.com/MRPT/mrpt/pull/1304

If all you want is to get rid of that error with a d/patch, it could be just changing these two lines:

https://github.com/MRPT/mrpt/blob/2.11.10/python/src/mrpt/system/datetime.cpp#L60 https://github.com/MRPT/mrpt/blob/2.11.10/python/src/mrpt/system/crc.cpp#L88

replacing long with time_t. I think that should be all...

jlblancoc commented 2 months ago

It seems all builds are now OK for Debian & Ubuntu Noble, right? If not, please feel free of reopening.