build: remove build stubs for external leveldb #23282
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Presumably these stubs indicate to packagers that external leveldb is meant to be supported in some way. It is not. Remove the stubs to avoid sending any mixed messages.
|
Concept ACK |
|
Concept ACK. Nice cleanup too. |
|
Concept ACK |
|
Concept ACK. |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
luke-jr
suggested changes
Oct 16, 2021
fanquake
approved these changes
Oct 20, 2021
There was a problem hiding this comment.
ACK 17ae260
Attempting to sabotage other people isn't going to make things any better.
I think calling this "sabotage" is pretty disingenuous. No-one is going out of their way to sabotage anyone else, and for that to be the case, there would need to be some number of users who are being drastically, adversely effected. I'm not currently aware of anyone who's actively building Bitcoin Core with a leveldb other than our internal subtree. We don't officially support doing that, and having build stubs that indicate otherwise is at best, misleading.
I know that Bitcoin Knots patches in support for --system-leveldb, but at the same time, marks it as DANGEROUS; NOT SUPPORTED, and I don't really understand why you'd expose an option like that. Do any Knots users actually use it, and if so, why? In any case, if this is something you want to continue to support in Knots, you can add a reversion of these changes to your existing --system-leveldb patchset.
From my point of view, this is a good cleanup for Bitcoin Core, which removes code that indicates support for a (potentially dangerous) option that we don't actually want to provide.
|
Guix Build: bash-5.1# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
7fda546a565750b11d43e57f4436c30945ad9471a20e60dc6b0a25e943b4eada guix-build-17ae2601c786/output/aarch64-linux-gnu/SHA256SUMS.part
e22ec3583a31455b7931f30578ea22a6e1c30843c24a99ffc8b5bb69b21d2b7b guix-build-17ae2601c786/output/aarch64-linux-gnu/bitcoin-17ae2601c786-aarch64-linux-gnu-debug.tar.gz
8b1f6c902f7547967f02c0c7fc53b37f5402ba6443e3ca46fc5d958fb95e1bf3 guix-build-17ae2601c786/output/aarch64-linux-gnu/bitcoin-17ae2601c786-aarch64-linux-gnu.tar.gz
cd434237183505055c1dfaa855f66a4dae3a44185239bff3a4927bac60423204 guix-build-17ae2601c786/output/arm-linux-gnueabihf/SHA256SUMS.part
b0fae8965485d983f57f849db5a9d2261039816a294ff5c70b15bf4fc0e9306e guix-build-17ae2601c786/output/arm-linux-gnueabihf/bitcoin-17ae2601c786-arm-linux-gnueabihf-debug.tar.gz
a45f552d53fd9ec5279f4ba5c0bb3c102dd8b5d856578a43151219931ded365a guix-build-17ae2601c786/output/arm-linux-gnueabihf/bitcoin-17ae2601c786-arm-linux-gnueabihf.tar.gz
45b7602147e151a96e8a0aa4ba17bffbfc28ab097dd97451f541d55d06f32428 guix-build-17ae2601c786/output/dist-archive/bitcoin-17ae2601c786.tar.gz
68d88b46788a12972c695f66a45e684ba314832e045167127ee6ff0de940fa05 guix-build-17ae2601c786/output/powerpc64-linux-gnu/SHA256SUMS.part
464f564b79b0dfc3ab7e3d961523b0bb79318c30c67fc1bb7cf7ac51bdcd0db0 guix-build-17ae2601c786/output/powerpc64-linux-gnu/bitcoin-17ae2601c786-powerpc64-linux-gnu-debug.tar.gz
fd38fe4b118710f526d3bc9c48945b7307a2449867ffc29cec55946e45e1413c guix-build-17ae2601c786/output/powerpc64-linux-gnu/bitcoin-17ae2601c786-powerpc64-linux-gnu.tar.gz
90c4f381753948e45acca504b4084806655a4da329b10fd444bb49346436f498 guix-build-17ae2601c786/output/powerpc64le-linux-gnu/SHA256SUMS.part
663e77fc697908c861454c70737502c0a8001e9a19ce8817de8e3b6e5a8ad7f1 guix-build-17ae2601c786/output/powerpc64le-linux-gnu/bitcoin-17ae2601c786-powerpc64le-linux-gnu-debug.tar.gz
4baebe4d2b4655ed27c2eb281121b47411a2f037b64ab47acf04007026d9e5b8 guix-build-17ae2601c786/output/powerpc64le-linux-gnu/bitcoin-17ae2601c786-powerpc64le-linux-gnu.tar.gz
05469c75a28e37431ebae5e6a2ead451c34cdda55d8d9ff84b1a716f7972ba5c guix-build-17ae2601c786/output/riscv64-linux-gnu/SHA256SUMS.part
bd390b4a9bdbf33685076856a07c8c7e62c6f9e4ea78f11d4f940204f0f38dd8 guix-build-17ae2601c786/output/riscv64-linux-gnu/bitcoin-17ae2601c786-riscv64-linux-gnu-debug.tar.gz
b79be4a47cb1d2212c4b683d5e5891b4927bd2e231d27322c7522a81715eaed6 guix-build-17ae2601c786/output/riscv64-linux-gnu/bitcoin-17ae2601c786-riscv64-linux-gnu.tar.gz
b95e043930107f25f0d1610aec3df8c1633a915d4ee39c997b644ba6c3547021 guix-build-17ae2601c786/output/x86_64-apple-darwin19/SHA256SUMS.part
206ec77c3230b89163fb8139b7bf6f3a089eb6f6b370726220e0f9fc76f8adcc guix-build-17ae2601c786/output/x86_64-apple-darwin19/bitcoin-17ae2601c786-osx-unsigned.dmg
5fd87acde9c4a8dcc1224dbe6aba22ae61e4c452ad14937d0cc46c165044e58a guix-build-17ae2601c786/output/x86_64-apple-darwin19/bitcoin-17ae2601c786-osx-unsigned.tar.gz
e5446a453a464f8e1864719d5db787982ff2cf2fc9a144df5dba73acb0e17ec6 guix-build-17ae2601c786/output/x86_64-apple-darwin19/bitcoin-17ae2601c786-osx64.tar.gz
207665a543cce2bcf34658a07c0d363e569bdd169b0a157eec01343a56f0e315 guix-build-17ae2601c786/output/x86_64-linux-gnu/SHA256SUMS.part
c0c0cff8712396c0fe794c0c77dcb4d622234ca8738fcc0e55637d6299697f81 guix-build-17ae2601c786/output/x86_64-linux-gnu/bitcoin-17ae2601c786-x86_64-linux-gnu-debug.tar.gz
35f8e0486726bbc4f813299a472d260236ff5b8dcb47c202e0fe612a7ac58fd9 guix-build-17ae2601c786/output/x86_64-linux-gnu/bitcoin-17ae2601c786-x86_64-linux-gnu.tar.gz
5cc4a2c0dd6f14c3ee16a63910cf6fea162f02d7fcb21db48eaea7b3860d139d guix-build-17ae2601c786/output/x86_64-w64-mingw32/SHA256SUMS.part
6b6f4aeb639e47df131d452e6114130bd1cfb6926c88ef4a9b2aa818fa6ea54c guix-build-17ae2601c786/output/x86_64-w64-mingw32/bitcoin-17ae2601c786-win-unsigned.tar.gz
2ae54d1db01c0916ceb4814ee7250f85a3d7d8edbc9b1baec7388f69d7023e7d guix-build-17ae2601c786/output/x86_64-w64-mingw32/bitcoin-17ae2601c786-win64-debug.zip
cc891d5cb8ba705bf5c8c0be4381105385a50f0d47b444ec2d12472dc34f891b guix-build-17ae2601c786/output/x86_64-w64-mingw32/bitcoin-17ae2601c786-win64-setup-unsigned.exe
48eb01508bbfb95e3bab436298cfcdda5b8a9dfb6b17806fa716ff9031d641dd guix-build-17ae2601c786/output/x86_64-w64-mingw32/bitcoin-17ae2601c786-win64.zip |
Guix builds: |
|
post-merge ACK 17ae260 |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Oct 21, 2021
17ae260 build: remove build stubs for external leveldb (Cory Fields) Pull request description: Presumably these stubs indicate to packagers that external leveldb is meant to be supported in some way. It is not. Remove the stubs to avoid sending any mixed messages. For context, this was reported on IRC: > \<Talkless> bitcoind fails to start with undefined symbol: _ZTIN7leveldb6LoggerE in Debian Sid after leveldb upgraded from 1.22 to 1.23: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=996486 ACKs for top commit: fanquake: ACK 17ae260 hebasto: ACK 17ae260. I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 2f1ac2cb30dac64791933a245a2b66ce237bde3955e6f4a6b7ec181248f77a9b1b10597d865d3e2c2b6def696af70de40e905ec274e4ae7cccd1daf461473957
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Apr 3, 2022
17ae260 build: remove build stubs for external leveldb (Cory Fields) Pull request description: Presumably these stubs indicate to packagers that external leveldb is meant to be supported in some way. It is not. Remove the stubs to avoid sending any mixed messages. For context, this was reported on IRC: > \<Talkless> bitcoind fails to start with undefined symbol: _ZTIN7leveldb6LoggerE in Debian Sid after leveldb upgraded from 1.22 to 1.23: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=996486 ACKs for top commit: fanquake: ACK 17ae260 hebasto: ACK 17ae260. I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 2f1ac2cb30dac64791933a245a2b66ce237bde3955e6f4a6b7ec181248f77a9b1b10597d865d3e2c2b6def696af70de40e905ec274e4ae7cccd1daf461473957
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
8 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Presumably these stubs indicate to packagers that external leveldb is meant to be supported in some way. It is not. Remove the stubs to avoid sending any mixed messages.
For context, this was reported on IRC: