Skip to content

patina_boot: Build on the stable toolchain - #213

Open
kat-perez wants to merge 3 commits into
OpenDevicePartnership:mainfrom
kat-perez:kat-perez/patina-boot-stable-toolchain
Open

patina_boot: Build on the stable toolchain#213
kat-perez wants to merge 3 commits into
OpenDevicePartnership:mainfrom
kat-perez:kat-perez/patina-boot-stable-toolchain

Conversation

@kat-perez

Copy link
Copy Markdown
Contributor

Summary

patina_boot pinned a nightly toolchain for two features. Neither is necessary, and the
pin actively prevented the crate from being used for its intended purpose.

  • never_type — the Result<!, EfiError> signatures become
    Result<core::convert::Infallible, EfiError>. This carries the same meaning here: every
    implementation and every caller either returns an error or diverges, so nothing ever
    constructs the success value.
  • coverage_attribute — now gated behind cfg(coverage)
    (#![cfg_attr(coverage, feature(coverage_attribute))] /
    #[cfg_attr(coverage, coverage(off))]), matching every upstream Patina crate. Coverage
    runs still opt in by adding the feature through RUSTFLAGS, so the #[coverage(off)]
    annotations keep working where they matter.

One unstable feature remains, and it is not ours: the patina SDK enables allocator_api
whenever alloc or test is active. Patina's own answer to that is to unlock it on a
pinned stable channel via RUSTC_BOOTSTRAP, restricted to the features Patina has
accepted through its unstable feature process. This PR adopts that same configuration, so
patina_boot is now built the same way as the rest of the ecosystem.

Why this matters

This is not a cleanup. Patina DXE core repositories set
-Z allow-features=c_variadic,allocator_api, which is an allowlist — it restricts what
RUSTC_BOOTSTRAP unlocks. never_type and coverage_attribute are not on it, so adding
patina_boot as a dependency of a DXE core failed to compile outright:

error[E0725]: the feature `coverage_attribute` is not in the list of allowed features
error[E0725]: the feature `never_type` is not in the list of allowed features

In other words, patina_boot could not be consumed by the thing it exists to run inside of.
After this change, a local prototype DXE core with BootDispatcher registered as a
component builds cleanly against the crate.

Changes

File Change
src/lib.rs Drop feature(never_type); gate coverage_attribute on cfg(coverage)
src/boot_orchestrator.rs, src/boot_dispatcher.rs, src/orchestrators/simple_boot_manager.rs Result<!, _>Result<Infallible, _>
src/boot_dispatcher.rs, src/helpers.rs, src/orchestrators/simple_boot_manager.rs #[coverage(off)]#[cfg_attr(coverage, coverage(off))]
.cargo/config.toml New. RUSTC_BOOTSTRAP + accepted-feature allowlist, mirroring upstream Patina
Cargo.toml Declare cfg(coverage) via unexpected_cfgs check-cfg
rust-toolchain.toml nightly-2026-02-271.95.0
.github/workflows/uefi-check.yml Same channel change for the patina_boot job, and correct the header comment

patina_tianocore still pins nightly-2025-12-12 and is untouched here.

Validation

Ran CI's exact commands on stable 1.95.0, all passing with zero warnings:

  • cargo fmt --check
  • cargo clippy --all-features -- -D warnings (host, x86_64-unknown-uefi, aarch64-unknown-uefi)
  • cargo doc --no-deps with RUSTDOCFLAGS=-D warnings
  • cargo build for both UEFI targets
  • cargo test — 94 unit tests, 6 doctests
  • cargo build --benches

Behaviour is unchanged; this is a compile-time and toolchain change only.

The crate pinned nightly for two features. `never_type` is replaced by
`core::convert::Infallible`, which carries the same meaning for these
signatures since every implementation only returns an error or diverges.
`coverage_attribute` is now gated behind `cfg(coverage)`, matching every
upstream Patina crate, so coverage runs still opt in via RUSTFLAGS.

The remaining unstable feature comes from the patina SDK itself, which
enables `allocator_api`. Adopt Patina's own approach for that: unlock it
on a stable channel through RUSTC_BOOTSTRAP with an explicit
allow-features list.

Without this, patina_boot cannot be consumed by a Patina DXE core. Those
repositories restrict allow-features to Patina's accepted set, so the
crate's feature gates were rejected outright.

Assisted-by: GitHub Copilot:claude-opus-5
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI 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.

🟢 Approval recommended

The changes are primarily toolchain/configuration and type/attribute substitutions with a single minor documentation consistency nit noted.

Pull request overview

This PR updates uefi/crates/patina_boot to build on a pinned stable Rust toolchain by removing the crate-owned nightly feature requirements, gating coverage-only attributes behind cfg(coverage), and adopting Patina’s stable RUSTC_BOOTSTRAP + -Z allow-features configuration so the crate can be consumed by Patina DXE cores.

Changes:

  • Switch patina_boot from nightly to stable 1.95.0 (crate toolchain pin + CI job toolchain).
  • Replace Result<!, EfiError> with Result<core::convert::Infallible, EfiError> and gate #[coverage(off)] behind cfg(coverage).
  • Add per-crate Cargo configuration to enable RUSTC_BOOTSTRAP with an accepted-feature allowlist; declare cfg(coverage) to avoid unexpected_cfgs warnings.
File summaries
File Description
uefi/crates/patina_boot/src/orchestrators/simple_boot_manager.rs Switches ! to Infallible and gates coverage attributes.
uefi/crates/patina_boot/src/lib.rs Drops nightly-only feature requirements; gates coverage_attribute behind cfg(coverage).
uefi/crates/patina_boot/src/helpers.rs Gates #[coverage(off)] behind cfg(coverage).
uefi/crates/patina_boot/src/boot_orchestrator.rs Updates trait signature to Infallible (doc wording needs a small follow-up).
uefi/crates/patina_boot/src/boot_dispatcher.rs Gates coverage attributes; updates test trait impl return type to Infallible.
uefi/crates/patina_boot/rust-toolchain.toml Pins stable toolchain 1.95.0.
uefi/crates/patina_boot/Cargo.toml Declares cfg(coverage) for unexpected_cfgs lint checking.
uefi/crates/patina_boot/.cargo/config.toml Adds RUSTC_BOOTSTRAP + -Z allow-features configuration for stable builds.
.github/workflows/uefi-check.yml Updates CI toolchain for patina_boot job to stable and refreshes header comment.
Review details
  • Files reviewed: 9/9 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread uefi/crates/patina_boot/src/boot_orchestrator.rs
The trait doc still called the Ok variant `!` after the signature moved to
Infallible. Also corrects "uninhabitable" to describe the type plainly.

Assisted-by: GitHub Copilot:claude-opus-5

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 4, 2026 19:26

Copilot AI 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.

🟢 Approval recommended

The changes are narrowly scoped to toolchain/configuration and mechanical type/attribute gating updates, with no functional behavior changes introduced.

Review details
  • Files reviewed: 9/9 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 4, 2026 19:29

Copilot AI 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.

🟢 Approval recommended

The changes are cohesive and self-consistent (API/doc updates, coverage gating, toolchain/CI alignment) with no verified correctness issues found in the modified regions.

Review details
  • Files reviewed: 9/9 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants