-
Notifications
You must be signed in to change notification settings - Fork 4
Remove common
#32
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
Remove common
#32
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 PR removes the common crate by moving its functionality to other crates in the workspace. The DxeReadinessCaptureSerDe struct has been moved to dxe_readiness_capture, while the serializable_fv module has been moved to the patina library (as indicated by the dependency on patina PR #1116).
Key Changes
- Removed the
commoncrate entirely, including its Cargo.toml and source files - Moved
DxeReadinessCaptureSerDestruct todxe_readiness_capture/src/lib.rs - Updated all imports to reference
patina::pi::serializable::serializable_fvinstead ofcommon::serializable_fv - Updated workspace dependencies to include
serdefeature for patina - Removed
commonfrom workspace members and build/test tasks
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
dxe_readiness_validator/src/validation_kind.rs |
Updated imports to use patina::pi::serializable::serializable_fv instead of common::serializable_fv |
dxe_readiness_validator/src/validate/fv.rs |
Updated imports and removed test-specific import of common types |
dxe_readiness_validator/src/validate.rs |
Changed import from common::DxeReadinessCaptureSerDe to dxe_readiness_capture::DxeReadinessCaptureSerDe |
dxe_readiness_validator/Cargo.toml |
Replaced common dependency with dxe_readiness_capture dependency |
dxe_readiness_capture/src/lib.rs |
Added DxeReadinessCaptureSerDe struct definition (moved from common); reorganized extern crate alloc placement |
dxe_readiness_capture/src/capture/fv.rs |
Updated import to use patina::pi::serializable::serializable_fv |
dxe_readiness_capture/src/capture.rs |
Updated imports to use new paths for types and added format to alloc imports |
dxe_readiness_capture/Cargo.toml |
Removed common dependency |
common/src/serializable_fv.rs |
Deleted entire file (163 lines) |
common/src/lib.rs |
Deleted entire file (27 lines) |
common/Cargo.toml |
Deleted entire file |
README.md |
Updated documentation to reflect two main crates instead of three, removed common crate section |
Makefile.toml |
Removed test-common task and updated test task dependencies |
Cargo.toml |
Removed common from workspace members, added serde feature to patina workspace dependency |
Cargo.lock |
Updated to reflect patina version 16.0.1 (local path dependency) and dependency changes |
Comments suppressed due to low confidence (1)
dxe_readiness_validator/src/validate/fv.rs:184
- Missing import for
PeHeaderInfoin test module. The removed import includedPeHeaderInfowhich is still used in test code (e.g., line 429). This type needs to be imported frompatina::pi::serializable::serializable_fv::PeHeaderInfoor added to the parent module's imports so it's available viause super::*.
use super::*;
use goblin::pe::{
header::COFF_MACHINE_X86_64,
subsystem::{IMAGE_SUBSYSTEM_EFI_BOOT_SERVICE_DRIVER, IMAGE_SUBSYSTEM_EFI_RUNTIME_DRIVER},
};
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
Copilot reviewed 14 out of 15 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
dxe_readiness_validator/src/validate/fv.rs:184
- Missing import for types used in tests. The removal of the import statement for
FirmwareFileSerDe,FirmwareSectionSerDe,FirmwareVolumeSerDe, andPeHeaderInfofromcommon::serializable_fvwill cause compilation errors in the test module. These types are used in tests (e.g.,PeHeaderInfoat line 429,FirmwareSectionSerDeat line 425,FirmwareFileSerDeat line 420), but are no longer imported after the deletion.
Add the necessary imports:
use patina::pi::serializable::serializable_fv::{FirmwareFileSerDe, FirmwareSectionSerDe, FirmwareVolumeSerDe, PeHeaderInfo};#[cfg(test)]
mod tests {
use super::*;
use goblin::pe::{
header::COFF_MACHINE_X86_64,
subsystem::{IMAGE_SUBSYSTEM_EFI_BOOT_SERVICE_DRIVER, IMAGE_SUBSYSTEM_EFI_RUNTIME_DRIVER},
};
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
## Description As per OpenDevicePartnership/patina-readiness-tool#26, move the `common` crate serializable structs to patina for consistency with `serializable_hob`. - [ ] Impacts functionality? - [ ] Impacts security? - [ ] Breaking change? - [ ] Includes tests? - [ ] Includes documentation? ## How This Was Tested Build and run readiness tool on Q35. ## Integration Instructions Depends on OpenDevicePartnership/patina-readiness-tool#32. --------- Co-authored-by: Sherry Fan <sherryfan@microsoft.com> Co-authored-by: Michael Kubacki <michael.kubacki@microsoft.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
176371e
into
OpenDevicePartnership:main
Description
As per #26, remove the common crate.
serializable_fvhas been moved topatina_pi.How This Was Tested
Build and run readiness tool on Q35.
Integration Instructions
Depends on OpenDevicePartnership/patina#1116.