Skip to content

Conversation

@ryanofsky
Copy link
Contributor

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.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. #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)

…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)
Copy link
Member

@jonatack jonatack left a 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
Copy link
Member

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

Suggested change
# Explicitly list dependencies on generated headers as described
# Explicitly list dependencies on generated headers as described in

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re: #25214 (comment)

if I understand your intent

Applied, thanks!

Copy link
Member

@hebasto hebasto left a 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

bitcoin/src/Makefile.am

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.

  1. Would it better to move this rule closer to

    bitcoin/src/Makefile.am

    Lines 1011 to 1013 in 4063a06

    libbitcoin_ipc_mpgen_input = \
    ipc/capnp/echo.capnp \
    ipc/capnp/init.capnp
    ?
  2. Why builds are mostly successful on the current master branch?

Also move manual dependency closer to actual build target
@ryanofsky ryanofsky changed the title libmultiprocess build fix: ipc/capnp/init.capnp.h: No such file or directory multiprocess build fix: ipc/capnp/init.capnp.h: No such file or directory May 25, 2022
Copy link
Contributor Author

@ryanofsky ryanofsky left a 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)

  1. Would it better to move this rule closer to

    bitcoin/src/Makefile.am

    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.

  1. 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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re: #25214 (comment)

if I understand your intent

Applied, thanks!

@dongcarl
Copy link
Contributor

Not able to reproduce the CI failure locally, but I think the fix is to move all multiprocess-related pieces in src/Makefile.am under the if BUILD_MULTIPROCESS guard?

@hebasto
Copy link
Member

hebasto commented May 26, 2022

Also deleted unused %.capnp: rule

Does it cause CI failures?

@ryanofsky
Copy link
Contributor Author

ryanofsky commented May 26, 2022

Updated fe3d3a8 -> 8135b58 (pr/mpdep.2 -> pr/mpdep.3, compare) to fix build failures in all builds except the multiprocess build probably caused by removing the %.capnp: rule (which I have no idea why since the rule should be unused)

make[3]: *** No rule to make target 'ipc/capnp/echo.capnp.c++', needed by 'ipc/capnp/echo.capnp.o'.  Stop.

https://cirrus-ci.com/task/4893644023922688
https://cirrus-ci.com/task/5175119000633344
https://cirrus-ci.com/task/6301018907475968
https://cirrus-ci.com/task/4752906535567360
https://cirrus-ci.com/task/6582493884186624
https://cirrus-ci.com/task/5593313590902784
https://cirrus-ci.com/task/5878806442409984
https://cirrus-ci.com/task/6441756395831296
https://cirrus-ci.com/task/6019543930765312
https://cirrus-ci.com/task/5034381512278016
https://cirrus-ci.com/task/6160281419120640
https://cirrus-ci.com/task/4612169047212032

re: #25214 (comment)

Not able to reproduce the CI failure locally, but I think the fix is to move all multiprocess-related pieces in src/Makefile.am under the if BUILD_MULTIPROCESS guard?

CI is actually failing in the cases where BUILD_MULTIPROCESS is false

re: #25214 (comment)

Also deleted unused %.capnp: rule

Does it cause CI failures?

Yes thanks this is probably the cause

@ryanofsky
Copy link
Contributor Author

Updated 8135b58 -> 44904aa (pr/mpdep.3 -> pr/mpdep.4, compare) just fixing commit message

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re-ACK 44904aa

@fanquake
Copy link
Member

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

@fanquake fanquake merged commit 345457b into bitcoin:master May 27, 2022
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 28, 2022
@DrahtBot
Copy link
Contributor

Guix builds

File commit 66bb4df
(master)
commit 22274858b114a9075aa1238c61982fc21829fd8d
(master and this pull)
SHA256SUMS.part e4bdbb88e3275eb2... a517fc0b0846ec82...
*-aarch64-linux-gnu-debug.tar.gz de78977a26391ae7... c011b045905b22e5...
*-aarch64-linux-gnu.tar.gz a98d33a6b93e072b... 745c65306ec119ee...
*-arm-linux-gnueabihf-debug.tar.gz e5eacac3c8892aaf... 4beaa5658dbc193c...
*-arm-linux-gnueabihf.tar.gz 3a7ef199e6bbd841... 45e7e43256a24d85...
*-arm64-apple-darwin-unsigned.dmg 0363b4aa9d46e8c7... d471679f19a75960...
*-arm64-apple-darwin-unsigned.tar.gz bda5465d94196f77... db833a11a9d3de55...
*-arm64-apple-darwin.tar.gz 9af9640b6821f36b... 2982d77ce8092e92...
*-powerpc64-linux-gnu-debug.tar.gz 88de93bffed22128... b61788a7a89247d5...
*-powerpc64-linux-gnu.tar.gz 3ac311cb66c19972... fef6115030f5d4c1...
*-powerpc64le-linux-gnu-debug.tar.gz 0e28a3de4fd1c5be... 685ad7f0e1f73e86...
*-powerpc64le-linux-gnu.tar.gz bb22a321067dab13... 49ab46361a8093ce...
*-riscv64-linux-gnu-debug.tar.gz 0d4ecbe3faea1bfe... 0a90e22f74488ce8...
*-riscv64-linux-gnu.tar.gz e9786bb6e9dfadf6... d967ce5d8ad84116...
*-win64-debug.zip ae171a04482bf017... 4b722a9af96cc529...
*-win64-setup-unsigned.exe feb6400df7006b8b... 95c40678f1fba83d...
*-win64-unsigned.tar.gz 29f70c014d6171b5... 028edece798118dc...
*-win64.zip 77908ae9514744f8... 9f4b5cbcb2b53ae3...
*-x86_64-apple-darwin-unsigned.dmg 71ae49f3502b8f62... 361d72ab17690d95...
*-x86_64-apple-darwin-unsigned.tar.gz 8622d322aa5a246c... b5fde8f0bb1e8bf0...
*-x86_64-apple-darwin.tar.gz dfb17fc2163b23a0... d935f700174f5288...
*-x86_64-linux-gnu-debug.tar.gz 58cf869b3c49af39... bbc5e67e7f426f57...
*-x86_64-linux-gnu.tar.gz c46351cdaed19059... 5eda24f024c64296...
*.tar.gz 848859e98e62a4ba... 11a96f5f5d8dd02e...
guix_build.log b0fe3349e765d71e... f7b1c83fcb2a4971...
guix_build.log.diff 2a0bc042e3bf2fb5...

@bitcoin bitcoin deleted a comment from pakizefar May 29, 2022
@bitcoin bitcoin locked and limited conversation to collaborators May 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants