Change to force a rebuild of the pollset on flag changes#1393
Change to force a rebuild of the pollset on flag changes#1393mikepurvis merged 3 commits intoros:melodic-develfrom
Conversation
This reverts commit 5e2b38e. This change may be considered for backporting to Lunar at a later time.
| void add_socket_to_watcher(int epfd, int fd) { | ||
| #if defined(HAVE_EPOLL) | ||
| struct epoll_event ev; | ||
| bzero(&ev, sizeof(ev)); |
There was a problem hiding this comment.
Fixing valgrind warnings.
| void set_events_on_socket(int epfd, int fd, int events) { | ||
| #if defined(HAVE_EPOLL) | ||
| struct epoll_event ev; | ||
| bzero(&ev, sizeof(ev)); |
There was a problem hiding this comment.
Fixing valgrind warnings.
| catkin_add_gtest(${PROJECT_NAME}-test_version test_version.cpp) | ||
| if(TARGET ${PROJECT_NAME}-test_version) | ||
| target_link_libraries(${PROJECT_NAME}-test_version) | ||
| target_link_libraries(${PROJECT_NAME}-test_version ${catkin_LIBRARIES}) |
There was a problem hiding this comment.
I could not build and run the unit tests without the following changes.
There was a problem hiding this comment.
Did you end up just running the tests on Linux but with a build where HAVE_EPOLL was forced to 0?
There was a problem hiding this comment.
That's correct yes.
There was a problem hiding this comment.
And prior to this fix the tests failed to complete successfully?
There was a problem hiding this comment.
Yes, sorry, I should have mentioned that.
Also, I am looking at the current build failure.
There was a problem hiding this comment.
My impression is that that test has been flakey for a while— it definitely has a timing sensitivity and does sometimes pass on the buildfarm. However, if you have further insights on it, that would be great.
|
I cannot figure out why the test fails on the build farm. They obviously pass just fine on my laptop which makes it hard to troubleshoot... |
|
LGTM, and has the thumbs-up from @mpflanzer in #1357 (comment). IMO this should merge now and we can treat the test regression as separate since it predates this change. |
This change fixes issues seen in #1357
Goes with #1281