search

ros / roscpp_core

ros distribution sandbox
89 stars 116 forks source link

Compile issues on Windows (RoboStack) #145

Closed Tobias-Fischer closed 9 months ago

Tobias-Fischer commented 9 months ago

Hi @peci1,

We are running into some build problems with the 0.7.3 version of rostime:

[3/4] Building CXX object CMakeFiles\rostime.dir\src\time.cpp.obj
FAILED: CMakeFiles/rostime.dir/src/time.cpp.obj 
C:\PROGRA~2\MICROS~2\2019\ENTERP~1\VC\Tools\MSVC\1429~1.301\bin\Hostx64\x64\cl.exe  /nologo /TP -DBOOST_CHRONO_DYN_LINK -DBOOST_CHRONO_NO_LIB -DBOOST_DATE_TIME_DYN_LINK -DBOOST_DATE_TIME_NO_LIB -DBOOST_SYSTEM_DYN_LINK -DBOOST_SYSTEM_NO_LIB -DBOOST_THREAD_DYN_LINK -DBOOST_THREAD_NO_LIB -DNOMINMAX -DROS_BUILD_SHARED_LIBS=1 -DWIN32_LEAN_AND_MEAN -D_USE_MATH_DEFINES -Drostime_EXPORTS -I%SRC_DIR%\ros-noetic-rostime\src\work\include -external:I%PREFIX%\Library\include -external:W0 /DWIN32 /D_WINDOWS /W3 /GR /EHsc /MD /O2 /Ob2 /DNDEBUG   /D _VARIADIC_MAX=10 /Zc:__cplusplus /showIncludes /FoCMakeFiles\rostime.dir\src\time.cpp.obj /FdCMakeFiles\rostime.dir\ /FS -c %SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(86): error C2766: explicit specialization; 'MAX' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros\duration.h(139): note: see previous definition of 'public: static ros::Duration const ros::DurationBase<ros::Duration>::MAX'

Full log: https://github.com/RoboStack/ros-noetic/actions/runs/7732212508/job/21081638956

It seems related to https://github.com/ros/roscpp_core/commit/94adcfbddcd02dbd8b1780dc31313759938d9e9d

Do you have an idea how to go about it?

Thanks, Tobi /cc @traversaro

peci1 commented 9 months ago

Hmm, I guess this patch was never intended to be run on Windows. IIRC winsock.h or something similar defines max macros... Not sure how to go about that. Undef them first? I might have seen something similar in gencpp or dynamic_reconfigure.

Or enclose the MAX in parentheses?

Do you have an easy way to try things out?

traversaro commented 9 months ago

I do not think this is related to the usual collision with MAX/MIN macros in winsock.h/windows.h .

If you see the complete error, it applies to all static attributes of the templated classes:

