-
Notifications
You must be signed in to change notification settings - Fork 26
wip: checking compilation without libclang #750
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
base: master
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThe workflow step that installs system dependencies on Linux in Possibly related PRs
Suggested reviewers
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Manual Deploy AvailableYou can trigger a manual deploy of this PR branch to testnet: Alternative: Comment
Comment updated automatically when the PR is synchronized. |
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.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
test-integration/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
.github/actions/setup-build-env/action.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 650
File: magicblock-chainlink/src/submux/subscription_task.rs:13-99
Timestamp: 2025-11-20T08:57:07.217Z
Learning: In the magicblock-validator repository, avoid posting review comments that merely confirm code is correct or matches intended behavior without providing actionable feedback, suggestions for improvement, or identifying potential issues. Such confirmatory comments are considered unhelpful noise by the maintainers.
🔇 Additional comments (1)
.github/actions/setup-build-env/action.yml (1)
33-33: No action required — the project contains no libclang-dependent crates.Verification of the codebase shows that neither
bindgennorclang-sysappear in any of the 20+Cargo.tomlfiles across the workspace. The project depends exclusively on Solana-related and standard Rust libraries. The removal oflibclang-devdoes not break the build because the project never required it for compilation.Likely an incorrect or invalid review comment.
| shell: "bash" | ||
| run: rustup toolchain install ${{ inputs.rust_toolchain_release }} --profile default | ||
|
|
||
| - name: Install system deps (libudev, LLVM/Clang) |
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.
Update step name to reflect actual packages installed.
The step name mentions "LLVM/Clang" but libclang-dev is no longer being installed. Update the name to match the actual behavior.
Apply this diff to fix the inconsistency:
- - name: Install system deps (libudev, LLVM/Clang)
+ - name: Install system deps (libudev)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Install system deps (libudev, LLVM/Clang) | |
| - name: Install system deps (libudev) |
🤖 Prompt for AI Agents
.github/actions/setup-build-env/action.yml around line 30: the step name
currently reads "Install system deps (libudev, LLVM/Clang)" but
libclang-dev/LLVM/Clang are no longer installed; update the step name to
accurately list the packages actually installed (for example "Install system
deps (libudev)" or "Install system deps (libudev, libxyz)" if there are other
packages) so the action name matches behavior; edit the name string on line 30
to the corrected concise description.
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.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
test-integration/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
Cargo.toml(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 650
File: magicblock-chainlink/src/submux/subscription_task.rs:13-99
Timestamp: 2025-11-20T08:57:07.217Z
Learning: In the magicblock-validator repository, avoid posting review comments that merely confirm code is correct or matches intended behavior without providing actionable feedback, suggestions for improvement, or identifying potential issues. Such confirmatory comments are considered unhelpful noise by the maintainers.
📚 Learning: 2025-11-07T18:19:48.996Z
Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 614
File: test-integration/programs/schedulecommit/Cargo.toml:19-20
Timestamp: 2025-11-07T18:19:48.996Z
Learning: In Solana programs (crates depending on solana-program), the empty feature flags `custom-heap = []` and `custom-panic = []` are standard declarations required to avoid compiler warnings. These should not be flagged as unused features in Cargo.toml files for Solana program crates.
Applied to files:
Cargo.toml
📚 Learning: 2025-11-24T14:21:00.996Z
Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 639
File: Cargo.toml:58-58
Timestamp: 2025-11-24T14:21:00.996Z
Learning: In the magicblock-validator codebase, magicblock-api/Cargo.toml intentionally uses borsh = "1.5.3" (instead of the workspace version 0.10.4) because it needs to deserialize types from the magic-domain-program external dependency, which requires borsh 1.5.x compatibility. This is an intentional exception for interoperability with the magic domain program.
Applied to files:
Cargo.toml
| version = "0.22.0" | ||
| # default-features = false | ||
| # features = ["lz4"] | ||
| features = ["lz4"] |
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.
Disable default features alongside the lz4 feature to avoid libclang dependency.
To enable only LZ4 compression: default-features = false features = ["lz4"]. By default, the bindgen-runtime feature enables the runtime feature of bindgen, which dynamically links to libclang and is suitable for most platforms and enabled by default. Since the PR objective is compilation without libclang, add default-features = false to the rocksdb dependency alongside the features = ["lz4"] specification.
🤖 Prompt for AI Agents
In Cargo.toml around line 213, the rocksdb dependency currently enables lz4 via
features = ["lz4"] but still pulls in bindgen-runtime/libclang; update the
rocksdb dependency to disable default features by adding default-features =
false alongside features = ["lz4"] so it compiles without libclang (i.e., set
default-features = false and keep features = ["lz4"] for the rocksdb entry).
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.