-
Notifications
You must be signed in to change notification settings - Fork 912
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: disable-grpc flag #7478
build: disable-grpc flag #7478
Conversation
11899b2
to
5cfe72b
Compare
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.
This seems like it adds a RUST dependent GRPC option; defaulting it to active won't have any impact unless the RUST compilation option is also set.
What's the end user goal here?
@@ -179,6 +179,7 @@ set_defaults() | |||
VALGRIND=${VALGRIND:-$(default_valgrind_setting)} | |||
TEST_NETWORK=${TEST_NETWORK:-regtest} | |||
RUST=${RUST:-$(default_rust_setting)} | |||
GRPC=1 |
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.
since this depends on RUST, seems logical to default it to the rust setting?
@@ -220,6 +221,8 @@ usage() | |||
echo " Compile with fuzzing" | |||
usage_with_default "--enable/disable-rust" "$RUST" "enable" "disable" | |||
echo " Compile with Rust support" | |||
usage_with_default "--enable/disable-grpc" "$GRPC" "enable" "disable" | |||
echo " Compile with cln-grpc plugin" |
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.
If this is enabled but RUST isn't, iiuc it won't actually compile the CLN-GRPC plugin right? Seems confusing.
Based on this discord conversation, some users might want to disable the grpc plugin at build time to reduce build times. The same can be achieved with |
I also agree it's better with the separate |
Let's cut down on the compilation configurations, and keep things as runtime configuration whenever possible. We already eliminated the experimental features compile time flag, and adding new ones may not be a good idea (it massively increases the support area). To disable at runtime we have |
related: #7473
When enabling the cln-grpc plugin to run by default, uses should have the option to opt out of it at build time.