Skip to content
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

fix clang-tidy CI job #2792

Merged
merged 2 commits into from
Aug 12, 2021
Merged

fix clang-tidy CI job #2792

merged 2 commits into from
Aug 12, 2021

Conversation

v4hn
Copy link
Contributor

@v4hn v4hn commented Jul 29, 2021

#2790 and #2674 currently show that the clang-tidy job broke, likely due to 55886f7 .

Recovering the clang-tidy job might fail for a variety of reasons, so I will mark this as WIP.
Anyone, feel free to fix and merge this if you have time.

@codecov
Copy link

codecov bot commented Jul 29, 2021

Codecov Report

Merging #2792 (7cb6abb) into master (8237887) will increase coverage by 0.01%.
The diff coverage is n/a.

❗ Current head 7cb6abb differs from pull request most recent head 2d4d1fe. Consider uploading reports for the commit 2d4d1fe to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2792      +/-   ##
==========================================
+ Coverage   60.45%   60.45%   +0.01%     
==========================================
  Files         366      366              
  Lines       31673    31673              
==========================================
+ Hits        19144    19146       +2     
+ Misses      12529    12527       -2     
Impacted Files Coverage Δ
...eit_ros/manipulation/pick_place/src/pick_place.cpp 89.48% <0.00%> (-3.15%) ⬇️
...ipulation/pick_place/src/manipulation_pipeline.cpp 71.30% <0.00%> (-0.92%) ⬇️
...on/pick_place/src/approach_and_translate_stage.cpp 73.38% <0.00%> (-0.64%) ⬇️
...dl_kinematics_plugin/src/kdl_kinematics_plugin.cpp 77.87% <0.00%> (-0.38%) ⬇️
...raint_samplers/src/default_constraint_samplers.cpp 80.72% <0.00%> (+0.30%) ⬆️
...meterization/work_space/pose_model_state_space.cpp 83.02% <0.00%> (+0.63%) ⬆️
.../ompl_interface/src/detail/constrained_sampler.cpp 59.46% <0.00%> (+16.22%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8237887...2d4d1fe. Read the comment docs.

@v4hn
Copy link
Contributor Author

v4hn commented Jul 29, 2021

does anyone see a connection between this pr and a bullet collision test failing with bad_alloc or has seen this failure before?

Copy link
Member

@henningkayser henningkayser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see anything related to bullet. Is this repeatable? Also, FCL seems to fail for the same reason now.

moveit_core/robot_state/src/robot_state.cpp Outdated Show resolved Hide resolved
@v4hn
Copy link
Contributor Author

v4hn commented Aug 6, 2021

I don't see anything related to bullet. Is this repeatable? Also, FCL seems to fail for the same reason now.

build/moveit_core/test_results/moveit_core/gtest-test_fcl_collision_detection_panda.xml: 18 tests, 0 errors, 2 failures, 0 skipped

Sorry, it is related to FCL indeed. I misread the logs before.
I can't reproduce this issue with my local system and clang 11.0.1, so this needs to be debugged in the CI environment.
@tylerjw and @rhaschke have way more experience in this, sorry 😕

@v4hn
Copy link
Contributor Author

v4hn commented Aug 8, 2021 via email

v4hn added a commit that referenced this pull request Aug 10, 2021
@v4hn
Copy link
Contributor Author

v4hn commented Aug 11, 2021

I just distilled my insights from late-night debugging into a -O0 to a single (though highly crucial) source file.
Everyone is welcome to pick it up from here and find the concrete optimization pass that introduces the problem or find a way to work around it in source code.

If CI succeeds now I would still propose to merge it for now with an urgent followup TODO to unblock CI.

3b9977d threw away the clang build we setup for travis.
However, we definitely want to have a CI job running with clang and clang-tidy
seems to reuse arguments generated by cmake for the gcc compiler...
FCLDistanceCheckPanda/DistanceFullPandaTest/0 fails with
unknown file
C++ exception with description "std::bad_alloc" thrown in the test body.
@v4hn v4hn changed the title [WIP] fix clang-tidy CI job fix clang-tidy CI job Aug 11, 2021
@v4hn
Copy link
Contributor Author

v4hn commented Aug 11, 2021

This succeeds as soon as moveit/moveit_visual_tools#96 and PickNikRobotics/rviz_visual_tools#195 are merged, so I'll remove the WIP label already.

@tylerjw
Copy link
Member

tylerjw commented Aug 11, 2021

This succeeds as soon as ros-planning/moveit_visual_tools#96 and ros-planning/moveit_visual_tools#96 are merged, so I'll remove the WIP label already.

Did you mean to link the same PR twice or is there another PR that is also needed?

@v4hn
Copy link
Contributor Author

v4hn commented Aug 12, 2021

Did you mean to link the same PR twice or is there another PR that is also needed?

updated above, sorry for the broken c&p.

@tylerjw
Copy link
Member

tylerjw commented Aug 12, 2021

I don't like the fix of changing the optimization level but I don't have time or any better ideas at the moment. I'll merge this and create an issue for fixing it better.

@tylerjw tylerjw merged commit 86174f3 into moveit:master Aug 12, 2021
rhaschke added a commit to ubi-agni/moveit that referenced this pull request Aug 17, 2021
which was disabled in moveit#2792
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants