Drake: Drake should build using -Werror

Created on 24 Mar 2016  路  12Comments  路  Source: RobotLocomotion/drake

Drake should build using -Werror... at least on a few of the "essential" supported configurations (the ones that most developers will typically use day-to-day; see also #1923.) By this, I mean (1) that the "getting started" recipes that developers who are going to contribute to Drake use initialize their build result in -Werror being used, and (2) that CI for the "essential" developer platforms replicate that setup, so the CI builds also use -Werror.

Warnings almost always identify meaningful problems, and should either be immediately resolved, or suppressed on an occurrence-by-occurrence basis.

high

Most helpful comment

I submitted RobotLocomotion/drake-ci#1 to turn off the warnings flags and it has been merged. Now, we can start incrementally putting them back in drake-distro/drake/CMakeLists.txt along with -Werror (on platforms that support it).

All 12 comments

+1

Note that CI builds (currently) run with the most verbose warning flags and there is still some work to be done to filter out some of the warnings in third party packages, so you may want to consider delaying this.

The default (required-to-pass-before-merging, etc.) CI builds should identically replicate the drake developers' setup, so my warnings should be the CI warnings. If we _also_ have a CI build with more warnings for informational purposes, great, but that should be orthogonal to getting -Werror on the baseline CI.

They will indeed replicate the developer builds eventually, but for now every single build on every type of job will fail, so you need to balance this will the continuing roll out of the CI infrastructure.

New PR builds should be closer to the developer builds. I am seeing one compiler warning on clang, none on GCC. We are working toward being identical.

Until I have build targets for the various tools that run nightly, they may fail if -Werror is enabled globally for GCC and Clang. I can enable manually it on Jenkins as a temporary measure, if this is high priority.

Would it make sense for me to add -Werror to the CMakeLists and have it be on by default, but offer an option to turn it off that the nightlies (or any other irregular configurations) would use?

Sure. That works.

I maybe stating, the obvious, but make sure it is only enabled for compilers that support it.

Now that I see the CI scripts linked from #1958, I have a better sense of the challenge. I think I will wait for that fix to merge before touching anything with -Werror. (Editing the drake build system and the CI settings in a single PR for this issue should simplify the work immensely.)

I submitted RobotLocomotion/drake-ci#1 to turn off the warnings flags and it has been merged. Now, we can start incrementally putting them back in drake-distro/drake/CMakeLists.txt along with -Werror (on platforms that support it).

I think this issue is complete for linux and mac (gcc and clang). I am content to let windows continue to spew ignored warnings (we can turn most of its good ones on for clang in the future, anyway), so I'm closing this issue.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

amcastro-tri picture amcastro-tri  路  4Comments

palmieri picture palmieri  路  4Comments

amcastro-tri picture amcastro-tri  路  5Comments

Islam0mar picture Islam0mar  路  4Comments

EricCousineau-TRI picture EricCousineau-TRI  路  3Comments