build: Update leveldb to 1.22+ #17398
Merged
+11,520
−7,937
Conversation
This file contains 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
|
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. |
laanwj
force-pushed
the
2019_11_leveldb_upstream
branch
from
e9cad79
to
cca3488
Compare
|
OK, travis build should pass now Testing notes:
|
laanwj
changed the title
laanwj
force-pushed
the
2019_11_leveldb_upstream
branch
2 times, most recently
from
43cbeb7
to
9e34948
Compare
|
Concept ACK for the new branch. it's really nice that we can again easily see the diff from upstream. Will try to read later the (reported) changes upstream leveldb did in the past ~160 commits. |
laanwj
force-pushed
the
2019_11_leveldb_upstream
branch
2 times, most recently
from
12849e2
to
f6a4f7c
Compare
|
Regarding the msvc build I didn't have any problems getting it to build after applying what I think are the relevant changes to the I do get two new warnings: Are any of the preprocessor defines below relevant for Windows? In case it's of any use the diff for |
Thanks, I'll include that!
I think they're only used in env_posix.cc? Yes, they are, so indeed they're irrelevant on windows. |
laanwj
force-pushed
the
2019_11_leveldb_upstream
branch
from
f6a4f7c
to
75f384d
Compare
|
AppVeyor fails on some warning that is promoted to error, for an unreachable destructor. Did you need some flag changes too? |
|
Concept ACK - have not reviewed the changes yet. Only done some minimal does it compile + start running tests. Started testing going between leveldb versions on macOS. macOS (fed6a9b + depends):checking for fdatasync... no
checking for F_FULLFSYNC... yes
checking for O_CLOEXEC... yes
checking for __builtin_prefetch... yes
checking for _mm_prefetch... yes
checking for strong getauxval support in the system headers... no
checking for weak getauxval support in the compiler... yes
....
LevelDB using max_open_files=1000 (default=1000)openBSD 6.6 (fed6a9b)checking for fdatasync... yes
checking for F_FULLFSYNC... no
checking for O_CLOEXEC... yes
checking for __builtin_prefetch... yes
checking for _mm_prefetch... yes
checking for strong getauxval support in the system headers... no
checking for weak getauxval support in the compiler... yes
....
LevelDB using max_open_files=1000 (default=1000)Windows 10 (WSL) (fed6a9b + depends)checking for fdatasync... no
checking for F_FULLFSYNC... no
checking for O_CLOEXEC... no
checking for __builtin_prefetch... yes
checking for _mm_prefetch... yes
checking for strong getauxval support in the system headers... no
checking for weak getauxval support in the compiler... yes
...
LevelDB using max_open_files=1000 (default=1000) |
|
@fanquake thanks for testing, good to hear that this is not causing build failure anywhere, and that it works with regard to file dscriptors setting |
laanwj
force-pushed
the
2019_11_leveldb_upstream
branch
from
fed6a9b
to
abd50f1
Compare
laanwj
changed the title
|
Squashed fixup commits and removed WIP tag. Should be ready for review/testing. |
|
ACK. Concept: very happy to get rid of all those local modifications. Code review: I reviewed the diff between upstream LevelDB and our new branch (but not the changes in upstream LevelDB), and the new commits here. I left two nit comments on one of the commits. Tests:
I did observe a segfault in the script verification thread once, which was very surprising. As the script verification threads shouldn't be doing any LevelDB stuff this should be unrelated, but scary regardless. I do have a large number of weird dmesg lines on this system, so this may be a hardware issue. I've been unable to reproduce the problem. |
|
tACK* e2ef5fb. *test on Windows (full sync, as well as catching up an existing node) |
|
Is commit abd50f1 still needed? |
|
Thanks for testing!
The subtree check in travis didn't pass without it. I could try removing it, but I think that's still the case. I don't understand how it's supposed to pass in the first place for the other dependencies without ever fetching them as described in the doc? Or does travis do this hidden? |
|
We only check that it is a subtree, not that the subtree corresponds to some subtree hosted somewhere else. |
Are you sure? Now I'm doubly confused why that commit makes a difference at all (the literal error message was something like "Could not find commit XXXXXX"). Did I subtree correctly? |
|
Maybe the script doesn't work for initial subtree check ins? |
|
I don't understand why that would be the case. What is the difference between this tree, and the tree when it is merged into master, that would make it pass a subtree check? In both cases the merge of (and if it is the case, what should we do, merge with this commit, or merge without it and hope everything will be alright after merging? Both seems suboptimal. Is this the first time that this comes up?) |
Gitian builds
|
|
Anything left to do here? If not, can this get some (re-)ACKs please? |
|
ACK 6648082. No problems using a data directory of "₿_🏃" on Windows 10. |
|
ACK 677fb8e |
laanwj
added a commit
that referenced
this pull request
Feb 10, 2020
677fb8e test: Add ubsan surpression for crc32c (Wladimir J. van der Laan) 8e68bb1 build: Disable msvc warning 4722 for leveldb build (Aaron Clauson) be23949 build: MSVC changes for leveldb update (Aaron Clauson) 9ebdf04 build: CRC32C build system integration (Wladimir J. van der Laan) 402252a build: Add LCOV exception for crc32c (Wladimir J. van der Laan) 3a037d0 test: Add crc32c exception to various linters and generation scripts (Wladimir J. van der Laan) 84ff1b2 test: Add crc32c to subtree check linter (Wladimir J. van der Laan) 7cf13a5 doc: Add crc32c subtree to developer notes (Wladimir J. van der Laan) 24d02a9 build: Update build system for new leveldb (Wladimir J. van der Laan) 2e18193 Squashed 'src/crc32c/' content from commit 224988680f7673cd7c769963d4035cb315aa3388 (Wladimir J. van der Laan) 6648082 Squashed 'src/leveldb/' changes from f545dfa..f8ae182c1e5176d12e816fb2217ae33a5472fdd7 (Wladimir J. van der Laan) Pull request description: This updates leveldb to currently newest upstream commit bitcoin-core/leveldb-subtree@0c40829: - CRC32C hardware acceleration is now an external library [crc32c](https://github.com/google/crc32c). This adds acceleration on ARM, and should be faster on x86 because of using prefetch. It also makes it easy to support similar instruction sets on other platforms in the future. - Thread handling uses C++11, instead of platform specific code. - Native windows environment was added. No need to maintain our own hacky one, anymore. - Upstream now builds using CMake. This doesn't mean we need to use that (phew), but internal configuration changed to a a series of checks, instead of OS profiles. This means the blanket error "Cannot build leveldb for $host. Please file a bug report' is removed. All changes: google/leveldb@a53934a...0c40829 Pretty much all our changes have been subsumed by upstream, so we figured it was cleaner to start over with a new branch from upstream with the still-relevant patches applied: https://github.com/bitcoin-core/leveldb/tree/bitcoin-fork-new There's quite some testing to be done (see below). See bitcoin-core/leveldb-subtree#25 and bitcoin-core/leveldb-subtree#26 for more history and context. TODO: - [x] Subtree `crc32c` - [x] Make linters happy about crc32 subtree - [x] Integrate `crc32c` library into build system - [x] MSVC build system ACKs for top commit: sipa: ACK 677fb8e Tree-SHA512: 37ee92a750e053e924bc4626b12bb3fd81faa9f8c5ebaa343931fee810c45ba05aa6051fdea82535fa351bf2be7297801b98af9469865fc5ead771650a5d6240
This was referenced Feb 10, 2020
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Feb 18, 2020
677fb8e test: Add ubsan surpression for crc32c (Wladimir J. van der Laan) 8e68bb1 build: Disable msvc warning 4722 for leveldb build (Aaron Clauson) be23949 build: MSVC changes for leveldb update (Aaron Clauson) 9ebdf04 build: CRC32C build system integration (Wladimir J. van der Laan) 402252a build: Add LCOV exception for crc32c (Wladimir J. van der Laan) 3a037d0 test: Add crc32c exception to various linters and generation scripts (Wladimir J. van der Laan) 84ff1b2 test: Add crc32c to subtree check linter (Wladimir J. van der Laan) 7cf13a5 doc: Add crc32c subtree to developer notes (Wladimir J. van der Laan) 24d02a9 build: Update build system for new leveldb (Wladimir J. van der Laan) 2e18193 Squashed 'src/crc32c/' content from commit 224988680f7673cd7c769963d4035cb315aa3388 (Wladimir J. van der Laan) 6648082 Squashed 'src/leveldb/' changes from f545dfa..f8ae182c1e5176d12e816fb2217ae33a5472fdd7 (Wladimir J. van der Laan) Pull request description: This updates leveldb to currently newest upstream commit bitcoin-core/leveldb-subtree@0c40829: - CRC32C hardware acceleration is now an external library [crc32c](https://github.com/google/crc32c). This adds acceleration on ARM, and should be faster on x86 because of using prefetch. It also makes it easy to support similar instruction sets on other platforms in the future. - Thread handling uses C++11, instead of platform specific code. - Native windows environment was added. No need to maintain our own hacky one, anymore. - Upstream now builds using CMake. This doesn't mean we need to use that (phew), but internal configuration changed to a a series of checks, instead of OS profiles. This means the blanket error "Cannot build leveldb for $host. Please file a bug report' is removed. All changes: google/leveldb@a53934a...0c40829 Pretty much all our changes have been subsumed by upstream, so we figured it was cleaner to start over with a new branch from upstream with the still-relevant patches applied: https://github.com/bitcoin-core/leveldb/tree/bitcoin-fork-new There's quite some testing to be done (see below). See bitcoin-core/leveldb-subtree#25 and bitcoin-core/leveldb-subtree#26 for more history and context. TODO: - [x] Subtree `crc32c` - [x] Make linters happy about crc32 subtree - [x] Integrate `crc32c` library into build system - [x] MSVC build system ACKs for top commit: sipa: ACK 677fb8e Tree-SHA512: 37ee92a750e053e924bc4626b12bb3fd81faa9f8c5ebaa343931fee810c45ba05aa6051fdea82535fa351bf2be7297801b98af9469865fc5ead771650a5d6240
fanquake
added a commit
that referenced
this pull request
May 11, 2020
6853727 build: Enable -Werror=sign-compare (Ben Woosley) eac6a30 refactor: Rework asmap Interpret to avoid ptrdiff_t (Ben Woosley) df37377 test: Fix outstanding -Wsign-compare errors (Ben Woosley) Pull request description: Disallowing sign-comparison mismatches can help to prevent the introduction of overflow and interpretation bugs. In this case, ~all~ most existing violations are in the tests, and most simply required annotating the literal as unsigned for comparison. This was previously prevented by violations in leveldb which were fixed upstream and merged in #17398. You can test that by building this branch against: 22d1118 vs 75fb37c ACKs for top commit: fjahr: re-ACK 6853727 practicalswift: ACK 6853727 Tree-SHA512: 14b5daa38c496fb51548feb30fb4dd179e6f76a8d355f52bc8e2a18f2f9340f0bc98dcf36d8b3d6521045d013891c3103749a4eda88ceef00202a6a0cf93f73c
random-zebra
added a commit
to PIVX-Project/PIVX
that referenced
this pull request
Jul 31, 2020
9029d86 add crc32c subtree to linter (Fuzzbawls) f9b99e9 cmake: crc32c and leveldb native integration (Fuzzbawls) 7478a81 build: CRC32C build system integration (Wladimir J. van der Laan) 18366bf build: add LCOV exception for crc32c (Fuzzbawls) d6e0aa2 test: add crc32c exception to generation scripts (Fuzzbawls) 6270118 doc: Add crc32c subtree to developer notes (Wladimir J. van der Laan) de6c30e build: Update build system for new leveldb (Wladimir J. van der Laan) c0752b7 Squashed 'src/crc32c/' content from commit 224988680f (Fuzzbawls) 9cabe58 Squashed 'src/leveldb/' changes from 64052c7..0c40829872 (Fuzzbawls) Pull request description: Backport of bitcoin#17398, original description: > This updates leveldb to currently newest upstream commit bitcoin-core/leveldb-subtree@0c40829: > >* CRC32C hardware acceleration is now an external library [crc32c](https://github.com/google/crc32c). This adds acceleration on ARM, and should be faster on x86 because of using prefetch. It also makes it easy to support similar instruction sets on other platforms in the future. >* Thread handling uses C++11, instead of platform specific code. >* Native windows environment was added. No need to maintain our own hacky one, anymore. >* Upstream now builds using CMake. This doesn't mean we need to use that (phew), but internal configuration changed to a series of checks, instead of OS profiles. This means the blanket error "Cannot build leveldb for $host. Please file a bug report' is removed. >All changes: google/leveldb@a53934a...0c40829 >Pretty much all our changes have been subsumed by upstream, so we figured it was cleaner to start over with a new branch from upstream with the still-relevant patches applied: https://github.com/bitcoin-core/leveldb/tree/bitcoin-fork-new Additional work was done to ensure compatibility with our CMake build system, but our primary Autotools build system maintains the approach of defining build targets is `Makefile.leveldb.include` and `Makefile.crc32c.include` ACKs for top commit: furszy: nice update, ACK 9029d86 random-zebra: ACK 9029d86 and merging... Tree-SHA512: d2907a1a63b4958f351144eb62aa5d1b8d4666e54a45a96f35d8e3f243ed45ceefbb24554a1f25f89e5f9aec8aa4c92cf89f93586e80576c44d55767548bbc42
sidhujag
pushed a commit
to syscoin-core/syscoin
that referenced
this pull request
Nov 10, 2020
677fb8e test: Add ubsan surpression for crc32c (Wladimir J. van der Laan) 8e68bb1 build: Disable msvc warning 4722 for leveldb build (Aaron Clauson) be23949 build: MSVC changes for leveldb update (Aaron Clauson) 9ebdf04 build: CRC32C build system integration (Wladimir J. van der Laan) 402252a build: Add LCOV exception for crc32c (Wladimir J. van der Laan) 3a037d0 test: Add crc32c exception to various linters and generation scripts (Wladimir J. van der Laan) 84ff1b2 test: Add crc32c to subtree check linter (Wladimir J. van der Laan) 7cf13a5 doc: Add crc32c subtree to developer notes (Wladimir J. van der Laan) 24d02a9 build: Update build system for new leveldb (Wladimir J. van der Laan) 2e18193 Squashed 'src/crc32c/' content from commit 224988680f7673cd7c769963d4035cb315aa3388 (Wladimir J. van der Laan) 6648082 Squashed 'src/leveldb/' changes from f545dfa..f8ae182c1e5176d12e816fb2217ae33a5472fdd7 (Wladimir J. van der Laan) Pull request description: This updates leveldb to currently newest upstream commit bitcoin-core/leveldb-subtree@0c40829: - CRC32C hardware acceleration is now an external library [crc32c](https://github.com/google/crc32c). This adds acceleration on ARM, and should be faster on x86 because of using prefetch. It also makes it easy to support similar instruction sets on other platforms in the future. - Thread handling uses C++11, instead of platform specific code. - Native windows environment was added. No need to maintain our own hacky one, anymore. - Upstream now builds using CMake. This doesn't mean we need to use that (phew), but internal configuration changed to a a series of checks, instead of OS profiles. This means the blanket error "Cannot build leveldb for $host. Please file a bug report' is removed. All changes: google/leveldb@a53934a...0c40829 Pretty much all our changes have been subsumed by upstream, so we figured it was cleaner to start over with a new branch from upstream with the still-relevant patches applied: https://github.com/bitcoin-core/leveldb/tree/bitcoin-fork-new There's quite some testing to be done (see below). See bitcoin-core/leveldb-subtree#25 and bitcoin-core/leveldb-subtree#26 for more history and context. TODO: - [x] Subtree `crc32c` - [x] Make linters happy about crc32 subtree - [x] Integrate `crc32c` library into build system - [x] MSVC build system ACKs for top commit: sipa: ACK 677fb8e Tree-SHA512: 37ee92a750e053e924bc4626b12bb3fd81faa9f8c5ebaa343931fee810c45ba05aa6051fdea82535fa351bf2be7297801b98af9469865fc5ead771650a5d6240
PastaPastaPasta
pushed a commit
to dashpay/dash
that referenced
this pull request
Jul 15, 2021
* Update to leveldb upstream using subtree merge * Import crc32c using subtree merge as as 'src/crc32c' * build: Update build system for new leveldb Upstream leveldb switched build systems, which means we need to define a few different values. Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com> * doc: Add crc32c subtree to developer notes * test: Add crc32c to subtree check linter * test: Add crc32c exception to various linters and generation scripts * build: Add LCOV exception for crc32c Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com> * build: CRC32C build system integration Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Oct 6, 2021
6853727 build: Enable -Werror=sign-compare (Ben Woosley) eac6a30 refactor: Rework asmap Interpret to avoid ptrdiff_t (Ben Woosley) df37377 test: Fix outstanding -Wsign-compare errors (Ben Woosley) Pull request description: Disallowing sign-comparison mismatches can help to prevent the introduction of overflow and interpretation bugs. In this case, ~all~ most existing violations are in the tests, and most simply required annotating the literal as unsigned for comparison. This was previously prevented by violations in leveldb which were fixed upstream and merged in bitcoin#17398. You can test that by building this branch against: 22d1118 vs 75fb37c ACKs for top commit: fjahr: re-ACK 6853727 practicalswift: ACK 6853727 Tree-SHA512: 14b5daa38c496fb51548feb30fb4dd179e6f76a8d355f52bc8e2a18f2f9340f0bc98dcf36d8b3d6521045d013891c3103749a4eda88ceef00202a6a0cf93f73c
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Oct 6, 2021
6853727 build: Enable -Werror=sign-compare (Ben Woosley) eac6a30 refactor: Rework asmap Interpret to avoid ptrdiff_t (Ben Woosley) df37377 test: Fix outstanding -Wsign-compare errors (Ben Woosley) Pull request description: Disallowing sign-comparison mismatches can help to prevent the introduction of overflow and interpretation bugs. In this case, ~all~ most existing violations are in the tests, and most simply required annotating the literal as unsigned for comparison. This was previously prevented by violations in leveldb which were fixed upstream and merged in bitcoin#17398. You can test that by building this branch against: 22d1118 vs 75fb37c ACKs for top commit: fjahr: re-ACK 6853727 practicalswift: ACK 6853727 Tree-SHA512: 14b5daa38c496fb51548feb30fb4dd179e6f76a8d355f52bc8e2a18f2f9340f0bc98dcf36d8b3d6521045d013891c3103749a4eda88ceef00202a6a0cf93f73c
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Oct 10, 2021
6853727 build: Enable -Werror=sign-compare (Ben Woosley) eac6a30 refactor: Rework asmap Interpret to avoid ptrdiff_t (Ben Woosley) df37377 test: Fix outstanding -Wsign-compare errors (Ben Woosley) Pull request description: Disallowing sign-comparison mismatches can help to prevent the introduction of overflow and interpretation bugs. In this case, ~all~ most existing violations are in the tests, and most simply required annotating the literal as unsigned for comparison. This was previously prevented by violations in leveldb which were fixed upstream and merged in bitcoin#17398. You can test that by building this branch against: 22d1118 vs 75fb37c ACKs for top commit: fjahr: re-ACK 6853727 practicalswift: ACK 6853727 Tree-SHA512: 14b5daa38c496fb51548feb30fb4dd179e6f76a8d355f52bc8e2a18f2f9340f0bc98dcf36d8b3d6521045d013891c3103749a4eda88ceef00202a6a0cf93f73c
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Oct 12, 2021
6853727 build: Enable -Werror=sign-compare (Ben Woosley) eac6a30 refactor: Rework asmap Interpret to avoid ptrdiff_t (Ben Woosley) df37377 test: Fix outstanding -Wsign-compare errors (Ben Woosley) Pull request description: Disallowing sign-comparison mismatches can help to prevent the introduction of overflow and interpretation bugs. In this case, ~all~ most existing violations are in the tests, and most simply required annotating the literal as unsigned for comparison. This was previously prevented by violations in leveldb which were fixed upstream and merged in bitcoin#17398. You can test that by building this branch against: 22d1118 vs 75fb37c ACKs for top commit: fjahr: re-ACK 6853727 practicalswift: ACK 6853727 Tree-SHA512: 14b5daa38c496fb51548feb30fb4dd179e6f76a8d355f52bc8e2a18f2f9340f0bc98dcf36d8b3d6521045d013891c3103749a4eda88ceef00202a6a0cf93f73c
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Dec 12, 2021
6853727 build: Enable -Werror=sign-compare (Ben Woosley) eac6a30 refactor: Rework asmap Interpret to avoid ptrdiff_t (Ben Woosley) df37377 test: Fix outstanding -Wsign-compare errors (Ben Woosley) Pull request description: Disallowing sign-comparison mismatches can help to prevent the introduction of overflow and interpretation bugs. In this case, ~all~ most existing violations are in the tests, and most simply required annotating the literal as unsigned for comparison. This was previously prevented by violations in leveldb which were fixed upstream and merged in bitcoin#17398. You can test that by building this branch against: 22d1118 vs 75fb37c ACKs for top commit: fjahr: re-ACK 6853727 practicalswift: ACK 6853727 Tree-SHA512: 14b5daa38c496fb51548feb30fb4dd179e6f76a8d355f52bc8e2a18f2f9340f0bc98dcf36d8b3d6521045d013891c3103749a4eda88ceef00202a6a0cf93f73c
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Dec 12, 2021
6853727 build: Enable -Werror=sign-compare (Ben Woosley) eac6a30 refactor: Rework asmap Interpret to avoid ptrdiff_t (Ben Woosley) df37377 test: Fix outstanding -Wsign-compare errors (Ben Woosley) Pull request description: Disallowing sign-comparison mismatches can help to prevent the introduction of overflow and interpretation bugs. In this case, ~all~ most existing violations are in the tests, and most simply required annotating the literal as unsigned for comparison. This was previously prevented by violations in leveldb which were fixed upstream and merged in bitcoin#17398. You can test that by building this branch against: 22d1118 vs 75fb37c ACKs for top commit: fjahr: re-ACK 6853727 practicalswift: ACK 6853727 Tree-SHA512: 14b5daa38c496fb51548feb30fb4dd179e6f76a8d355f52bc8e2a18f2f9340f0bc98dcf36d8b3d6521045d013891c3103749a4eda88ceef00202a6a0cf93f73c
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This updates leveldb to currently newest upstream commit bitcoin-core/leveldb-subtree@0c40829:
All changes: google/leveldb@a53934a...0c40829
Pretty much all our changes have been subsumed by upstream, so we figured it was cleaner to start over with a new branch from upstream with the still-relevant patches applied: https://github.com/bitcoin-core/leveldb/tree/bitcoin-fork-new
There's quite some testing to be done (see below). See bitcoin-core/leveldb-subtree#25 and bitcoin-core/leveldb-subtree#26 for more history and context.
TODO:
crc32ccrc32clibrary into build system