Details

Reviewers

deadalnix
jasonbcox

Group Reviewers

Restricted Project

Commits

rSTAGINGa48b9f299778: Make c++14 standard the default for compilation
rABCa48b9f299778: Make c++14 standard the default for compilation

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

Fabien created this revision.Jan 31 2019, 16:37

Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 31 2019, 16:37

Herald added a reviewer: deadalnix. · View Herald Transcript

Herald added a subscriber: schancel. · View Herald Transcript

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

Fabien added a child revision: D2463: Migrate MakeUnique to c++14 std::make_unique.Jan 31 2019, 16:39

Fabien added a child revision: D2464: Use c++14 generic std::rbegin() and std::rend() instead of class methods.Jan 31 2019, 16:42

Harbormaster completed remote builds in B4794: Diff 7067.Jan 31 2019, 16:54

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

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

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.

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.

deadalnix accepted this revision.Apr 1 2019, 21:24

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

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

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

Harbormaster completed remote builds in B5350: Diff 7897.Apr 2 2019, 07:56

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

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

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

Harbormaster completed remote builds in B5364: Diff 7917.Apr 2 2019, 19:29

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

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

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.

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

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

Harbormaster completed remote builds in B5387: Diff 7956.Apr 5 2019, 09:34

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

Harbormaster completed remote builds in B5388: Diff 7958.Apr 5 2019, 09:39

deadalnix accepted this revision.Apr 5 2019, 14:32

Rebase

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

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

Harbormaster completed remote builds in B5412: Diff 7991.Apr 8 2019, 07:42

jasonbcox accepted this revision.Apr 8 2019, 16:23

This revision is now accepted and ready to land.Apr 8 2019, 16:23