Page MenuHomePhabricator

Make c++14 standard the default for compilation
ClosedPublic

Authored by Fabien on Jan 31 2019, 16:37.

Details

Summary

As per title.
Note that this will make it difficult to run bitcoin-abc on OpenBSD with
the current documentation, due to the old gcc version which does not
support c++14.

Test Plan
make check

Run gitian builds

Diff Detail

Repository
rABC Bitcoin ABC
Branch
c++14
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 5387
Build 8836: Bitcoin ABC Buildbot (legacy)
Build 8835: arc lint + arc unit

Event Timeline

deadalnix requested changes to this revision.Jan 31 2019, 22:06

We need to have someone test this on OSX.

Also it'd be better if we do this after the freeze date. Requesting change to clear my queue, but please request review again after Feb, 15.

This revision now requires changes to proceed.Jan 31 2019, 22:06
Fabien planned changes to this revision.Jan 31 2019, 23:09

I would also have someone to test on OSX, atm I only checked that it builds on gitian.
I'm keeping it out of queue for now.

Fabien requested review of this revision.Feb 28 2019, 08:10

Back to the queue

Can you remove documentation mentioning support for plateform for which C++14 do not work yet ? Once this is done, I have no problem with that, to the contrary :)

I rechecked the documentation, here is the status.

There is documentation for:

  • Linux, will build with c++14 on most distributions. Gitian builds done with success.
  • Windows, builds with c++ on WSL. Gitian builds done with success.
  • OSX, builds with c++14. Gitian builds done with success. I did not run the software because I have no OSX machine at hand.
  • OpenBSD, I couldn't manage to make it build with c++14. Support gets dropped in D2461, i.e. build instructions and references are removed from the documentation.
  • FreeBSD, builds with c++14 (with no GUI, and after applying this patch). The build instructions are located at the end of the build-unix.md file. They need some update, and be moved to their own documentation file (will do in another diff). The build has been tested with the current latest FreeBSD 12.0.
  • Cross build for ARM has been tested through Gitian builds. I did no other testing, especially I did not run the binary.

I don't think there is another platform mentioned in the doc, if you think that I'm missing one please let me know.
So far the only support that needs to get dropped to enable c++14 is OpenBSD.

This revision is now accepted and ready to land.Apr 1 2019, 21:24

Can you please add release notes about this ?

I will add the entry if you think it's useful.
I have been asking myself about this, but thought it was unnecessary because it's not user facing.

Rebase and add an entry in the release notes

jasonbcox requested changes to this revision.Apr 2 2019, 18:53
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
doc/release-notes.md
15 ↗(On Diff #7897)

This needs rebasing on 0.19.3 release notes.

Also, a recommended rewording for those running binaries since they may be confused by this:

- Code standard updated to C++14. If building from source, please make sure you have a compatible compiler installed.

This revision now requires changes to proceed.Apr 2 2019, 18:53

Rebase and reword release notes as suggested

deadalnix requested changes to this revision.Apr 4 2019, 19:34
deadalnix added inline comments.
doc/release-notes.md
7 ↗(On Diff #7917)

Remove If building from source, please make sure you have a compatible compiler installer.

If you think this is important to precise it, then add it to the build instructions.

This revision now requires changes to proceed.Apr 4 2019, 19:34

Move the compiler warning to the build instructions

Also include instructions for windows build. If the build is done
with WSL as per the instructions there will be no issue, but when
building from another linux the compiler version may vary.

This revision is now accepted and ready to land.Apr 8 2019, 16:23
This revision was automatically updated to reflect the committed changes.