build: remove mostly pointless BOOST

fanquake commented at 8:22 am on February 15, 2021: member

Performing a series of link checks for a Boost component that is header-only doesn’t make much sense, and currently means we just have another confusing Boost macro in our tree. I’m not sure why this was originally done this way; maybe Sjors or luke-jr can elaborate (#15382 (929cda5470f98d1ef85c05b1cad4e2fb9227e3b0))?

The macro also has the side-effect of producing confusing error messages. i.e in #20744, the CI is currently failing with:

0checking for boostlib >= 1.58.0 (105800) lib path in "/tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/lib"... yes
1checking for boostlib >= 1.58.0 (105800)... yes
2checking whether the Boost::Process library is available... yes
3configure: error: Could not find a version of the Boost::Process library!

This isn’t useful, given there is no such thing as a Boost::Process library.

This PR just removes the macro entirely, but maintains a --with-boost-process (defaulting to off), flag to configure. Hopefully this will also be removed, in favour of --enable/disable-external-signer if/when #16546 is merged.

in configure.ac:1401 in 1b960f0780 outdated

1398+dnl Opt-in to Boost Process
1399+if test "x$boost_process" != xno; then
1400+AC_MSG_CHECKING(for Boost Process)
1401+AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <boost/process.hpp>]],
1402+ [[ boost::process::child* child = new boost::process::child; delete child; ]])],
1403+ [ AC_MSG_RESULT(yes); AC_DEFINE([HAVE_BOOST_PROCESS],,[define if the Boost::Process library is available])],

hebasto commented at 8:26 am on February 15, 2021:

there is no such thing as a Boost::Process library.

:)

fanquake commented at 8:49 am on February 15, 2021:

Heh. Thanks

fanquake force-pushed on Feb 15, 2021

fanquake requested review from luke-jr on Feb 15, 2021

fanquake requested review from Sjors on Feb 15, 2021

laanwj commented at 8:31 am on February 15, 2021: member

Concept ACK

These vague configure errors are very common and often spook users, have done so for years:

0configure: error: Could not find a version of the Boost::Process library!

Good to get rid of one instance at least.

Boost Process is header only,

TIL. This is pretty good news. So the ‘unique’ boost parts that we need: process and multi_index, are both header only. This means that soon we no longer have to compile boost at all, just copy headers. Assuming of course that header-only boost doesn’t depend on libraries deeper in the hierarchy (this is very possible).

This PR just removes the macro entirely, but maintains a –with-boost-process(defaulting to off), flag to configure.

Right! it is better to have configure flags that semantically make sense to users not just ‘use implementation specific thing’.

sipa commented at 8:35 am on February 15, 2021: member

boost::multi_index depends on the boost serialization library if its serialization functions are used (which we don’t). According to the documentation, if you don’t, then multi_index is purely headers only.

fanquake added the label Build system on Feb 15, 2021

DrahtBot commented at 8:40 am on February 15, 2021: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#16546 (External signer support - Wallet Box edition by Sjors)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

MarcoFalke added the label Needs gitian build on Feb 15, 2021

MarcoFalke added the label Needs Guix build on Feb 15, 2021

practicalswift commented at 10:42 am on February 15, 2021: contributor

Concept ACK

Sjors commented at 3:52 pm on February 15, 2021: member

In #15382 I just copied what I saw for another Boost dependency :-)

Sjors approved

Sjors commented at 7:00 pm on February 15, 2021: member

tACK ffe3d65

I’m using this in #16546 now (will rebase that after this is merged).

DrahtBot commented at 6:23 pm on February 16, 2021: member

Gitian builds

