-
Notifications
You must be signed in to change notification settings - Fork 38.7k
multiprocess build fix: ipc/capnp/init.capnp.h: No such file or directory #25214
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…tory Error was reported by SatoriHoshiAiko in bitcoin#25207 and happens unpredictably because make doesn't always build dependencies in the same order. The source file src/ipc/capnp/protocol.cpp includes some generated headers so needs to have an explicit dependency specified in the makefile so the headers will be generated before the file is compiled. bitcoin#19160 added the explicit dependency, but it was incorrect because it referred to an old file path from before the source file was renamed (ipc.cpp -> protocol.cpp)
jonatack
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Concept ACK
Noted warning in linked section: "Adding explicit dependencies like this can be a bit dangerous if you are not careful enough. This is due to the way Automake tries not to overwrite your rules (it assumes you know better than it)."
src/Makefile.am
Outdated
| libbitcoin_util_a-clientversion.$(OBJEXT): obj/build.h | ||
|
|
||
| ipc/capnp/libbitcoin_ipc_a-ipc.$(OBJEXT): $(libbitcoin_ipc_mpgen_input:=.h) | ||
| # Explicitly list dependencies on generated headers as described |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if I understand your intent
| # Explicitly list dependencies on generated headers as described | |
| # Explicitly list dependencies on generated headers as described in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hebasto
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 4063a06, tested on Ubuntu 22.04 with
$ make -C depends MULTIPROCESS=1 NO_QT=1 NO_ZMQ=1 NO_WALLET=1 NO_UPNP=1 NO_NATPMP=1
$ ./autogen.sh
$ ./configure CONFIG_SITE=$PWD/depends/x86_64-pc-linux-gnu/share/config.site
$ make clean
$ make -C src ipc/capnp/libbitcoin_ipc_a-protocol.o
make: Entering directory '/home/hebasto/GitHub/bitcoin/src'
GEN ipc/capnp/init.capnp.h
GEN ipc/capnp/echo.capnp.h
CXX ipc/capnp/libbitcoin_ipc_a-protocol.o
make: Leaving directory '/home/hebasto/GitHub/bitcoin/src'
Indeed, this change fixes a "typo" in the object file name as the libbitcoin_ipc.a library does not contain ipc.cpp, rather protocol.cpp
Lines 1019 to 1029 in 4063a06
| libbitcoin_ipc_a_SOURCES = \ | |
| ipc/capnp/context.h \ | |
| ipc/capnp/init-types.h \ | |
| ipc/capnp/protocol.cpp \ | |
| ipc/capnp/protocol.h \ | |
| ipc/context.h \ | |
| ipc/exception.h \ | |
| ipc/interfaces.cpp \ | |
| ipc/process.cpp \ | |
| ipc/process.h \ | |
| ipc/protocol.h |
But still curios about two points.
- Would it better to move this rule closer to ?
Lines 1011 to 1013 in 4063a06
libbitcoin_ipc_mpgen_input = \ ipc/capnp/echo.capnp \ ipc/capnp/init.capnp - Why builds are mostly successful on the current master branch?
Also move manual dependency closer to actual build target
ryanofsky
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated 4063a06 -> fe3d3a8 (pr/mpdep.1 -> pr/mpdep.2, compare) with some suggested changes. Also deleted unused %.capnp: rule
re: #25214 (review)
- Would it better to move this rule closer to
Lines 1011 to 1013 in 4063a06
libbitcoin_ipc_mpgen_input = \ ipc/capnp/echo.capnp \ ipc/capnp/init.capnp
Yes I agree that is probably better, so moved it now. Before it was trying to be close to the libbitcoin_util_a-clientversion.$(OBJEXT) rule, which was a similar manual rule, but not really related.
- Why builds are mostly successful on the current master branch?
I think it's just because make is normally pretty deterministic. Also because there are a number of generated .cpp files that make knows it needs to compile, and as long as generates any one of these files before it tries to compile protocol.cpp, the build will succeed because all the generated files are generated by one command.
src/Makefile.am
Outdated
| libbitcoin_util_a-clientversion.$(OBJEXT): obj/build.h | ||
|
|
||
| ipc/capnp/libbitcoin_ipc_a-ipc.$(OBJEXT): $(libbitcoin_ipc_mpgen_input:=.h) | ||
| # Explicitly list dependencies on generated headers as described |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
Not able to reproduce the CI failure locally, but I think the fix is to move all multiprocess-related pieces in |
Does it cause CI failures? |
|
Updated fe3d3a8 -> 8135b58 ( https://cirrus-ci.com/task/4893644023922688 re: #25214 (comment)
CI is actually failing in the cases where BUILD_MULTIPROCESS is false re: #25214 (comment)
Yes thanks this is probably the cause |
|
Updated 8135b58 -> 44904aa ( |
hebasto
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re-ACK 44904aa
|
Guix Build: 31193ef2e731887c640d8a4b29086e9f8fd61d53da36e3f6ef652e185997f92b guix-build-44904aa632cb/output/arm-linux-gnueabihf/SHA256SUMS.part
f2cb3314c3031d8c1b8c2070de374d6b5365f6918eb06a228f23e76715912edf guix-build-44904aa632cb/output/arm-linux-gnueabihf/bitcoin-44904aa632cb-arm-linux-gnueabihf-debug.tar.gz
ac2cb000c98c3065c9626101c7e0f29a34dbd672842907cc1883334a723c44c9 guix-build-44904aa632cb/output/arm-linux-gnueabihf/bitcoin-44904aa632cb-arm-linux-gnueabihf.tar.gz
ad3953b224989767d2ac8b5a760758bd7c43d6782232aeff5d39f3025b5851b9 guix-build-44904aa632cb/output/arm64-apple-darwin/SHA256SUMS.part
2df81fccf4025abbe530c3a8926c542752f5cb871c5e8206cdb0e47644af2118 guix-build-44904aa632cb/output/arm64-apple-darwin/bitcoin-44904aa632cb-arm64-apple-darwin-unsigned.dmg
073eb4e0ffab4e6af81b743b540ca7996b012780fb0924e98e1fdac69b2b47aa guix-build-44904aa632cb/output/arm64-apple-darwin/bitcoin-44904aa632cb-arm64-apple-darwin-unsigned.tar.gz
ab911354f6a25d4c347ddd52c87d5cf9cce0a9ccff7ac4fc34506da1d0bcdc4d guix-build-44904aa632cb/output/arm64-apple-darwin/bitcoin-44904aa632cb-arm64-apple-darwin.tar.gz
4e21f8360717394995348f63558018fab1a301be1599baff640740379c44b644 guix-build-44904aa632cb/output/dist-archive/bitcoin-44904aa632cb.tar.gz
eed85b7686fe4dd41a7819e394fa3b76c5e045b5b2f2c47dd3ffa213dfc2e6b3 guix-build-44904aa632cb/output/powerpc64-linux-gnu/SHA256SUMS.part
fce8db89918479a4b4271e74efcdec4a3daa7d84831621828e91d0a3aad0e4d4 guix-build-44904aa632cb/output/powerpc64-linux-gnu/bitcoin-44904aa632cb-powerpc64-linux-gnu-debug.tar.gz
2565f9be3d193a2df7c31361322209f99933f0c6f5e02aab3101a410ac02a37f guix-build-44904aa632cb/output/powerpc64-linux-gnu/bitcoin-44904aa632cb-powerpc64-linux-gnu.tar.gz
ed5c1994a9452ce735f5b765876b5e39f4abe09b6ecc0ec9ffed1282a1e20c4a guix-build-44904aa632cb/output/powerpc64le-linux-gnu/SHA256SUMS.part
dfaf5fe341f7d924f36a3e753711d5aa4433dbb9b9ffbb5eb149565695a3f8d3 guix-build-44904aa632cb/output/powerpc64le-linux-gnu/bitcoin-44904aa632cb-powerpc64le-linux-gnu-debug.tar.gz
a8a7a0cc750d38231d46aa7cbc9d986c18b69b24764766ad1ab68696131a295a guix-build-44904aa632cb/output/powerpc64le-linux-gnu/bitcoin-44904aa632cb-powerpc64le-linux-gnu.tar.gz
033f456cc8cb899a4253f037fa09825565dbb4f08450e0ac3ffc8ec81dc4d8a1 guix-build-44904aa632cb/output/riscv64-linux-gnu/SHA256SUMS.part
fcbb0829fd2bb46103df2ba4aea2f30dea5ba7b7d2094091aa2bcd9ec23c4687 guix-build-44904aa632cb/output/riscv64-linux-gnu/bitcoin-44904aa632cb-riscv64-linux-gnu-debug.tar.gz
1e7d4bab3bd3487a6452068ffcf5e9f77f58eee6f3e449b7f7d4f8a379852f68 guix-build-44904aa632cb/output/riscv64-linux-gnu/bitcoin-44904aa632cb-riscv64-linux-gnu.tar.gz
87e77865b399211c5bf0ce3758647d7c303e0b3b5965cf34f4f889ae0aa916df guix-build-44904aa632cb/output/x86_64-apple-darwin/SHA256SUMS.part
c5c9746a0b4f6b9c2b3ca041de34460d50a99388f49180c1e2bafde9d395b66d guix-build-44904aa632cb/output/x86_64-apple-darwin/bitcoin-44904aa632cb-x86_64-apple-darwin-unsigned.dmg
49be50427d0dba8dc557d8e294344599d6564191db6a28da18fb48ab307c104a guix-build-44904aa632cb/output/x86_64-apple-darwin/bitcoin-44904aa632cb-x86_64-apple-darwin-unsigned.tar.gz
899bc6526fffec73cf05f1bbef45bb56d3feecab90e4fd99852a570046b3d33c guix-build-44904aa632cb/output/x86_64-apple-darwin/bitcoin-44904aa632cb-x86_64-apple-darwin.tar.gz
c237eff5c2c267acdb4a9f4dccf0cbc605bfd666f137310ee429550fa55cbe08 guix-build-44904aa632cb/output/x86_64-linux-gnu/SHA256SUMS.part
9107bc6931949d4797ff5796a088ceeab0097d777461aef8c24ed4f824407aa4 guix-build-44904aa632cb/output/x86_64-linux-gnu/bitcoin-44904aa632cb-x86_64-linux-gnu-debug.tar.gz
cd2e8526be24ec10eb2dd51c5d1dd449e90526036d9c935f2205f701a7798a5c guix-build-44904aa632cb/output/x86_64-linux-gnu/bitcoin-44904aa632cb-x86_64-linux-gnu.tar.gz
3785f386cc60ec273bef5aa7417da1a9bb2ee681ac08a7240eabbcd4698f1290 guix-build-44904aa632cb/output/x86_64-w64-mingw32/SHA256SUMS.part
fd2dedcb4f47c3d69b2da5ca90fbabe2ccb225857010e30b5cbf212384350478 guix-build-44904aa632cb/output/x86_64-w64-mingw32/bitcoin-44904aa632cb-win64-debug.zip
17b6ccde0d29d3355bf4455ba5ced92abd2ea4b595b5b85e444a02772a55fec5 guix-build-44904aa632cb/output/x86_64-w64-mingw32/bitcoin-44904aa632cb-win64-setup-unsigned.exe
9e3fcbb19ace957858dd00bce93bec486b58414b87d14e888b99e3d910af1fae guix-build-44904aa632cb/output/x86_64-w64-mingw32/bitcoin-44904aa632cb-win64-unsigned.tar.gz
6144326fcc219feb32d06cc1baa3f66ddce83fac019ae9398271bf8ce929bec8 guix-build-44904aa632cb/output/x86_64-w64-mingw32/bitcoin-44904aa632cb-win64.zip |
Error was reported by SatoriHoshiAiko in #25207 and happens unpredictably because make doesn't always build dependencies in the same order.
The source file
src/ipc/capnp/protocol.cppincludes some generated headers so needs to have an explicit dependency specified in the makefile so the headers will be generated before the file is compiled. #19160 added the explicit dependency, but it was incorrect because it referred to an old file path from before the source file was renamed (ipc.cpp->protocol.cpp)