Load app metadata from companion app_<target>.json file for Rust binaries#61
Load app metadata from companion app_<target>.json file for Rust binaries#61fbeutin-ledger wants to merge 1 commit intomasterfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #61 +/- ##
==========================================
- Coverage 94.74% 94.22% -0.53%
==========================================
Files 14 14
Lines 571 623 +52
==========================================
+ Hits 541 587 +46
- Misses 30 36 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
9bcbd9f to
ee7b771
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds support for loading application metadata from companion JSON files for Rust-based Ledger binaries. Rust applications don't embed app_name, app_version, and app_flags directly in the ELF binary; instead, these values are stored in a separate app_<target>.json file located in the same directory as the binary.
Key Changes:
- Added automatic detection of Rust applications via
rust_sdk_nameorrust_sdk_versionELF sections - Implemented JSON metadata loading for Rust apps with comprehensive error handling for missing files, malformed JSON, and partial data
- Added
app_flagsfield to theSectionsdataclass to support this metadata
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/ledgered/binary.py | Adds is_rust_app property, _load_metadata_from_json() method, and app_flags field to enable JSON-based metadata loading for Rust binaries |
| tests/unit/test_binary.py | Comprehensive test coverage including Rust app detection, JSON loading success/failure cases, and verification that C apps ignore JSON files |
| CHANGELOG.md | Documents the new feature in version 0.15.0 release notes |
Comments suppressed due to low confidence (1)
src/ledgered/binary.py:18
- The field
app_flagsis being added here, but according to the full file content, there appears to be anotherapp_flagsfield definition at line 31 in theSectionsdataclass. Having duplicate field definitions in a dataclass will cause a syntax error. Please verify that there are no duplicate field definitions in theSectionsclass.
app_name: Optional[str] = None
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
2cbada2 to
195a93a
Compare
195a93a to
abfa2c0
Compare
| def _load_metadata_from_json(self) -> None: | ||
| """Load app metadata from companion app_<target>.json file. | ||
|
|
||
| Rust applications don't embed app_name, app_version, and app_flags in the ELF. |
There was a problem hiding this comment.
Not true anymore with this PR.
When it will be merged, this PR is no more needed?
PS. Also, all these data can be retrieved from Cargo.toml.
|
Not needed anymore, merged on SDK side |
No description provided.