[kernel 1/n] Introduce initial libbitcoinkernel #24322

pull dongcarl wants to merge 11 commits into bitcoin:master from dongcarl:2022-02-libbitcoinkernel-p1-minimal changing 13 files +290 −211
  1. dongcarl commented at 7:13 pm on February 11, 2022: member

    Part of: #24303

    This PR introduces a libbitcoinkernel static library linking in the minimal list of files necessary to use our consensus engine as-is. bitcoin-chainstate introduced in #24304 now will link against libbitcoinkernel.

    Most of the changes are related to the build system.

    Please read the commit messages for more details.

  2. dongcarl added the label Build system on Feb 11, 2022
  3. dongcarl added the label Utils/log/libs on Feb 11, 2022
  4. dongcarl force-pushed on Feb 11, 2022
  5. dongcarl force-pushed on Feb 11, 2022
  6. DrahtBot commented at 5:55 am on February 13, 2022: member

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

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #24994 (build: Build libbitcoinconsensus from its own convenience library by hebasto)
    • #24972 (build: Make libunivalue a non-Libtool convenience library by hebasto)
    • #24774 (build, refactor: Rely on AC_DEFINE macro by hebasto)
    • #24773 ([POC] Enable AVX2 implementation of SHA256 for MSVC builds by hebasto)
    • #23670 (build: Build test binaries in make check, not in make 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.

  7. dongcarl force-pushed on Feb 13, 2022
  8. dongcarl force-pushed on Feb 13, 2022
  9. DrahtBot added the label Needs rebase on Feb 14, 2022
  10. dongcarl force-pushed on Feb 14, 2022
  11. dongcarl commented at 8:12 pm on February 14, 2022: member
    Rebased
  12. ryanofsky commented at 2:56 pm on February 15, 2022: member
    It might make sense to add a some note about the libbitcoinkernel to https://github.com/bitcoin/bitcoin/blob/master/doc/shared-libraries.md. I don’t think it would need to say much, but would be good to mention that it exists and API is under development and not stable. I also started writing a doc/design/libraries.md file in #24352 that mentions libbitcoinkernel
  13. dongcarl force-pushed on Feb 22, 2022
  14. DrahtBot removed the label Needs rebase on Feb 22, 2022
  15. dongcarl marked this as ready for review on Mar 3, 2022
  16. dongcarl force-pushed on Mar 3, 2022
  17. dongcarl commented at 7:42 pm on March 3, 2022: member

    Pushed 07d1e5ced8 -> e322677b3b:

    • Rebased over master
  18. laanwj added this to the "Blockers" column in a project

  19. DrahtBot added the label Needs rebase on Mar 12, 2022
  20. in src/Makefile.leveldb.include:6 in e4efd86123 outdated
    1@@ -2,6 +2,9 @@
    2 # Distributed under the MIT software license, see the accompanying
    3 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
    4 
    5+
    6+noinst_LTLIBRARIES += leveldb/libleveldb.la leveldb/libmemenv.la
    


    ryanofsky commented at 8:31 pm on March 14, 2022:

    In commit “build: Create .la library for leveldb” (e4efd8612300d7713e35d5c0fa33bfd89943a95c)

    ~Can you add a comment to say what these are used for and why they are noinst? If you are making a libbitcoin_kernel shared library that depends on leveldb shared libraries presuambly all the libraries need to be installed, or you need to make the leveldb libraries into “convenience libraries” that will be embedded in the libbitcoin_kernel shared library.~

    UPDATE: Looking at later commits, it seems this is going the convenience-library route. But then I don’t see a reason to declare these as separate libraries and declare every library twice instead of once. Following seems to work with and keeps one library definition:

     0--- a/src/Makefile.leveldb.include
     1+++ b/src/Makefile.leveldb.include
     2@@ -2,11 +2,11 @@
     3 # Distributed under the MIT software license, see the accompanying
     4 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
     5 
     6-LIBLEVELDB_INT = leveldb/libleveldb.a
     7-LIBMEMENV_INT  = leveldb/libmemenv.a
     8+LIBLEVELDB_INT = leveldb/libleveldb.la
     9+LIBMEMENV_INT  = leveldb/libmemenv.la
    10 
    11-EXTRA_LIBRARIES += $(LIBLEVELDB_INT)
    12-EXTRA_LIBRARIES += $(LIBMEMENV_INT)
    13+EXTRA_LTLIBRARIES = $(LIBLEVELDB_INT)
    14+EXTRA_LTLIBRARIES += $(LIBMEMENV_INT)
    15 
    16 LIBLEVELDB = $(LIBLEVELDB_INT) $(LIBCRC32C)
    17 LIBMEMENV = $(LIBMEMENV_INT)
    
    0sed -i s/libleveldb_a/libleveldb_la/g src/Makefile.leveldb.include
    1sed -i s/libmemenv_a/libmemenv_la/g src/Makefile.leveldb.include
    

    I think this would also allow dropping extra library declarations and noinst_ lines in next two commits:

    • 0dc4f08fed816996c906943d75a8ca2288fe646a build: Create .la library for crc32c (2/10)
    • cee91e43d72d8c6cdd703f159483671088ae0744 build: Create .la library for bitcoincrypto (3/10)

    dongcarl commented at 4:06 am on March 16, 2022:

    Yup! Great idea, see latest push as of e51ead8496f010315b21fb48465b120e5cb8e334

    Let me know if this addresses everything

  21. in src/Makefile.am:34 in cee91e43d7 outdated
    30@@ -23,6 +31,7 @@ LIBBITCOIN_CONSENSUS=libbitcoin_consensus.a
    31 LIBBITCOIN_CLI=libbitcoin_cli.a
    32 LIBBITCOIN_UTIL=libbitcoin_util.a
    33 LIBBITCOIN_CRYPTO_BASE=crypto/libbitcoin_crypto_base.a
    34+LIBBITCOINCRYPTO_BASE=crypto/libbitcoin_crypto_base.la
    


    ryanofsky commented at 8:45 pm on March 14, 2022:

    In commit “build: Create .la library for bitcoincrypto” (cee91e43d72d8c6cdd703f159483671088ae0744)

    Could we call these variables:

    • LIBBITCOIN_CRYPTO_BASE
    • LIBBITCOIN_CRYPTO_BASE_LA

    or:

    • LIBBITCOIN_CRYPTO_BASE
    • LIBBITCOIN_CRYPTO_BASE_SHARED

    instead of:

    • LIBBITCOIN_CRYPTO_BASE
    • LIBBITCOINCRYPTO_BASE

    Using a missing underscore to distinguish static from dynamic seems like an obscure thing to do. I see that there is something similar where libbitcoinconsensus is shared and libbitcoin_consensus is static, but I think this is an isolated case which isn’t great to turn into a pattern


    dongcarl commented at 4:07 am on March 16, 2022:

    Since e51ead8496f010315b21fb48465b120e5cb8e334 we’re no longer going to be building both .a’s and .la’s, only .la’s, which circumvents this problem.

    I do agree that naming can be clearer here so am going to followup with a PR that renames all our .la variables to LTLIB*.

  22. in src/Makefile.am:805 in 08f2be497e outdated
    806@@ -805,8 +807,38 @@ bitcoin_util_LDADD = \
    807 #
    808 
    809 # bitcoin-chainstate binary #
    810-bitcoin_chainstate_SOURCES = \
    811-  bitcoin-chainstate.cpp \
    812+bitcoin_chainstate_SOURCES = bitcoin-chainstate.cpp
    


    ryanofsky commented at 9:20 pm on March 14, 2022:

    In commit “build: Extract the libbitcoinkernel library” (08f2be497ee4769ff9d29d010433bf52c45ec103)

    Commit message seems out of date “This _SOURCES list of files may seem daunting,”


    dongcarl commented at 4:08 am on March 16, 2022:
    Removed as of 0057c5096ec798324208485642039d69b4b6f518
  23. in configure.ac:1654 in 08f2be497e outdated
    1649   AC_DEFINE([HAVE_CONSENSUS_LIB], [1], [Define this symbol if the consensus lib has been built])
    1650   AC_CONFIG_FILES([libbitcoinconsensus.pc:libbitcoinconsensus.pc.in])
    1651 fi
    1652+
    1653+AM_CONDITIONAL([BUILD_BITCOIN_KERNEL_LIB], [test "$build_experimental_kernel_lib" != "no" && ( test "$build_experimental_kernel_lib" = "yes" || test "$build_bitcoin_chainstate" = "yes" )])
    1654+AM_COND_IF([BUILD_BITCOIN_KERNEL_LIB], [AC_DEFINE([HAVE_KERNEL_LIB], [1], [Define this symbol if the experimental kernel lib has been built])])
    


    ryanofsky commented at 9:31 pm on March 14, 2022:

    In commit “build: Extract the libbitcoinkernel library” (08f2be497ee4769ff9d29d010433bf52c45ec103)

    Pretty sure there should not be a HAVE_KERNEL_LIB preprocessor macro and it would be better to drop this line. Code should generally not compile differently depending on whether some other build output is enabled.


    dongcarl commented at 4:08 am on March 16, 2022:
    Removed as of 0057c5096ec798324208485642039d69b4b6f518
  24. in src/Makefile.am:45 in 08f2be497e outdated
    40@@ -40,6 +41,7 @@ LIBBITCOIN_ZMQ=libbitcoin_zmq.a
    41 endif
    42 if BUILD_BITCOIN_LIBS
    43 LIBBITCOINCONSENSUS=libbitcoinconsensus.la
    44+LIBBITCOINKERNEL=libbitcoinkernel.la
    


    ryanofsky commented at 9:40 pm on March 14, 2022:

    In commit “build: Extract the libbitcoinkernel library” (08f2be497ee4769ff9d29d010433bf52c45ec103)

    Maybe add if BUILD_BITCOIN_KERNEL_LIB for consistency with other code above and below which is defining library variables conditionally (BUILD_BITCOIN_LIBS, ENABLE_ZMQ, ENABLE_WALLET). I’d probably prefer to drop the conditionals for brevity, but I guess they provide a kind of documentation about conditions which libraries may get built.


    dongcarl commented at 4:08 am on March 16, 2022:
    Done as of 0057c5096ec798324208485642039d69b4b6f518
  25. in src/Makefile.am:11 in 08eb041e43 outdated
     7@@ -8,7 +8,7 @@ print-%: FORCE
     8 
     9 DIST_SUBDIRS = secp256k1
    10 
    11-AM_LDFLAGS = $(LIBTOOL_LDFLAGS) $(HARDENED_LDFLAGS) $(GPROF_LDFLAGS) $(SANITIZER_LDFLAGS) $(LTO_LDFLAGS)
    12+AM_LDFLAGS = $(RELDFLAGS) $(LIBTOOL_LDFLAGS) $(HARDENED_LDFLAGS) $(GPROF_LDFLAGS) $(SANITIZER_LDFLAGS) $(LTO_LDFLAGS)
    


    ryanofsky commented at 9:47 pm on March 14, 2022:

    In commit “build: Absorb RELDFLAGS into AM_LDFLAGS” (08eb041e431f95098d7bad1cdac8ae0e806412d8)

    I don’t understand:

    • How this change is related to this PR
    • Why it is better for RELDFLAGS to be added to AM_LDFLAGS
    • Why RELDFLAGS was not previously part of AM_LDFLAGS

    dongcarl commented at 4:09 am on March 16, 2022:

    Lmk if these commits’ messages make it clear:

    • b9fd57b4e53f9237d70c97425d45829bf2555aab
    • a9eed6f181f0ddb4439243a979bb3cffe44ba33c
    • c8f5c84c3cb47fd8ecae3b26ba0e8c2c7cb8eac8

    ryanofsky commented at 5:01 am on March 29, 2022:

    In commit “build: Absorb RELDFLAGS into AM_LDFLAGS” (aa0017a5d931355faa221074ca615ae282c7d239)

    re: #24322 (review)

    Thanks, commit message now saying this is a “cleanup” does narrow down reason for the change a bit, but it still seems pretty opaque. I would maybe say “This change simplifies the build by avoiding need to repeatedly specify RELDFLAGS in individual targets, and makes the effect of the –enable-reduce-exports option more obvious in configure output” as explanations for why the change is good.


    dongcarl commented at 5:57 pm on April 14, 2022:
    Fixed as of 3961a789735c874ed9b3ff4bbd64c2b850282c07, thanks for helping with the wording!
  26. in configure.ac:1535 in 047aff7747 outdated
    1507@@ -1508,7 +1508,7 @@ dnl Check for reduced exports
    1508 if test "$use_reduce_exports" = "yes"; then
    1509   AX_CHECK_COMPILE_FLAG([-fvisibility=hidden], [CXXFLAGS="$CXXFLAGS -fvisibility=hidden"],
    1510   [AC_MSG_ERROR([Cannot set hidden symbol visibility. Use --disable-reduce-exports.])], [$CXXFLAG_WERROR])
    1511-  AX_CHECK_LINK_FLAG([-Wl,--exclude-libs,ALL], [RELDFLAGS="-Wl,--exclude-libs,ALL"], [], [$LDFLAG_WERROR])
    1512+  AX_CHECK_LINK_FLAG([-Wl,--exclude-libs,ALL], [RE_LDFLAGS="-Wl,--exclude-libs,ALL"], [], [$LDFLAG_WERROR])
    


    ryanofsky commented at 9:53 pm on March 14, 2022:

    In commit “build: Rename RELDFLAGS to RE_LDFLAGS” (047aff774770fb55f9cdd5bd22b51d2be0e2007b)

    Would be nice to say reason for the change in commit message: RE_LDFLAGS name is supposed to be consistent with RE_CXXFLAGS name in the next commit. I do think HIDE_SYMBOLS_CXXFLAGS / HIDE_SYMBOLS_LDFLAGS names might be more clear FWIW


    dongcarl commented at 4:10 am on March 16, 2022:

    Done: a9eed6f181f0ddb4439243a979bb3cffe44ba33c

    Going to leave the name alone for now if that’s okay?

  27. in src/Makefile.am:830 in a324a9582d outdated
    835@@ -836,7 +836,11 @@ lib_LTLIBRARIES += $(LIBBITCOINKERNEL)
    836 libbitcoinkernel_la_LDFLAGS = $(AM_LDFLAGS) -no-undefined $(PTHREAD_FLAGS)
    837 libbitcoinkernel_la_LIBADD = $(LIBSECP256K1) leveldb/libleveldb.la leveldb/libmemenv.la $(LIBUNIVALUE) crc32c/libcrc32c.la crc32c/libcrc32c_sse42.la $(LIBBITCOINCRYPTO)
    838 libbitcoinkernel_la_CPPFLAGS = $(AM_CPPFLAGS) -I$(builddir)/obj -I$(srcdir)/secp256k1/include -DBUILD_BITCOIN_INTERNAL $(BOOST_CPPFLAGS) $(LEVELDB_CPPFLAGS) -I$(srcdir)/$(UNIVALUE_INCLUDE_DIR_INT)
    839-libbitcoinkernel_la_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
    840+
    841+# libbitcoinkernel requires default symbol visibility, explicitly specify that
    842+# here so that things still work even when user configures with
    843+#   --enable-reduce-exports
    844+libbitcoinkernel_la_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) -fvisibility=default
    


    ryanofsky commented at 10:01 pm on March 14, 2022:

    In commit “build: Put reduced exports flags in RE_CXXFLAGS” (a324a9582d69bbc96d656b65b1b55d343aadb506)

    This functional change seems only tangentially related to the cleanups in the rest of this commit, and like it should be in it’s own behavior-changing commit or a part of the earlier commit adding this line in the first place (commit 08f2be497ee4769ff9d29d010433bf52c45ec103 “build: Extract the libbitcoinkernel library”)


    dongcarl commented at 4:10 am on March 16, 2022:
    Squashed it into 0057c5096ec798324208485642039d69b4b6f518 and added context in commit message
  28. in src/bitcoin-chainstate.cpp:31 in 61ab5fa1d0 outdated
    27@@ -28,7 +28,7 @@
    28 #include <functional>
    29 #include <iosfwd>
    30 
    31-const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;
    32+__attribute__ ((visibility("default"))) const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;
    


    ryanofsky commented at 10:10 pm on March 14, 2022:

    In commit “b-cs: Explicitly mark G_TRANSLATION_FUN as visible” (61ab5fa1d0a8d0e77b49a24b1b7035deaea9b7c8)

    Does the fact that this commit fixes a link error imply that previous commits will not build? If so, this seems like something that we might want to fix so the previous commits are functional.

    In any case, it would be good to get rid of this line entirely at some point. The application shouldn’t need to export its own symbols to use the library. The translation global just be a global in the library if it has to be a global.


    dongcarl commented at 4:10 am on March 16, 2022:
    Done a36c0ff4af800e5499bbe7aca3137581251e71b3
  29. in src/Makefile.am:837 in 08f2be497e outdated
    834+# bitcoinkernel library #
    835+if BUILD_BITCOIN_KERNEL_LIB
    836+lib_LTLIBRARIES += $(LIBBITCOINKERNEL)
    837+
    838+libbitcoinkernel_la_LDFLAGS = $(AM_LDFLAGS) -no-undefined $(RELDFLAGS) $(PTHREAD_FLAGS)
    839+libbitcoinkernel_la_LIBADD = $(LIBSECP256K1) leveldb/libleveldb.la leveldb/libmemenv.la $(LIBUNIVALUE) crc32c/libcrc32c.la crc32c/libcrc32c_sse42.la $(LIBBITCOINCRYPTO)
    


    ryanofsky commented at 10:19 pm on March 14, 2022:

    In commit “build: Absorb RELDFLAGS into AM_LDFLAGS” (08eb041e431f95098d7bad1cdac8ae0e806412d8)

    It seems like this is referring to leveldb/libleveldb.la leveldb/libmemenv.la crc32c/libcrc32c.la library files directly is breaking the convention of referring to dependencies through variables like LIBSECP256K1 LIBUNIVALUE. It makes this code more tied to implementation details of leveldb and crc libraries that it would otherwise need to be.


    dongcarl commented at 4:11 am on March 16, 2022:
    Done 0057c5096ec798324208485642039d69b4b6f518
  30. ryanofsky commented at 10:29 pm on March 14, 2022: member

    e322677b3b784fc48d928ae9daa7ab1ca90bfb8a mostly looks good but I think it would be good to avoid declaring leveldb, crc, and other libraries twice as .a and .la versions, and just build .la versions in the vanilla way shown https://www.gnu.org/software/libtool/manual/html_node/Using-Automake.html

    Also the implementation has changed a little and I think you could update the PR description (drop the WTF paragraph)

  31. dongcarl force-pushed on Mar 16, 2022
  32. dongcarl commented at 4:05 am on March 16, 2022: member

    Pushed e322677b3b784fc48d928ae9daa7ab1ca90bfb8a -> e51ead8496f010315b21fb48465b120e5cb8e334

    • Revamped most commits to replace .a’s with .la’s instead of creating additional .la’s
    • Added LIBTOOL_APP_LDFLAGS to _LDFLAGS of .la’s, see 567927c467e25a9f9b864f2dcef6bfab9ac15359’s commit message for more details
    • Added a commit that removes a vestigial LIBLEVELDB_SSE42: bba336bdde5431f9cd615a056a7939f5cecd0a4a
    • Added a commit that defines G_TRANSLATION_FUN in bitcoinkernel.cpp: a36c0ff4af800e5499bbe7aca3137581251e71b3
    • Addressed most review comments (thanks Ryan! ❤️ )
  33. dongcarl force-pushed on Mar 16, 2022
  34. dongcarl commented at 4:14 am on March 16, 2022: member

    Push e51ead8496f010315b21fb48465b120e5cb8e334 -> 32183205c21411c630efd818b7b303c16da2a323

    • Rebased over master
  35. DrahtBot removed the label Needs rebase on Mar 16, 2022
  36. dongcarl force-pushed on Mar 16, 2022
  37. dongcarl commented at 5:31 pm on March 16, 2022: member

    Push 32183205c21411c630efd818b7b303c16da2a323 -> d31c6015796f58dbede31fb313c382ba214b4b73

    • Reorder bench_bitcoin’s LDADD list to compensate for ld.bfd’s inability to resolve out of order references: 36494c5c02ea5e40cf18eca4094592508d47474a
  38. MarcoFalke added the label DrahtBot Guix build requested on Mar 16, 2022
  39. dongcarl added this to the "WIP PR" column in a project

  40. dongcarl moved this from the "WIP PR" to the "Ready for Review PR" column in a project

  41. DrahtBot commented at 9:12 am on March 18, 2022: member

    Guix builds

    File commit 3617d225628f629bbfa686937edf63f66f12e138(master) commit 5af182b06732e794d644d85160362ebd918218a5(master and this pull)
    SHA256SUMS.part e7ba8719d2c4f234... 8b5ad15f0c1e1116...
    *-aarch64-linux-gnu-debug.tar.gz a746a7274326c658... d6e05cd3958aaf88...
    *-aarch64-linux-gnu.tar.gz 13abb17969dec0f9... dc312e06662d0a77...
    *-arm-linux-gnueabihf-debug.tar.gz 0d8b3ed7f5a4505c... 56e2bf60a6d77f62...
    *-arm-linux-gnueabihf.tar.gz 864724a4a70b392f... 9a0625e3df03429f...
    *-arm64-apple-darwin.tar.gz d27ab01a8ca28a60... b654622a0b71f923...
    *-osx-unsigned.dmg 671a8ea24e3566d1... 0bd04a027c2746a2...
    *-osx-unsigned.tar.gz 2a92ef33dd53d204... b9b00be5e71f9e1e...
    *-osx64.tar.gz ee05d39268fd39c5... f8f3ff5743a36f25...
    *-powerpc64-linux-gnu-debug.tar.gz 47a30aa13d503221... cf1f9461915a69d7...
    *-powerpc64-linux-gnu.tar.gz 37f8390fefc3a3ea... cb84e1cf89ff0b87...
    *-powerpc64le-linux-gnu-debug.tar.gz 93d826410d4ba1d9... e324d16f5a6cc98f...
    *-powerpc64le-linux-gnu.tar.gz 87fae117e74bf977... 223680b3af707eda...
    *-riscv64-linux-gnu-debug.tar.gz 2b817268f578aad0... 2b8bec27c007d82a...
    *-riscv64-linux-gnu.tar.gz 4afb4438cb0f9ccd... 1fb59ff92c8fd125...
    *-win-unsigned.tar.gz 80d77ae299db20e9... 3a53afd1d33365d7...
    *-win64-debug.zip 3539d8d6afa7bb75... febf072e12485f06...
    *-win64-setup-unsigned.exe e3a887b96733e898... 261af5ffabfe5c0d...
    *-win64.zip 707b8e9cc1f1276f... fd40eb49973ed35c...
    *-x86_64-linux-gnu-debug.tar.gz a291d4e1058df942... a8e56859b73a17d0...
    *-x86_64-linux-gnu.tar.gz ecc53de536b18b34... 1398b65b5559636e...
    *.tar.gz 940e6af52dd7969e... bc6e6c7cf6b1e005...
    guix_build.log c4153607b4f4255a... ba6200ad3171cba7...
    guix_build.log.diff 243b914317f0cf53...
  42. DrahtBot removed the label DrahtBot Guix build requested on Mar 18, 2022
  43. jonatack commented at 10:26 pm on March 24, 2022: member

    Debug-compiled with Debian clang 15.0.0, clang 13.0.1, and gcc 11.2.0. Seeing the following with clang 15.0.0 only (Debian testing, so feel free to ignore).

    0Debian clang version 15.0.0-++20220320111423+51e6059c1277-1~exp1~20220320111511.381
    1...
    2  CXX      qt/libbitcoinqt_a-bitcoingui.o
    3clang: warning: argument unused during compilation: '-pthread' [-Wunused-command-line-argument]
    4clang: warning: argument unused during compilation: '-pthread' [-Wunused-command-line-argument]
    
  44. jonatack commented at 5:46 pm on March 25, 2022: member

    ACK d31c6015796f58dbede31fb313c382ba214b4b73 for each commit, code review and made debug builds (autogen/configure/make distclean/make) on debian testing with both gcc 11.2.0 and clang 15.0.0 (and also clang 13.0.1 on the final commit) and ran some unit tests

    0--enable-debug
    1--enable-bench 
    2--enable-suppress-external-warnings
    3--enable-experimental-util-chainstate / --with-experimental-kernel-lib
    

    The updates in response to the A+ review feedback by @ryanofsky look good from what I can tell.

    Starting with commit 764c1f18c031166b406be439486956fb84d3b9fe “build: Extract the libbitcoinkernel library”, both gcc and clang print the following while building

    0copying selected object files to avoid basename conflicts...
    

    (and the probably ignorable clang 15 warnings described in #24322 (comment) appear starting with the same commit)

    0clang: warning: argument unused during compilation: '-pthread' [-Wunused-command-line-argument]
    1clang: warning: argument unused during compilation: '-pthread' [-Wunused-command-line-argument]
    
  45. jonatack commented at 5:58 pm on March 25, 2022: member
    Correcting myself after re-verifying building with clang 13.0.1: it prints the warnings too.
  46. dongcarl commented at 7:26 pm on March 25, 2022: member

    Thanks for the review @jonatack!

    Starting with commit 764c1f18c031166b406be439486956fb84d3b9fe “build: Extract the libbitcoinkernel library”, both gcc and clang print the following while building

    0copying selected object files to avoid basename conflicts...
    

    These are probably not too significant. While developing libbitcoinkernel we’ll have some duplicate compiling of object files, but they only happen when you configure with the experimental flags and should go away in the end.

    (and the probably ignorable clang 15 warnings described in #24322 (comment) appear starting with the same commit)

    0clang: warning: argument unused during compilation: '-pthread' [-Wunused-command-line-argument]
    1clang: warning: argument unused during compilation: '-pthread' [-Wunused-command-line-argument]
    

    Yeah probably not too big of a problem. (Naively I think we should probably turn off -Wunused-command-line-argument at some point…)

  47. in src/Makefile.crc32c.include:38 in 636db73ae3 outdated
    32@@ -33,41 +33,44 @@ else
    33 CRC32C_CPPFLAGS_INT += -DBYTE_ORDER_BIG_ENDIAN=0
    34 endif
    35 
    36-crc32c_libcrc32c_a_CPPFLAGS = $(AM_CPPFLAGS) $(CRC32C_CPPFLAGS_INT) $(CRC32C_CPPFLAGS)
    37-crc32c_libcrc32c_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
    38+crc32c_libcrc32c_la_CPPFLAGS = $(AM_CPPFLAGS) $(CRC32C_CPPFLAGS_INT) $(CRC32C_CPPFLAGS)
    39+crc32c_libcrc32c_la_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
    40+crc32c_libcrc32c_la_LDFLAGS = $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS)
    


    ryanofsky commented at 3:18 am on March 29, 2022:

    In commit “build: Create .la library for crc32c” (636db73ae358eb07c0fe3c721d6677b7867b380f)

    I think it’s probably bad to apply LIBTOOL_APP_LDFLAGS flags to a library, when it seems like the whole reason that variable exists is to separate application flags from library flags. Plus, the line would be a lot more obvious if it just said -all-static literally.

    It would also be nice to add simple explanatory comment above like “# Specify -all-static so libtool will only build a static version of this library. We don’t need a dynamic version, and a dynamic version can’t be used on windows anyway because the library doesn’t currently export DLL symbols.”

    The longer comment in the commit message is also OK, but I think comments like these need to be in the source. Nobody could be expected to understand how these makefiles work if information about how they work is buried in commit history.


    dongcarl commented at 5:57 pm on April 14, 2022:
    Fixed as of 3961a789735c874ed9b3ff4bbd64c2b850282c07
  48. in src/Makefile.leveldb.include:42 in 06816ad68c outdated
    36@@ -37,111 +37,113 @@ else
    37 LEVELDB_CPPFLAGS_INT += -DLEVELDB_PLATFORM_POSIX
    38 endif
    39 
    40-leveldb_libleveldb_a_CPPFLAGS = $(AM_CPPFLAGS) $(LEVELDB_CPPFLAGS_INT) $(LEVELDB_CPPFLAGS)
    41-leveldb_libleveldb_a_CXXFLAGS = $(filter-out -Wconditional-uninitialized -Werror=conditional-uninitialized -Wsuggest-override -Werror=suggest-override, $(AM_CXXFLAGS)) $(PIE_FLAGS)
    42+leveldb_libleveldb_la_CPPFLAGS = $(AM_CPPFLAGS) $(LEVELDB_CPPFLAGS_INT) $(LEVELDB_CPPFLAGS)
    43+leveldb_libleveldb_la_CXXFLAGS = $(filter-out -Wconditional-uninitialized -Werror=conditional-uninitialized -Wsuggest-override -Werror=suggest-override, $(AM_CXXFLAGS)) $(PIE_FLAGS)
    44+leveldb_libleveldb_la_LDFLAGS = $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS)
    


    ryanofsky commented at 3:26 am on March 29, 2022:

    In commit “build: Create .la library for crc32c” (636db73ae358eb07c0fe3c721d6677b7867b380f)

    Same feedback here as the last commit. I think it would be clearer and more appropriate to replace $(LIBTOOL_APP_LDFLAGS) with -all-static and add a comment like “# Specify -all-static so libtool will only build a static version of this library. We don’t need a dynamic version, and trying to use it breaks the windows build because we aren’t currently enabling the LEVELDB_SHARED_LIBRARY and LEVELDB_COMPILE_LIBRARY flags to export DLL symbols.”


    dongcarl commented at 5:57 pm on April 14, 2022:
    Fixed as of 3961a789735c874ed9b3ff4bbd64c2b850282c07
  49. in src/Makefile.am:462 in 2084db2830 outdated
    456@@ -457,9 +457,10 @@ libbitcoin_wallet_tool_a_SOURCES = \
    457   $(BITCOIN_CORE_H)
    458 
    459 # crypto primitives library
    460-crypto_libbitcoin_crypto_base_a_CPPFLAGS = $(AM_CPPFLAGS)
    461-crypto_libbitcoin_crypto_base_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
    462-crypto_libbitcoin_crypto_base_a_SOURCES = \
    463+crypto_libbitcoin_crypto_base_la_CPPFLAGS = $(AM_CPPFLAGS)
    464+crypto_libbitcoin_crypto_base_la_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
    465+crypto_libbitcoin_crypto_base_la_LDFLAGS = $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS)
    


    ryanofsky commented at 3:34 am on March 29, 2022:

    In commit “build: Create .la library for bitcoincrypto” (2084db283053f48cdcbb967905f13d234175a3c8)

    Same feedback as previous two commits: Would be clearer and more appropriate to replace $(LIBTOOL_APP_LDFLAGS) with -all-static and add a comment like “# Specify -all-static so libtool will only build a static version of this library. We don’t need a dynamic version, and trying to use it breaks the windows build because the library isn’t currently exporting DLL symbols.”


    dongcarl commented at 5:57 pm on April 14, 2022:
    Fixed as of 3961a789735c874ed9b3ff4bbd64c2b850282c07
  50. in src/Makefile.am:934 in 89a194f669 outdated
    873@@ -874,6 +874,8 @@ bitcoin_chainstate-clientversion.$(OBJEXT): obj/build.h
    874 
    875 # bitcoinconsensus library #
    876 if BUILD_BITCOIN_LIBS
    877+lib_LTLIBRARIES += $(LIBBITCOINCONSENSUS)
    


    ryanofsky commented at 4:47 am on March 29, 2022:

    In commit “build: Separate lib_LTLIBRARIES initialization” (89a194f669f8e1bd0ba979b07d9a282dcc6f7533)

    Note (just for understanding): There is no change of behavior here even though this line is moved inside the if BUILD_BITCOIN_LIBS block because LIBBITCOINCONSENSUS variable is empty when BUILD_BITCOIN_LIBS isn’t true.


    dongcarl commented at 3:38 pm on April 18, 2022:
    Leaving open for the note.
  51. in src/Makefile.am:811 in 764c1f18c0 outdated
    808+
    809+# bitcoinkernel library #
    810+if BUILD_BITCOIN_KERNEL_LIB
    811+lib_LTLIBRARIES += $(LIBBITCOINKERNEL)
    812+
    813+libbitcoinkernel_la_LDFLAGS = $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) $(PTHREAD_FLAGS)
    


    ryanofsky commented at 5:55 am on March 29, 2022:

    In commit “build: Extract the libbitcoinkernel library” (764c1f18c031166b406be439486956fb84d3b9fe)

    I’m confused how this could be building a shared library if LIBTOOL_APP_LDFLAGS includes -all-static “If output-file is a library, then only create a static library.”

    Also it doesn’t seem right to specify an APP_ flag for a library.


    dongcarl commented at 6:03 pm on April 14, 2022:

    I’ve been going down another deep rabbit hole with this, and it is surprisingly hard to create a shared mingw-w64 dll with libstdc++, libpthread, etc statically embedded…

    What I’m going to do is just leave it as a static library for now, so that we can make progress on the main thrust of the project (even with just the static library, we will surface re-couplings via linker errors), open an issue detailing my spelunking, and work on getting the build correct in the mean time. It is very likely that LIBTOOL_IS_A_FOOL


    ajtowns commented at 9:34 am on April 20, 2022:
    If this is just creating another static library now, I don’t really understand why it’s still so complicated? Can’t you just add the static library, and not do all the libtool stuff?

    ryanofsky commented at 8:41 pm on April 20, 2022:

    If this is just creating another static library now, I don’t really understand why it’s still so complicated? Can’t you just add the static library, and not do all the libtool stuff?

    A new comment https://github.com/dongcarl/bitcoin/blob/3961a789735c874ed9b3ff4bbd64c2b850282c07/src/Makefile.am#L811-L813 mentions plans to build libbitcoinkernel as a shared library instead of a static library in the future. IIUC, libtool is needed to do that to make sure static libraries linked into the dynamic library are built with correct PIC flags.


    ajtowns commented at 5:26 am on April 22, 2022:
    Right, but why not leave the libtool / .la commits until the future PR that does build libbitcoinkernel as a shared library?

    ryanofsky commented at 12:00 pm on April 22, 2022:

    Right, but why not leave the libtool / .la commits until the future PR that does build libbitcoinkernel as a shared library?

    ~Dude, I don’t know~ [EDIT: Sorry for exasperated tone. Woke up grumpy this morning!]. I think libbitcoinkernel is just big project and we’re trying to make incremental progress.

    These commits are simple, basically just replacing _a with _la, and would be annoying to maintain separately because they’ll conflict with pretty much every other change to these files. They also clean up some obsolete things, make the libraries compatible with dynamic linking generically (not specific to libbitcoinkernel), and document the libraries varying abilities to be dynamically linked themselves. So they are good changes.

    It’d be fine to move these commits into separate PRs if that would actually help get this PR reviewed. But otherwise I think questions about how PRs are subdivided are not that useful, and that it would be a loss and obstacle to progress if these changes had delayed indefinitely until they were part of some more comprehensive “future PR”.


    dongcarl commented at 10:00 pm on April 27, 2022:

    With the latest push, we build shared libraries for all platforms except Windows! 🎉

    I think libbitcoinkernel is just big project and we’re trying to make incremental progress.

    ☺️

  52. in src/Makefile.am:792 in 764c1f18c0 outdated
    789+bitcoin_chainstate_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
    790+bitcoin_chainstate_LDFLAGS = $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) $(PTHREAD_FLAGS)
    791+bitcoin_chainstate_LDADD = $(LIBBITCOINKERNEL)
    792+#
    793+
    794+# bitcoinconsensus library #
    


    ryanofsky commented at 6:04 am on March 29, 2022:

    In commit “build: Extract the libbitcoinkernel library” (764c1f18c031166b406be439486956fb84d3b9fe)

    Not very important, but if targets were declared in order: bitcoin_chainstate, bitcoinkernel, bitcoinconsensus, less code would be moving around and related chainstate/kernel declarations would be closer together


    dongcarl commented at 5:56 pm on April 14, 2022:
    Fixed as of 3961a789735c874ed9b3ff4bbd64c2b850282c07
  53. ryanofsky approved
  54. ryanofsky commented at 6:19 am on March 29, 2022: member

    Code review ACK d31c6015796f58dbede31fb313c382ba214b4b73, but I do think LIBTOOL_APP_LDFLAGS variable is misused throughout this PR and would be happier if it could be cleaned up before this was merged.

    Apart from that though, everything else looks great. Nice work enabling the libtool builds and dealing with the windows build errors!

  55. in src/kernel/bitcoinkernel.cpp:5 in d31c601579 outdated
    0@@ -0,0 +1,4 @@
    1+#include <functional>
    


    ariard commented at 4:22 pm on April 4, 2022:
    nit: You can add the Copyright.

    dongcarl commented at 5:56 pm on April 14, 2022:
    Fixed as of 3961a789735c874ed9b3ff4bbd64c2b850282c07
  56. in configure.ac:1681 in d31c601579 outdated
    1651 if test "$build_bitcoin_libs" = "yes"; then
    1652   AC_DEFINE([HAVE_CONSENSUS_LIB], [1], [Define this symbol if the consensus lib has been built])
    1653   AC_CONFIG_FILES([libbitcoinconsensus.pc:libbitcoinconsensus.pc.in])
    1654 fi
    1655+
    1656+AM_CONDITIONAL([BUILD_BITCOIN_KERNEL_LIB], [test "$build_experimental_kernel_lib" != "no" && ( test "$build_experimental_kernel_lib" = "yes" || test "$build_bitcoin_chainstate" = "yes" )])
    


    ariard commented at 4:31 pm on April 4, 2022:
    I don’t remember if bitcoin-chainstate is intended to be a long-term binary though you could add a rule in the top makefile to build it from the root bitcoind directory. I was confused to fail on it at first.

    ryanofsky commented at 5:24 pm on April 4, 2022:

    I don’t remember if bitcoin-chainstate is intended to be a long-term binary though you could add a rule in the top makefile to build it from the root bitcoind directory. I was confused to fail on it at first.

    It could be convenient to add the rule, but I think think this is not intended to stick around long term. Instead it will be moved to an examples folder and given a cmake file (:partying_face:) and I think just detached from the main build: https://github.com/dongcarl/bitcoin/tree/2022-01-v8-on-new-kirby/src/kernel/example/bitcoin-chainstate

  57. ariard commented at 4:43 pm on April 4, 2022: member

    Light Code Review ACK d31c601

    Built bitcoin-chainstate, linked with libbitcoinnkernel.so on debian 10.

  58. DrahtBot added the label Needs rebase on Apr 5, 2022
  59. build: Don't add unrelated libs to LIBTEST_*
    This was used to, in effect, manually emulate --start-group/--end-group.
    However, we can just order the libraries correctly and avoid specifying
    libraries multiple times on the link line.
    
    Note: lld (not ld.bfd) knows how to resolve out-of-order references and
          doesn't seem to need the reodering
    1392e8e2d8
  60. build: Remove vestigial LIBLEVELDB_SSE42
    - LIBLEVELDB_SSE42_INT was defined, but never referenced anywhere
    - LIBLEVELDB_SSE42 is referenced, but never defined anywhere
    
    Apparently leveldb used to have platform-specific crc32 code before it
    got split off into a separate lib.
    64caf94479
  61. dongcarl commented at 0:27 am on April 12, 2022: member
    @ryanofsky I actually didn’t look into the LEVELDB_SHARED_LIBRARY and LEVELDB_COMPILE_LIBRARY flags… It doesn’t seem to hard to set them correctly, perhaps that’d be a more elegant solution to my problem. Will try it.
  62. ryanofsky commented at 0:59 am on April 12, 2022: member

    @ryanofsky I actually didn’t look into the LEVELDB_SHARED_LIBRARY and LEVELDB_COMPILE_LIBRARY flags… It doesn’t seem to hard to set them correctly, perhaps that’d be a more elegant solution to my problem. Will try it.

    To be sure, I wasn’t suggesting in #24322 (review) that you should set those flags. Those flags would only be needed if you wanted to drop -all-static on windows, so the comment I suggested adding explaining -all-static mentioned them. I think it is probably a good thing to keep using -all-static for internal leveldb, crc, and crypto libraries, since would keep these libraries statically linked and internal to the libbitcoinkernel shared library. You could drop -all-static, and go the route of building these libraries as shared libraries, but think there probably isn’t much benefit to doing that. My suggestions were just about making usage of -all-static more explicit and better documented.

  63. dongcarl force-pushed on Apr 14, 2022
  64. dongcarl commented at 5:58 pm on April 14, 2022: member

    Pushed d31c6015796f58dbede31fb313c382ba214b4b73 -> 3961a789735c874ed9b3ff4bbd64c2b850282c07

    • Addressed all review comments, thanks Ryan and Antoine!
  65. dongcarl commented at 6:04 pm on April 14, 2022: member
    An important update for reviewers: #24322 (review)
  66. DrahtBot removed the label Needs rebase on Apr 14, 2022
  67. in src/kernel/bitcoinkernel.cpp:10 in 9b6296dcd4 outdated
    0@@ -0,0 +1,8 @@
    1+// Copyright (c) 2022 The Bitcoin Core developers
    2+// Distributed under the MIT software license, see the accompanying
    3+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
    4+
    5+#include <functional>
    6+#include <string>
    7+
    8+extern const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;
    


    ryanofsky commented at 9:12 pm on April 20, 2022:

    In commit “b-cs: Define G_TRANSLATION_FUN in bitcoinkernel.cpp” (9b6296dcd4f0601b6c9ff5958edf385ca12e6d75)

    It might be good to add a comment explaining this like “Define G_TRANSLATION_FUN symbol in libbitcoinkernel library so users of the library aren’t required to export this symbol”


    dongcarl commented at 9:36 pm on April 21, 2022:
    Done!
  68. in configure.ac:1527 in bd1376c178 outdated
    1523@@ -1524,7 +1524,7 @@ AM_CONDITIONAL([ENABLE_SYSCALL_SANDBOX], [test "$use_syscall_sandbox" != "no"])
    1524 
    1525 dnl Check for reduced exports
    1526 if test "$use_reduce_exports" = "yes"; then
    1527-  AX_CHECK_COMPILE_FLAG([-fvisibility=hidden], [CORE_CXXFLAGS="$CORE_CXXFLAGS -fvisibility=hidden"],
    1528+  AX_CHECK_COMPILE_FLAG([-fvisibility=hidden], [RE_CXXFLAGS="$RE_CXXFLAGS -fvisibility=hidden"],
    


    ryanofsky commented at 9:32 pm on April 20, 2022:

    In commit “build: Put reduced exports flags in RE_CXXFLAGS” (bd1376c178df10cf64faa8d2e810149ff0ff0d49)

    The commit message rationale about “we really shouldn’t be setting CXXFLAGS” doesn’t seem to apply anymore now that current code is setting CORE_CXXFLAGS. But maybe another reason for changing this is to make R_CXXFLAGS consististent with RE_LDFLAGS?

    Commit message should probably be fixed in any case


    dongcarl commented at 9:37 pm on April 21, 2022:
    I removed the REFLAGS changes from this PR, thanks for the suggestion!
  69. ryanofsky approved
  70. ryanofsky commented at 9:39 pm on April 20, 2022: member

    Code review ACK 3961a789735c874ed9b3ff4bbd64c2b850282c07. Since last review, this was rebased, some review suggestions were implemented, and comments and commit messages were updated. Main change is using -all-static more explicitly instead of LIBTOOL_APP_LDFLAGS.

    I do wonder if they RE_CXXFLAGS/RE_LDFLAGS changes could be split off from this PR and moved into a separate PR. They make this more complicated, and don’t seem to be actually necessary here.

    re: #24322 (review)

    I’ve been going down another deep rabbit hole with this, and it is surprisingly hard to create a shared mingw-w64 dll with libstdc++, libpthread, etc statically embedded…

    I don’t really think it would be an improvement to make leveldb/crc/crypto libraries dynamic instead of static. Even if you want libbitcoinkernel to be dynamic, linking these smaller libraries into it statically should be fine, and could even have benefits like allowing an application that uses libbitcoinkernel and leveldb to use a different version of leveldb than libitcoinkernel uses.

    re: #24322#issue-1132989320

    Part of: #24303 Depends on: #24304

    This PR introduces a libbitcoinkernel library linking in the minimal list of files necessary to use our consensus engine as-is. This list is moved from the bitcoin-chainstate executable introduced in the dependent PR #24304, and bitcoin-chainstate now will link against libbitcoinkernel.

    Most of the changes are related to the build system.

    Please read the commit messages for more details.

    Note: This was split out from #24304 per this suggestion

    Relevant comments/review threads prior to split

    * **Topic**: Adding a CI job to test the new build procedures
      
      * [[kernel 0/n] Introduce `bitcoin-chainstate` [#24304](/bitcoin-bitcoin/24304/) (comment)](/bitcoin-bitcoin/24304/#issuecomment-1035466827)
      * [[kernel 0/n] Introduce `bitcoin-chainstate` [#24304](/bitcoin-bitcoin/24304/) (comment)](/bitcoin-bitcoin/24304/#issuecomment-1035479455)
      * [[kernel 0/n] Introduce `bitcoin-chainstate` [#24304](/bitcoin-bitcoin/24304/) (comment)](/bitcoin-bitcoin/24304/#issuecomment-1035491348)
      * [[kernel 0/n] Introduce `bitcoin-chainstate` [#24304](/bitcoin-bitcoin/24304/) (comment)](https://github.com/bitcoin/bitcoin/pull/24304#discussion_r804422582)
      * **Resolution**: Added to the "no wallet" job, see: [ci: Build bitcoin-chainstate + libbitcoinkernel](https://github.com/bitcoin/bitcoin/pull/24322/commits/d1f4494abc3f606e2fe4109edec68ce0c75eb282)
    

    You may ask: WTF?! Why is index/*.cpp, etc. being linked in to a consensus engine library?

    This PR is meant only to capture the state of dependencies in our consensus engine as of right now. There are many things to decouple from consensus, which will be done in subsequent PRs. Listing the files out right now in libbitcoinkernel_la_SOURCES is purely to give us a clear picture of the task at hand, it is not to say that these dependencies belongs in a consensus engine library in any way.

    Followups:

    * [[kernel 1/n] Introduce initial `libbitcoinkernel` [#24322](/bitcoin-bitcoin/24322/) (comment)](https://github.com/bitcoin/bitcoin/pull/24322#discussion_r827608636)
    

    I think basically this whole PR description is obsolete, and would be good to rewrite. Possible description might be something like “introduce libbitcoinkernel library and link it into chainstate test program. Use libtool to build static libraries that libbitcoinkernel depends on, so it can be built as a dynamic library in the future.”

  71. dongcarl force-pushed on Apr 21, 2022
  72. dongcarl commented at 9:36 pm on April 21, 2022: member

    Pushed 3961a789735c874ed9b3ff4bbd64c2b850282c07 -> 05ad18f563d1ab1c1c6a11fcd080038a78401546

  73. ryanofsky approved
  74. ryanofsky commented at 0:12 am on April 22, 2022: member

    Code review ACK 05ad18f563d1ab1c1c6a11fcd080038a78401546. Only changes since last review are dropping reduced export cleanup commits, and changing some comments.

    It would be good to have more reviewers, now that PR is simpler and more focused, and all the changes are pretty comprehensively explained. Hopefully reviewers won’t be intimidated by build arcana and will ask questions if the changes and explanations aren’t clear enough.

    re: #24322#issue-1132989320

    You may ask: WTF?! Why is index/*.cpp, etc. being linked in to a consensus engine library?

    Just a thought, but I think this paragraph of PR description is more confusing than helpful, and it’s hard to figure out what’ it’s referring to since there are no new index/*.cpp lines being added here. Would recommend dropping this section from the PR description. If you do want to call attention to the fact libbitcoinkernel_la_SOURCES includes some files that we plan to remove in the future, I think the best place to write that would be a comment directly on libbitcoinkernel_la_SOURCES.

  75. dongcarl commented at 5:30 pm on April 22, 2022: member

    @ryanofsky

    Just a thought, but I think this paragraph of PR description is more confusing than helpful, and it’s hard to figure out what’ it’s referring to since there are no new index/*.cpp lines being added here. Would recommend dropping this section from the PR description. If you do want to call attention to the fact libbitcoinkernel_la_SOURCES includes some files that we plan to remove in the future, I think the best place to write that would be a comment directly on libbitcoinkernel_la_SOURCES.

    Done in latest push!

  76. in src/Makefile.bench.include:55 in 1392e8e2d8 outdated
    51@@ -52,13 +52,13 @@ nodist_bench_bench_bitcoin_SOURCES = $(GENERATED_BENCH_FILES)
    52 bench_bench_bitcoin_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(EVENT_CFLAGS) $(EVENT_PTHREADS_CFLAGS) -I$(builddir)/bench/
    53 bench_bench_bitcoin_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
    54 bench_bench_bitcoin_LDADD = \
    55+  $(LIBTEST_UTIL) \
    


    theuni commented at 7:36 pm on April 22, 2022:
    Nice, does this mean preserve-dup-deps can go away now: https://github.com/bitcoin/bitcoin/blob/master/src/Makefile.am#L14 ?

    dongcarl commented at 3:45 pm on April 25, 2022:
    Perhaps? Although I think GNU bfd ld still has requirements on library ordering so we should probably keep it…

    theuni commented at 8:02 pm on April 25, 2022:

    Yes, but only to default-enforce the (very sane imo) rule of no circular dependencies.

    IIRC preserve-dup-deps was added at one point because we had a real circular dependency. If either of:

    • There was never actually a circular dependency and the flag was added as an overkill way to solve the ordering problem
    • There was but no longer is a circular dependency

    then imo the flag should be removed to put us back in the “strict ordering” mode, which would be helpful for pointing out accidental new circular dependencies.

    But if I’m wrong about those two conditions, just ignore me :)


    dongcarl commented at 9:55 pm on April 27, 2022:
    Okay I see! Let’s do this as a followup.
  77. in src/Makefile.am:813 in 743306d15a outdated
    801-  bitcoin-chainstate.cpp \
    802+bitcoin_chainstate_SOURCES = bitcoin-chainstate.cpp
    803+bitcoin_chainstate_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES)
    804+bitcoin_chainstate_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
    805+bitcoin_chainstate_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) $(PTHREAD_FLAGS)
    806+bitcoin_chainstate_LDADD = $(LIBBITCOINKERNEL)
    


    theuni commented at 7:49 pm on April 22, 2022:
    Shouldn’t all the bitcoin_chainstate binary stuff be under the BUILD_BITCOIN_KERNEL_LIB condition below? Otherwise it’ll just fail to link I think?

    dongcarl commented at 4:08 pm on April 25, 2022:
    Fixed at configure time as of: a415b5080d288707c60634dab730ef7a5b6bb971
  78. in src/Makefile.am:830 in 743306d15a outdated
    819+libbitcoinkernel_la_CPPFLAGS = $(AM_CPPFLAGS) -I$(builddir)/obj -I$(srcdir)/secp256k1/include -DBUILD_BITCOIN_INTERNAL $(BOOST_CPPFLAGS) $(LEVELDB_CPPFLAGS) -I$(srcdir)/$(UNIVALUE_INCLUDE_DIR_INT)
    820+
    821+# libbitcoinkernel requires default symbol visibility, explicitly specify that
    822+# here so that things still work even when user configures with
    823+#   --enable-reduce-exports
    824+libbitcoinkernel_la_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) -fvisibility=default
    


    theuni commented at 7:56 pm on April 22, 2022:

    Mmm, this is ok as a quick hack, but I’d prefer to describe it here as such. Mind adding that it’s temporary until we actually define what’s to be exported?

    Eventually we’ll need our own set of macros and we’ll need to decorate our exported functions ala: https://github.com/bitcoin/bitcoin/blob/master/src/script/bitcoinconsensus.h#L11

    Otherwise the lib ends up with everything exported and stripping/lto can’t help.


    dongcarl commented at 4:08 pm on April 25, 2022:
    Fixed as of a415b5080d288707c60634dab730ef7a5b6bb971
  79. theuni commented at 8:08 pm on April 22, 2022: member

    Looking great!

    Without building locally… does the switch to libtool libs mean that more objects end up being built (other than the second set for the lib)? Or does -all-static basically fix that?

  80. theuni commented at 8:21 pm on April 22, 2022: member
    Just being paranoid… it’d be helpful imo to compare the object hashes before/after these changes just to verify that all flags are propagating to libtool correctly and that bitcoind is either unchanged or understandably changed.
  81. hebasto commented at 12:48 pm on April 25, 2022: member
    Concept ACK.
  82. hebasto commented at 1:55 pm on April 25, 2022: member

    Commit code reviewing:

    • 1392e8e2d8cfe4115f0a152aca16ffe3f0f4573a “build: Don’t add unrelated libs to LIBTEST*”_
    • 64caf944797bc35c3044fe5675389656f9511a41 “build: Remove vestigial LIBLEVELDB_SSE42”
    • 3acbba81e196a7870e050f211f6e18e6bdd85a39 “build: Create .la library for crc32c” Outdated commit message in part “Also append LIBTOOL_APP_LDFLAGS…”
    • 854d11fcbe9a59ba980a9b8e9b1a971b00ac40e6 “build: Create .la library for leveldb” The same.
    • 1eef3c9cc7d39c05fbce0d1327eb57f92d7899c0 “build: Create .la library for bitcoincrypto” The same.
    • 8c4c87234c9d08b676ab56492ab618d801ab70d6 “build: Separate lib_LTLIBRARIES initialization”
    • e215e9d87c3b985dc111a3efed5ad822a9804918 _“b-cs: Define G_TRANSLATION_FUN in bitcoinkernel.cpp”
    • 743306d15ab50d2d05be353b59e9797091873e54 “build: Extract the libbitcoinkernel library” In commit message:
    • “append LIBTOOL_APP_LDFLAGS” is outdated
    • typo: libbitcionkernel – > libbitcoinkernel
    • 05ad18f563d1ab1c1c6a11fcd080038a78401546 “ci: Build libbitcoinkernel”
    • 33837804cf25c1093159936128ab3daa7472601c “docs: Add libbitcoinkernel_la_SOURCES explanation”

    1. If the -all-static Libtool flag is required for Windows builds only, why do not apply it conditionally?

    2. Besides the -all-static mode, Libtool also suggests similar -static and -static-libtool-libs ones. Why the former has been chosen?

  83. ryanofsky commented at 3:21 pm on April 25, 2022: member

    re: #24322#pullrequestreview-950505318

    Without building locally… does the switch to libtool libs mean that more objects end up being built (other than the second set for the lib)? Or does -all-static basically fix that?

    I haven’t confirmed this but IIUC the same number of objects should be built right now because libbitcoinkernel is still static. If it is switched to be dynamic, more objects might be built because more things need to be built with PIC flags.

    re: #24322 (comment)

    1. If the -all-static Libtool flag is required for Windows builds only, why do not apply it conditionally?

    Maybe we will. We are not locked into a decision here but -all-static flag works well, and fixes immediate problems on windows. Trying other flags requires testing and experimentation, which is probably not worth the effort . I do think statically linking libbitcoinkernel dependencies even when libbitcoinkernel is dynamic could have other benefits like making the library easier to link against, and letting libbitcoinkernel applications be able to use a different leveldb version than libbitcoinkernel uses (as mentioned #24322#pullrequestreview-947768622). But this is just why it static linking seems like a good choice for these libraries at the moment. The only thing actually requiring these libraries to be statically linked is that they don’t export symbols on windows, which IMO is not worth making effort to implement now.

    1. Besides the -all-static mode, Libtool also suggests similar -static and -static-libtool-libs ones. Why the former has been chosen?

    This is interesting. I didn’t realize it before, but all three options seem identical when applied to libraries, and only different when applied to executables. The documentation for all three options at https://www.gnu.org/software/libtool/manual/html_node/Link-mode.html says “If output-file is a library, then only create a static library.”

    I’d probably prefer -static over -all-static for simpliciity, but I think -all-static is fine, and any of these options would be fine. History of the PR is that original version was using $LIBTOOL_APP_LDFLAGS, and $LIBTOOL_APP_LDFLAGS uses -all-static, and I thought it was inappropriate to use $LIBTOOL_APP_LDFLAGS, so I suggested using -all-static directly.

  84. dongcarl commented at 3:40 pm on April 25, 2022: member

    @theuni

    does the switch to libtool libs mean that more objects end up being built (other than the second set for the lib)? Or does -all-static basically fix that?

    Unfortunately, libtool seems to always want to build twice… Not sure if we can fix that.

    0$ find src/crypto -name '*sha256_sse41*'
    1src/crypto/.libs/libbitcoin_crypto_sse41_la-sha256_sse41.o
    2src/crypto/libbitcoin_crypto_sse41_la-sha256_sse41.lo
    3src/crypto/libbitcoin_crypto_sse41_la-sha256_sse41.o
    4src/crypto/sha256_sse41.cpp
    5src/crypto/.deps/libbitcoin_crypto_sse41_la-sha256_sse41.Plo
    

    Funnily enough these files are identical, BUT are not symlinks or hard links

    0$ diff --report-identical src/crypto/{.libs/,}libbitcoin_crypto_sse41_la-sha256_sse41.o
    1Files src/crypto/.libs/libbitcoin_crypto_sse41_la-sha256_sse41.o and src/crypto/libbitcoin_crypto_sse41_la-sha256_sse41.o are identical
    
    0$ stat src/crypto/.libs/libbitcoin_crypto_sse41_la-sha256_sse41.o | head -n3
    1  File: src/crypto/.libs/libbitcoin_crypto_sse41_la-sha256_sse41.o
    2  Size: 173240          Blocks: 344        IO Block: 4096   regular file
    3Device: 9,10    Inode: 22444798    Links: 1
    
    0$ stat src/crypto/libbitcoin_crypto_sse41_la-sha256_sse41.o | head -n3
    1  File: src/crypto/libbitcoin_crypto_sse41_la-sha256_sse41.o
    2  Size: 173240          Blocks: 344        IO Block: 4096   regular file
    3Device: 9,10    Inode: 21931470    Links: 1
    

    I also tried specifying -prefer-pic, which according to the manual makes sure “Libtool will try to build only PIC objects.” Nothing changed.

    Behaviour like this (largely from libtool) has exhausted my willpower quite a bit, and I think we should just leave for later.

  85. ryanofsky approved
  86. ryanofsky commented at 3:55 pm on April 25, 2022: member
    Code review ACK 33837804cf25c1093159936128ab3daa7472601c. Only change sinse last review is new commit adding suggested comment
  87. dongcarl force-pushed on Apr 25, 2022
  88. hebasto commented at 4:58 pm on April 25, 2022: member

    Tested ae94c62b1d0ac86bb7011434f8f51ce9cbb91164 on Ubuntu 22.04:

     0$ cd BUILD_WINDOWS
     1$ ../autogen.sh
     2$ ../configure CONFIG_SITE=$PWD/../depends/x86_64-w64-mingw32/share/config.site --with-experimental-kernel-lib --disable-tests --disable-bench --without-utils --disable-fuzz-binary
     3$ make clean
     4$ make
     5...
     6  CXXLD    bitcoind.exe
     7  CXXLD    libbitcoinkernel.la
     8libtool: warning: undefined symbols not allowed in x86_64-w64-mingw32 shared libraries; building static only
     9copying selected object files to avoid basename conflicts...
    10...
    
  89. in ci/test/00_setup_env_native_nowallet_libbitcoinkernel.sh:14 in ae94c62b1d outdated
    10@@ -11,4 +11,4 @@ export DOCKER_NAME_TAG=ubuntu:18.04  # Use bionic to have one config run the tes
    11 export PACKAGES="python3-zmq clang-8 llvm-8 libc++abi-8-dev libc++-8-dev"  # Use clang-8 to test C++17 compatibility, see doc/dependencies.md
    12 export DEP_OPTS="NO_WALLET=1 CC=clang-8 CXX='clang++-8 -stdlib=libc++'"
    13 export GOAL="install"
    14-export BITCOIN_CONFIG="--enable-reduce-exports CC=clang-8 CXX='clang++-8 -stdlib=libc++' --enable-experimental-util-chainstate"
    15+export BITCOIN_CONFIG="--enable-reduce-exports CC=clang-8 CXX='clang++-8 -stdlib=libc++' --enable-experimental-util-chainstate --with-experimental-kernel-lib --enable-shared"
    


    hebasto commented at 5:08 pm on April 25, 2022:

    I cannot see any difference in the built artifacts that depends on the --enable-shared option.

    What is expected?


    dongcarl commented at 9:58 pm on April 27, 2022:
    With the latest push (035fa1f07aacb7bce74c0884ae28c8cf00fe3b1b), I elected to keep it to be explicit about the fact the bitcoin-chainstate is linking against a shared libbitcoinkernel
  90. theuni commented at 9:10 pm on April 25, 2022: member

    @theuni

    does the switch to libtool libs mean that more objects end up being built (other than the second set for the lib)? Or does -all-static basically fix that?

    Unfortunately, libtool seems to always want to build twice… Not sure if we can fix that.

    I think I’ve come up with a reasonably clean solution to this here: https://github.com/theuni/bitcoin/commit/9ffbd707afc8188fe0b8cc5316f6c583f7ae21d0

    libtool tries to build twice by default: once for static, once for shared. With this change, there’s only static by default, so default configs continue to only build one set of objects.

    With this commit, if you want shared libs/dlls, you’d need to configure with --enable-shared, which will then build everything twice. For a project that’s mostly about executables, that seems reasonable to me. (Also, amazingly --enable-shared --disable-static does the right thing and also builds a single copy of everything for shared libs)

    This also means we don’t have to special-case the Windows case in this pr. We’ll just know that --enable-shared is busted on windows (and maybe warn as such for that config?)

    The only downside I can think of is that devs aren’t going to be linking the lib every day as part of their typical workflows so link problems may show up around release time. IMO that’s a reasonable trade-off.

  91. theuni commented at 4:42 pm on April 26, 2022: member

    This commit fixes Windows dll linking for me: https://github.com/theuni/bitcoin/commit/9612f5f0f1e3295065dbd8a0e0f472f8f020bbd9 . Untested, but it now links. I’ll be testing it out as a next step.

    Thanks to @dongcarl for pointing out the problem.

    Edit: I’m unable to find what’s actually affected by this define in my local mingw tree and in depends. Maybe gcc does something internally with it or something?

    Edit2: Found it!: https://github.com/mingw-w64/mingw-w64/blob/f79f89f976dc5345090db557fb20d5deba913d9f/mingw-w64-libraries/winpthreads/include/pthread.h#L86

    That seems like heavy-handed behavior for mingw-w64, much more useful would be for them to define and use PTHREADS_DLL_EXPORT instead. I’ll work to understand this better and upstream a patch if necessary.

  92. build: Create .la library for crc32c
    Libtool will yell at you if you try to link a shared library against
    static ones.
    
    This change creates a libtool archive library for crc32c and allows a
    shared library to be linked against it portably.
    
    Also specify -static in both:
    
    - ..._la_CXXFLAGS so that libtool will avoid building two versions of
      each object (one PIC, one non-PIC). We just need the one that is
      suitable for static linking.
    - ..._la_LDFLAGS so that libtool will create a static library.
    
    [META] This change is done in preparation for a future commit where we
           link the libbitcoinkernel library against this one.
    05d1525b6d
  93. build: Create .la library for leveldb
    Libtool will yell at you if you try to link a shared library against
    static ones.
    
    This change creates a libtool archive library for leveldb and allows a
    shared library to be linked against it portably.
    
    Also specify -static in both:
    
    - ..._la_CXXFLAGS so that libtool will avoid building two versions of
      each object (one PIC, one non-PIC). We just need the one that is
      suitable for static linking.
    - ..._la_LDFLAGS so that libtool will create a static library.
    
    If we don't specify this, then libtool will build two versions of each
    object and prefer the non-static PIC version of the object, which is
    built with -DDLL_EXPORT -DPIC for mingw-w64 targets. This can cause
    symbol resolution problems when we link this library against an
    executable that does specify -all-static, since that will be built
    without the -DDLL_EXPORT flag.
    
    This is especially important for leveldb and memenv since they link
    against libwinpthreads, which has difference symbols depending on
    whether DLL_EXPORT is defined or not.
    
    [META] This change is done in preparation for a future commit where we
           link the libbitcoinkernel library against this one.
    
    Appendix:
    
    The specific linker errors when linking memenv built without -all-static
    against a bitcoind with -all-static look like:
    
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:749: undefined reference to `__imp_pthread_mutex_lock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:749: undefined reference to `__imp_pthread_mutex_lock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o): in function `__gthread_mutex_unlock':
    /gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:749: undefined reference to `__imp_pthread_mutex_lock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:749: undefined reference to `__imp_pthread_mutex_lock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o): in function `__gthread_mutex_unlock':
    /gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:740: undefined reference to `__imp_pthread_mutex_destroy'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:749: undefined reference to `__imp_pthread_mutex_lock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o): in function `__gthread_mutex_unlock':
    /gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:740: undefined reference to `__imp_pthread_mutex_destroy'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:749: undefined reference to `__imp_pthread_mutex_lock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o): in function `__gthread_mutex_unlock':
    /gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:740: undefined reference to `__imp_pthread_mutex_destroy'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/distsrc-base/distsrc-99874bd94511-x86_64-w64-mingw32/src/leveldb/helpers/memenv/memenv.cc:230: undefined reference to `__imp_pthread_mutex_lock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o): in function `__gthread_mutex_unlock':
    /gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:740: undefined reference to `__imp_pthread_mutex_destroy'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:740: undefined reference to `__imp_pthread_mutex_destroy'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:740: undefined reference to `__imp_pthread_mutex_destroy'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/distsrc-base/distsrc-99874bd94511-x86_64-w64-mingw32/src/leveldb/helpers/memenv/memenv.cc:230: undefined reference to `__imp_pthread_mutex_lock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o): in function `__gthread_mutex_unlock':
    /gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:740: undefined reference to `__imp_pthread_mutex_destroy'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:740: undefined reference to `__imp_pthread_mutex_destroy'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:740: undefined reference to `__imp_pthread_mutex_destroy'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:749: undefined reference to `__imp_pthread_mutex_lock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o): in function `__gthread_mutex_unlock':
    /gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:740: undefined reference to `__imp_pthread_mutex_destroy'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:749: undefined reference to `__imp_pthread_mutex_lock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o): in function `__gthread_mutex_unlock':
    /gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:740: undefined reference to `__imp_pthread_mutex_destroy'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:749: undefined reference to `__imp_pthread_mutex_lock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o): in function `__gthread_mutex_unlock':
    /gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:740: undefined reference to `__imp_pthread_mutex_destroy'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:749: undefined reference to `__imp_pthread_mutex_lock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:749: undefined reference to `__imp_pthread_mutex_lock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/distsrc-base/distsrc-99874bd94511-x86_64-w64-mingw32/src/leveldb/helpers/memenv/memenv.cc:285: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/bits/stl_map.h:501: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:733: undefined reference to `__imp_pthread_mutex_init'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o): in function `std::_Vector_base<char*, std::allocator<char*> >::_Vector_impl_data::_Vector_impl_data()':
    /gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/bits/stl_vector.h:97: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o): in function `std::mutex::lock()':
    /gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/bits/std_mutex.h:104: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:749: undefined reference to `__imp_pthread_mutex_lock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o): in function `leveldb::Status::IOError(leveldb::Slice const&, leveldb::Slice const&)':
    /distsrc-base/distsrc-99874bd94511-x86_64-w64-mingw32/src/./leveldb/include/leveldb/status.h:53: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:749: undefined reference to `__imp_pthread_mutex_lock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o): in function `leveldb::Status::IOError(leveldb::Slice const&, leveldb::Slice const&)':
    /distsrc-base/distsrc-99874bd94511-x86_64-w64-mingw32/src/./leveldb/include/leveldb/status.h:53: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:749: undefined reference to `__imp_pthread_mutex_lock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o): in function `leveldb::Status::IOError(leveldb::Slice const&, leveldb::Slice const&)':
    /distsrc-base/distsrc-99874bd94511-x86_64-w64-mingw32/src/./leveldb/include/leveldb/status.h:53: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:749: undefined reference to `__imp_pthread_mutex_lock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:749: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/bits/stl_map.h:1069: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/bits/stl_tree.h:350: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o): in function `leveldb::Status::IOError(leveldb::Slice const&, leveldb::Slice const&)':
    /distsrc-base/distsrc-99874bd94511-x86_64-w64-mingw32/src/./leveldb/include/leveldb/status.h:53: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/distsrc-base/distsrc-99874bd94511-x86_64-w64-mingw32/src/./leveldb/include/leveldb/status.h:53: more undefined references to `__imp_pthread_mutex_unlock' follow
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:740: undefined reference to `__imp_pthread_mutex_destroy'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o): in function `std::mutex::lock()':
    /gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/bits/std_mutex.h:104: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:749: undefined reference to `__imp_pthread_mutex_lock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/bits/stl_map.h:1069: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o): in function `leveldb::Status::IOError(leveldb::Slice const&, leveldb::Slice const&)':
    /distsrc-base/distsrc-99874bd94511-x86_64-w64-mingw32/src/./leveldb/include/leveldb/status.h:53: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:740: undefined reference to `__imp_pthread_mutex_destroy'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:749: undefined reference to `__imp_pthread_mutex_lock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o): in function `std::mutex::lock()':
    /gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/bits/std_mutex.h:100: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/distsrc-base/distsrc-99874bd94511-x86_64-w64-mingw32/src/leveldb/helpers/memenv/memenv.cc:268: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:733: undefined reference to `__imp_pthread_mutex_init'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o): in function `std::_Vector_base<char*, std::allocator<char*> >::_Vector_impl_data::_Vector_impl_data()':
    /gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/bits/stl_vector.h:97: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o): in function `std::mutex::lock()':
    /gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/bits/std_mutex.h:104: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: /gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/bits/std_mutex.h:104: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:733: undefined reference to `__imp_pthread_mutex_init'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:749: undefined reference to `__imp_pthread_mutex_lock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:749: undefined reference to `__imp_pthread_mutex_lock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    x86_64-w64-mingw32-ld: leveldb/.libs/libmemenv.a(libmemenv_la-memenv.o):/gnu/store/yn52na8xbgzpiq7fdpm9pfyyf5w3z60m-gcc-cross-x86_64-w64-mingw32-10.3.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:779: undefined reference to `__imp_pthread_mutex_unlock'
    8bdfe057c7
  94. build: Create .la library for bitcoincrypto
    Libtool will yell at you if you try to link a shared library against
    static ones.
    
    This change creates a libtool archive library for bitcoincrypto and
    allows a shared library to be linked against it portably.
    
    Also specify -static in both:
    
    - ..._la_CXXFLAGS so that libtool will avoid building two versions of
      each object (one PIC, one non-PIC). We just need the one that is
      suitable for static linking.
    - ..._la_LDFLAGS so that libtool will create a static library.
    
    [META] This change is done in preparation for a future commit where we
           link the libbitcoinkernel library against this one.
    c1e16cb31f
  95. build: Separate lib_LTLIBRARIES initialization
    Set lib_LTLIBRARIES with '=' to an empty value at the top of the
    Makefile.am and append to it from the library-local block for
    readability.
    
    Here's the error you get if you don't set lib_LTLIBRARIES to be empty:
    
        error: lib_LTLIBRARIES must be set with '=' before using '+='
    
    [META] In a subsequent commit, we're going to introduce a library and
           append it to lib_LTLIBRARIES in its local block, this makes
           things more readable.
    83a0bb7cc9
  96. b-cs: Define G_TRANSLATION_FUN in bitcoinkernel.cpp
    [META] This is done in preparation for extracting libbitcoinkernel in a
           following commit. It seems logical that generally users of a
           library shouldn't need to export its own symbols to use the
           library.
    1df44dd20c
  97. dongcarl force-pushed on Apr 26, 2022
  98. dongcarl commented at 4:03 pm on April 27, 2022: member
    With permission from theuni, let’s continue the conversation about fixing the mingw-w64 dll’s in #25008. We won’t be fixing that in this PR.
  99. theuni commented at 8:48 pm on April 27, 2022: member

    @dongcarl and I discussed a few of the remaining issues today, and I’ve pushed up a branch on top of this one which addresses all the build issues I’m aware of: https://github.com/theuni/bitcoin/commits/24322-win-fixes

    Feel free to take any/all of those commits. The “provide a way to avoid exporting on Windows by default” commit is a hack, I’m sure there’s a less invasive way to do that.

    Edit: These are mostly fixes for Windows, which @dongcarl has said he’s not targeting here, so those fixes can be PR’d as a follow-up as necessary.

    I think https://github.com/theuni/bitcoin/commit/ee29b8862fe909009357b6da88053ee2ad9f7d99 belongs in this PR, though.

  100. build: Extract the libbitcoinkernel library
    I strongly recommend reviewing with the following git-diff flags:
      --patience --color-moved=dimmed-zebra
    
    Extract out a libbitcoinkernel library linking in all files necessary
    for using our consensus engine as-is. Link bitcoin-chainstate against
    it.
    
    See previous commit "build: Add example bitcoin-chainstate executable"
    for more context.
    
    We explicitly specify -fvisibility=default, which effectively overrides
    the effects of --enable-reduced-exports since libbitcoinkernel requires
    default symbol visibility
    
    When compiling for mingw-w64, specify -static in both:
    
    - ..._la_CXXFLAGS so that libtool will avoid building two versions of
      each object (one PIC, one non-PIC). We just need the one that is
      suitable for static linking.
    - ..._la_LDFLAGS so that libtool will create a static library.
    
    If we don't specify this, then libtool will prefer the non-static PIC
    version of the object, which is built with -DDLL_EXPORT -DPIC for
    mingw-w64 targets. This can cause symbol resolution problems when we
    link this library against an executable that does specify -all-static,
    since that will be built without the -DDLL_EXPORT flag.
    
    Unfortunately, this means that for mingw-w64 we can only build a static
    version of the library for now. This will be fixed.
    
    However, on other targets, the shared library creation works fine.
    
    -----
    
    Note to users: You need to either specify:
    
      --enable-experimental-util-chainstate
    
    or,
    
      --with-experimental-kernel-lib
    
    To build the libbitcionkernel library. See the configure help for more
    details.
    
    build shared libbitcoinkernel where we can
    26b2e7ffb3
  101. ci: Build libbitcoinkernel 94ad45deb2
  102. docs: Add libbitcoinkernel_la_SOURCES explanation 3f0595095d
  103. build: Remove LIBTOOL_APP_LDFLAGS for bitcoin-chainstate
    See added comment.
    
    Note that this won't actually have any effect until we add the mingw-w64
    DLL fix since LIBTOOL_APP_LDFLAGS is undefined for other platforms.
    035fa1f07a
  104. dongcarl force-pushed on Apr 27, 2022
  105. dongcarl commented at 9:53 pm on April 27, 2022: member

    From ae94c62b1d0ac86bb7011434f8f51ce9cbb91164 -> 035fa1f07aacb7bce74c0884ae28c8cf00fe3b1b

    • Use just -static instead of -all-static (there’s no difference for libraries)
    • Use -static in CXXFLAGS and LDFLAGS to ensure that objects aren’t compiled twice for no good reason
    • Update comments and commit messages accordingly
    • Allow building shared libbitcoinkernel on all platforms other than windows (marked as a TODO, tracked here: #25008)
    • Remove overzealous LIBTOOL_APP_LDFLAGS so that we can link bitcoin-chainstate against both shared and static versions of libbitcoinkernel
  106. dongcarl commented at 9:56 pm on April 27, 2022: member

    @hebasto

    1. If the -all-static Libtool flag is required for Windows builds only, why do not apply it conditionally?

    Incorporated as of 035fa1f07aacb7bce74c0884ae28c8cf00fe3b1b

    1. Besides the -all-static mode, Libtool also suggests similar -static and -static-libtool-libs ones. Why the former has been chosen?

    -static chosen as of 035fa1f07aacb7bce74c0884ae28c8cf00fe3b1b

  107. theuni approved
  108. theuni commented at 1:22 am on April 28, 2022: member

    I’ve been experimenting with this for a few platforms in various configs today. At this point everything seems to build/link as expected with the exception of Windows, and those patches are in the pipeline.

    I don’t love the idea of merging this without anything testing the resulting shared libs for viability, but there’s so much work left to do here that I think this is a reasonable stopping point, so imo it’s ok to follow up with some trivial tests.

    I won’t be surprised if the libtool switch causes some unforeseen build breakage for some of the more esoteric platforms over the next few weeks. If so, I’m hoping we can solve those quickly as they arise.

    This may be my favorite PR ever. It’s a privilege to ACK 035fa1f07aacb7bce74c0884ae28c8cf00fe3b1b.

    Well done :)

  109. in src/Makefile.am:32 in 26b2e7ffb3 outdated
    28@@ -29,6 +29,7 @@ LIBBITCOIN_NODE=libbitcoin_node.a
    29 LIBBITCOIN_COMMON=libbitcoin_common.a
    30 LIBBITCOIN_CONSENSUS=libbitcoin_consensus.a
    31 LIBBITCOIN_CLI=libbitcoin_cli.a
    32+LIBBITCOIN_KERNEL=libbitcoin_kernel.a
    


    theuni commented at 4:33 am on April 28, 2022:
    Nit: I think this is extraneous?
  110. fanquake commented at 2:14 pm on April 28, 2022: member

    I don’t love the idea of merging this without anything testing the resulting shared libs for viability, but there’s so much work left to do here that I think this is a reasonable stopping point, so imo it’s ok to follow up with some trivial tests.

    Ideally we’ll have the Windows libtool issues sorted next. Further changes / nits can be addressed in the next PRs in the kernel series.

  111. fanquake merged this on Apr 28, 2022
  112. fanquake closed this on Apr 28, 2022

  113. fanquake moved this from the "Ready for Review PRs" to the "Done or Closed or Rethinking" column in a project

  114. in src/Makefile.crc32c.include:42 in 05d1525b6d outdated
    53+
    54+# Specify -static in both CXXFLAGS and LDFLAGS so libtool will only build a
    55+# static version of this library. We don't need a dynamic version, and a dynamic
    56+# version can't be used on windows anyway because the library doesn't currently
    57+# export DLL symbols.
    58+crc32c_libcrc32c_la_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) -static
    


    ryanofsky commented at 3:08 pm on April 28, 2022:

    In commit “build: Create .la library for leveldb” (8bdfe057c796dde1cd2e5a37a73e87a879e9fe56)

    Note: I was confused how adding -static to CXXFLAGS could even do anything, but apparently -static is a compile mode flags as well as a link mode flag for libtool (https://www.gnu.org/software/libtool/manual/html_node/Compile-mode.html https://www.gnu.org/software/automake/manual/html_node/Libtool-Flags.html), and the compile mode overrides the override for something, idk.


    theuni commented at 6:20 pm on April 28, 2022:

    Heh, yes, that’s about it. libtool manages to defy intuition at about every turn, but at least the outcome here is reasonable (I hope!).

    By default for every lib it wants to build a shared and static version, and because of the differing export semantics it wants to compile one set of objects to go into the shared library, and another set for the static library.

    Adding -static in LDFLAGS tells it to disable any linking, but it’ll still build the “shared” objects as well. Adding -static in CXXFLAGS tells it that there will be nothing to link in the end, so there’s no point in building the “shared” objects.

    As for argument parsing, yes, it’s really nasty. libtool gobbles up parameters passed to the compiler/linker and attempts to substitute the correct actions. It gets hairy when there’s ambiguity with keywords (like “-static”).

    It’s a dinosaur :(


    hebasto commented at 8:52 pm on April 28, 2022:

    Adding -static in CXXFLAGS tells it that there will be nothing to link in the end, so there’s no point in building the “shared” objects.

    Hmm, I’m not sure what’s going on exactly, but removing -static from leveldb_libleveldb_la_CXXFLAGS

     0--- a/src/Makefile.leveldb.include
     1+++ b/src/Makefile.leveldb.include
     2@@ -43,7 +43,7 @@ leveldb_libleveldb_la_CPPFLAGS = $(AM_CPPFLAGS) $(LEVELDB_CPPFLAGS_INT) $(LEVELD
     3 # static version of this library. We don't need a dynamic version, and a dynamic
     4 # version can't be used on windows anyway because the library doesn't currently
     5 # export DLL symbols.
     6-leveldb_libleveldb_la_CXXFLAGS = $(filter-out -Wconditional-uninitialized -Werror=conditional-uninitialized -Wsuggest-override -Werror=suggest-override, $(AM_CXXFLAGS)) $(PIE_FLAGS) -static
     7+leveldb_libleveldb_la_CXXFLAGS = $(filter-out -Wconditional-uninitialized -Werror=conditional-uninitialized -Wsuggest-override -Werror=suggest-override, $(AM_CXXFLAGS)) $(PIE_FLAGS)
     8 leveldb_libleveldb_la_LDFLAGS = $(AM_LDFLAGS) -static
     9 
    10 leveldb_libleveldb_la_SOURCES=
    

    changes nothing in building libleveldb.la – I’ve compared verbose build logs and build directories.

    But many other -libtool: compile: calls are not invoked twice, and .libs/libbitcoinconsensus.so.0.0.0 is not created.


    I used the following configuration:

    0$ ./configure --enable-suppress-external-warnings --disable-wallet --without-gui --without-utils --disable-tests --disable-bench --disable-fuzz-binary --without-zmq CC=clang CXX=clang++
    
  115. ryanofsky commented at 3:14 pm on April 28, 2022: member

    Code review ACK 035fa1f07aacb7bce74c0884ae28c8cf00fe3b1b

    Want cmake

  116. theuni commented at 5:09 pm on April 28, 2022: member

    Code review ACK 035fa1f

    Want cmake

    +1

    Lots of time and effort here went into fighting with the buildsystem, and that’s no good. It’s working against us rather than for us more and more it seems.

    I’ve switched to using CMake for my personal projects, and these days it’s much more pleasant to work with than it used to be.

    I know I’ve been one of the loudest and last objectors to it over the years, and yeah, I’m feeling bad lately for stalling the inevitable switch. Sorry :(

  117. hebasto commented at 6:56 pm on April 28, 2022: member

    Want cmake

    +1

    +1

  118. laanwj removed this from the "Blockers" column in a project

  119. sidhujag referenced this in commit 132861e2a8 on Apr 29, 2022
  120. real-or-random referenced this in commit 44c2452fd3 on May 19, 2022
  121. laanwj referenced this in commit 7008087548 on May 24, 2022
  122. in src/Makefile.am:820 in 035fa1f07a
    817+
    818+# bitcoinkernel library #
    819+if BUILD_BITCOIN_KERNEL_LIB
    820+lib_LTLIBRARIES += $(LIBBITCOINKERNEL)
    821+
    822+libbitcoinkernel_la_LDFLAGS = $(AM_LDFLAGS) -no-undefined $(RELDFLAGS) $(PTHREAD_FLAGS)
    


    MarcoFalke commented at 2:29 pm on July 4, 2022:

    Is this needed? I get a warning that it is unused:

    0...
    1  CXXLD    libbitcoinkernel.la
    2clang-13: warning: argument unused during compilation: '-pthread' [-Wunused-command-line-argument]
    3clang-13: warning: argument unused during compilation: '-pthread' [-Wunused-command-line-argument]
    4copying selected object files to avoid basename conflicts...
    
  123. DrahtBot locked this on Jul 4, 2023

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-07-03 10:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me