[3/4] Building CXX object CMakeFiles\rostime.dir\src\time.cpp.obj
FAILED: CMakeFiles/rostime.dir/src/time.cpp.obj 
C:\PROGRA~2\MICROS~2\2019\ENTERP~1\VC\Tools\MSVC\1429~1.301\bin\Hostx64\x64\cl.exe  /nologo /TP -DBOOST_CHRONO_DYN_LINK -DBOOST_CHRONO_NO_LIB -DBOOST_DATE_TIME_DYN_LINK -DBOOST_DATE_TIME_NO_LIB -DBOOST_SYSTEM_DYN_LINK -DBOOST_SYSTEM_NO_LIB -DBOOST_THREAD_DYN_LINK -DBOOST_THREAD_NO_LIB -DNOMINMAX -DROS_BUILD_SHARED_LIBS=1 -DWIN32_LEAN_AND_MEAN -D_USE_MATH_DEFINES -Drostime_EXPORTS -I%SRC_DIR%\ros-noetic-rostime\src\work\include -external:I%PREFIX%\Library\include -external:W0 /DWIN32 /D_WINDOWS /W3 /GR /EHsc /MD /O2 /Ob2 /DNDEBUG   /D _VARIADIC_MAX=10 /Zc:__cplusplus /showIncludes /FoCMakeFiles\rostime.dir\src\time.cpp.obj /FdCMakeFiles\rostime.dir\ /FS -c %SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(86): error C2766: explicit specialization; 'MAX' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros\duration.h(139): note: see previous definition of 'public: static ros::Duration const ros::DurationBase<ros::Duration>::MAX'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(87): error C2766: explicit specialization; 'MIN' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros\duration.h(140): note: see previous definition of 'public: static ros::Duration const ros::DurationBase<ros::Duration>::MIN'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(88): error C2766: explicit specialization; 'ZERO' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros\duration.h(141): note: see previous definition of 'public: static ros::Duration const ros::DurationBase<ros::Duration>::ZERO'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(89): error C2766: explicit specialization; 'NANOSECOND' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros\duration.h(148): note: see previous definition of 'public: static ros::Duration const ros::DurationBase<ros::Duration>::NANOSECOND'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(90): error C2766: explicit specialization; 'MICROSECOND' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros\duration.h(147): note: see previous definition of 'public: static ros::Duration const ros::DurationBase<ros::Duration>::MICROSECOND'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(91): error C2766: explicit specialization; 'MILLISECOND' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros\duration.h(146): note: see previous definition of 'public: static ros::Duration const ros::DurationBase<ros::Duration>::MILLISECOND'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(92): error C2766: explicit specialization; 'SECOND' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros\duration.h(145): note: see previous definition of 'public: static ros::Duration const ros::DurationBase<ros::Duration>::SECOND'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(93): error C2766: explicit specialization; 'MINUTE' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros\duration.h(144): note: see previous definition of 'public: static ros::Duration const ros::DurationBase<ros::Duration>::MINUTE'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(94): error C2766: explicit specialization; 'HOUR' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros\duration.h(143): note: see previous definition of 'public: static ros::Duration const ros::DurationBase<ros::Duration>::HOUR'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(95): error C2766: explicit specialization; 'DAY' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros\duration.h(142): note: see previous definition of 'public: static ros::Duration const ros::DurationBase<ros::Duration>::DAY'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(97): error C2766: explicit specialization; 'MAX' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros\duration.h(175): note: see previous definition of 'public: static ros::WallDuration const ros::DurationBase<ros::WallDuration>::MAX'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(98): error C2766: explicit specialization; 'MIN' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros\duration.h(176): note: see previous definition of 'public: static ros::WallDuration const ros::DurationBase<ros::WallDuration>::MIN'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(99): error C2766: explicit specialization; 'ZERO' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros\duration.h(177): note: see previous definition of 'public: static ros::WallDuration const ros::DurationBase<ros::WallDuration>::ZERO'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(100): error C2766: explicit specialization; 'DAY' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros\duration.h(178): note: see previous definition of 'public: static ros::WallDuration const ros::DurationBase<ros::WallDuration>::DAY'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(101): error C2766: explicit specialization; 'HOUR' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros\duration.h(179): note: see previous definition of 'public: static ros::WallDuration const ros::DurationBase<ros::WallDuration>::HOUR'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(102): error C2766: explicit specialization; 'MINUTE' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros\duration.h(180): note: see previous definition of 'public: static ros::WallDuration const ros::DurationBase<ros::WallDuration>::MINUTE'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(103): error C2766: explicit specialization; 'SECOND' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros\duration.h(181): note: see previous definition of 'public: static ros::WallDuration const ros::DurationBase<ros::WallDuration>::SECOND'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(104): error C2766: explicit specialization; 'MILLISECOND' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros\duration.h(182): note: see previous definition of 'public: static ros::WallDuration const ros::DurationBase<ros::WallDuration>::MILLISECOND'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(105): error C2766: explicit specialization; 'MICROSECOND' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros\duration.h(183): note: see previous definition of 'public: static ros::WallDuration const ros::DurationBase<ros::WallDuration>::MICROSECOND'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(106): error C2766: explicit specialization; 'NANOSECOND' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros\duration.h(184): note: see previous definition of 'public: static ros::WallDuration const ros::DurationBase<ros::WallDuration>::NANOSECOND'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(111): error C2766: explicit specialization; 'MAX' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros/time.h(224): note: see previous definition of 'public: static ros::Time const ros::TimeBase<ros::Time,ros::Duration>::MAX'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(112): error C2766: explicit specialization; 'MIN' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros/time.h(225): note: see previous definition of 'public: static ros::Time const ros::TimeBase<ros::Time,ros::Duration>::MIN'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(113): error C2766: explicit specialization; 'ZERO' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros/time.h(226): note: see previous definition of 'public: static ros::Time const ros::TimeBase<ros::Time,ros::Duration>::ZERO'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(114): error C2766: explicit specialization; 'UNINITIALIZED' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros/time.h(227): note: see previous definition of 'public: static ros::Time const ros::TimeBase<ros::Time,ros::Duration>::UNINITIALIZED'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(116): error C2766: explicit specialization; 'MAX' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros/time.h(261): note: see previous definition of 'public: static ros::WallTime const ros::TimeBase<ros::WallTime,ros::WallDuration>::MAX'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(117): error C2766: explicit specialization; 'MIN' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros/time.h(262): note: see previous definition of 'public: static ros::WallTime const ros::TimeBase<ros::WallTime,ros::WallDuration>::MIN'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(118): error C2766: explicit specialization; 'ZERO' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros/time.h(263): note: see previous definition of 'public: static ros::WallTime const ros::TimeBase<ros::WallTime,ros::WallDuration>::ZERO'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(119): error C2766: explicit specialization; 'UNINITIALIZED' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros/time.h(264): note: see previous definition of 'public: static ros::WallTime const ros::TimeBase<ros::WallTime,ros::WallDuration>::UNINITIALIZED'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(121): error C2766: explicit specialization; 'MAX' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros/time.h(300): note: see previous definition of 'public: static ros::SteadyTime const ros::TimeBase<ros::SteadyTime,ros::WallDuration>::MAX'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(122): error C2766: explicit specialization; 'MIN' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros/time.h(301): note: see previous definition of 'public: static ros::SteadyTime const ros::TimeBase<ros::SteadyTime,ros::WallDuration>::MIN'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(123): error C2766: explicit specialization; 'ZERO' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros/time.h(302): note: see previous definition of 'public: static ros::SteadyTime const ros::TimeBase<ros::SteadyTime,ros::WallDuration>::ZERO'
%SRC_DIR%\ros-noetic-rostime\src\work\src\time.cpp(124): error C2766: explicit specialization; 'UNINITIALIZED' has already been defined
%SRC_DIR%\ros-noetic-rostime\src\work\include\ros/time.h(303): note: see previous definition of 'public: static ros::SteadyTime const ros::TimeBase<ros::SteadyTime,ros::WallDuration>::UNINITIALIZED'
ninja: build stopped: subcommand failed.
traversaro commented 9 months ago

