Multiprocess build support #18677

ryanofsky · 2020-04-16T20:44:38Z

This PR is part of the process separation project.

This PR consists of build changes only. It adds an --enable-multiprocess autoconf option (off by default and marked experimental), that builds new bitcoin-node and bitcoin-gui binaries. These currently function the same as existing bitcoind and bitcoin-qt binaries, but are extended in #10102 with IPC features to execute node, wallet, and gui functions in separate processes.

In addition to adding the --enable-multiprocess config flag, it also adds a depends package and autoconf rules to build with the libmultiprocess library, and it adds new travis configuration to exercise the build code and run functional tests with the new binaries.

The changes in this PR were originally part of #10102 but were moved into #16367 to be able to develop and review the multiprocess build changes independently of the code changes. #16367 was briefly merged and then reverted in #18588. Only change since #16367 has been dropping the native_boost.mk depends package which was pointed out to be no longer necessary in #16367 (comment) and #18588 (review)

maflcko · 2020-04-16T20:56:59Z

re-ACK c8672df, only change since my last review is dropping boost and small doc fixup. This review does not cover libmultiprocess 🚕

Show signature and timestamp

Signature:

Timestamp of file with hash 5ffc2ebe9acc3666c3cfa34fc843d6447defa592787071c5cc86bf728ace8075 -

DrahtBot · 2020-04-16T22:41:25Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

test: Replace TEST_PREVIOUS_RELEASES env var with test_framework option #19014 (test: Replace TEST_PREVIOUS_RELEASES env var with test_framework option by MarcoFalke)
build: Decouple clientversion.cpp from the git repo #18980 (build: Decouple clientversion.cpp from the git repo by hebasto)
ci: Run fuzz testing test cases (bitcoin-core/qa-assets) under valgrind to catch memory errors #18912 (ci: Run fuzz testing test cases (bitcoin-core/qa-assets) under valgrind to catch memory errors by practicalswift)
net: Add NAT-PMP port forwarding support #18077 (net: Add NAT-PMP port forwarding support by hebasto)
UI external signer support (e.g. hardware wallet) #16549 (UI external signer support (e.g. hardware wallet) by Sjors)
External signer support - Wallet Box edition #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.

fanquake

Thanks for reopening and the responses here. I also read through the IRC logs from last night.

Few comments other than on the code. Nothing blocking while this is all still experimental.

Cross-compiling for macOS (on Debian 10.3) with MULTIPROCESS=1 currently fails while building capnp:

make -C depends/ HOST=x86_64-apple-darwin16 MULTIPROCESS=1
...
libtool: link: clang++ -target x86_64-apple-darwin16 -mmacosx-version-min=10.12 --sysroot /bitcoin/depends/SDKs/MacOSX10.14.sdk -stdlib=libc++ -I./src -I./src -DKJ_HEADER_WARNINGS -DCAPNP_HEADER_WARNINGS -DCAPNP_INCLUDE_DIR=\"/bitcoin/depends/x86_64-apple-darwin16/include\" -D_THREAD_SAFE -pipe -O2 -D_THREAD_SAFE -D_THREAD_SAFE -o .libs/capnp src/capnp/compiler/module-loader.o src/capnp/compiler/capnp.o -Wl,-bind_at_load  -L/bitcoin/depends/x86_64-apple-darwin16/lib ./.libs/libcapnpc.dylib ./.libs/libcapnp-json.dylib /bitcoin/depends/work/build/x86_64-apple-darwin16/capnp/0.7.0-87ee694a224/.libs/libcapnp.dylib ./.libs/libcapnp.dylib /bitcoin/depends/work/build/x86_64-apple-darwin16/capnp/0.7.0-87ee694a224/.libs/libkj.dylib ./.libs/libkj.dylib
echo capnp capnpc-c++ src/capnp/test.capnp src/capnp/test-import.capnp src/capnp/test-import2.capnp src/capnp/compat/json-test.capnp | (read CAPNP CAPNPC_CXX SOURCES && ./$CAPNP compile --src-prefix=./src -o./$CAPNPC_CXX:src -I./src $SOURCES)
./capnp: line 117: /bitcoin/depends/work/build/x86_64-apple-darwin16/capnp/0.7.0-87ee694a224/.libs/capnp: cannot execute binary file: Exec format error
./capnp: line 117: /bitcoin/depends/work/build/x86_64-apple-darwin16/capnp/0.7.0-87ee694a224/.libs/capnp: Success
make[1]: *** [Makefile:4326: test_capnpc_middleman] Error 126
make[1]: Leaving directory '/bitcoin/depends/work/build/x86_64-apple-darwin16/capnp/0.7.0-87ee694a224'
make: *** [funcs.mk:254: /bitcoin/depends/work/build/x86_64-apple-darwin16/capnp/0.7.0-87ee694a224/./.stamp_built] Error 2
make: Leaving directory '/bitcoin/depends'

Would you prefer these issues opened here or the libmultiprocess repo?

Can we pass --with-openssl=no (+ any other relevant flags) to the capnp configure? I assume we aren't using any parts of it that require OpenSSL, and we might as well opt out of any autodetection and/or additional compilation.

Also, I didn't notice before, but just wanted to note that building capnp currently requires gnu extensions. This is at odds with our current compiler requirements.

fanquake · 2020-04-16T23:55:04Z

depends/packages/boost.mk

@@ -1,7 +1,7 @@
 package=boost
 $(package)_version=1_70_0
 $(package)_download_path=https://dl.bintray.com/boostorg/release/1.70.0/source/
-$(package)_file_name=$(package)_$($(package)_version).tar.bz2
+$(package)_file_name=boost_$($(package)_version).tar.bz2


This seems leftover from the boost changes. I'd suggest dropping as it would likely just be changed back. i.e: #17928

re: #18677 (comment)

This seems leftover from the boost changes. I'd suggest dropping as it would likely just be changed back. i.e: #17928

Substituting $(package) in variable names seems pretty clearly correct and helps with package inheritance while substituting $(package) in external URLs seems pretty clearly incorrect and breaks package inheritance. But I can make a separate commit or PR for this change if it would be better

doc/multiprocess.md

src/Makefile.am

fanquake · 2020-04-17T01:05:27Z

depends/builders/darwin.mk

-build_darwin_CC:=$(shell xcrun -f clang)
-build_darwin_CXX:=$(shell xcrun -f clang++)
+build_darwin_CC:=clang
+build_darwin_CXX:=clang++


Can you clarify why this is needed? I did look at the commit comments about passing --sysroot and missing includes, however from what I can tell, there should be no change in behaviour with this diff. I did run a few compilation tests and didn't see any obvious difference in compile flags/includes etc.

My assumption is that the problems seen (chaincodelabs/libmultiprocess#5, #16367 (comment)) might have been due to an issue with the development environments i.e missing SDK headers, not installing the CommandLineTools etc.

Mostly curious because if this was an issue it seems like it shouldn't be multiprocess specific? It also seems strange to drop the xcrun call only for clang/clang++, and not for the rest of the tools.

re: #18677 (comment)

Can you clarify why this is needed? I did look at the commit comments about passing --sysroot and missing includes, however from what I can tell, there should be no change in behaviour with this diff. I did run a few compilation tests and didn't see any obvious difference in compile flags/includes etc.

The comment the PR description links to is #16367 (comment). If you follow the instructions there creating test.cpp echo '#include <stdlib.h>' > test.cpp and running /Library/Developer/CommandLineTools/usr/bin/clang++ -c test.cpp, then you can see if the problem affects your system.

It has something to do with the SDK version:

build fails with OSX Mojave neovim/neovim#9050 (comment)

https://discourse.brew.sh/t/clang-can-no-longer-find-usr-include-header-files-fatal-error-stdlib-h-file-not-found/4523

https://stackoverflow.com/questions/52509602/cant-compile-c-program-on-a-mac-after-upgrade-to-mojave/52530212#52530212

My original fix from those comments was ryanofsky@d6921b1, but Cory's suggestion ryanofsky@04bec8d seemed better: just use native compiler for native packages, not the sdk compiler.

My assumption is that the problems seen (chaincodelabs/libmultiprocess#5, #16367 (comment)) might have been due to an issue with the development environments i.e missing SDK headers, not installing the CommandLineTools etc.

Not unless running the shell xcrun -f clang compiler without a sysroot option is supposed to work.

Mostly curious because if this was an issue it seems like it shouldn't be multiprocess specific?

It's not, just a bad interaction with native packages that don't specify --sysroot, which we didn't have previously.

It also seems strange to drop the xcrun call only for clang/clang++, and not for the rest of the tools.

That's a good point. It didn't occur to me to test changing all the other tools paths when I adopted Cory's fix, and at the moment I don't have a convenient way to retest. My original fix ryanofsky@d6921b1 doesn't have this inconsistency, and I'd be happy to switch back to it if it's preferable.

It'd be nice to have automated test coverage for this. It seems curious that the one osx job we have on travis does not test depends, IIUC?

maflcko · 2020-04-17T11:48:15Z

src/Makefile.am

-bitcoind_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
-bitcoind_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS)
+# bitcoind & bitcoin-node binaries #
+bitcoin_common_sources = bitcoind.cpp


Sorry to be bikeshedding this again, but what about naming this node_sources or daemon_sources? As of now it sounds confusingly similar to libbitcoin_common, which is an utility library for all Bitcoin Core util binaries

re: #18677 (comment)

Sorry to be bikeshedding this again, but what about naming this node_sources or daemon_sources? As of now it sounds confusingly similar to libbitcoin_common, which is an utility library for all Bitcoin Core util binaries

Switched to daemon_sources as suggested. Possible this is just bikeshedding with past-Marco #16367 (comment), but I think all the suggestions have been good, and the confusion with libbitcoin_common didn't occur to me before this

ryanofsky

Updated c8672df -> f80a689 (pr/ipc-build2.1 -> pr/ipc-build2.2, compare) with x86_64-apple-darwin16 fixes and suggested automake renames

re: #18677 (review)

Cross-compiling for macOS (on Debian 10.3) with MULTIPROCESS=1 currently fails while building capnp:
make -C depends/ HOST=x86_64-apple-darwin16 MULTIPROCESS=1
...

Good catch, I hadn't tested this configuration. I fixed this and more problems with it in the latest push.

Would you prefer these issues opened here or the libmultiprocess repo?

This was a depends system issue so it made sense here, but in general it can be hard to pin down where issues arise so either place should be fine

Can we pass --with-openssl=no (+ any other relevant flags) to the capnp configure? I assume we aren't using any parts of it that require OpenSSL, and we might as well opt out of any autodetection and/or additional compilation.

I think you'd have to write an openssl depends package and add it to $(package)_dependencies in capnp.mk for there to be anything to detect. But maybe requesting to be built without openssl when openssl isn't available would be a good way to demonstrate not liking openssl

Also, I didn't notice before, but just wanted to note that building capnp currently requires gnu extensions. This is at odds with our current compiler requirements.

I don't think gnu extensions are required, and I don't see any problem building without them (following regular build steps but passing CXXFLAGS=-std=c++14 to configure). Digging into the history of this check it looks like it was added to work around a cygwin bug where snprintf wasn't declared with -std=c++0x, but was with -std=gnu++0x

https://groups.google.com/d/msg/capnproto/l46nHozrhCY/vJTHMkJC-ZEJ
https://groups.google.com/d/msg/capnproto/l46nHozrhCY/UG__SqiVbywJ
https://stackoverflow.com/questions/20149633/how-to-use-snprintf-in-g-std-c11-version-4-8-2

ryanofsky · 2020-04-17T20:23:12Z

depends/builders/darwin.mk

-build_darwin_CC:=$(shell xcrun -f clang)
-build_darwin_CXX:=$(shell xcrun -f clang++)
+build_darwin_CC:=clang
+build_darwin_CXX:=clang++


re: #18677 (comment)

Can you clarify why this is needed? I did look at the commit comments about passing --sysroot and missing includes, however from what I can tell, there should be no change in behaviour with this diff. I did run a few compilation tests and didn't see any obvious difference in compile flags/includes etc.

The comment the PR description links to is #16367 (comment). If you follow the instructions there creating test.cpp echo '#include <stdlib.h>' > test.cpp and running /Library/Developer/CommandLineTools/usr/bin/clang++ -c test.cpp, then you can see if the problem affects your system.

It has something to do with the SDK version:

build fails with OSX Mojave neovim/neovim#9050 (comment)

https://discourse.brew.sh/t/clang-can-no-longer-find-usr-include-header-files-fatal-error-stdlib-h-file-not-found/4523

https://stackoverflow.com/questions/52509602/cant-compile-c-program-on-a-mac-after-upgrade-to-mojave/52530212#52530212

My original fix from those comments was ryanofsky@d6921b1, but Cory's suggestion ryanofsky@04bec8d seemed better: just use native compiler for native packages, not the sdk compiler.

My assumption is that the problems seen (chaincodelabs/libmultiprocess#5, #16367 (comment)) might have been due to an issue with the development environments i.e missing SDK headers, not installing the CommandLineTools etc.

Not unless running the shell xcrun -f clang compiler without a sysroot option is supposed to work.

Mostly curious because if this was an issue it seems like it shouldn't be multiprocess specific?

It's not, just a bad interaction with native packages that don't specify --sysroot, which we didn't have previously.

It also seems strange to drop the xcrun call only for clang/clang++, and not for the rest of the tools.

That's a good point. It didn't occur to me to test changing all the other tools paths when I adopted Cory's fix, and at the moment I don't have a convenient way to retest. My original fix ryanofsky@d6921b1 doesn't have this inconsistency, and I'd be happy to switch back to it if it's preferable.

It'd be nice to have automated test coverage for this. It seems curious that the one osx job we have on travis does not test depends, IIUC?

ryanofsky · 2020-04-17T20:23:22Z

depends/packages/boost.mk

@@ -1,7 +1,7 @@
 package=boost
 $(package)_version=1_70_0
 $(package)_download_path=https://dl.bintray.com/boostorg/release/1.70.0/source/
-$(package)_file_name=$(package)_$($(package)_version).tar.bz2
+$(package)_file_name=boost_$($(package)_version).tar.bz2


re: #18677 (comment)

This seems leftover from the boost changes. I'd suggest dropping as it would likely just be changed back. i.e: #17928

Substituting $(package) in variable names seems pretty clearly correct and helps with package inheritance while substituting $(package) in external URLs seems pretty clearly incorrect and breaks package inheritance. But I can make a separate commit or PR for this change if it would be better

doc/multiprocess.md

src/Makefile.am

ryanofsky · 2020-04-20T17:08:22Z

src/Makefile.am

-bitcoind_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
-bitcoind_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS)
+# bitcoind & bitcoin-node binaries #
+bitcoin_common_sources = bitcoind.cpp


re: #18677 (comment)

Sorry to be bikeshedding this again, but what about naming this node_sources or daemon_sources? As of now it sounds confusingly similar to libbitcoin_common, which is an utility library for all Bitcoin Core util binaries

Switched to daemon_sources as suggested. Possible this is just bikeshedding with past-Marco #16367 (comment), but I think all the suggestions have been good, and the confusion with libbitcoin_common didn't occur to me before this

Catalina SDK clang stopped automatically searching the SDK include paths when invoked without --sysroot: bitcoin#16367 (comment) Homebrew/homebrew-core#45061 This hasn't been a problem for current native depends packages because are passing their own --sysroot values, and hasn't been a problem for current host packages because they use `darwin_` commands instead of `build_darwin_` commands. But the current `build_darwin_CC` and `build_darwin_CXX` commands are still unnecessarily fragile, and incompatible with new native depends packages added in bitcoin#18677. Cory Fields <cory-nospam-@coryfields.com> suggested in bitcoin#16367 (comment) switching compiler from SDK clang to native clang (from $PATH) to avoid this problem. This is easy and makes a certain amount of sense for building native packages, as opposed to host packages. But fanquake <fanquake@gmail.com> pointed out in bitcoin#18677 (comment) that it would be inconsistent use switch to non-SDK compilers while still using other SDK tools like ranlib and install_name_tool. So simplest, minimal fix seems to be just adding the missing --sysroot option.

luke-jr · 2020-04-24T18:45:33Z

I thought we were going to pivot this to just add a -process=node/wallet/gui option to the same bitcoin-qt binary?

ryanofsky · 2020-04-27T19:02:12Z

Updated f80a689 -> fba09de (pr/ipc-build2.2 -> pr/ipc-build2.3, compare) to try to fix travis error https://travis-ci.org/github/bitcoin/bitcoin/jobs/677378707 missing pthread.h
Updated fba09de -> 474f4f1 (pr/ipc-build2.3 -> pr/ipc-build2.4, compare) fixing travis error and moving mac os builder workaround to #18743

re: #18677 (comment)

I thought we were going to pivot this

I think the current naming is better, but I pointed out in the IRC meeting it is easy to change. If it's important, would suggest opening a followup PR and considering this as usability change to discuss ideally after having some experience using the feature

depends: Add --sysroot option to mac os native compile flags Catalina SDK clang stopped automatically searching the SDK include paths when invoked without --sysroot: bitcoin#16367 (comment) Homebrew/homebrew-core#45061 This hasn't been a problem for current native depends packages because are passing their own --sysroot values, and hasn't been a problem for current host packages because they use `darwin_` commands instead of `build_darwin_` commands. But the current `build_darwin_CC` and `build_darwin_CXX` commands are still unnecessarily fragile, and incompatible with new native depends packages added in bitcoin#18677. Cory Fields <cory-nospam-@coryfields.com> suggested in bitcoin#16367 (comment) switching compiler from SDK clang to native clang (from $PATH) to avoid this problem. This is easy and makes a certain amount of sense for building native packages, as opposed to host packages. But fanquake <fanquake@gmail.com> pointed out in bitcoin#18677 (comment) that it would be inconsistent use switch to non-SDK compilers while still using other SDK tools like ranlib and install_name_tool. So simplest, minimal fix seems to be just adding the missing --sysroot option.

configure.ac

src/Makefile.am

hebasto · 2020-05-02T06:51:13Z

7c99f2c:
Maybe add fallback to default value for the TEST_RUNNER_ENV variable in the 00_setup_env.sh script for consistency?

ryanofsky

Updated 474f4f1 -> abf920e (pr/ipc-build2.4 -> pr/ipc-build2.5, compare) with suggestions

Thanks for review!

re: #18677 (comment)

7c99f2c:
Maybe add fallback to default value for the TEST_RUNNER_ENV variable in the 00_setup_env.sh script for consistency?

Added

configure.ac

src/Makefile.am

hebasto

ACK e2bab2a, tested on Linux Mint 19.3 (x86_64):

$ make -C depends NO_QT=1 MULTIPROCESS=1
$ ./autogen.sh
$ CONFIG_SITE=$PWD/depends/x86_64-pc-linux-gnu/share/config.site ./configure
$ make check
$ BITCOIND=bitcoin-node ./test/functional/test_runner.py

Also built depends without NO_QT=1 and tested ./src/bitcoin-gui 👍

Sjors

tACK e2bab2a on macOS 10.15.4

Depends builds now. Without depends still works fine too, using capnp 0.7.0 from homebrew, and the latest libmultiprocess self compiled. Ideally you should tag a libmultiprocess (pre-)release after this lands in master, or right before 0.21 branches off.

I'm no longer able to reproduce a problem with BITCOIND=bitcoin-node test/functional/feature_backwards_compatibility.py

Direct link to IRC meeting log for April 16th: http://www.erisian.com.au/meetbot/bitcoin-core-dev/2020/bitcoin-core-dev.2020-04-16-19.03.log.html#l-243

I thought we were going to pivot this to just add a -process=node/wallet/gui option to the same bitcoin-qt binary?

think the current naming is better, but I pointed out in the IRC meeting it is easy to change.

I think separate binaries is a better approach for now. For development I'd like to have them both, so I don't have to reconfigure to see if a bug is related to the multiprocess stuff. I imagine at some point we'll feel comfortable shipping the multi-process binaries, with their dependencies, but not comfortable enough to make them the default.

Sjors · 2020-05-19T11:21:44Z

depends/packages/native_libmultiprocess.mk

@@ -0,0 +1,18 @@
+package=native_libmultiprocess
+$(package)_version=5741d750a04e644a03336090d8979c6d033e32c0


If you need to change anything else: maybe bump to 9f5b8355649aca94b4b529d89ba439aa3b2f9642 for consistency with building from master (but the new commits shouldn't be relevant here)

Sjors · 2020-05-19T12:14:50Z

depends/packages/native_capnp.mk

+$(package)_download_file=capnproto-c++-$($(package)_version).tar.gz
+$(package)_file_name=capnproto-cxx-$($(package)_version).tar.gz
+$(package)_sha256_hash=c9a4c0bd88123064d483ab46ecee777f14d933359e23bff6fb4f4dbd28b4cd41
+


As @fanquake suggested elsewhere, you could add this:

define $(package)_set_vars $(package)_config_opts=--without-openssl endef

Note that at least on macOS it won't pick up my homebrew installed openssl@1.1: stable 1.1.1g even when I use --with-openssl, but I think it's worth explicitly opting out anyway.

You mentioned:

But maybe requesting to be built without openssl when openssl isn't available would be a good way to demonstrate not liking openssl

It's also consistent with the practice in other dependencies to actively opt out of stuff (see e.g. #17730, #16370)

re: #18677 (comment)

It's also consistent with the practice in other dependencies to actively opt out of stuff (see e.g. #17730, #16370)

Thanks, I will make that change in a followup, or here if another update is required (reluctant to make changes now it seems like all the build problems so far have been ironed out).

At the time I made that comment about OpenSSL I mistakenly thought the depends build was more hermetic, and that it wouldn't pick up system dependencies in host package builds. This was before I knew about #18915

ryanofsky · 2020-05-19T13:52:49Z

I think it might be a good time to merge this PR. It seems to be at a point where it is working for the people who've tested it on different platforms. There are definitely improvements to be made going forward, but we know build changes are fragile and a fix for one bug tends to lead to other bugs, so it would be great to be done with this big PR when it's in a pretty good state, and start to track future issues in smaller more focused PRs.

Sjors · 2020-05-19T15:24:17Z

Maybe a Gitian build to top it off?

1d9040e1d0e33a0aeebc2ee3a3be56b20df9fe985acc90be8d7d4aa7e3909356  bitcoin-3aec27edc9d4-aarch64-linux-gnu.tar.gz
3cb8c621421daf7de5cf02004a88eb462d0dcc855f0235cc40f01e65ea9dd115  bitcoin-3aec27edc9d4-win64-setup-unsigned.exe
baed7424d10644a1ff7443801a1aaaa7326ad37fdf587a098950e05c0fbc7ad1  bitcoin-3aec27edc9d4-osx-unsigned.dmg

fanquake · 2020-05-20T11:12:47Z

I think it might be a good time to merge this PR.

Thanks. I'm planning on merging this shortly.

practicalswift · 2020-05-20T15:21:28Z

ACK e2bab2a

Thanks for doing this excellent architectural work: a dream come true (even if optional)!

With this dream off the table I think I only have Sandboxed API integration, control-flow integrity and killing the UUM bug class for good using -ftrivial-auto-var-init=zero left to dream about from a security perspective :)

Thanks @ryanofsky: you're great, and so is your patience -- this has been a very long road :)

DrahtBot · 2020-05-21T04:55:59Z

Gitian builds

File	commit `0aa2ff0` (master)	commit `b8eebc5` (master and this pull)
*-aarch64-linux-gnu-debug.tar.gz	`f4e97320d67e9f9d...`	`ae39b2293d95e03f...`
*-aarch64-linux-gnu.tar.gz	`708dd8c7998c93bb...`	`0f85de77dcb78b72...`
*-arm-linux-gnueabihf-debug.tar.gz	`8306690022f814f2...`	`67e97aae8e4cf93b...`
*-arm-linux-gnueabihf.tar.gz	`027f84bd9489b511...`	`044ea5779bc42b8d...`
*-osx-unsigned.dmg	`02a0b8f2ba11dfa4...`	`0e12b3b08e9a993f...`
*-osx64.tar.gz	`52e97780af471514...`	`3d3a0cb246ec2fce...`
*-riscv64-linux-gnu-debug.tar.gz	`cba762eed7d969a3...`	`2801f933cf057b6b...`
*-riscv64-linux-gnu.tar.gz	`2c732808ce42f68c...`	`04e4d5ab4627cc1f...`
*-win64-debug.zip	`f947a4d958558143...`	`40ed56707d48b50e...`
*-win64-setup-unsigned.exe	`cda69226dcaaa83c...`	`bd1b04bf38fce1d0...`
*-win64.zip	`b5f95d502e377110...`	`274344dd485b3896...`
*-x86_64-linux-gnu-debug.tar.gz	`9a26cbd755166033...`	`3d1b6e10e5228a55...`
*-x86_64-linux-gnu.tar.gz	`d2d737f786ee7a7b...`	`d1e526a4024cf79b...`
*.tar.gz	`d0b818b8368fb371...`	`57e4c4fb9899e323...`
bitcoin-core-linux-0.21-res.yml	`b6f91507a347a74f...`	`7cd851b20104fca5...`
bitcoin-core-osx-0.21-res.yml	`e2fe3f4cfb0c2efe...`	`30368228d2d82424...`
bitcoin-core-win-0.21-res.yml	`a587bff12db575a7...`	`9a1db49db80d364d...`
linux-build.log	`bc5a88fd2de120fe...`	`af81c60b53c76c59...`
osx-build.log	`d36c40220263504e...`	`9c0599a713ab0521...`
win-build.log	`d3d4007978e2e2b2...`	`3825b58385bba7d1...`
bitcoin-core-linux-0.21-res.yml.diff		`857dd69b4ceed69b...`
bitcoin-core-osx-0.21-res.yml.diff		`245076a96f4893ee...`
bitcoin-core-win-0.21-res.yml.diff		`62b1aeb2ec857d8d...`
linux-build.log.diff		`7fd8b3b45ed65662...`
osx-build.log.diff		`e3f930c3cc1a04cd...`
win-build.log.diff		`b7e5e7f383c24859...`

maflcko · 2020-05-21T13:23:20Z

Checked that the ci config file is indeed native and runs on aarch64:

Options used to compile and link:
  multiprocess  = yes
  with wallet   = yes
  with gui / qt = yes
    with qr     = yes
  with zmq      = yes
  with test     = yes
    with fuzz   = no
  with bench    = yes
  with upnp     = yes
  use asm       = yes
  sanitizers    = 
  debug enabled = no
  gprof enabled = no
  werror        = no

  target os     = linux
  build os      = 

  CC            = /usr/bin/ccache gcc
  CFLAGS        = -pipe -O2 
  CPPFLAGS      =   -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -I/bitcoin-core/depends/aarch64-unknown-linux-gnu/share/../include/  -DHAVE_BUILD_INFO -D__STDC_FORMAT_MACROS
  CXX           = /usr/bin/ccache g++ -std=c++11
  CXXFLAGS      =   -fstack-reuse=none -Wstack-protector -fstack-protector-all     -pipe -O2 
  LDFLAGS       = -pthread  -Wl,-z,relro -Wl,-z,now -pie  -L/bitcoin-core/depends/aarch64-unknown-linux-gnu/share/../lib 
  ARFLAGS       = cr

e2bab2a multiprocess: add multiprocess travis configuration (Russell Yanofsky) 603fd6a depends: add MULTIPROCESS depends option (Russell Yanofsky) 5d1377b build: multiprocess autotools changes (Russell Yanofsky) Pull request description: This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10). --- This PR consists of build changes only. It adds an `--enable-multiprocess` autoconf option (off by default and marked experimental), that builds new `bitcoin-node` and `bitcoin-gui` binaries. These currently function the same as existing `bitcoind` and `bitcoin-qt` binaries, but are extended in bitcoin#10102 with IPC features to execute node, wallet, and gui functions in separate processes. In addition to adding the `--enable-multiprocess` config flag, it also adds a depends package and autoconf rules to build with the [libmultiprocess](https://github.com/chaincodelabs/libmultiprocess) library, and it adds new travis configuration to exercise the build code and run functional tests with the new binaries. The changes in this PR were originally part of bitcoin#10102 but were moved into bitcoin#16367 to be able to develop and review the multiprocess build changes independently of the code changes. bitcoin#16367 was briefly merged and then reverted in bitcoin#18588. Only change since bitcoin#16367 has been dropping the `native_boost.mk` depends package which was pointed out to be no longer necessary in bitcoin#16367 (comment) and bitcoin#18588 (review) ACKs for top commit: practicalswift: ACK e2bab2a Sjors: tACK e2bab2a on macOS 10.15.4 hebasto: ACK e2bab2a, tested on Linux Mint 19.3 (x86_64): Tree-SHA512: b5a76eab5abf63d9d8b6d628cbdff4cc1888eef15cafa0a5d56369e2f9d02595fed623f4b74b2cf2830c42c05a774f0943e700f9c768a82d9d348cad199e135c

