-
Notifications
You must be signed in to change notification settings - Fork 116
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Compile issues on Windows (RoboStack) #145
Comments
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? |
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:
|
On Windows:
change |
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:
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... |
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
|
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 3421689 . |
Seems like this did the job - thanks @traversaro! |
Hi @peci1,
We are running into some build problems with the 0.7.3 version of
rostime
:Full log: https://github.com/RoboStack/ros-noetic/actions/runs/7732212508/job/21081638956
It seems related to 94adcfb
Do you have an idea how to go about it?
Thanks,
Tobi
/cc @traversaro
The text was updated successfully, but these errors were encountered: