build: make dynamic linking opt-in instead of default #9626
Conversation
| sudo apt install make build-essential cmake protobuf-compiler curl openssl libssl-dev libsasl2-dev libcurl4-openssl-dev pkg-config postgresql-client tmux lld libzstd-dev | ||
| sudo apt install make build-essential cmake protobuf-compiler curl openssl libssl-dev libsasl2-dev libcurl4-openssl-dev pkg-config postgresql-client tmux lld |
There was a problem hiding this comment.
I think more can be removed, but I'm not brave enough to do it. 😅
There was a problem hiding this comment.
I guess this can be aligned with ci/Dockerfile. 👀
src/risedevtool/config/src/main.rs
Outdated
There was a problem hiding this comment.
Moving to a different crate so that it compiles fast & won't fail.
Codecov Report
@@ Coverage Diff @@
## main #9626 +/- ##
==========================================
- Coverage 70.99% 70.77% -0.22%
==========================================
Files 1245 1233 -12
Lines 208372 206790 -1582
==========================================
- Hits 147927 146353 -1574
+ Misses 60445 60437 -8
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 212 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Makefile.toml
Outdated
| set_env ENABLE_TELEMETRY "false" | ||
|
|
||
| is_sanitizer_enabled = get_env ENABLE_SANITIZER | ||
| is_all_in_one_enabled = get_env ENABLE_ALL_IN_ONE | ||
| is_hdfs_backend = get_env ENABLE_HDFS | ||
| is_release = get_env ENABLE_RELEASE_PROFILE | ||
| is_release = set ${is_release} or false |
There was a problem hiding this comment.
workaround sagiegurari/duckscript#326
| --features "${RISINGWAVE_FEATURES}" \ | ||
| ${RISINGWAVE_FEATURE_FLAGS} \ |
There was a problem hiding this comment.
I considered make rw-static-link default feature, and add --no-default-features for dynamic linking. But not sure.
There was a problem hiding this comment.
Both LGTM as long as we handle and hide all this stuff in RiseDev. 🤣
| openssl = { version = "0.10", optional = true, features = ["vendored"] } | ||
| rdkafka = { workspace = true, optional = true, features = ["cmake-build", "ssl-vendored", "gssapi-vendored"] } | ||
| openssl-sys = { version = "0.9", optional = true, features = ["vendored"] } | ||
| sasl2-sys = { version = "0.1", optional = true, features = ["gssapi-vendored"] } |
There was a problem hiding this comment.
I think if any crate enabled vendored ssl, the eventual result is enabling openssl-sys/vendored. So I just use its own feature here to be more transparent.
| @@ -9,14 +9,15 @@ repository = { workspace = true } | |||
| # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html | |||
|
|
|||
| [features] | |||
| # Use a feature flag to control enable or not, otherwise `cargo test` will compile all dependencies. | |||
There was a problem hiding this comment.
I don’t understand the comment, so removed 😅
| @@ -49,6 +54,12 @@ else | |||
| set_env BUILD_HDFS_BACKEND_CMD "" | |||
| end | |||
|
|
|||
| if ${is_not_release} and ${is_dynamic_linking} | |||
There was a problem hiding this comment.
Note: if not ${is_release} and ${is_dynamic_linking} doesn't work. sagiegurari/duckscript#326
|
@BugenZhao request approval |
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Checklist For Contributors
./risedev check(or alias,./risedev c)Checklist For Reviewers
Documentation
Click here for Documentation
Types of user-facing changes
Please keep the types that apply to your changes, and remove the others.
Release note