build: Remove `--enable-threadlocal`

fanquake commented at 3:05 am on May 18, 2024: member

Includes a commit from #30099. Closes #29952.

DrahtBot commented at 3:05 am on May 18, 2024: contributor

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	laanwj, hebasto, maflcko, theuni

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

DrahtBot added the label Build system on May 18, 2024

laanwj commented at 3:31 am on May 18, 2024: member

Concept ACK.

fanquake force-pushed on May 18, 2024

DrahtBot commented at 3:33 am on May 18, 2024: contributor

🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

Possibly this is due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

Leave a comment here, if you need help tracking down a confusing failure.

Debug: https://github.com/bitcoin/bitcoin/runs/25126121769

DrahtBot added the label CI failed on May 18, 2024

DrahtBot removed the label CI failed on May 18, 2024

hebasto commented at 6:59 am on May 18, 2024: member

Concept ACK.

hebasto added the label Needs CMake port on May 18, 2024

theuni commented at 7:28 pm on May 20, 2024: member

Concept ACK

hebasto commented at 10:13 pm on May 20, 2024: member

Concept ACK.

I forgot to mention that it would be great to ditch it before CMake :)

build: Enable `thread_local` for MinGW-w64 builds

The assumption in the commit 188ca75e5fe4837d16241446558c7566912f67b2
about the broken `thread_local` implementation in GCC was misguided
because the initial failure was not due to GCC, but a bug in the Wine
runtime, as evidenced, for example, here:
 - https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=917307
 - https://bugs.freedesktop.org/show_bug.cgi?id=108662

Consequently, it is safe to re-enable `thread_local` support for
MinGW-w64 builds.

5bba43312c

doc: remove comment about using thread_local

Followup to https://github.com/bitcoin/bitcoin/pull/30095#discussion_r1605655974.

1e7c20bc19

build: remove --enable-threadlocal 17fe948cce

fanquake force-pushed on May 21, 2024

laanwj commented at 11:05 am on May 21, 2024: member

Code review ACK 17fe948cce2eb75f0f3f4b0db9d0d90648c7d4af This removes, to my knowledge, all mentions of thread-local in the build system or comments. Will test the guix binary on windows.

DrahtBot requested review from hebasto on May 21, 2024

DrahtBot requested review from theuni on May 21, 2024

fanquake marked this as ready for review on May 21, 2024

fanquake added the label DrahtBot Guix build requested on May 21, 2024

hebasto approved

hebasto commented at 12:44 pm on May 21, 2024: member

ACK 17fe948cce2eb75f0f3f4b0db9d0d90648c7d4af.

Guix build:

0x86_64
1b5dd1ec801cb549590bb922867b9254de51c3688220ce4d29ae03077e0047448  guix-build-17fe948cce2e/output/dist-archive/bitcoin-17fe948cce2e.tar.gz
29bc0b56bf0440f04937646ded534e47df860c4ca2b9b96699c1f5b431ae71508  guix-build-17fe948cce2e/output/x86_64-w64-mingw32/SHA256SUMS.part
33e1cceb01e86b7e47d68ae100b60747333a88aad5b352c7c3e9113d700465f1c  guix-build-17fe948cce2e/output/x86_64-w64-mingw32/bitcoin-17fe948cce2e-win64-debug.zip
4e0c36335cfc1c399d40231adffd1d8afd33261dc59c0523823c50ab6ee00f5e8  guix-build-17fe948cce2e/output/x86_64-w64-mingw32/bitcoin-17fe948cce2e-win64-setup-unsigned.exe
58a0044e4e5357e4e0eb1577a704a4cc0ddb46617151eee607842010c120738a8  guix-build-17fe948cce2e/output/x86_64-w64-mingw32/bitcoin-17fe948cce2e-win64-unsigned.tar.gz
6de79709714a39ebcb5988fad5add91e7ed71e9fc313143f0cca5dcc6814dd34b  guix-build-17fe948cce2e/output/x86_64-w64-mingw32/bitcoin-17fe948cce2e-win64.zip