File	commit 489030f2a8f89e7ae5031351fc3d0db83e3911ea(master)	commit 852ca3fc17182225cb5060eeeee97c93f5c0973e(master and this pull)
*-aarch64-linux-gnu-debug.tar.gz	`006c783b5ee9b7e2...`	`fe9fc8a97dd8b6a3...`
*-aarch64-linux-gnu.tar.gz	`f6bef6690ef6ee58...`	`b7523eacb2deeb33...`
*-arm-linux-gnueabihf-debug.tar.gz	`fbc6003d4f87b3f3...`	`20973d54f06cd1ba...`
*-arm-linux-gnueabihf.tar.gz	`1e0bc41a2f22e3c8...`	`d49c6a4eee0652ba...`
*-osx-unsigned.dmg	`2cdfd57d35b2b300...`	`e047362508f6074f...`
*-osx64.tar.gz	`f39dc996a04495fd...`	`2a362ad99c51700c...`
*-powerpc64-linux-gnu-debug.tar.gz	`d070af570efa110a...`	`e412b10c366d3a6c...`
*-powerpc64-linux-gnu.tar.gz	`2bc7cef416bf048c...`	`178aaf5fd05c0346...`
*-powerpc64le-linux-gnu-debug.tar.gz	`776436d5885dcf6d...`	`3d005f86d07e2f6a...`
*-powerpc64le-linux-gnu.tar.gz	`affb8ec092f9a7ac...`	`f88b15a62cd678ad...`
*-riscv64-linux-gnu-debug.tar.gz	`5e8d5a7ead21c571...`	`c4491755d166a5b4...`
*-riscv64-linux-gnu.tar.gz	`690ced04615f2f50...`	`b554f845614515ba...`
*-win64-debug.zip	`3c45039ca679a2ce...`	`df47a846952e57ed...`
*-win64-setup-unsigned.exe	`c923d94eb6ad6625...`	`031985f27a853601...`
*-win64.zip	`115a276ef2566d76...`	`5ee192be900ebf46...`
*-x86_64-linux-gnu-debug.tar.gz	`1b3ea996252c2e3b...`	`7a30c803cb4806d1...`
*-x86_64-linux-gnu.tar.gz	`1289b942fddcd737...`	`182cb9d654f76e1e...`
*.tar.gz	`5097aaab3eff1d43...`	`c3f11d0514f1fc35...`
bitcoin-core-linux-22-res.yml	`1dddf5ad4c54df22...`	`4047756e6aaf810e...`
bitcoin-core-osx-22-res.yml	`e3519662695f59ec...`	`21a95049dcb3d4ef...`
bitcoin-core-win-22-res.yml	`4841d2d23b8e1a96...`	`bd0f6bb110823ef0...`
linux-build.log	`6fa01f16dc2596f6...`	`0815a978af442895...`
osx-build.log	`c346ccb31425f8cd...`	`862fa59fc9961dfa...`
win-build.log	`651a38db57f23a64...`	`4140bb5fde76348b...`
bitcoin-core-linux-22-res.yml.diff		`b1fb2411aa6dda21...`
bitcoin-core-osx-22-res.yml.diff		`dd6ce9bd434ebd95...`
bitcoin-core-win-22-res.yml.diff		`7f6d85c65519543f...`
linux-build.log.diff		`4ee6c877077e1a0e...`
osx-build.log.diff		`6a502bc8572af803...`
win-build.log.diff		`cd106f020b1fa412...`

DrahtBot removed the label Needs gitian build on Feb 16, 2021

practicalswift commented at 7:19 pm on February 16, 2021: contributor

cr ACK ffe3d65aaa18df9413c35e6914b42f7a400bf0b6: patch looks correct

in configure.ac:1401 in ffe3d65aaa outdated

1398+dnl Opt-in to Boost Process
1399+if test "x$boost_process" != xno; then
1400+AC_MSG_CHECKING(for Boost Process)
1401+AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <boost/process.hpp>]],
1402+ [[ boost::process::child* child = new boost::process::child; delete child; ]])],
1403+ [ AC_MSG_RESULT(yes); AC_DEFINE([HAVE_BOOST_PROCESS],,[define if Boost::Process is available])],

laanwj commented at 8:05 pm on February 16, 2021:

Shouldn’t the configure run fail when --with-boost-process is set but this test fails?

fanquake commented at 0:05 am on February 17, 2021:

Thanks, yes, this is now failing when Boost Process isn’t available.

laanwj commented at 2:03 pm on February 17, 2021:

Thanks. Yes this is better. I guess in the future when the option is changed to a semantic feature like --enable-external-signer it would make sense to mention in the error message what it’s required for. Not now though.

build: remove mostly pointless BOOST_PROCESS macro

