-
Notifications
You must be signed in to change notification settings - Fork 5
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
cmake: Port changes from PR28981 #154
Conversation
Backport changes from bitcoin#28981.
ea7e4a2
to
b8fd285
Compare
AUTO | ||
) | ||
cmake_dependent_option(WITH_EXTERNAL_SIGNER "Enable external signer support." ON "NOT WIN32" OFF) | ||
set(ENABLE_EXTERNAL_SIGNER ${WITH_EXTERNAL_SIGNER}) |
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.
Why this set
? Is it just because the option name and the internal name are different?
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.
Correct. See:
bitcoin/cmake/bitcoin-config.h.in
Lines 145 to 146 in b8fd285
/* Define if external signer support is enabled. */ | |
#cmakedefine ENABLE_EXTERNAL_SIGNER |
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.
Thanks!
@@ -55,54 +55,4 @@ function(add_boost_if_needed) | |||
endif() | |||
endif() | |||
|
|||
if(WITH_EXTERNAL_SIGNER) |
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.
good to get rid of this
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.
Code review ACK b8fd285
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.
#154 (comment) this is unfortunate, but I think aligning the naming is something that should be done in another PR in concert with the other options.
LGTM ACK
This PR ports changes from bitcoin#28981.