Skip to content

Add unstable feature and improve CI for feature flags #590

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

Merged
merged 3 commits into from
Nov 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -159,3 +159,13 @@ jobs:

- name: Build
run: cargo xtask build

build_feature_permutations:
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately, this will increase our CI times :/
But for what it's worth, this is a good thing to have, I think

Copy link
Member Author

Choose a reason for hiding this comment

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

It might not increase the time actually since the jobs are run in parallel. The times vary from one run to another, but for example in https://github.com/rust-osdev/uefi-rs/actions/runs/3557618653/ the new job took 3m26s, but the aarch64 job took 3m39s, the lint job took 3m40s, and the Windows job took 3m50s. So CI times seem likely to remain about the same as before.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, then forget what I've said. Probably, it is not a big problem then :)

name: Check that the build works for all feature combinations
runs-on: ubuntu-latest
steps:
- name: Checkout sources
uses: actions/checkout@v3

- name: Build
run: cargo xtask build --feature-permutations
1 change: 1 addition & 0 deletions uefi/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ logger = []
# were observed on the VirtualBox UEFI implementation (see uefi-rs#121).
# In those cases, this feature can be excluded by removing the default features.
panic-on-logger-errors = []
unstable = []

[dependencies]
bitflags = "1.3.1"
Expand Down
9 changes: 8 additions & 1 deletion uefi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@
//! is not a high-performance logger.
//! - `panic-on-logger-errors` (enabled by default): Panic if a text
//! output error occurs in the logger.
//! - `unstable`: Enable functionality that depends on [unstable
//! features] in the nightly compiler. Note that currently the `uefi`
//! crate _always_ requires unstable features even if the `unstable`
//! feature is not enabled, but once a couple more required features
Copy link
Member

Choose a reason for hiding this comment

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

nit: there is a small risk that we may not update this comment here when uefi works on stable eventually.

//! are stabilized we intend to make the `uefi` crate work on the
//! stable channel by default.
//!
//! The `global_allocator` and `logger` features require special
//! handling to perform initialization and tear-down. The
Expand All @@ -48,13 +54,14 @@
//!
//! [`GlobalAlloc`]: alloc::alloc::GlobalAlloc
//! [`uefi-services`]: https://crates.io/crates/uefi-services
//! [unstable features]: https://doc.rust-lang.org/unstable-book/

#![feature(abi_efiapi)]
#![feature(maybe_uninit_slice)]
#![feature(negative_impls)]
#![feature(ptr_metadata)]
#![feature(error_in_core)]
#![cfg_attr(feature = "alloc", feature(vec_into_raw_parts))]
#![cfg_attr(feature = "unstable", feature(error_in_core))]
#![cfg_attr(docsrs, feature(doc_auto_cfg))]
#![no_std]
// Enable some additional warnings and lints.
Expand Down
1 change: 1 addition & 0 deletions uefi/src/result/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,5 @@ impl<Data: Debug + Display> Display for Error<Data> {
}
}

#[cfg(feature = "unstable")]
impl<Data: Debug + Display> core::error::Error for Error<Data> {}
1 change: 1 addition & 0 deletions xtask/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ clap = { version = "4.0.4", features = ["derive"] }
fatfs = { git = "https://github.com/rafalh/rust-fatfs.git", rev = "87fc1ed5074a32b4e0344fcdde77359ef9e75432" }
fs-err = "2.6.0"
heck = "0.4.0"
itertools = "0.10.5"
# Use git as a temporary workaround for https://github.com/rust-osdev/uefi-rs/issues/573.
mbrman = { git = "https://github.com/cecton/mbrman.git", rev = "78565860e55c272488d94480e72a4e2cd159ad8e" }
nix = "0.25.0"
Expand Down
35 changes: 33 additions & 2 deletions xtask/src/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,24 +45,55 @@ impl Package {

#[derive(Clone, Copy, Debug)]
pub enum Feature {
GlobalAllocator,
// `uefi` features.
Alloc,
GlobalAllocator,
Logger,
PanicOnLoggerErrors,
Unstable,

// `uefi-services` features.
PanicHandler,
Qemu,
ServicesLogger,

// `uefi-test-runner` features.
Ci,
}

impl Feature {
fn as_str(&self) -> &'static str {
match self {
Self::GlobalAllocator => "global_allocator",
Self::Alloc => "alloc",
Self::GlobalAllocator => "global_allocator",
Self::Logger => "logger",
Self::PanicOnLoggerErrors => "panic-on-logger-errors",
Self::Unstable => "unstable",

Self::PanicHandler => "uefi-services/panic_handler",
Self::Qemu => "uefi-services/qemu",
Self::ServicesLogger => "uefi-services/logger",

Self::Ci => "uefi-test-runner/ci",
}
}

/// Get the features for the given package.
pub fn package_features(package: Package) -> Vec<Self> {
match package {
Package::Uefi => vec![
Self::Alloc,
Self::GlobalAllocator,
Self::Logger,
Self::PanicOnLoggerErrors,
Self::Unstable,
],
Package::UefiServices => vec![Self::PanicHandler, Self::Qemu, Self::ServicesLogger],
Package::UefiTestRunner => vec![Self::Ci],
_ => vec![],
}
}

/// Set of features that enables more code in the root uefi crate.
pub fn more_code() -> Vec<Self> {
vec![Self::GlobalAllocator, Self::Alloc, Self::Logger]
Expand Down
27 changes: 27 additions & 0 deletions xtask/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,39 @@ mod util;
use anyhow::Result;
use cargo::{fix_nested_cargo_env, Cargo, CargoAction, Feature, Package, TargetTypes};
use clap::Parser;
use itertools::Itertools;
use opt::{Action, BuildOpt, ClippyOpt, DocOpt, Opt, QemuOpt};
use std::process::Command;
use tempfile::TempDir;
use util::{command_to_string, run_cmd};

fn build_feature_permutations(opt: &BuildOpt) -> Result<()> {
for package in [Package::Uefi, Package::UefiServices] {
let all_package_features = Feature::package_features(package);
for features in all_package_features.iter().powerset() {
let features = features.iter().map(|f| **f).collect();

let cargo = Cargo {
action: CargoAction::Build,
features,
packages: vec![package],
release: opt.build_mode.release,
target: Some(*opt.target),
warnings_as_errors: true,
target_types: TargetTypes::BinsExamplesLib,
};
run_cmd(cargo.command()?)?;
}
}

Ok(())
}

fn build(opt: &BuildOpt) -> Result<()> {
if opt.feature_permutations {
return build_feature_permutations(opt);
}

let cargo = Cargo {
action: CargoAction::Build,
features: Feature::more_code(),
Expand Down
5 changes: 5 additions & 0 deletions xtask/src/opt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ pub struct BuildOpt {

#[clap(flatten)]
pub build_mode: BuildModeOpt,

/// Build multiple times to check that different feature
/// combinations work.
#[clap(long, action)]
pub feature_permutations: bool,
}

/// Run clippy on all the packages.
Expand Down