build: Build libbitcoinconsensus from its own convenience library #24994

pull hebasto wants to merge 2 commits into bitcoin:master from hebasto:220426-consensus changing 5 files +139 −54
  1. hebasto commented at 5:54 pm on April 26, 2022: member

    This PR has the following 4 benefits:

    First benefit

    This PR prunes all of unnecessary symbols being exported from libbitcoinconsensus.so:

    0$ objdump -T bitcoin-c28bb15a117e-x86_64-linux-gnu/bitcoin-c28bb15a117e/lib/libbitcoinconsensus.so.0 | grep -v UND
1
2bitcoin-c28bb15a117e-x86_64-linux-gnu/bitcoin-c28bb15a117e/lib/libbitcoinconsensus.so.0:     file format elf64-x86-64
3
4DYNAMIC SYMBOL TABLE:
50000000000003000 g    DF .init	0000000000000000  Base        _init
60000000000045494 g    DF .fini	0000000000000000  Base        _fini
70000000000004de0 g    DF .text	0000000000000037  Base        bitcoinconsensus_version
80000000000005230 g    DF .text	0000000000000049  Base        bitcoinconsensus_verify_script_with_amount
90000000000005280 g    DF .text	000000000000006d  Base        bitcoinconsensus_verify_script

    For more details regarding this bug please refer to #26698.

    Second benefit

    It fixes libbitcoinconsensus Windows builds with DEBUG=1.

    On master (833add0f48b0fad84d7b8cf9373a349e7aef20b4):

    0$ ./autogen.sh
1$ ./configure --host=x86_64-w64-mingw32 CPPFLAGS='-D_GLIBCXX_DEBUG' --disable-bench --disable-fuzz-binary --disable-tests --disable-wallet --without-daemon --without-gui --without-utils
2$ make clean
3$ make
4...
5  CXXLD    libbitcoinconsensus.la
6/usr/bin/x86_64-w64-mingw32-ld: consensus/.libs/libbitcoinconsensus_la-merkle.o:/usr/lib/gcc/x86_64-w64-mingw32/10-posix/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:749: undefined reference to `__imp_pthread_mutex_lock'
7/usr/bin/x86_64-w64-mingw32-ld: consensus/.libs/libbitcoinconsensus_la-merkle.o:/usr/lib/gcc/x86_64-w64-mingw32/10-posix/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
8...

    With this PR:

    0$ ./autogen.sh
1$ ./configure --host=x86_64-w64-mingw32 CPPFLAGS='-D_GLIBCXX_DEBUG' --disable-bench --disable-fuzz-binary --disable-tests --disable-wallet --without-daemon --without-gui --without-utils
2$ make clean
3$ make
4# no errors

    From libstdc++ docs:

    Note that this flag [-D_GLIBCXX_DEBUG] changes the sizes and behavior of standard class templates such as std::vector, and therefore you can only link code compiled with debug mode and code compiled without debug mode if no instantiation of a container is passed between the two translation units.

    On the master branch, a linker adds to binaries every specified object file. And it fails, for example, for https://github.com/bitcoin/bitcoin/blob/f58c1f1a446716a245ca204e2ac1a219455f1340/src/util/strencodings.h#L58-L59 when building libbitcoinconsensus-0.dll which in turn

    effectively consists of two parts; the DLL itself which contains the shared object code, and an import library which consists of the stub functions which are actually linked into the executable, at a rate of one stub per entry point.

    Third benefit

    The resulted binaries got smaller because a linker only extracts from a convenience library those object files that are actually referenced in the code being linked (that could be easy to verify by comparing nm outputs):

    • master (695ca641a4e3ae065121815a968c769198aa73de)
    0$ ls -l bitcoin-695ca641a4e3-x86_64-linux-gnu/bitcoin-695ca641a4e3/lib/libbitcoinconsensus.so.0.0.0 
1-rwxr-xr-x 1 hebasto hebasto 1517816 Jun  4 21:52 bitcoin-695ca641a4e3-x86_64-linux-gnu/bitcoin-695ca641a4e3/lib/libbitcoinconsensus.so.0.0.0
    • this PR (4.3% smaller)
    0$ ls -l bitcoin-c28bb15a117e-x86_64-linux-gnu/bitcoin-c28bb15a117e/lib/libbitcoinconsensus.so.0.0.0 
1-rwxr-xr-x 1 hebasto hebasto 1452280 Apr 27 12:59 bitcoin-c28bb15a117e-x86_64-linux-gnu/bitcoin-c28bb15a117e/lib/libbitcoinconsensus.so.0.0.0

    Fourth benefit

    This solution does not introduce another Libtool hack as a competitive one does.

    Guix builds on x86_64:

     0$ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
 16dbc71f90085bb785b88d6e0fabdfb49e4df01dddc7adcf36847eab1c39323c2  guix-build-c28bb15a117e/output/aarch64-linux-gnu/SHA256SUMS.part
 27892c618ad9d29f999528451dc894fd8b9647479bd38b7308d7ad4d687734a6b  guix-build-c28bb15a117e/output/aarch64-linux-gnu/bitcoin-c28bb15a117e-aarch64-linux-gnu-debug.tar.gz
 3f161c35d495c33a558a049899ad3d782bfeb6ff33f2e5d0346b624339c7ed4d5  guix-build-c28bb15a117e/output/aarch64-linux-gnu/bitcoin-c28bb15a117e-aarch64-linux-gnu.tar.gz
 45898572c86b5b20e71e007e3b3759883e567df34200775aa7e8baf4787330b14  guix-build-c28bb15a117e/output/arm-linux-gnueabihf/SHA256SUMS.part
 594386f5febb0aa9e77e4bdd079cb550cda39c21d27deae64591bfbb3aa879d3d  guix-build-c28bb15a117e/output/arm-linux-gnueabihf/bitcoin-c28bb15a117e-arm-linux-gnueabihf-debug.tar.gz
 68cca048298f11ec932e07c7b5e239a0274b8345f5e048c03d22da2de73f2d2ca  guix-build-c28bb15a117e/output/arm-linux-gnueabihf/bitcoin-c28bb15a117e-arm-linux-gnueabihf.tar.gz
 79a11382e702e0f0635e17659664c11139015cbac9731c19a522bf893051e5ad9  guix-build-c28bb15a117e/output/arm64-apple-darwin/SHA256SUMS.part
 84e7eaa152d8671c40e661f62cee95a2dc4b03102faadf68e4996d4079673e1ae  guix-build-c28bb15a117e/output/arm64-apple-darwin/bitcoin-c28bb15a117e-arm64-apple-darwin-unsigned.dmg
 95fa820623ce24c5f866e4a7a585019a32e09aeede58457521d615f719b6c9ac9  guix-build-c28bb15a117e/output/arm64-apple-darwin/bitcoin-c28bb15a117e-arm64-apple-darwin-unsigned.tar.gz
104b1d616e7a100fb225b8fd283ceb6045806ab108783fffcb7a761ccb101f7a7c  guix-build-c28bb15a117e/output/arm64-apple-darwin/bitcoin-c28bb15a117e-arm64-apple-darwin.tar.gz
1145eab8b3a4932163c0d3a27b5d1dbf06c3ddbc025409ce12674faddba974bbf0  guix-build-c28bb15a117e/output/dist-archive/bitcoin-c28bb15a117e.tar.gz
1287618c7fe2b3c34866bdb32b52486e4a76b960bf3dd8ba4febeb9477ea7e3a55  guix-build-c28bb15a117e/output/powerpc64-linux-gnu/SHA256SUMS.part
1360bbc756a733cd8f902652a2201a422236411ef23996972b7d407eb1a4768f2e  guix-build-c28bb15a117e/output/powerpc64-linux-gnu/bitcoin-c28bb15a117e-powerpc64-linux-gnu-debug.tar.gz
14643b3d279eb3bc021f1cb4d461ead0a4abb6c366101c0fc41a99b7da6ea21ca1  guix-build-c28bb15a117e/output/powerpc64-linux-gnu/bitcoin-c28bb15a117e-powerpc64-linux-gnu.tar.gz
150a715a8e37a749a627a09f022aad4e1b5bd8c925d24ac714c7c68223b5a1afb4  guix-build-c28bb15a117e/output/powerpc64le-linux-gnu/SHA256SUMS.part
16598eedd190b26a32ebe836eb6cb5ebd7b040d5fe9b0d514d62e6397f8ac32973  guix-build-c28bb15a117e/output/powerpc64le-linux-gnu/bitcoin-c28bb15a117e-powerpc64le-linux-gnu-debug.tar.gz
175ca09731ecb34a37166cbaefbcd956eb1eba32255e7113309c720c4fe6afe8c4  guix-build-c28bb15a117e/output/powerpc64le-linux-gnu/bitcoin-c28bb15a117e-powerpc64le-linux-gnu.tar.gz
180b30ecdc2a34605c7f5ca320e02d4acc13cc0a79fb6bf16795550950c45d4205  guix-build-c28bb15a117e/output/riscv64-linux-gnu/SHA256SUMS.part
199a4ef88387e75f8024630f600789b905bb5e0ebca695cf52dd8e43d382eba08c  guix-build-c28bb15a117e/output/riscv64-linux-gnu/bitcoin-c28bb15a117e-riscv64-linux-gnu-debug.tar.gz
20d6efeea1aab7ec4d5cf44e2687ba998c3be950d802b8932f807c30c223a8ef06  guix-build-c28bb15a117e/output/riscv64-linux-gnu/bitcoin-c28bb15a117e-riscv64-linux-gnu.tar.gz
2130fc887a3507ea8762021b567b4ef4b88a8a247c3cbb5612ee440aee3ecd7806  guix-build-c28bb15a117e/output/x86_64-apple-darwin/SHA256SUMS.part
22ce5b00636bbc350c03bdb768bb644bc2acc4443e5b5e21d5ec8382129043e06b  guix-build-c28bb15a117e/output/x86_64-apple-darwin/bitcoin-c28bb15a117e-x86_64-apple-darwin-unsigned.dmg
238d604959ee976ae6c792a8df19fd481f798bfb2290d2bb9b4fd2c383e1980339  guix-build-c28bb15a117e/output/x86_64-apple-darwin/bitcoin-c28bb15a117e-x86_64-apple-darwin-unsigned.tar.gz
24029985c80ec64e156612b61b89c8499ecd3f5f81a38660b7636094772a361f2a  guix-build-c28bb15a117e/output/x86_64-apple-darwin/bitcoin-c28bb15a117e-x86_64-apple-darwin.tar.gz
25ebf0c0a4de026fffc85bdc203adcddf7dddb3b7e290e49db458e3f5e7100d52f  guix-build-c28bb15a117e/output/x86_64-linux-gnu/SHA256SUMS.part
26dec8a0fb339a40b7e3fe05fc95b0c0de368b37f5ee53d76ac3631af7ac386ad4  guix-build-c28bb15a117e/output/x86_64-linux-gnu/bitcoin-c28bb15a117e-x86_64-linux-gnu-debug.tar.gz
2777472639784a37b22a97da3893e9288b009770250f2ae773880c8406b9f02464  guix-build-c28bb15a117e/output/x86_64-linux-gnu/bitcoin-c28bb15a117e-x86_64-linux-gnu.tar.gz
2837c60fce475b0b788e859926fe51151519feeab0dd16b83361c967c4a229e687  guix-build-c28bb15a117e/output/x86_64-w64-mingw32/SHA256SUMS.part
29321051dbdd456ead04a295bcc1545d91ec30a70dbc5b2655dde0c64babfa454b  guix-build-c28bb15a117e/output/x86_64-w64-mingw32/bitcoin-c28bb15a117e-win64-debug.zip
3084acb869ce0ce2f53564b617464e9cb5086761c137115d5eb6e8b3e94ce5b808  guix-build-c28bb15a117e/output/x86_64-w64-mingw32/bitcoin-c28bb15a117e-win64-setup-unsigned.exe
3186fbf61f2a208510b3fdda92bb0c0d6489605d6ac54e676cd85e8f06f4f90d5a  guix-build-c28bb15a117e/output/x86_64-w64-mingw32/bitcoin-c28bb15a117e-win64-unsigned.tar.gz
324d6f4e9d3ba152033b484e6c0037047264c9d3dacf9a6c874f597c2b94aba8ec  guix-build-c28bb15a117e/output/x86_64-w64-mingw32/bitcoin-c28bb15a117e-win64.zip

    Fixes bitcoin/bitcoin#19772. Fixes bitcoin/bitcoin#26698.

  2. DrahtBot added the label Build system on Apr 26, 2022
  3. DrahtBot added the label Consensus on Apr 26, 2022
  4. hebasto force-pushed on Apr 27, 2022
  5. hebasto force-pushed on Apr 27, 2022
  6. hebasto renamed this:
    build: Fix `libbitcoinconsensus` Windows builds with `DEBUG=1`
    build: Build `libbitcoinconsensus` from its own convenience library
    on Apr 27, 2022
  7. fanquake commented at 8:19 am on April 27, 2022: member

    Concept NACK.

    Second, this PR fix libbitcoinconsensus Windows builds with DEBUG=1.

    Shuffling a bunch of code, creating a new library, and introducing a new circular dependency to fix a Windows only, debug build of libbitcoinconsensus (which, given how long it hasn’t worked for, can’t be a problem for many people) does not seem like a good tradeoff.

    I would wait and see what falls out of the discussion in #24322, as I think it’s very likely that is going to produce a more general solution to this problem, and solving #19772.

  8. DrahtBot commented at 8:44 am on April 27, 2022: contributor

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

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Concept NACK fanquake
    Concept ACK laanwj, jarolrod

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #26504 (Add UNREACHABLE macro and drop -Wreturn-type/C4715 warnings suppressions by hebasto)

    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.

  9. hebasto force-pushed on Apr 27, 2022
  10. hebasto commented at 11:33 am on April 27, 2022: member

    Updated.

    Shuffling a bunch of code

    Much less now.

    introducing a new circular dependency

    Fixed.

    to fix a Windows only, debug build of libbitcoinconsensus (which, given how long it hasn’t worked for, can’t be a problem for many people)

    Actually, it is a problem for a few developers who cannot test libbitcoinconsensus-0.dll using libstdc++ debug mode. I think, all Bitcoin Core users who run it on Windows would benefit.

    creating a new library

    I see this as a nice abstraction layer that has an accurate list of sources, for example.

  11. hebasto force-pushed on Apr 27, 2022
  12. hebasto marked this as ready for review on Apr 27, 2022
  13. hebasto commented at 3:35 pm on April 27, 2022: member

    MSVC build fixed.

    Ready for review now :)

  14. DrahtBot added the label Needs rebase on Apr 28, 2022
  15. fanquake commented at 7:56 am on April 29, 2022: member
    I’m still a concept NACK, and would rather we use Corys fix from #25008.
  16. hebasto force-pushed on Apr 30, 2022
  17. DrahtBot removed the label Needs rebase on Apr 30, 2022
  18. luke-jr commented at 0:46 am on May 7, 2022: member

    the resulted binaries are smaller, because a linker only extracts from a convenience library those object files that are actually referenced in the code being linked.

    Is this well-defined behaviour, or would we be relying on an optimisation that the user might disable (more likely with debug builds, I might note)?

  19. hebasto marked this as a draft on Jun 5, 2022
  20. refactor: Move serialization code out from `bitcoinconsensus` module 
    This change is a prerequisite for the following fix for
`libbitcoinconsensus-0.dll` builds with `DEBUG=1`.
    f100fc1ed4
  21. build: Add `libscriptvalidation` convenience library 
    This change fixes `libbitcoinconsensus-0.dll` builds with `DEBUG=1`, and
removes unneeded exported symbols from `libbitcoinconsensus.so`.
    c28bb15a11
  22. hebasto marked this as ready for review on Jun 5, 2022
  23. hebasto force-pushed on Jun 5, 2022
  24. hebasto commented at 10:43 am on June 5, 2022: member

    Updated f17c358c77873a0511539e634adbb05b61149a12 -> c28bb15a117e4191b7589e084c943827b3cd9e92 (pr24994.06 -> pr24994.07):

    • rebased
    • fixed symbol export on Linux

    the resulted binaries are smaller, because a linker only extracts from a convenience library those object files that are actually referenced in the code being linked.

    Is this well-defined behaviour, or would we be relying on an optimisation that the user might disable (more likely with debug builds, I might note)?

    I failed to find supportive statements in the Autotools docs, only in other sources.

  25. hebasto commented at 11:15 am on June 5, 2022: member
    The PR description has been updated.
  26. laanwj commented at 7:29 am on June 27, 2022: member
    Concept ACK
  27. in src/Makefile.am:943 in c28bb15a11 
    938@@ -937,12 +939,50 @@ endif # BUILD_BITCOIN_KERNEL_LIB
939 if BUILD_BITCOIN_LIBS
940 lib_LTLIBRARIES += $(LIBBITCOINCONSENSUS)
941 
942+noinst_LTLIBRARIES += libscriptvalidation.la
943+libscriptvalidation_la_SOURCES =
    laanwj commented at 8:00 am on June 27, 2022:
    Might want to add this to doc/design/libraries.md
  28. jarolrod commented at 5:01 am on July 12, 2022: member

    Concept ACK

    Guix hashes:

    x86:

     0$ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
 1
 26dbc71f90085bb785b88d6e0fabdfb49e4df01dddc7adcf36847eab1c39323c2  guix-build-c28bb15a117e/output/aarch64-linux-gnu/SHA256SUMS.part
 37892c618ad9d29f999528451dc894fd8b9647479bd38b7308d7ad4d687734a6b  guix-build-c28bb15a117e/output/aarch64-linux-gnu/bitcoin-c28bb15a117e-aarch64-linux-gnu-debug.tar.gz
 4f161c35d495c33a558a049899ad3d782bfeb6ff33f2e5d0346b624339c7ed4d5  guix-build-c28bb15a117e/output/aarch64-linux-gnu/bitcoin-c28bb15a117e-aarch64-linux-gnu.tar.gz
 55898572c86b5b20e71e007e3b3759883e567df34200775aa7e8baf4787330b14  guix-build-c28bb15a117e/output/arm-linux-gnueabihf/SHA256SUMS.part
 694386f5febb0aa9e77e4bdd079cb550cda39c21d27deae64591bfbb3aa879d3d  guix-build-c28bb15a117e/output/arm-linux-gnueabihf/bitcoin-c28bb15a117e-arm-linux-gnueabihf-debug.tar.gz
 78cca048298f11ec932e07c7b5e239a0274b8345f5e048c03d22da2de73f2d2ca  guix-build-c28bb15a117e/output/arm-linux-gnueabihf/bitcoin-c28bb15a117e-arm-linux-gnueabihf.tar.gz
 89a11382e702e0f0635e17659664c11139015cbac9731c19a522bf893051e5ad9  guix-build-c28bb15a117e/output/arm64-apple-darwin/SHA256SUMS.part
 94e7eaa152d8671c40e661f62cee95a2dc4b03102faadf68e4996d4079673e1ae  guix-build-c28bb15a117e/output/arm64-apple-darwin/bitcoin-c28bb15a117e-arm64-apple-darwin-unsigned.dmg
105fa820623ce24c5f866e4a7a585019a32e09aeede58457521d615f719b6c9ac9  guix-build-c28bb15a117e/output/arm64-apple-darwin/bitcoin-c28bb15a117e-arm64-apple-darwin-unsigned.tar.gz
114b1d616e7a100fb225b8fd283ceb6045806ab108783fffcb7a761ccb101f7a7c  guix-build-c28bb15a117e/output/arm64-apple-darwin/bitcoin-c28bb15a117e-arm64-apple-darwin.tar.gz
1245eab8b3a4932163c0d3a27b5d1dbf06c3ddbc025409ce12674faddba974bbf0  guix-build-c28bb15a117e/output/dist-archive/bitcoin-c28bb15a117e.tar.gz
1387618c7fe2b3c34866bdb32b52486e4a76b960bf3dd8ba4febeb9477ea7e3a55  guix-build-c28bb15a117e/output/powerpc64-linux-gnu/SHA256SUMS.part
1460bbc756a733cd8f902652a2201a422236411ef23996972b7d407eb1a4768f2e  guix-build-c28bb15a117e/output/powerpc64-linux-gnu/bitcoin-c28bb15a117e-powerpc64-linux-gnu-debug.tar.gz
15643b3d279eb3bc021f1cb4d461ead0a4abb6c366101c0fc41a99b7da6ea21ca1  guix-build-c28bb15a117e/output/powerpc64-linux-gnu/bitcoin-c28bb15a117e-powerpc64-linux-gnu.tar.gz
160a715a8e37a749a627a09f022aad4e1b5bd8c925d24ac714c7c68223b5a1afb4  guix-build-c28bb15a117e/output/powerpc64le-linux-gnu/SHA256SUMS.part
17598eedd190b26a32ebe836eb6cb5ebd7b040d5fe9b0d514d62e6397f8ac32973  guix-build-c28bb15a117e/output/powerpc64le-linux-gnu/bitcoin-c28bb15a117e-powerpc64le-linux-gnu-debug.tar.gz
185ca09731ecb34a37166cbaefbcd956eb1eba32255e7113309c720c4fe6afe8c4  guix-build-c28bb15a117e/output/powerpc64le-linux-gnu/bitcoin-c28bb15a117e-powerpc64le-linux-gnu.tar.gz
190b30ecdc2a34605c7f5ca320e02d4acc13cc0a79fb6bf16795550950c45d4205  guix-build-c28bb15a117e/output/riscv64-linux-gnu/SHA256SUMS.part
209a4ef88387e75f8024630f600789b905bb5e0ebca695cf52dd8e43d382eba08c  guix-build-c28bb15a117e/output/riscv64-linux-gnu/bitcoin-c28bb15a117e-riscv64-linux-gnu-debug.tar.gz
21d6efeea1aab7ec4d5cf44e2687ba998c3be950d802b8932f807c30c223a8ef06  guix-build-c28bb15a117e/output/riscv64-linux-gnu/bitcoin-c28bb15a117e-riscv64-linux-gnu.tar.gz
2230fc887a3507ea8762021b567b4ef4b88a8a247c3cbb5612ee440aee3ecd7806  guix-build-c28bb15a117e/output/x86_64-apple-darwin/SHA256SUMS.part
23ce5b00636bbc350c03bdb768bb644bc2acc4443e5b5e21d5ec8382129043e06b  guix-build-c28bb15a117e/output/x86_64-apple-darwin/bitcoin-c28bb15a117e-x86_64-apple-darwin-unsigned.dmg
248d604959ee976ae6c792a8df19fd481f798bfb2290d2bb9b4fd2c383e1980339  guix-build-c28bb15a117e/output/x86_64-apple-darwin/bitcoin-c28bb15a117e-x86_64-apple-darwin-unsigned.tar.gz
25029985c80ec64e156612b61b89c8499ecd3f5f81a38660b7636094772a361f2a  guix-build-c28bb15a117e/output/x86_64-apple-darwin/bitcoin-c28bb15a117e-x86_64-apple-darwin.tar.gz
26ebf0c0a4de026fffc85bdc203adcddf7dddb3b7e290e49db458e3f5e7100d52f  guix-build-c28bb15a117e/output/x86_64-linux-gnu/SHA256SUMS.part
27dec8a0fb339a40b7e3fe05fc95b0c0de368b37f5ee53d76ac3631af7ac386ad4  guix-build-c28bb15a117e/output/x86_64-linux-gnu/bitcoin-c28bb15a117e-x86_64-linux-gnu-debug.tar.gz
2877472639784a37b22a97da3893e9288b009770250f2ae773880c8406b9f02464  guix-build-c28bb15a117e/output/x86_64-linux-gnu/bitcoin-c28bb15a117e-x86_64-linux-gnu.tar.gz
2937c60fce475b0b788e859926fe51151519feeab0dd16b83361c967c4a229e687  guix-build-c28bb15a117e/output/x86_64-w64-mingw32/SHA256SUMS.part
30321051dbdd456ead04a295bcc1545d91ec30a70dbc5b2655dde0c64babfa454b  guix-build-c28bb15a117e/output/x86_64-w64-mingw32/bitcoin-c28bb15a117e-win64-debug.zip
3184acb869ce0ce2f53564b617464e9cb5086761c137115d5eb6e8b3e94ce5b808  guix-build-c28bb15a117e/output/x86_64-w64-mingw32/bitcoin-c28bb15a117e-win64-setup-unsigned.exe
3286fbf61f2a208510b3fdda92bb0c0d6489605d6ac54e676cd85e8f06f4f90d5a  guix-build-c28bb15a117e/output/x86_64-w64-mingw32/bitcoin-c28bb15a117e-win64-unsigned.tar.gz
334d6f4e9d3ba152033b484e6c0037047264c9d3dacf9a6c874f597c2b94aba8ec  guix-build-c28bb15a117e/output/x86_64-w64-mingw32/bitcoin-c28bb15a117e-win64.zip

    arm64:

     0$ env HOSTS='arm-linux-gnueabihf arm64-apple-darwin powerpc64-linux-gnu powerpc64le-linux-gnu riscv64-linux-gnu x86_64-apple-darwin x86_64-linux-gnu x86_64-w64-mingw32' ./contrib/guix/guix-build
 1$ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
 2
 35c6d15960a7a1c80a1cc226bfe67cff37d662e532f96ed78303242daf3f45a81  guix-build-c28bb15a117e/output/arm-linux-gnueabihf/SHA256SUMS.part
 4b52139b5e1f884f656528e45425188628a423c1ada1487e5c1300518752c8aa6  guix-build-c28bb15a117e/output/arm-linux-gnueabihf/bitcoin-c28bb15a117e-arm-linux-gnueabihf-debug.tar.gz
 5b35c0e48a30f1bbc747f7ce0e8b20fb3ce080563e4ff9d1a895fb13b1aca789e  guix-build-c28bb15a117e/output/arm-linux-gnueabihf/bitcoin-c28bb15a117e-arm-linux-gnueabihf.tar.gz
 6b3e397f7c6c1802aeff8d2d710aa8624e2e8e2c5beedf107efc9401383e795c5  guix-build-c28bb15a117e/output/arm64-apple-darwin/SHA256SUMS.part
 7408039e82e94a5ac15e4c3bb77d5854408edc925d9a9f9fd9d554825252e85b2  guix-build-c28bb15a117e/output/arm64-apple-darwin/bitcoin-c28bb15a117e-arm64-apple-darwin-unsigned.dmg
 8dbadef87ccc4ff66cee28f46991f400f9eed034329b219f5dae5cc43203d25e9  guix-build-c28bb15a117e/output/arm64-apple-darwin/bitcoin-c28bb15a117e-arm64-apple-darwin-unsigned.tar.gz
 930b0ab410aa5f90981b397c62d5af499121ea35fd8dcc21788e724ee6c151592  guix-build-c28bb15a117e/output/arm64-apple-darwin/bitcoin-c28bb15a117e-arm64-apple-darwin.tar.gz
1045eab8b3a4932163c0d3a27b5d1dbf06c3ddbc025409ce12674faddba974bbf0  guix-build-c28bb15a117e/output/dist-archive/bitcoin-c28bb15a117e.tar.gz
1192f2b5781794fc7c21b7a105a17de668e2ed86c263e442373166cb2fd2bd03df  guix-build-c28bb15a117e/output/powerpc64-linux-gnu/SHA256SUMS.part
1296ed974c3e2c126beb0fdb9480dda57a0423eba91abe299ebfc8713e3da28c38  guix-build-c28bb15a117e/output/powerpc64-linux-gnu/bitcoin-c28bb15a117e-powerpc64-linux-gnu-debug.tar.gz
13fdee462d3bbf0e6fa99cba5c52ddd2777c6d072dd9479f76ec72dffb6097ab24  guix-build-c28bb15a117e/output/powerpc64-linux-gnu/bitcoin-c28bb15a117e-powerpc64-linux-gnu.tar.gz
142ba05e32b2163a6f6a6b9940042b3e0a93d1e5d45c14726e596cbdc925b6d01b  guix-build-c28bb15a117e/output/powerpc64le-linux-gnu/SHA256SUMS.part
15a4fa6fc0ca7adbd2c7b6cf2016a23bdce3a00c993b97f44a258c1726fbece40f  guix-build-c28bb15a117e/output/powerpc64le-linux-gnu/bitcoin-c28bb15a117e-powerpc64le-linux-gnu-debug.tar.gz
1652f0742a773b0ec98e90e9d4c292bf9e5a3164428a2ddc7b1e038b6c93e21fcf  guix-build-c28bb15a117e/output/powerpc64le-linux-gnu/bitcoin-c28bb15a117e-powerpc64le-linux-gnu.tar.gz
17c21e177d34389670b3a0023e8abc5333c3ceda9c95c91736a90f5ade31d25d22  guix-build-c28bb15a117e/output/riscv64-linux-gnu/SHA256SUMS.part
188bd4cd247d3529c7fe012cb0cad32970066c3773b72bb64da1e0f52761504c0a  guix-build-c28bb15a117e/output/riscv64-linux-gnu/bitcoin-c28bb15a117e-riscv64-linux-gnu-debug.tar.gz
19b0000b299473f8d0cddf433a06b9f7de7e420b325a78621e1457862f1e05ebe7  guix-build-c28bb15a117e/output/riscv64-linux-gnu/bitcoin-c28bb15a117e-riscv64-linux-gnu.tar.gz
2030fc887a3507ea8762021b567b4ef4b88a8a247c3cbb5612ee440aee3ecd7806  guix-build-c28bb15a117e/output/x86_64-apple-darwin/SHA256SUMS.part
21ce5b00636bbc350c03bdb768bb644bc2acc4443e5b5e21d5ec8382129043e06b  guix-build-c28bb15a117e/output/x86_64-apple-darwin/bitcoin-c28bb15a117e-x86_64-apple-darwin-unsigned.dmg
228d604959ee976ae6c792a8df19fd481f798bfb2290d2bb9b4fd2c383e1980339  guix-build-c28bb15a117e/output/x86_64-apple-darwin/bitcoin-c28bb15a117e-x86_64-apple-darwin-unsigned.tar.gz
23029985c80ec64e156612b61b89c8499ecd3f5f81a38660b7636094772a361f2a  guix-build-c28bb15a117e/output/x86_64-apple-darwin/bitcoin-c28bb15a117e-x86_64-apple-darwin.tar.gz
24bf7e61d641eeebebff6e0e2842dccb176810ccae8202f2c324748359e55923e8  guix-build-c28bb15a117e/output/x86_64-linux-gnu/SHA256SUMS.part
25aa659e0337ad47f2cbaa6d9f5b44c0bab1b9682fdc32f5335f3a8353dcecae5c  guix-build-c28bb15a117e/output/x86_64-linux-gnu/bitcoin-c28bb15a117e-x86_64-linux-gnu-debug.tar.gz
269941bc1b1f0a9f2f5ec0ce162260cd3fb931fd021c00d1160fdfbde3f361adfc  guix-build-c28bb15a117e/output/x86_64-linux-gnu/bitcoin-c28bb15a117e-x86_64-linux-gnu.tar.gz
273f2fee0c8b3ecfa1d1b1d322bc83225ed9a101004a337f54b208539df58a2981  guix-build-c28bb15a117e/output/x86_64-w64-mingw32/SHA256SUMS.part
28eee8fcc25cfeeb96761ce7ed6cca5af639a020882339738ab970c23ca887368a  guix-build-c28bb15a117e/output/x86_64-w64-mingw32/bitcoin-c28bb15a117e-win64-debug.zip
2984acb869ce0ce2f53564b617464e9cb5086761c137115d5eb6e8b3e94ce5b808  guix-build-c28bb15a117e/output/x86_64-w64-mingw32/bitcoin-c28bb15a117e-win64-setup-unsigned.exe
3086fbf61f2a208510b3fdda92bb0c0d6489605d6ac54e676cd85e8f06f4f90d5a  guix-build-c28bb15a117e/output/x86_64-w64-mingw32/bitcoin-c28bb15a117e-win64-unsigned.tar.gz
310c5fece9865d51a9b1cccb07cd4e71f2a241a7bbdeca859c2659f6a5769b6e84  guix-build-c28bb15a117e/output/x86_64-w64-mingw32/bitcoin-c28bb15a117e-win64.zip
  29. hebasto commented at 2:12 pm on December 14, 2022: member

    @ryanofsky

    Could you be interesting in reviewing of this PR?

  30. fanquake commented at 9:42 am on February 23, 2023: member
    Concept NACK. In general, I don’t like this approach of refactoring code, and adding libraries, to workaround issues with build tooling, especially when its a problem that only exists for a single platform (let alone the DEBUG build, and ). I think this should just be closed in light of #27146 and the migration towards CMake.
  31. hebasto closed this on Feb 23, 2023
  32. ryanofsky commented at 3:04 pm on February 23, 2023: contributor

    The PR description seems to say that this fixes a bug and that there is alternative fix in #25008.

    Is https://github.com/theuni/bitcoin/commit/9612f5f0f1e3295065dbd8a0e0f472f8f020bbd9 the alternative fix?

    Neither of the two fixes seems to have an explanation of how they fix the problem. I also don’t see a description of what the bug is other than “We’re unable to build a dll for libbitcoinkernel right now because of unique problems arising out of the combination of mingw-w64 + winpthread + libtool + dll's” from #25008, which is extremely vague.

    I can see that this PR is doing a refactoring, and that fanquake doesn’t like the refactoring. I don’t see an explanation of why the refactoring is necessary, or why fanquake doesn’t like it. I’m not sure if fanquake is mainly objecting to this particular refactoring, or doesn’t like refactorings only addressing build problems. I also don’t know what the circular dependency referenced in #24994 (comment) is, so maybe that is a key factor.

    This is all very hard to decipher. If I’m the only one who’s confused, and everyone else understands how these fixes work, I guess that’s fine. But if there is not mutual understanding of both approaches, it’s not surprising there would be conflict trying to decide which one is better.

  33. bitcoin deleted a comment on Feb 23, 2023
  34. fanquake commented at 3:30 pm on February 23, 2023: member

    or why fanquake doesn’t like it. @ryanofsky I don’t like it because it’s clearly the wrong solution to what is ultimately a build/tooling problem. The code is fine, and works perfectly fine when compiled on all other platforms. It doesn’t work, when building a Windows DLL, and only when using -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC (DEBUG=1).

    So this is clearly a build system/tooling problem, and the the idea that we should refactor perfectly working code, introduce new libs etc, to fix a windows only issue, that only occurs when using certain preprocessing flags, seems completely incorrect.

  35. ryanofsky commented at 3:51 pm on February 23, 2023: contributor

    @fanquake that all makes sense at a high level, but without knowing the details of what exactly is causing the bug, I don’t actually know that “the code is fine.”

    If the code works on all platforms and configurations except one, it does strongly suggest that there is no problem with the code, so I see where you are coming from. But there is some reason this fix works, so it could be fixing a problem that is masked on other platforms, and it could be making improvements that would make the build less fragile and have other benefits.

    I think you right to push back though because it’s not clear what exactly is wrong with the current code that causes it not to work on this one platform and configuration.

  36. hebasto commented at 2:52 pm on December 19, 2023: member

    Concept NACK. In general, I don’t like this approach of refactoring code, and adding libraries, to workaround issues with build tooling, especially when its a problem that only exists for a single platform (let alone the DEBUG build, and ). I think this should just be closed in light of #27146 and the migration towards CMake.

    I disagree with this NACK. @fanquake’s push back is mostly focused on the Windows-related issue mentioned in the PR description. However, none alternatives are suggested to solve the main issue, i.e., #26698.

    Cross-posting my comment from #26698 (comment):

    The underlying problem is described in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=36022, and it was not considered as a bug.

    I think that moving code that uses C++ template functions/classes out from the bitcoinconsensus.{h,cpp} compilation unit, which were suggested in #24994, is a step in the right direction.

    The code is fine, and works perfectly fine when compiled on all other platforms.

    If the code of a shared library compiles and works, but a linker exports symbols, which are not a part of the library’s API, this code is not fine.

    cc @ryanofsky @TheCharlatan

  37. fanquake commented at 2:58 pm on December 19, 2023: member

    If the code of a shared library compiles and works, but a linker exports symbols, which are not a part of the library’s API, this code is not fine.

    How does the above indicate a problem with our code? Wouldn’t this point to the linker being the issue? Does the same happen with lld? If not, then it’s a problem with ld/libstdc++, not our code?

  38. hebasto commented at 3:00 pm on December 19, 2023: member

    Wouldn’t this point to the linker being the issue?

    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=36022 – Status: RESOLVED INVALID

  39. ryanofsky commented at 5:02 pm on December 19, 2023: contributor

    cc @ryanofsky

    I’d like to help with this, but I just don’t understand most of the discussion so far. I see that this fixes some kind of a bug on windows and is supposed to have other benefits mentioned in the PR description, but I don’t see a clear description of the bug anywhere. Maybe someone can open an issue with a description of the bug and why it happens? The issue could link to whatever PRs are being proposed to fix it.

  40. hebasto commented at 3:25 pm on December 22, 2023: member

    I’d like to help with this, but I just don’t understand most of the discussion so far.

    My apologies about that.

    I see that this fixes some kind of a bug on windows and is supposed to have other benefits mentioned in the PR description, but I don’t see a clear description of the bug anywhere.

    #24994#issue-1216293178:

    This PR prunes all of unnecessary symbols being exported from libbitcoinconsensus.so

    I’ve added an explicit mentioning of the #26698 issue.

    Maybe someone can open an issue with a description of the bug and why it happens? The issue could link to whatever PRs are being proposed to fix it.

    It is done in #26698.

  41. bitcoin locked this on Dec 21, 2024


hebasto fanquake DrahtBot luke-jr laanwj jarolrod ryanofsky

Labels
Build system Consensus

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-21 15:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me