Do you have an easy way to try things out?

On Windows:

mamba create -n rostimedev -c conda-forge -c robostack-staging cmake vs2022_win-64 pkg-config ninja pip boost-cpp ros-noetic-catkin ros-noetic-cpp-common
mamba activate rostimedev 
git clone https://github.com/ros/roscpp_core
cd .\roscpp_core\rostime
mkdir build
cd build
cmake -GNinja ..
ninja

change vs2022_win-64 to vs2019_win-64 if you are using Visual Studio 2019.

peci1 commented 9 months ago

I'm sorry, but I'm clueless here. I've spent more than an hour trying to figure it out... I'd say Windows doesn't deserve more...

A quick hack would be to ifdef these constants to be only in non-Windows builds. But that could cause problems downstream if somebody started using the constants...

When compiling the tests, there are also these errors about dllimport:

error C2491: ros::DurationBase::MAX: definition of dllimport static data member not allowed

I'm more and more skeptical to the whole Conda ROS on Windows thing. ROS 1 was never designed to run on Windows. Trying to do that without virtualizing Linux is just asking for a lot of work with unsure benefit...

peci1 commented 9 months ago

Here's a godbolt of the minimum code that causes the error. Surprisingly, in C++20, it compiles, while in C++17 it does not: https://godbolt.org/z/Pq3z4K3K6 .

traversaro commented 9 months ago

Here's a godbolt of the minimum code that causes the error. Surprisingly, in C++20, it compiles, while in C++17 it does not: https://godbolt.org/z/Pq3z4K3K6 .

Thanks for the suggestion! Unfortunately I tried to do this on rostime itself, and the compilation works fine, but then the test compilation fails with:

[1/2] Building CXX object CMakeFiles\rostime-test_duration.dir\test\duration.cpp.obj
FAILED: CMakeFiles/rostime-test_duration.dir/test/duration.cpp.obj
C:\PROGRA~1\MIB055~1\2022\PROFES~1\VC\Tools\MSVC\1438~1.331\bin\Hostx64\x64\cl.exe  /nologo /TP -DBOOST_CHRONO_DYN_LINK -DBOOST_CHRONO_NO_LIB -DBOOST_DATE_TIME_DYN_LINK -DBOOST_DATE_TIME_NO_LIB -DBOOST_SYSTEM_DYN_LINK -DBOOST_SYSTEM_NO_LIB -DBOOST_THREAD_DYN_LINK -DBOOST_THREAD_NO_LIB -DGTEST_LINKED_AS_SHARED_LIBRARY=1 -DNOMINMAX -DROS_BUILD_SHARED_LIBS=1 -DWIN32_LEAN_AND_MEAN -D_USE_MATH_DEFINES -IC:\src\roscpp_core\rostime\include -external:IC:\Users\straversaro\AppData\Local\miniforge3\envs\rostimedev\Library\include -external:W0 /DWIN32 /D_WINDOWS /W3 /GR /EHsc /MDd /Zi /Ob0 /Od /RTC1 -std:c++20   /D _VARIADIC_MAX=10 /Zc:__cplusplus /showIncludes /FoCMakeFiles\rostime-test_duration.dir\test\duration.cpp.obj /FdCMakeFiles\rostime-test_duration.dir\ /FS -c C:\src\roscpp_core\rostime\test\duration.cpp
C:\src\roscpp_core\rostime\include\ros/time.h(224): error C2491: 'ros::TimeBase<ros::Time,ros::Duration>::MAX': definition of dllimport static data member not allowed
C:\src\roscpp_core\rostime\include\ros/time.h(225): error C2491: 'ros::TimeBase<ros::Time,ros::Duration>::MIN': definition of dllimport static data member not allowed
C:\src\roscpp_core\rostime\include\ros/time.h(226): error C2491: 'ros::TimeBase<ros::Time,ros::Duration>::ZERO': definition of dllimport static data member not allowed
C:\src\roscpp_core\rostime\include\ros/time.h(227): error C2491: 'ros::TimeBase<ros::Time,ros::Duration>::UNINITIALIZED': definition of dllimport static data member not allowed
C:\src\roscpp_core\rostime\include\ros/time.h(261): error C2491: 'ros::TimeBase<ros::WallTime,ros::WallDuration>::MAX': definition of dllimport static data member not allowed
C:\src\roscpp_core\rostime\include\ros/time.h(262): error C2491: 'ros::TimeBase<ros::WallTime,ros::WallDuration>::MIN': definition of dllimport static data member not allowed
C:\src\roscpp_core\rostime\include\ros/time.h(263): error C2491: 'ros::TimeBase<ros::WallTime,ros::WallDuration>::ZERO': definition of dllimport static data member not allowed
C:\src\roscpp_core\rostime\include\ros/time.h(264): error C2491: 'ros::TimeBase<ros::WallTime,ros::WallDuration>::UNINITIALIZED': definition of dllimport static data member not allowed
C:\src\roscpp_core\rostime\include\ros/time.h(300): error C2491: 'ros::TimeBase<ros::SteadyTime,ros::WallDuration>::MAX': definition of dllimport static data member not allowed
C:\src\roscpp_core\rostime\include\ros/time.h(301): error C2491: 'ros::TimeBase<ros::SteadyTime,ros::WallDuration>::MIN': definition of dllimport static data member not allowed
C:\src\roscpp_core\rostime\include\ros/time.h(302): error C2491: 'ros::TimeBase<ros::SteadyTime,ros::WallDuration>::ZERO': definition of dllimport static data member not allowed
C:\src\roscpp_core\rostime\include\ros/time.h(303): error C2491: 'ros::TimeBase<ros::SteadyTime,ros::WallDuration>::UNINITIALIZED': definition of dllimport static data member not allowed
C:\src\roscpp_core\rostime\test\duration.cpp(91): warning C4305: 'argument': truncation from '__int64' to 'double'
C:\src\roscpp_core\rostime\test\duration.cpp(92): warning C4305: 'argument': truncation from '__int64' to 'double'
ninja: build stopped: subcommand failed.
traversaro commented 9 months ago

A quick hack would be to ifdef these constants to be only in non-Windows builds. But that could cause problems downstream if somebody started using the constants...

I can't think of other solutions, and I guess that it is better to have this rather then not have fresh ROS Noetic builds, if there are indeed downstream uses of this (assuming that are few) we can just patch them as well to use directly the required constants instead.

traversaro commented 9 months ago

A quick hack would be to ifdef these constants to be only in non-Windows builds. But that could cause problems downstream if somebody started using the constants...

I can't think of other solutions, and I guess that it is better to have this rather then not have fresh ROS Noetic builds, if there are indeed downstream uses of this (assuming that are few) we can just patch them as well to use directly the required constants instead.

@Tobias-Fischer if you agree, the patch for doing this is https://github.com/ros/roscpp_core/commit/3421689c9adf746dcf51a0d8ef8cdaa60fd4b112 .

Tobias-Fischer commented 9 months ago

Seems like this did the job - thanks @traversaro!