travis: Build with --enable-werror under OS X #10923

Build with --enable-werror under OS X.

As suggested by @TheBlueMatt in #10866. This will allow catching violations using Travis CI which does a clang build for OS X.

Looks good to me, though I'm confused, in #10866 you indicated that these checks should fail, but travis passed here?

@TheBlueMatt Yes, that is a bit weird. I verified this change locally using these four tests before submitting the PR:

$ CC=clang CXX=clang++ ./configure --enable-werror && make clean && make check; echo $?
…
# fails as expected:
Makefile:5576: recipe for target 'libbitcoin_server_a-net_processing.o' failed
2
$ CC=clang CXX=clang++ ./configure && make clean && make check; echo $?
…
# passes as expected:
0
$ CC=gcc CXX=g++ ./configure --enable-werror && make clean && make check; echo $?
…
# passes as expected:
0
$ CC=gcc CXX=g++ ./configure && make clean && make check; echo $?
…
# passes as expected:
0
$ clang++ --version | head -1
clang version 3.8.0-2ubuntu4 (tags/RELEASE_380/final)

Locally everything works as expected.

Does the Travis CI build run with --enable-werror? I cannot find that in .travis.yml.

No, --enable-werror is currently not set on travis.

Great! Lets enable it by default at least on the osx build. Its deliberately heavily limited so no reason not to (also maybe do it by default, @theuni?).

Yea, I like this!

My only concern is that the annotations are infectious, so this may cause us to have to add them while doing large refactors where one annotation already exists. That's a good problem to have though, IMO.

utACK after #10866 goes in.

Looks like the travis failure was from building fontconfig, not the expected annotation-missing failure. Is it unrelated?

@TheBlueMatt Yes it appears to fail before hitting expected annotation-missing failure. I'm not sure about the root cause here. Can we as a first step re-trigger the Travis CI build?

@practicalswift OK, it has been kicked, I dont know how building fontconfig would have failed because of this, but hopefully its deterministic if it is an issue with this PR.

@TheBlueMatt Travis CI now complains about a locking issue but it is not the locking issue I'm encountering locally (locally I'm getting the error described in #10866). Is the locking issue reported by Travis a false positive?

@practicalswift hmm, well I got it to work by changing the sync.h primitives to std from boost (see https://travis-ci.org/TheBlueMatt/bitcoin/jobs/265262602 and https://github.com/TheBlueMatt/bitcoin/commits/2017-08-test-10923)

@TheBlueMatt Oh, nice! What is the best way to proceed from here?

Should the changing of sync.h primitives from boost to std be put in a separate PR of yours, or should I simply incorporate your changes into this PR?

These are the changes: https://github.com/practicalswift/bitcoin/compare/thread-safety-analysis...TheBlueMatt:2017-08-test-10923

@practicalswift as per meeting split the "enable werror on Travis" part from the Wthread-safety-analysis part. Maybe just leave this PR as the first and move the second back to the did-thread-safety-analysis pr?

On August 16, 2017 4:09:42 PM EDT, practicalswift notifications@github.com wrote:

@TheBlueMatt Oh, nice! What is the best way to proceed from here? Shall I incorporate your changes into this PR or should I cherry-pick the individual commits?

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/bitcoin/bitcoin/pull/10923#issuecomment-322884443

@TheBlueMatt What meeting? What am I missing? :-)

@TheBlueMatt PR updated. Looks good? :-)

@ptracticalswift the IRC meeting, Werror was discussed. Change the PR title?

@practicalswift wait, huh, I meant just do Werror here, and the thread-safety-analysis thing can go in #10866.

@TheBlueMatt Got it! I haven't attended any Bitcoin IRC channels/meetings - are there logs available somewhere? :-)

@practicalswift https://botbot.me/freenode/bitcoin-core-dev/

ACK

ACK a65e02803cc76e9250f67a34c610234c263d4ab2

Prefixed the issue title with "travis:" otherwise it looks like all OSX builds will enable -werror, which is not the intent.