-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
build: partial revert dash#4311 (add libgmp detection, make immer a package) #4911
Conversation
I think you need to add a comment to |
|
Why start with immer though? I think the main obstacle here is |
Marked as draft until libblsdash and relic has been moved to source tree |
This pull request has conflicts, please rebase. |
git-subtree-dir: src/immer git-subtree-split: 9cb6a5a845df766a4cb3526d8a4584bf99bac8d5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one suggestion, LGTM otherwise
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, immer source matches, PR compiles locally
ACK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK for merging via merge commit
…as c0b716f2 f18e839 build: drop symlinks in immer subtree (Kittywhiskers Van Gogh) d761111 build: fix gitian builds (Kittywhiskers Van Gogh) a9f46b3 Squashed 'src/immer/' changes from 9cb6a5a845..5875f7739a (Kittywhiskers Van Gogh) e4ee302 revert: fix gitian builds (Kittywhiskers Van Gogh) fb00300 revert: drop symlinks in immer subtree (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * Dependency for #6380 #6380 will be using `std::ranges` as a replacement for [dash#4622](#4622) and currently, it will fail to compile due to the current version of `immer` (last updated two years ago in #4911) not meeting constraints ([source](https://en.cppreference.com/w/cpp/language/constraints)) set by `std::ranges` (see below). ``` ./evo/deterministicmns.h:243:16: error: no matching function for call to object of type 'const __count_if_fn' return ranges::count_if(mnMap, [](const auto& p) { return IsMNValid(*p.second); }); ^~~~~~~~~~~~~~~~ /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/ranges_algo.h:331:7: note: candidate template ignored: substitution failure [with _Range = const MnMap &, _Proj = identity, _Pred = (lambda at ./evo/deterministicmns.h:243:40)]: constraints not satisfied for alias template 'range_difference_t' [with _Range = const immer::map<uint256, std::shared_ptr<const CDeterministicMN>, CDeterministicMNList::ImmerHasher> &] operator()(_Range&& __r, _Pred __pred, _Proj __proj = {}) const ^ /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/ranges_algo.h:316:7: note: candidate function template not viable: requires at least 3 arguments, but 2 were provided operator()(_Iter __first, _Sent __last, ^ ``` This has been resolved by updating `immer` to the latest available release, [v0.8.1](https://github.com/arximboldi/immer/releases/tag/v0.8.1). Expected subtree hash **without changes** are `a5ded361aec714bc74149909c5e1984c10ab132c4c539c63fe57362dccd95556` (see [here](#6323 (review)) for verification instructions). ## Breaking Changes None expected. ## Checklist - [x] I have performed a self-review of my own code **(note: N/A)** - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)** - [x] I have made corresponding changes to the documentation **(note: N/A)** - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: PastaPastaPasta: utACK f18e839 UdjinM6: utACK f18e839 Tree-SHA512: 99d1b577eb4cf3fcc3ebfd28f19006700f085d23ccdabe802a2aa5844e4b39314347b0c0e49a352c5fc034d6584a0109edf08fa3c0846514967f1fb16e7f127a
Additional Notes
Dash's build system currently possesses a limitation where you cannot build it without the in-tree depends directory. Back when that pull request was made, we were exclusively using the build system used by Bitcoin (GNU Autotools) and were fine with the additional constraint as there was only a singular build system and a sole direction for support.
Unfortunately, if we need to support systems like CMake (for better IDE support) and MSVC (downstreaming from Core for native builds instead of cross-compiles), we need to remove the depends lock-in as maintaining the particular -isms of those respective build systems within depends will prove to be more taxing than beneficial, hence necessitating the move to the source tree