Page MenuHomePhabricator
Differential D2643

[CMAKE] Make RelWithDebInfo the default CMake configuration
ClosedPublic
Actions

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

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Commits
rSTAGING4adac02d223b: [CMAKE] Make RelWithDebInfo the default CMake configuration
rABC4adac02d223b: [CMAKE] Make RelWithDebInfo the default CMake configuration
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
Branch
cmake_default_RelDebWithInfo
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 5597
Build 9256: Bitcoin ABC Buildbot (legacy)
Build 9255: arc lint + arc unit

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 reviewer: deadalnix. · View Herald Transcript
Herald added a subscriber: schancel. · View Herald Transcript
teamcity added a subscriber: teamcity.Mar 4 2019, 21:04

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

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
Harbormaster completed remote builds in B5134: Diff 7582.Mar 4 2019, 21:16
teamcity added a comment.Mar 4 2019, 21:16

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

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

teamcity added a comment.Mar 5 2019, 16:47

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

teamcity added a comment.Mar 5 2019, 16:59

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B5149: Diff 7602.Mar 5 2019, 16:59
Fabien edited the summary of this revision. (Show Details)Mar 5 2019, 17:04
Fabien added a parent revision: D2651: [CMAKE] Remove the -DNDEBUG flag from the release build configurations.
Fabien removed a parent revision: D2639: [CMAKE] Override the flags for the RelWithDebInfo build configuration.
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

teamcity added a comment.Apr 18 2019, 06:03

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

teamcity added a comment.Apr 18 2019, 06:18

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B5500: Diff 8119.Apr 18 2019, 06:18
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

teamcity added a comment.Apr 25 2019, 09:32

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

teamcity added a comment.Apr 25 2019, 09:46

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing failed.

Harbormaster failed remote builds in B5597: Diff 8266!Apr 25 2019, 09:46
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
Closed by commit rABC4adac02d223b: [CMAKE] Make RelWithDebInfo the default CMake configuration (authored by Fabien). · Explain Why
This revision was automatically updated to reflect the committed changes.
Fabien added a commit: rSTAGING4adac02d223b: [CMAKE] Make RelWithDebInfo the default CMake configuration.Sep 18 2019, 23:15

Revision Contents
Changeset List

PathSize
6 lines

Diff 8266

View Options

CMakeLists.txt

Loading...