Bump minimum CMake version up to 3.16

[hebasto]

Considering the time when the new CMake-based build system will be available for users, it seems reasonable to drop support
for CMake versions older than 3.16.

Here are benefits of such version bumping:

• code simplification (see individual commits)
• the cmake --build tool gained --verbose and -v options to specify verbose build output
• the cmake-gui dialog gained new -S and -B arguments to explicitly specify source and build directories
• the FindSQLite3 module was added to find the SQLite v3.x library (not implemented in this PR yet)
• the cmake --build tool --target parameter gained support for multiple targets. It now also has a short form -t alias
• the cmake command gained a new --install option

[hebasto]

cc @fanquake @theuni @TheCharlatan @luke-jr @willcl-ark @stickies-v @ajtowns @w0xlt @pablomartin4btc @vasild

[ajtowns]

seems sensible to me

[TheCharlatan]

ACK b517d5c

[theuni]

ACK b517d5c

[theuni]

This unset is new in the rebase, no? Is it needed for some reason? Is this a pattern we should be reviewing for?

[hebasto]

The reason is to keep the global namespace tidy.

[fanquake]

Can someone link to the discussion about the CMake PIE checks, their output, and how it differs from what we do with master?
I guess we are fine with what is being done here, but I didn't see the difference being discussed anywhere. (I also don't see anything PIE related in the hardening PR, to account for any difference.)

i.e, one example on Fedora Linux:

# Master

src/init.cpp -fPIE
bitcoind -fPIE -pie

# CMake

src/init.cpp -fPIC
bitcoind -fPIE

[hebasto]

Can someone link to the discussion about the CMake PIE checks, their output, and how it differs from what we do with master? I guess we are fine with what is being done here, but I didn't see the difference being discussed anywhere. (I also don't see anything PIE related in the hardening PR, to account for any difference.)

i.e, one example on Fedora Linux:

# Master

src/init.cpp -fPIE
bitcoind -fPIE -pie

# CMake

src/init.cpp -fPIC
bitcoind -fPIE

From CMake docs:

• https://cmake.org/cmake/help/latest/prop_tgt/POSITION_INDEPENDENT_CODE.html
• https://cmake.org/cmake/help/latest/policy/CMP0083.html
• https://cmake.org/cmake/help/latest/module/CheckPIESupported.html

[fanquake]

Sure, I can lookup the docs, but I'm asking why have we changed this, does the difference matter, etc?

[fanquake]

In the docs I also see:

Set to TRUE: flags to produce a position independent executable are passed to the linker step. For example -pie for GCC.

but that doesn't seem to be the case here? So is the check not working as intended, is our behaviour differing from the docs?

[hebasto]

Autotools:

$ file src/bitcoind
src/bitcoind: ELF 64-bit LSB pie executable, x86-64, version 1 (GNU/Linux), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, BuildID[sha1]=9b7e56fa795f50af442bd6930d9b9b21e683403d, for GNU/Linux 3.2.0, with debug_info, not stripped

CMake:

$ file build/src/bitcoind 
build/src/bitcoind: ELF 64-bit LSB pie executable, x86-64, version 1 (GNU/Linux), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, BuildID[sha1]=a028648ba6ed1937ae5be5d826adbca412251245, for GNU/Linux 3.2.0, with debug_info, not stripped

[hebasto]

The link command for bitcoind does include -fPIE -pie:

$ cat build/src/CMakeFiles/bitcoind.dir/link.txt
/usr/bin/c++ -O2 -g -Wl,-z,relro -Wl,-z,now -Wl,-z,separate-code -fPIE -pie CMakeFiles/bitcoind.dir/bitcoind.cpp.o CMakeFiles/bitcoind.dir/init/bitcoind.cpp.o -o bitcoind  libbitcoin_node.a wallet/libbitcoin_wallet.a /usr/lib/x86_64-linux-gnu/libsqlite3.so /usr/lib/x86_64-linux-gnu/libdb_cxx.so libbitcoin_common.a ../libsecp256k1.a util/libbitcoin_util.a ../libminisketch.a /usr/lib/x86_64-linux-gnu/libevent_pthreads.so /usr/lib/x86_64-linux-gnu/libevent.so /usr/lib/x86_64-linux-gnu/libnatpmp.so /usr/lib/x86_64-linux-gnu/libminiupnpc.so zmq/libbitcoin_zmq.a ../libleveldb.a ../libcrc32c.a univalue/libunivalue.a /usr/lib/x86_64-linux-gnu/libzmq.so 

[theuni]

CMake added the necessary linker behavior here: https://cmake.org/cmake/help/latest/policy/CMP0083.html

This is kinda like bitcoin#28672 IMO. pic/pie are arguably used out of practicality/compatibility more these days than for hardening itself. I think it's reasonable to disconnect this from hardening.

[theuni]

Wait, no. I misunderstood what was happening here. I thought we agreed not to force this on, letting CMake do the right thing depending on what was being built? Wasn't this only turned on as a stop-gap for initial review?

[hebasto]

Wait, no. I misunderstood what was happening here. I thought we agreed not to force this on, letting CMake do the right thing depending on what was being built? Wasn't this only turned on as a stop-gap for initial review?

Just to clarify, this PR did not aim to alter the PIC/PIE behavior. It dropped the code branch for CMake versions older than 3.16.

[theuni]

Yes, understood. And I think above I was misremembering, that's how we handled it for libsecp. For Core, yeah, just turn it on.

All good. Sorry for the noise.

[hebasto]

Speaking of PIC/PIE, I really doubt that the check, or at least a warning, is required for a Windows target (both MSVC and cross-compiling).

[hebasto]

• the FindSQLite3 module was added to find the SQLite v3.x library (not implemented in this PR yet)

Implemented in #39.