Updated templates' bins to fix compilation errors. #613
Conversation
WalkthroughRefactors template build entrypoints to apply Changes
Sequence Diagram(s)sequenceDiagram
participant Deployer as DeployerExt
participant Container as DeployedContractsContainer
participant Contract as HostContract (deployed)
Note over Deployer,Container: Simple path (no package_name)
Deployer->>Container: contract_ref::<Contract>(env)
alt contract exists
Container-->>Deployer: Ok(HostRef)
else not found
Deployer->>Deployer: try_deploy(...) -> contract
Deployer->>Container: add_contract(&contract)
Container-->>Deployer: Ok(())
Deployer-->>Deployer: return HostRef
end
Note over Deployer,Container: CFG-enabled path (named package)
Deployer->>Container: contract_ref_named::<Contract>(env, package_name)
alt contract exists
Container-->>Deployer: Ok(HostRef)
else not found
Deployer->>Deployer: try_deploy_with_cfg(...) -> contract
Deployer->>Container: add_contract_named(&contract, package_name)
Container-->>Deployer: Ok(())
Deployer-->>Deployer: return HostRef
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used🧬 Code graph analysis (2)odra-cli/src/utils.rs (3)
odra-cli/src/container.rs (4)
🔇 Additional comments (3)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
templates/full/bin/build_schema.rs (1)
2-3: Broad lint allowances for templates.Allowing
unused_imports,redundant_imports, andclippy::single_component_path_importsis reasonable for template code where generated projects may have varying import needs. However, these suppressions prevent detection of actual import hygiene issues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
templates/blank/bin/build_contract.rs(1 hunks)templates/blank/bin/build_schema.rs(1 hunks)templates/cep18/bin/build_contract.rs(1 hunks)templates/cep18/bin/build_schema.rs(1 hunks)templates/cep95/bin/build_contract.rs(1 hunks)templates/cep95/bin/build_schema.rs(1 hunks)templates/full/bin/build_contract.rs(1 hunks)templates/full/bin/build_schema.rs(1 hunks)templates/workspace/flapper/bin/build_contract.rs(1 hunks)templates/workspace/flapper/bin/build_schema.rs(1 hunks)templates/workspace/flipper/bin/build_contract.rs(1 hunks)templates/workspace/flipper/bin/build_schema.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (12)
templates/workspace/flapper/bin/build_contract.rs (5)
templates/blank/bin/build_contract.rs (1)
main(8-8)templates/cep18/bin/build_contract.rs (1)
main(8-8)templates/cep95/bin/build_contract.rs (1)
main(8-8)templates/full/bin/build_contract.rs (1)
main(8-8)templates/workspace/flipper/bin/build_contract.rs (1)
main(8-8)
templates/cep18/bin/build_schema.rs (2)
odra-macros/src/utils/attr.rs (1)
odra_module(11-13)odra-build/src/lib.rs (1)
schema(10-21)
templates/blank/bin/build_contract.rs (5)
templates/cep18/bin/build_contract.rs (1)
main(8-8)templates/cep95/bin/build_contract.rs (1)
main(8-8)templates/full/bin/build_contract.rs (1)
main(8-8)templates/workspace/flapper/bin/build_contract.rs (1)
main(8-8)templates/workspace/flipper/bin/build_contract.rs (1)
main(8-8)
templates/cep18/bin/build_contract.rs (5)
templates/blank/bin/build_contract.rs (1)
main(8-8)templates/cep95/bin/build_contract.rs (1)
main(8-8)templates/full/bin/build_contract.rs (1)
main(8-8)templates/workspace/flapper/bin/build_contract.rs (1)
main(8-8)templates/workspace/flipper/bin/build_contract.rs (1)
main(8-8)
templates/full/bin/build_contract.rs (5)
templates/blank/bin/build_contract.rs (1)
main(8-8)templates/cep18/bin/build_contract.rs (1)
main(8-8)templates/cep95/bin/build_contract.rs (1)
main(8-8)templates/workspace/flapper/bin/build_contract.rs (1)
main(8-8)templates/workspace/flipper/bin/build_contract.rs (1)
main(8-8)
templates/workspace/flapper/bin/build_schema.rs (2)
odra-macros/src/utils/attr.rs (1)
odra_module(11-13)odra-build/src/lib.rs (1)
schema(10-21)
templates/workspace/flipper/bin/build_contract.rs (5)
templates/blank/bin/build_contract.rs (1)
main(8-8)templates/cep18/bin/build_contract.rs (1)
main(8-8)templates/cep95/bin/build_contract.rs (1)
main(8-8)templates/full/bin/build_contract.rs (1)
main(8-8)templates/workspace/flapper/bin/build_contract.rs (1)
main(8-8)
templates/workspace/flipper/bin/build_schema.rs (2)
odra-macros/src/utils/attr.rs (1)
odra_module(11-13)odra-build/src/lib.rs (1)
schema(10-21)
templates/blank/bin/build_schema.rs (5)
odra-macros/src/utils/attr.rs (1)
odra_module(11-13)templates/cep18/bin/build_schema.rs (4)
module_schema(8-8)casper_contract_schema(9-9)main(13-17)main(20-20)modules/bin/build_schema.rs (2)
module_schema(7-7)casper_contract_schema(8-8)examples/bin/build_schema.rs (3)
module_schema(7-7)casper_contract_schema(8-8)main(12-16)odra-build/src/lib.rs (1)
schema(10-21)
templates/full/bin/build_schema.rs (4)
odra-macros/src/utils/attr.rs (1)
odra_module(11-13)templates/blank/bin/build_schema.rs (4)
module_schema(8-8)casper_contract_schema(9-9)main(13-17)main(20-20)modules/bin/build_schema.rs (2)
module_schema(7-7)casper_contract_schema(8-8)odra-build/src/lib.rs (1)
schema(10-21)
templates/cep95/bin/build_contract.rs (6)
templates/blank/bin/build_contract.rs (1)
main(8-8)templates/cep18/bin/build_contract.rs (1)
main(8-8)templates/cep95/bin/build_schema.rs (2)
main(13-17)main(20-20)templates/full/bin/build_contract.rs (1)
main(8-8)templates/workspace/flapper/bin/build_contract.rs (1)
main(8-8)templates/workspace/flipper/bin/build_contract.rs (1)
main(8-8)
templates/cep95/bin/build_schema.rs (2)
odra-macros/src/utils/attr.rs (1)
odra_module(11-13)templates/blank/bin/build_schema.rs (4)
module_schema(8-8)casper_contract_schema(9-9)main(13-17)main(20-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Evaluate benchmark
- GitHub Check: Test templates
- GitHub Check: Test
- GitHub Check: Calculate test coverage
🔇 Additional comments (13)
templates/workspace/flapper/bin/build_contract.rs (1)
3-8: Conditional entry point fix looks good.
no_mainnow only applies on wasm32 and the stubmain()keeps host builds happy, so this bin compiles cleanly across targets.templates/cep95/bin/build_contract.rs (1)
3-8: Template bin configuration is ready.
Target-specificno_mainplus the guarded emptymain()resolve the cross-target build failures for this template.templates/full/bin/build_contract.rs (1)
3-8: Full template binary looks good.
The selectiveno_mainand host-only stub entry point bring this bin in line with the fixed compilation flow.templates/blank/bin/build_contract.rs (1)
3-8: Blank template bin verified.
The wasm-onlyno_mainand fallbackmain()maintain compatibility across targets.templates/cep18/bin/build_contract.rs (1)
3-8: CEP18 contract bin ready.
The cfg-guarded entry points match the intended cross-target behavior.templates/workspace/flipper/bin/build_contract.rs (1)
3-8: Workspace flipper bin looks solid.
Applyingno_mainonly on wasm32 and adding the host stub entry point fixes the prior compilation gap.templates/cep95/bin/build_schema.rs (1)
2-20: CFG split resolves schema build edge cases.
Gating the externs and realmain()on non-emptyodra_moduleplus non-wasm32 avoids undefined symbol/linker issues, while the fallback noopmain()keeps alternative configs compiling.templates/workspace/flapper/bin/build_schema.rs (1)
2-20: Workspace schema builder configuration is on point.
The tightened cfg conditions plus fallback entry point ensure schema generation only runs where supported and still compiles under wasm or empty-module setups.templates/cep18/bin/build_schema.rs (2)
2-3: Lint allowances match other templates.Consistent with
templates/full/bin/build_schema.rs- these broad allowances are acceptable for template code.
6-20: Cfg conditions correctly implemented.The conditional compilation logic matches the pattern established in other template files and properly ensures a
main()function exists in all build configurations.templates/blank/bin/build_schema.rs (1)
2-20: Changes consistent with other templates.The lint allowances and cfg conditions match the pattern in
templates/full/bin/build_schema.rsandtemplates/cep18/bin/build_schema.rs. This consistency across templates is good for maintainability.templates/workspace/flipper/bin/build_schema.rs (1)
2-20: Workspace template correctly updated.The changes mirror those in other templates but use the concrete crate name
flipper(line 4) instead of a placeholder, which is appropriate for a workspace example template.templates/full/bin/build_schema.rs (1)
6-20: Cfg logic verified as correct and exhaustive.The verification confirms that
odra_modulebehavior is consistent when unset versus set to empty string—both result in identical cfg behavior sinceodra-build/src/lib.rsdefaults unsetODRA_MODULEto"". The conditions are mutually exclusive and exhaustive across all build scenarios (allbuild_schema.rsfiles across templates and workspace projects implement the same proven pattern).
![llamapreview[bot]](https://avatars.githubusercontent.com/in/1023541?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Code Review by LlamaPReview
🎯 TL;DR & Recommendation
Recommendation: Approve with suggestions
This PR fixes compilation errors in template binaries but introduces minor suggestions for improving maintainability and reducing potential confusion in build configurations.
🌟 Strengths
- Resolves compilation errors across multiple contract templates.
- Enhances build configurations for better cross-platform support.
| Priority | File | Category | Impact Summary | Anchors |
|---|---|---|---|---|
| P2 | templates/blank/bin/build_schema.rs | Architecture | Build fragility from conditional compilation dependency | |
| P2 | templates/blank/bin/build_schema.rs | Maintainability | Unsafe blocks risking undefined behavior | |
| P2 | templates/blank/bin/build_contract.rs | Architecture | Confusion from dual compilation targets | |
| P2 | templates/blank/bin/build_schema.rs | Maintainability | Lint suppressions hiding code smells |
🔍 Notable Themes
- Consistent use of conditional compilation across templates that could benefit from centralized configuration management.
- Opportunities to replace unsafe code with safer alternatives and improve lint handling.
💡 Have feedback? We'd love to hear it in our GitHub Discussions.
✨ This review was generated by LlamaPReview Advanced, which is free for all open-source projects. Learn more.
| #[cfg(all(not(odra_module = ""), not(target_arch = "wasm32")))] | ||
| fn main() { | ||
| odra_build::schema(unsafe { crate::module_schema() }, unsafe { | ||
| crate::casper_contract_schema() | ||
| }); | ||
| } |
There was a problem hiding this comment.
P2 | Confidence: High
The unsafe blocks for accessing module_schema() and casper_contract_schema() functions are unnecessary and introduce potential undefined behavior risks. Since these are extern "Rust" functions declared in the same compilation unit, they should be called directly without unsafe blocks. The current implementation suggests a misunderstanding of Rust's unsafe semantics.
| #[cfg(all(not(odra_module = ""), not(target_arch = "wasm32")))] | |
| fn main() { | |
| odra_build::schema(unsafe { crate::module_schema() }, unsafe { | |
| crate::casper_contract_schema() | |
| }); | |
| } | |
| #[cfg(all(not(odra_module = ""), not(target_arch = "wasm32")))] | |
| fn main() { | |
| odra_build::schema(module_schema(), casper_contract_schema()); | |
| } |
| #![doc = "Binary for building wasm files from odra contracts."] | ||
| #![no_std] | ||
| #![no_main] | ||
| #![cfg_attr(target_arch = "wasm32", no_main)] |
There was a problem hiding this comment.
P2 | Confidence: High
The dual compilation target approach (WASM vs native) is architecturally sound but creates potential confusion. The empty main() function for native targets serves as a placeholder but provides no meaningful functionality or documentation about its purpose. This could lead to maintenance confusion about why these binaries exist for native compilation.
| #![cfg_attr(target_arch = "wasm32", no_main)] | |
| #[cfg(not(target_arch = "wasm32"))] | |
| fn main() { | |
| // Placeholder for native compilation - actual contract logic is WASM-only | |
| } |
| #![allow(unused_imports, redundant_imports)] | ||
| #![allow(clippy::single_component_path_imports)] |
There was a problem hiding this comment.
P2 | Confidence: High
The blanket allowance of lints (redundant_imports, single_component_path_imports) suppresses legitimate warnings that could help maintain code quality. While this may reduce build noise, it hides potential code smells and reduces the compiler's ability to catch actual issues during development.
| #[cfg(all(not(odra_module = ""), not(target_arch = "wasm32")))] | ||
| extern "Rust" { | ||
| fn module_schema() -> odra::contract_def::ContractBlueprint; | ||
| fn casper_contract_schema() -> odra::schema::casper_contract_schema::ContractSchema; | ||
| } |
There was a problem hiding this comment.
P2 | Confidence: Medium
Speculative: The conditional compilation using odra_module environment variable creates potential build-time fragility. This introduces a dependency on external build configuration that may not be consistently set across different development environments or CI/CD pipelines. The condition not(odra_module = "") relies on specific build tooling to define this variable, which could lead to inconsistent behavior between local development and production builds.
Summary by CodeRabbit
Chores
Refactor