Page MenuHomePhabricator

[CMAKE] Make RelWithDebInfo the default CMake configuration
ClosedPublic

Authored by Fabien on Mar 4 2019, 21:03.

Details

Summary

This set RelWithDebInfo as the default build type for CMake. This
makes the default optimization flags identical to those from
configure.

Depends on D2651

Test Plan
mkdir buildcmake && cd buildcmake
cmake -GNinja ..
ninja -v

Check that the -g -O2 flags are set in the compile commands for both
CC and CXX compile lines.

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Fabien created this revision.Mar 4 2019, 21:03
Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 4 2019, 21:03
Herald added a subscriber: schancel. · View Herald Transcript
deadalnix requested changes to this revision.Mar 4 2019, 21:15

This generates a ton of warning, so there is likely something not correct about what RelWithDebInfo does. It's not -O2 or -g though.

This revision now requires changes to proceed.Mar 4 2019, 21:15
Fabien added a comment.Mar 4 2019, 21:23

@deadalnix can you provide an example of what warning you are getting ?

Fabien updated this revision to Diff 7602.Mar 5 2019, 16:46

Rebase

deadalnix requested changes to this revision.Mar 5 2019, 20:51

Please do not change the config to something that's not working as expected. RelWithDebInfo is not working properly yet.

This revision now requires changes to proceed.Mar 5 2019, 20:51
Fabien requested review of this revision.Mar 13 2019, 14:56

Requesting review again since D2651 is landed.
This will add -g -O2 by default.

deadalnix requested changes to this revision.Apr 17 2019, 23:20
deadalnix added inline comments.
src/CMakeLists.txt
23 ↗(On Diff #7602)

Not if you change the configuration after the fact...

31 ↗(On Diff #7602)

This comes after flags have been modified the line just before. This is probably a bad idea.

This revision now requires changes to proceed.Apr 17 2019, 23:20
Fabien updated this revision to Diff 8119.Apr 18 2019, 06:02

Change configuration before removing the NDEBUG flag

deadalnix accepted this revision.Apr 19 2019, 12:52
This revision is now accepted and ready to land.Apr 19 2019, 12:52
deadalnix requested changes to this revision.Apr 19 2019, 12:53
deadalnix added inline comments.
src/CMakeLists.txt
24 ↗(On Diff #8119)

Maybe it's better to put this in the root CMakeLists.txt

This revision now requires changes to proceed.Apr 19 2019, 12:53
Fabien updated this revision to Diff 8266.Apr 25 2019, 09:31

Move to the root CMakeLists.txt

deadalnix accepted this revision.May 4 2019, 17:50
This revision is now accepted and ready to land.May 4 2019, 17:50
This revision was landed with ongoing or failed builds.May 6 2019, 08:05
This revision was automatically updated to reflect the committed changes.