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

Conversation

nicholasbishop
Copy link
Member

@nicholasbishop nicholasbishop commented Nov 27, 2022

Note: the first commit in this PR is from #589 so that we don't have to resolve merge conflicts later.

This PR adds an unstable feature to the uefi crate. It will be used to gate new unstable features as we work towards being able to compile on the stable channel. Re naming, both nightly and unstable are used frequently by other crates. nightly is slightly more common, but I think unstable is more accurate since unstable features can be enabled on non-nightly builds of the compiler, and the documentation for these features is the Unstable Book, as opposed to the "Nightly Book".

Also in this PR is a new build mode: cargo xtask build --feature-permutations. This builds both uefi and uefi-services a bunch of times with different feature flags set, to make sure that compilation still works in all cases. There's a new CI job to run this command as well.

Checklist

  • Sensible git history (for example, squash "typo" or "fix" commits). See the Rewriting History guide for help.
  • Update the changelog (if necessary)

//! - `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.

The unstable feature will be used to gate the use of new unstable
features. We hope to make the uefi crate work on the stable channel once
the `abi_efiapi` and `default_alloc_error_handler` features are
stabilized, so we don't want to start relying unconditionally on new
unstable features.

For now the new feature just gates `error_in_core`, but some additional
features will be gated here soon.
The `--feature-permutations` flag runs a bunch of builds of just the
`uefi` and `uefi-services` crates to test that the build works no matter
what features are selected.
@@ -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 :)

@phip1611 phip1611 force-pushed the bishop-add-unstable-feat branch from 527cd72 to 87df1a5 Compare November 27, 2022 09:29
@phip1611 phip1611 merged commit 01b27af into rust-osdev:main Nov 27, 2022
@nicholasbishop nicholasbishop deleted the bishop-add-unstable-feat branch November 27, 2022 18:46
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.

2 participants