maflcko commented at 1:01 pm on May 21, 2024: member

utACK 17fe948cce2eb75f0f3f4b0db9d0d90648c7d4af

fanquake commented at 1:56 pm on May 21, 2024: member

Guix Build (aarch64):

 077a67096a2225cafa44637d69e9618cb275b26eeda00585dfa246dfbec7fc818  guix-build-17fe948cce2e/output/aarch64-linux-gnu/SHA256SUMS.part
 157867960d6c8597648cc49efe4165e543b1181ad0a3a97a16f66bd41b93afe8b  guix-build-17fe948cce2e/output/aarch64-linux-gnu/bitcoin-17fe948cce2e-aarch64-linux-gnu-debug.tar.gz
 276c872bcb98e8fc46a5cb2ebd13c489caa6b7d3b0b6db92c467aae27b30308fc  guix-build-17fe948cce2e/output/aarch64-linux-gnu/bitcoin-17fe948cce2e-aarch64-linux-gnu.tar.gz
 35093f7155c56e94ad994dcbe586fef1b8cfedc214e037a4dfdb1b2012059b5c9  guix-build-17fe948cce2e/output/arm-linux-gnueabihf/SHA256SUMS.part
 406f2bb56d6aae8b1dc70e5d09a111739a7d34b8b07797f674f6c17fbf7ec74ab  guix-build-17fe948cce2e/output/arm-linux-gnueabihf/bitcoin-17fe948cce2e-arm-linux-gnueabihf-debug.tar.gz
 583a5f2bcb1ecd4e7cf1cc85db394396aaf46b92252555f42237a44cde0bc9b96  guix-build-17fe948cce2e/output/arm-linux-gnueabihf/bitcoin-17fe948cce2e-arm-linux-gnueabihf.tar.gz
 613c6c28083906e8ac81d4f320a6d4d9f7508919530d33f371596baaea8eba066  guix-build-17fe948cce2e/output/arm64-apple-darwin/SHA256SUMS.part
 70dcf263fb0c3945af210977dd5a363e3cff162c4a7b47d8fed3cef4b708b9365  guix-build-17fe948cce2e/output/arm64-apple-darwin/bitcoin-17fe948cce2e-arm64-apple-darwin-unsigned.tar.gz
 83c9da1723c61bcab2e6a856f0a7cbb3ad53cea7b18b8761b1d50d0cfd3012305  guix-build-17fe948cce2e/output/arm64-apple-darwin/bitcoin-17fe948cce2e-arm64-apple-darwin-unsigned.zip
 97b6debab244c7c50e6eb47c180bc149de511f8b2304870c5e2347fdac3107fa1  guix-build-17fe948cce2e/output/arm64-apple-darwin/bitcoin-17fe948cce2e-arm64-apple-darwin.tar.gz