Summary: ``` Catalina SDK clang stopped automatically searching the SDK include paths when invoked without --sysroot: bitcoin/bitcoin#16367 (comment) Homebrew/homebrew-core#45061 This hasn't been a problem for current native depends packages because are passing their own --sysroot values, and hasn't been a problem for current host packages because they use `darwin_` commands instead of `build_darwin_` commands. But the current `build_darwin_CC` and `build_darwin_CXX` commands are still unnecessarily fragile, and incompatible with new native depends packages added in bitcoin/bitcoin#18677. Cory Fields <cory-nospam-@coryfields.com> suggested in bitcoin/bitcoin#16367 (comment) switching compiler from SDK clang to native clang (from $PATH) to avoid this problem. This is easy and makes a certain amount of sense for building native packages, as opposed to host packages. But fanquake <fanquake@gmail.com> pointed out in bitcoin/bitcoin#18677 (comment) that it would be inconsistent use switch to non-SDK compilers while still using other SDK tools like ranlib and install_name_tool. So simplest, minimal fix seems to be just adding the missing --sysroot option. ``` Backport of core [[bitcoin/bitcoin#18743 | PR18743]]. Note that it doesn't fix any real life problem for now. Test Plan: Run the OSX Gitian build. Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D7522

Catalina SDK clang stopped automatically searching the SDK include paths when invoked without --sysroot: bitcoin/bitcoin#16367 (comment) Homebrew/homebrew-core#45061 This hasn't been a problem for current native depends packages because are passing their own --sysroot values, and hasn't been a problem for current host packages because they use `darwin_` commands instead of `build_darwin_` commands. But the current `build_darwin_CC` and `build_darwin_CXX` commands are still unnecessarily fragile, and incompatible with new native depends packages added in bitcoin/bitcoin#18677. Cory Fields <cory-nospam-@coryfields.com> suggested in bitcoin/bitcoin#16367 (comment) switching compiler from SDK clang to native clang (from $PATH) to avoid this problem. This is easy and makes a certain amount of sense for building native packages, as opposed to host packages. But fanquake <fanquake@gmail.com> pointed out in bitcoin/bitcoin#18677 (comment) that it would be inconsistent use switch to non-SDK compilers while still using other SDK tools like ranlib and install_name_tool. So simplest, minimal fix seems to be just adding the missing --sysroot option.

Catalina SDK clang stopped automatically searching the SDK include paths when invoked without --sysroot: bitcoin#16367 (comment) Homebrew/homebrew-core#45061 This hasn't been a problem for current native depends packages because are passing their own --sysroot values, and hasn't been a problem for current host packages because they use `darwin_` commands instead of `build_darwin_` commands. But the current `build_darwin_CC` and `build_darwin_CXX` commands are still unnecessarily fragile, and incompatible with new native depends packages added in bitcoin#18677. Cory Fields <cory-nospam-@coryfields.com> suggested in bitcoin#16367 (comment) switching compiler from SDK clang to native clang (from $PATH) to avoid this problem. This is easy and makes a certain amount of sense for building native packages, as opposed to host packages. But fanquake <fanquake@gmail.com> pointed out in bitcoin#18677 (comment) that it would be inconsistent use switch to non-SDK compilers while still using other SDK tools like ranlib and install_name_tool. So simplest, minimal fix seems to be just adding the missing --sysroot option.

…ompile flags 1e94a2b depends: Add --sysroot option to mac os native compile flags (Russell Yanofsky) Pull request description: Catalina SDK clang stopped automatically searching the SDK include paths when invoked without `--sysroot`: - bitcoin#16367 (comment) - Homebrew/homebrew-core#45061 This hasn't been a problem for current native depends packages because are passing their own `--sysroot` values, and hasn't been a problem for current host packages because they use `darwin_` commands instead of `build_darwin_` commands. But the current `build_darwin_CC` and `build_darwin_CXX` commands are still unnecessarily fragile, and incompatible with new native depends packages added in bitcoin#18677. Cory Fields (theuni) suggested in bitcoin#16367 (comment) switching compiler from SDK clang to native clang (from $PATH) to avoid this problem. This is easy and makes a certain amount of sense for building native packages, as opposed to host packages. But Michael (fanquake) pointed out in bitcoin#18677 (comment) that it would be inconsistent to switch to non-SDK compilers while still using other SDK tools like `ranlib` and `install_name_tool`. So simplest, minimal fix seems to be just adding the missing `--sysroot` option. ACKs for top commit: ryanofsky: > ACK [1e94a2b](bitcoin@1e94a2b) - I think this change is ok, and I prefer it to the previous patch. fanquake: ACK 1e94a2b - I think this change is ok, and I prefer it to the previous patch. Thanks for the summary in the PR description. I played around with Xcode and the CLT; I think previously I didn't fully grok the slight differences between the two. Tree-SHA512: 4d4bbb7f49acb76d934a872a15b4e14f36290b508cb9e728815f959767ec174bcfb6d2ca7dcd995cc550d86980d64d4247ea5ecfca2301f0953006e50744fdb4

The bare minimum necessary change for bitcoin#19764. Marked as "partial merge" for attribution only, otherwise considered unmerged

laanwj added the Build system label Apr 16, 2020

This was referenced Apr 17, 2020

net: Add NAT-PMP port forwarding support #18077

Merged

depends: update to Boost 1.72 #17941

Closed

fanquake reviewed Apr 17, 2020

View reviewed changes

This was referenced Apr 17, 2020

util: add RunCommandParseJSON #15382

Merged

Multiprocess bitcoin #10102

Draft

maflcko reviewed Apr 17, 2020

View reviewed changes

ryanofsky force-pushed the pr/ipc-build2 branch from c8672df to f80a689 Compare April 20, 2020 18:23

ryanofsky commented Apr 20, 2020

View reviewed changes

This was referenced Apr 22, 2020

Erlay: bandwidth-efficient transaction relay protocol #18261

Closed

build: Suppress -Wdeprecated-copy warnings #18738

Merged

ryanofsky mentioned this pull request Apr 22, 2020

depends: Add --sysroot option to mac os native compile flags #18743

Merged

ryanofsky force-pushed the pr/ipc-build2 branch from f80a689 to fba09de Compare April 22, 2020 21:52

This was referenced Apr 23, 2020

depends: Allow building with system clang #17919

Merged

External signer support - Wallet Box edition #16546

Merged

ryanofsky force-pushed the pr/ipc-build2 branch from fba09de to 474f4f1 Compare April 23, 2020 19:54

DrahtBot mentioned this pull request Apr 23, 2020

UI external signer support (e.g. hardware wallet) #16549

Closed

hebasto reviewed May 2, 2020

View reviewed changes

configure.ac Show resolved Hide resolved

hebasto reviewed May 2, 2020

View reviewed changes

src/Makefile.am Show resolved Hide resolved

ryanofsky force-pushed the pr/ipc-build2 branch from 474f4f1 to abf920e Compare May 6, 2020 16:15

ryanofsky commented May 6, 2020

View reviewed changes

configure.ac Show resolved Hide resolved

src/Makefile.am Show resolved Hide resolved

This was referenced May 6, 2020

rpc: prevent circular deps by extracting into separate include #18871

Closed

travis: Remove valgrind #18899

Merged

ryanofsky force-pushed the pr/ipc-build2 branch from e488518 to e2bab2a Compare May 13, 2020 01:12

ryanofsky mentioned this pull request May 14, 2020

macOS: Evaluate removing xcruns in darwin builder #18959

Closed

DrahtBot mentioned this pull request May 15, 2020

build: Decouple clientversion.cpp from the git repo #18980

Closed

hebasto approved these changes May 16, 2020

View reviewed changes

Sjors approved these changes May 19, 2020

View reviewed changes

maflcko added DrahtBot Guix build requested Needs gitian build labels May 19, 2020

DrahtBot mentioned this pull request May 19, 2020

test: Replace TEST_PREVIOUS_RELEASES env var with test_framework option #19014

Merged

DrahtBot removed the Needs gitian build label May 21, 2020

fanquake merged commit 97b21b3 into bitcoin:master May 21, 2020

maflcko removed the DrahtBot Guix build requested label May 31, 2020

ryanofsky added this to Done in Process Separation Jun 3, 2020

hebasto mentioned this pull request Jun 23, 2020

build: Require pkg-config for all of the hosts #18307

Merged

kwvg added a commit to kwvg/dash that referenced this pull request Aug 27, 2021

partial merge bitcoin#18677: Multiprocess build support

a003178

The bare minimum necessary change for bitcoin#19764. Marked as "partial merge" for attribution only, otherwise considered unmerged

str4d mentioned this pull request Sep 15, 2021

Add CMake to depends system zcash/zcash#5301

Merged

fanquake mentioned this pull request Oct 20, 2021

Add minisketch subtree and integrate into build/test #23114

Merged

bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Multiprocess build support #18677

Multiprocess build support #18677

ryanofsky commented Apr 16, 2020

maflcko commented Apr 16, 2020

DrahtBot commented Apr 16, 2020 •

edited

fanquake left a comment

fanquake Apr 16, 2020

ryanofsky Apr 17, 2020

fanquake Apr 17, 2020

ryanofsky Apr 17, 2020

maflcko Apr 17, 2020

ryanofsky Apr 20, 2020

ryanofsky left a comment

ryanofsky Apr 17, 2020

ryanofsky Apr 17, 2020

ryanofsky Apr 20, 2020

luke-jr commented Apr 24, 2020

ryanofsky commented Apr 27, 2020

hebasto commented May 2, 2020

ryanofsky left a comment

hebasto left a comment

Sjors left a comment

Sjors May 19, 2020

Sjors May 19, 2020

ryanofsky May 19, 2020

ryanofsky commented May 19, 2020

Sjors commented May 19, 2020 •

edited

fanquake commented May 20, 2020

practicalswift commented May 20, 2020 •

edited

DrahtBot commented May 21, 2020

maflcko commented May 21, 2020

		@@ -0,0 +1,18 @@
		package=native_libmultiprocess
		$(package)_version=5741d750a04e644a03336090d8979c6d033e32c0

Multiprocess build support #18677

Multiprocess build support #18677

Conversation

ryanofsky commented Apr 16, 2020

maflcko commented Apr 16, 2020

DrahtBot commented Apr 16, 2020 • edited

Conflicts

fanquake left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryanofsky left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luke-jr commented Apr 24, 2020

ryanofsky commented Apr 27, 2020

hebasto commented May 2, 2020

ryanofsky left a comment

Choose a reason for hiding this comment

hebasto left a comment

Choose a reason for hiding this comment

Sjors left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryanofsky commented May 19, 2020

Sjors commented May 19, 2020 • edited

fanquake commented May 20, 2020

practicalswift commented May 20, 2020 • edited

DrahtBot commented May 21, 2020

Gitian builds

maflcko commented May 21, 2020

DrahtBot commented Apr 16, 2020 •

edited

Sjors commented May 19, 2020 •

edited

practicalswift commented May 20, 2020 •

edited