msvc: remove direct Bitcoin Core compat.h include
#66
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
My (untested) assumption is that this was only being used for a ssize_t definition. Remove the include, and define ssize_t to SSIZE_T. I'm planning on making a similar change in the Core codebase. See: https://docs.microsoft.com/en-us/windows/win32/winprog/windows-data-types#ssize_t
|
@fanquake Does Bitcoin Core compile on MSVC with this change? Because minisketch itself has no MSVC CI. |
|
@sipa I cherry-picked a subtree update + this change into bitcoin/bitcoin#25493, to test the MSVC CI, and it's compiled / run the unit tests fine. Functional tests running at the moment: https://cirrus-ci.com/task/6565150772690944. |
|
MSVC build has finished all green in our repo. |
|
ACK 64f1758 |
fanquake
added a commit
to fanquake/bitcoin
that referenced
this pull request
Jun 29, 2022
47f0a2d26f Merge bitcoin-core/minisketch#66: msvc: remove direct Bitcoin Core `compat.h` include 64f17584c7 msvc: remove Core compat.h include a223557ce1 Merge bitcoin-core/minisketch#65: Update broken links 127f7763e8 Update broken links git-subtree-dir: src/minisketch git-subtree-split: 47f0a2d26f6ca0f6ab3ba5e72064a9d28745de77
achow101
added a commit
to bitcoin-core/gui
that referenced
this pull request
Jun 29, 2022
28a28a0 Squashed 'src/minisketch/' changes from 7eeb778fef..47f0a2d26f (fanquake) Pull request description: Contains: * bitcoin-core/minisketch#65 * bitcoin-core/minisketch#66 Required for #25493. ACKs for top commit: achow101: ACK dc375e5 hebasto: ACK dc375e5, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: fbcd6cdc551770ff67d1df65ab171ce43c9eb7e7668da76da5c5b06865ed550527abcff661741a86c1535018a85a165619ff94ae3e6c7a695374b6c4f843c5ca
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Jun 29, 2022
28a28a0 Squashed 'src/minisketch/' changes from 7eeb778fef..47f0a2d26f (fanquake) Pull request description: Contains: * bitcoin-core/minisketch#65 * bitcoin-core/minisketch#66 Required for bitcoin#25493. ACKs for top commit: achow101: ACK dc375e5 hebasto: ACK dc375e5, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: fbcd6cdc551770ff67d1df65ab171ce43c9eb7e7668da76da5c5b06865ed550527abcff661741a86c1535018a85a165619ff94ae3e6c7a695374b6c4f843c5ca
janus
pushed a commit
to BitgesellOfficial/bitgesell
that referenced
this pull request
Aug 4, 2022
47f0a2d26f Merge bitcoin-core/minisketch#66: msvc: remove direct Bitcoin Core `compat.h` include 64f17584c7 msvc: remove Core compat.h include a223557ce1 Merge bitcoin-core/minisketch#65: Update broken links 127f7763e8 Update broken links git-subtree-dir: src/minisketch git-subtree-split: 47f0a2d26f6ca0f6ab3ba5e72064a9d28745de77
vmta
added a commit
to umkoin/umkoin
that referenced
this pull request
Oct 6, 2024
eb37a9b8e Merge bitcoin-core/minisketch#87: Avoid copy in self-assign fe6557642 Merge bitcoin-core/minisketch#88: build: Add `-Wundef` 8ea298bfa Avoid copy in self-assign 978a3d886 build: Add `-Wundef` 338704417 Merge bitcoin-core/minisketch#86: doc: fix typo in sketch_impl.h 15c2d13b6 doc: fix typo in sketch_impl.h 7be08b8a4 Merge bitcoin-core/minisketch#85: Fixes for integer precision loss 00fb4a4d8 Avoid or make integer precision conversion explicit 9d62a4d27 Avoid the need to cast/convert to size_t for vector operations 19e06cc7a Prevent overflows from large capacity/max_elements 3472e2f5e Merge bitcoin-core/minisketch#81: Avoid overflowing shift by special casing inverse of 1 653d8b2e2 Avoid overflowing shift by special casing inverse of 1 33b7c200b Merge bitcoin-core/minisketch#80: Add c++20 version of CountBits 4a48f31a3 Merge bitcoin-core/minisketch#83: ci: Fix "s390x (big-endian)" task 82b6488ac Add c++20 version of CountBits 0498084d3 ci: Fix "s390x (big-endian)" task 71709dca9 Merge bitcoin-core/minisketch#82: ci: Fix `x86_64-w64-mingw32` task 9e6127fa9 Merge bitcoin-core/minisketch#74: Avoid >> above type width in BitWriter ed420bc17 ci: Fix `x86_64-w64-mingw32` task fe1040f22 Drop -Wno-shift-count-overflow compile flag 154bcd43b Avoid >> above type width in BitWriter 67b87acdb Merge bitcoin-core/minisketch#78: ci: Update macOS image for CI 7de725041 ci: Update macOS image for CI 83d812ea9 Merge bitcoin-core/minisketch#73: ci: Use correct variable to designate C++ compiler e051a7d69 ci: Install wine32 package for Windows tests 2d2c695d7 build: Drop unused `CC` variable 1810fcbd1 ci: Use correct variable to designate C++ compiler 022b95904 Merge bitcoin-core/minisketch#77: Add missing include 08443c489 Add missing include a571ba20f Merge bitcoin-core/minisketch#68: Add missed `#include <string>` b9a7f7e2b Merge bitcoin-core/minisketch#69: refactor: Drop unused `total` local variables 8a5af94ed Merge bitcoin-core/minisketch#70: build: Remove `-Qunused-arguments` workaround for clang + ccache c36f1f03a Merge bitcoin-core/minisketch#72: Fix MSVC implementation of `CountBits()` function 0078bedda Ignore `HAVE_CLZ` macro when building with MSVC 1c772918c Fix MSVC implementation of `CountBits()` function 98f87c55f build: Remove `-Qunused-arguments` workaround for clang + ccache 11a1e25c8 refactor: Drop unused `total` local variables ed6c8fcfd Add missed `#include <string>` 47f0a2d26 Merge bitcoin-core/minisketch#66: msvc: remove direct Bitcoin Core `compat.h` include 64f17584c msvc: remove Core compat.h include git-subtree-dir: src/minisketch git-subtree-split: eb37a9b8e79f9e49d73b96a49bf97a96d9eb676c
kwvg
added a commit
to kwvg/dash
that referenced
this pull request
Oct 20, 2024
47f0a2d26f Merge bitcoin-core/minisketch#66: msvc: remove direct Bitcoin Core `compat.h` include 64f17584c7 msvc: remove Core compat.h include a223557ce1 Merge bitcoin-core/minisketch#65: Update broken links 127f7763e8 Update broken links git-subtree-dir: src/minisketch git-subtree-split: 47f0a2d26f6ca0f6ab3ba5e72064a9d28745de77
tomt1664
pushed a commit
to tomt1664/elements
that referenced
this pull request
Apr 24, 2025
a571ba20f9 Merge bitcoin-core/minisketch#68: Add missed `#include <string>` b9a7f7e2bc Merge bitcoin-core/minisketch#69: refactor: Drop unused `total` local variables 8a5af94edc Merge bitcoin-core/minisketch#70: build: Remove `-Qunused-arguments` workaround for clang + ccache c36f1f03a3 Merge bitcoin-core/minisketch#72: Fix MSVC implementation of `CountBits()` function 0078bedda6 Ignore `HAVE_CLZ` macro when building with MSVC 1c772918c4 Fix MSVC implementation of `CountBits()` function 98f87c55f4 build: Remove `-Qunused-arguments` workaround for clang + ccache 11a1e25c81 refactor: Drop unused `total` local variables ed6c8fcfd9 Add missed `#include <string>` 47f0a2d26f Merge bitcoin-core/minisketch#66: msvc: remove direct Bitcoin Core `compat.h` include 64f17584c7 msvc: remove Core compat.h include a223557ce1 Merge bitcoin-core/minisketch#65: Update broken links 127f7763e8 Update broken links git-subtree-dir: src/minisketch git-subtree-split: a571ba20f9dd1accab6a2309d066369878042ca6
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
My (untested) assumption is that this was only being used for a ssize_t
definition. Remove the include, and define ssize_t to SSIZE_T.
I'm planning on making a similar change in the Core codebase, and will pull a subtree update if this is merged.
See:
https://docs.microsoft.com/en-us/windows/win32/winprog/windows-data-types#ssize_t