10b5dd1ec801cb549590bb922867b9254de51c3688220ce4d29ae03077e0047448  guix-build-17fe948cce2e/output/dist-archive/bitcoin-17fe948cce2e.tar.gz
115743e13fa3b8505a66412f1845e8b0d8f908822b417b4d94412fb381b30db64a  guix-build-17fe948cce2e/output/powerpc64-linux-gnu/SHA256SUMS.part
12c394fe9822cba9275d17dc040772325e8c4ae091b3dc4137a3b3856af9358028  guix-build-17fe948cce2e/output/powerpc64-linux-gnu/bitcoin-17fe948cce2e-powerpc64-linux-gnu-debug.tar.gz
135d5ed565449d736520ee8d8d2f15bb55c99f6bea2b5a8bebc30aca75055f67c7  guix-build-17fe948cce2e/output/powerpc64-linux-gnu/bitcoin-17fe948cce2e-powerpc64-linux-gnu.tar.gz
14f5565a2e56d65c262f561b9ea509afd22f4d6ff55907cc259fe114dd6880f7d8  guix-build-17fe948cce2e/output/riscv64-linux-gnu/SHA256SUMS.part
158821cd02599d6d26858d7414147ad93e8c7c9f6ff272fde44162dc291332f3ad  guix-build-17fe948cce2e/output/riscv64-linux-gnu/bitcoin-17fe948cce2e-riscv64-linux-gnu-debug.tar.gz
16ea4844a3048eda3c31339754ef42efc30a5b2def1b776ffab990add0c523e6a2  guix-build-17fe948cce2e/output/riscv64-linux-gnu/bitcoin-17fe948cce2e-riscv64-linux-gnu.tar.gz
17cd0e7b8739c7039205dfc714df6889646a4f5bb55e0cb1819797990bf96fc8f0  guix-build-17fe948cce2e/output/x86_64-apple-darwin/SHA256SUMS.part
18d680caba616d24ef7ec28a7100d093ae428908d1b12ea27352fb575269a49c87  guix-build-17fe948cce2e/output/x86_64-apple-darwin/bitcoin-17fe948cce2e-x86_64-apple-darwin-unsigned.tar.gz
19fb234d002cb3c68e7bba82cbb10677e4473f24a617f09e2c17f0b69dd2c47fa3  guix-build-17fe948cce2e/output/x86_64-apple-darwin/bitcoin-17fe948cce2e-x86_64-apple-darwin-unsigned.zip
2015c310bef26654d0a9a7098a54593fb58266d7de2d5be28a307d98e64716fe83  guix-build-17fe948cce2e/output/x86_64-apple-darwin/bitcoin-17fe948cce2e-x86_64-apple-darwin.tar.gz
214ba52b40fad3b0ccb3a4c9a812a5dc4aab0047b2a908dd18f58bf417a71c60e3  guix-build-17fe948cce2e/output/x86_64-linux-gnu/SHA256SUMS.part
22f5eba6a58f87896795928c5010cbee34453b4fa223e37a6b0cb7da65327f8afd  guix-build-17fe948cce2e/output/x86_64-linux-gnu/bitcoin-17fe948cce2e-x86_64-linux-gnu-debug.tar.gz
23af9816b48a68116a61ba2e2af1a22f5aa508b7f699acecfb80b31ef8da976c69  guix-build-17fe948cce2e/output/x86_64-linux-gnu/bitcoin-17fe948cce2e-x86_64-linux-gnu.tar.gz
249bc0b56bf0440f04937646ded534e47df860c4ca2b9b96699c1f5b431ae71508  guix-build-17fe948cce2e/output/x86_64-w64-mingw32/SHA256SUMS.part
253e1cceb01e86b7e47d68ae100b60747333a88aad5b352c7c3e9113d700465f1c  guix-build-17fe948cce2e/output/x86_64-w64-mingw32/bitcoin-17fe948cce2e-win64-debug.zip
26e0c36335cfc1c399d40231adffd1d8afd33261dc59c0523823c50ab6ee00f5e8  guix-build-17fe948cce2e/output/x86_64-w64-mingw32/bitcoin-17fe948cce2e-win64-setup-unsigned.exe
278a0044e4e5357e4e0eb1577a704a4cc0ddb46617151eee607842010c120738a8  guix-build-17fe948cce2e/output/x86_64-w64-mingw32/bitcoin-17fe948cce2e-win64-unsigned.tar.gz
28de79709714a39ebcb5988fad5add91e7ed71e9fc313143f0cca5dcc6814dd34b  guix-build-17fe948cce2e/output/x86_64-w64-mingw32/bitcoin-17fe948cce2e-win64.zip

theuni approved

theuni commented at 4:36 pm on May 21, 2024: member

utACK 17fe948cce2eb75f0f3f4b0db9d0d90648c7d4af

fanquake merged this on May 21, 2024

fanquake closed this on May 21, 2024

fanquake deleted the branch on May 21, 2024

DrahtBot commented at 9:57 pm on May 21, 2024: contributor

Guix builds (on x86_64) [untrusted test-only build, possibly unsafe, not for production use]

