-
Notifications
You must be signed in to change notification settings - Fork 246
Internal evercbor #7524
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: main
Are you sure you want to change the base?
Internal evercbor #7524
Conversation
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.
Pull request overview
This pull request reorganizes the EverCBOR library by moving it from 3rdparty/exported to 3rdparty/internal, reflecting a change in how this dependency is managed within the project.
Key Changes:
- Updated CMake configuration to reference the new internal location
- Added EverCBOR header files with CBOR type definitions and API declarations
- Included KaRaMeL runtime library header for supporting the EverCBOR implementation
Reviewed changes
Copilot reviewed 1 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| cmake/evercbor.cmake | Updated paths and variable references to point to the internal directory instead of exported |
| 3rdparty/internal/evercbor/krmllib.h | Added KaRaMeL runtime library header with macro definitions and utility functions |
| 3rdparty/internal/evercbor/internal/CBORNondet.h | Added internal CBOR header with function declarations for serialization and comparison |
| 3rdparty/internal/evercbor/CBORNondetType.h | Added CBOR type definitions including structures for integers, strings, arrays, maps, and iterators |
| 3rdparty/internal/evercbor/CBORNondet.h | Added public CBOR API header with function declarations for parsing, serialization, and data access |
|
|
||
| target_include_directories( | ||
| evercbor PUBLIC $<BUILD_INTERFACE:${CCF_3RD_PARTY_EXPORTED_DIR}/evercbor> | ||
| evercbor PUBLIC $<BUILD_INTERFACE:${CCF_3RD_PARTY_INTERNAL_DIR}/evercbor> |
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.
I don't think this is right, I think the target_include_directories should be PRIVATE instead, and probably not refer to the install interface, since the point of moving from external to internal is to not export these headers.
For this to work, we need to make sure that none of the headers we export in include/ccf include headers from evercbor. I do not know if that is (already) the case.
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 PR simply moves it from exported to internal to avoid them being packaged
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.
but we still export the include directory as a public dependency, so that seems like a breaking change
No description provided.