Performing a series of link checks for a Boost component that is
header-only doesn't make much sense, and currently means we just have
another confusing Boost macro in our tree. I'm not sure why this was
originally done this way; maybe Sjors or luke-jr can elaborate
(#15382 (929cda5470f98d1ef85c05b1cad4e2fb9227e3b0))?

The macro also has the side-effect of producing confusing error
messages. i.e in #20744, the CI is currently failing with:
```bash
checking for boostlib >= 1.58.0 (105800) lib path in "/tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/lib"... yes
checking for boostlib >= 1.58.0 (105800)... yes
checking whether the Boost::Process library is available... yes
configure: error: Could not find a version of the Boost::Process library!
```

This isn't useful, given there is no such thing as a `Boost::Process`
library.

This PR just removes the macro entirely, but maintains a `--with-boost-process`
(defaulting to off), flag to configure. Hopefully this will also be
removed, in favour of `--enable-disable-external-signer` if/when #16546
is merged.

7bf04e358a

fanquake force-pushed on Feb 17, 2021

laanwj commented at 2:04 pm on February 17, 2021: member

ACK 7bf04e358a6550ac9851f1b2d87795927fc5ff4b

laanwj merged this on Feb 17, 2021

laanwj closed this on Feb 17, 2021

fanquake deleted the branch on Feb 17, 2021

sidhujag referenced this in commit 5d5698c0ae on Feb 17, 2021

DrahtBot commented at 4:26 pm on February 17, 2021: member

Guix builds

File	commit 92fee79dab384acea47bf20741a9847a58253330(master)	commit d3c2cced4353898bb361211f97dbff222285caf4(master and this pull)
*-aarch64-linux-gnu-debug.tar.gz	`5a111c6410cbe1b7...`	`7100acb8809cd131...`
*-aarch64-linux-gnu.tar.gz	`c34ee7b51294fa34...`	`dd76a640115a61e3...`
*-arm-linux-gnueabihf-debug.tar.gz	`319722a3e897b44d...`	`2e9669d5e76d89ad...`
*-arm-linux-gnueabihf.tar.gz	`918dd4eaecbf674e...`	`560e5f196908e989...`
*-osx-unsigned.dmg	`3ea1713dd2c2e93d...`
*-osx-unsigned.tar.gz	`c7ef47d2605fc49f...`
*-osx64.tar.gz	`7d01ab16be9c6551...`
*-riscv64-linux-gnu-debug.tar.gz	`2f73512e5994d117...`	`e79c7e181aec1645...`
*-riscv64-linux-gnu.tar.gz	`309229dbee5ca9ff...`	`6e77c4edd59743c3...`
*-win-unsigned.tar.gz	`ca693e0ccf46f6c0...`
*-win64-debug.zip	`613ce4f1a39725a8...`
*-win64-setup-unsigned.exe	`232fd5a4ecc88e13...`	`8484b9eecb53eb20...`
*-win64.zip	`dc3928950e7dbd51...`
*-x86_64-linux-gnu-debug.tar.gz	`82122cab5cffa47d...`	`392a3c646cd41820...`
*-x86_64-linux-gnu.tar.gz	`a5f5834e0adf7c7b...`	`dc613f2e97d0e5f4...`
*.tar.gz	`eb4d9b2072405139...`	`d471b8500a828cee...`
guix_build.log	`10224f79e4d826e6...`	`3b5df31e1c7f0832...`
guix_build.log.diff		`6bc7919cba5576fc...`

DrahtBot removed the label Needs Guix build on Feb 17, 2021

laanwj commented at 12:48 pm on February 18, 2021: member

Looks like the GUIX build failed? Oh, it’s unrelated:

0/gnu/store/4yisib9qc6l5a8kbb6pmkmaj0zx7fj67-profile/bin/x86_64-w64-mingw32-objcopy:bitcoin-d3c2cced4353/bin/test_bitcoin.exe.dbg[.debug_loc]: No space left on device

MarcoFalke commented at 1:32 pm on February 18, 2021: member

Will increase the disk size soon(tm)(c)(2021)

laanwj referenced this in commit a9335e4f12 on Feb 23, 2021

DrahtBot locked this on Aug 16, 2022

build: remove mostly pointless BOOST_PROCESS macro #21182

Conflicts

Gitian builds

Guix builds