File	commit a786fd2041913d82ca90b561de309421bd24e41b(master)	commit 64989516def98b155d926ff2c4c1b035ce5fb69e(master and this pull)
SHA256SUMS.part	`14f282b5bf7ebcd4...`	`d2b11fe03f620927...`
*-aarch64-linux-gnu-debug.tar.gz	`b2364625629230d1...`	`2dc133001ba32c41...`
*-aarch64-linux-gnu.tar.gz	`b05fc0561baa0966...`	`ca83cce58855e8fd...`
*-arm-linux-gnueabihf-debug.tar.gz	`4545637daf89882c...`	`f5a547a8e76ad429...`
*-arm-linux-gnueabihf.tar.gz	`27115f4c7e251175...`	`98807d914e281f9b...`
*-arm64-apple-darwin-unsigned.tar.gz	`cf3950f9d05fb1c0...`	`eaacda993a335185...`
*-arm64-apple-darwin-unsigned.zip	`b1c32e004722a3b4...`	`063170bd94f3b1ed...`
*-arm64-apple-darwin.tar.gz	`f09a38acef4b141b...`	`55ad1ef99ba5c4d9...`
*-powerpc64-linux-gnu-debug.tar.gz	`75cfe8d73120b00d...`	`a7df880ae406cbe7...`
*-powerpc64-linux-gnu.tar.gz	`d4f8c1296886fac4...`	`c7e83c889c0d3a43...`
*-riscv64-linux-gnu-debug.tar.gz	`a8f56aee14e85f29...`	`a455fad048c5e7ee...`
*-riscv64-linux-gnu.tar.gz	`d8a4ae4295e6aa9f...`	`14d3728c6525ad17...`
*-x86_64-apple-darwin-unsigned.tar.gz	`ff9ae232120c85f9...`	`42fdf06b2a1dfdba...`
*-x86_64-apple-darwin-unsigned.zip	`9917159a6fd59954...`	`e7d6a12616f63b91...`
*-x86_64-apple-darwin.tar.gz	`61112274cace86c4...`	`de4fd675e3ef30f3...`
*-x86_64-linux-gnu-debug.tar.gz	`557d05bc585a2910...`	`54eabd6e7f0e4a49...`
*-x86_64-linux-gnu.tar.gz	`4928355769a494d7...`	`da0144b6aeabbc01...`
*.tar.gz	`49c2acd89613cf65...`	`e296b8be8a64650c...`
guix_build.log	`88d5371b29f8bb75...`	`f2b4d1ac803fde2b...`
guix_build.log.diff		`4ced442c8b170aa4...`

DrahtBot removed the label DrahtBot Guix build requested on May 21, 2024

in src/test/util_threadnames_tests.cpp:53 in 17fe948cce

49@@ -52,11 +50,6 @@ std::set<std::string> RenameEnMasse(int num_threads)
50  */
51 BOOST_AUTO_TEST_CASE(util_threadnames_test_rename_threaded)
52 {
53-#if !defined(HAVE_THREAD_LOCAL)
54-    // This test doesn't apply to platforms where we don't have thread_local.
55-    return;
56-#endif
57-
58     std::set<std::string> names = RenameEnMasse(100);

vasild commented at 5:07 am on May 22, 2024:

With this it will be possible to remove export NOWARN_CXXFLAGS="-Wno-error=unreachable-code" from dev environment on FreeBSD, thanks!

vasild commented at 5:09 am on May 22, 2024: contributor

ACK 17fe948cce2eb75f0f3f4b0db9d0d90648c7d4af

hebasto referenced this in commit 6230d47c71 on May 25, 2024

hebasto commented at 2:25 pm on June 5, 2024: member

Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/214 and https://github.com/hebasto/bitcoin/pull/220.

hebasto removed the label Needs CMake port on Jun 5, 2024

build: Remove --enable-threadlocal #30137

Code Coverage

Reviews

Guix builds (on x86_64) [untrusted test-only build, possibly unsafe, not for production use]

build: Remove `--enable-threadlocal` #30137