Skip to content

[FCP] RFC: Consolidate common core code into a single internal library crate#1594

Merged
cfernald merged 3 commits into
OpenDevicePartnership:mainfrom
cfernald:rfc_common_lib
Jul 7, 2026
Merged

[FCP] RFC: Consolidate common core code into a single internal library crate#1594
cfernald merged 3 commits into
OpenDevicePartnership:mainfrom
cfernald:rfc_common_lib

Conversation

@cfernald

@cfernald cfernald commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Description

This RFC proposes creating an internal crate (core/patina_internal_lib) for code shared by core implementations. This single crate would remove overhead and simplify changing internal code and interfaces. This will become increasingly important with the introduction of the Standalone MM core.

This RFC builds on the crate organization and naming conventions established by RFC: Categorizing and Renaming Patina Crates for Consistency.

  • Impacts functionality?
  • Impacts security?
  • Breaking change?
  • Includes tests?
  • Includes documentation?

How This Was Tested

N/A

Integration Instructions

N/A

This RFC proposes creating an internal crate (`core/patina_internal_lib`) for
code shared by core implementations. This single crate would remove overhead and
simplify changing internal code and interfaces. This will become increasingly
important with the introduction of the Standalone MM core.

This RFC builds on the crate organization and naming conventions established by
[RFC: Categorizing and Renaming Patina Crates for Consistency](./0007-patina-crates-naming-and-categorization.md).
@cfernald cfernald added the rfc A request-for-change proposal label Jun 27, 2026
@github-actions github-actions Bot added impact:non-functional Does not have a functional impact type:documentation Improvements or additions to documentation labels Jun 27, 2026
@patina-automation

patina-automation Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

✅ QEMU Validation Passed

All QEMU validation jobs completed successfully.

Note: Q35 is only built on Windows hosts (QEMU boot is disabled due to a QEMU vfat issue).

Workflow run: https://github.com/OpenDevicePartnership/patina/actions/runs/28843110485

Boot Time to EFI Shell

Platform Elapsed
ArmVirt (Linux Host) 21.0s
Q35 (Linux Host) 29.0s

Dependencies

Repository Ref
patina e1d7950
patina-dxe-core-qemu 90708a7
patina-fw-patcher 5b82a50
patina-qemu firmware v4.0.1
patina-qemu build script 1885d34

This comment was automatically generated by the Patina QEMU PR Validation Post workflow.

@codecov

codecov Bot commented Jun 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@joschock

Copy link
Copy Markdown
Contributor

Agree with the general motivation of the RFC, but one of the original motivations for several smaller crates was to help clarify/enforce separation of concerns. With everything in a large monocrate, it becomes cheaper to cross-couple things. Are there any organizational or technical mechanisms that we could retain to help with that motivation?

@os-d os-d left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t have strong feelings on this one. A small nit, naming is hard, but patina_internal_lib feels a bit too generic, maybe patina_internal_core to indicate its use?

Comment thread docs/src/rfc/text/0000-shared-core-library.md
@cfernald

cfernald commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Agree with the general motivation of the RFC, but one of the original motivations for several smaller crates was to help clarify/enforce separation of concerns. With everything in a large monocrate, it becomes cheaper to cross-couple things. Are there any organizational or technical mechanisms that we could retain to help with that motivation?

@joschock, this is true. I think this will be similar to the SDK where we perhaps need to define some rules and enforce them as practice instead of technologically. That is, on us as the maintainers and reviews to prevent bad code design from making it in.

I was thinking about adding a "Here are the rules for this crate", but I think until the crate matures it's probably difficult to say what they should be exactly. Do you have any ideas what types of rules you'd like to see?

@makubacki

Copy link
Copy Markdown
Collaborator

I don’t have strong feelings on this one. A small nit, naming is hard, but patina_internal_lib feels a bit too generic, maybe patina_internal_core to indicate its use?

Yeah, patina_internal_lib also struck me as too generic when I first saw it. patina_internal_core seems reasonable.

@makubacki makubacki left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense to step back and reduce our crate footprint for content we do not intend to be consumed outside a small set of core code.

It's less Patina-related crate noise in the registry and scales more sustainably as noted in the RFC. To my understanding, our sharing boundary at the crate level is largely "core consumption only" and the content separation, documentation, and access details can be handled within the module system. Also, as noted we can always split off crates meaningfully when we find justification to do so.

@cfernald

Copy link
Copy Markdown
Contributor Author

Added draft PR: #1597

@cfernald cfernald changed the title RFC: Consolidate common core code into a single internal library crate [FCP] RFC: Consolidate common core code into a single internal library crate Jul 2, 2026
@cfernald

cfernald commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

It seems we have reached agreement. Moving to FCP.

@cfernald
cfernald enabled auto-merge (squash) July 7, 2026 04:54
@cfernald
cfernald merged commit 2df25af into OpenDevicePartnership:main Jul 7, 2026
11 checks passed
cfernald added a commit that referenced this pull request Jul 7, 2026
Merges existing `patina_internal_depex` and
`patina_internal_collections` into a new more common
`patina_internal_core` to reduce overhead and process for adding new
shared code for the Patina DXE, MM, and supervisor cores.

Implementation for RFC:
#1594
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

impact:non-functional Does not have a functional impact rfc A request-for-change proposal type:documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants