Skip to content

workspace: uefi (main library) is in a dedicated directory now #566

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 2 commits into from
Nov 17, 2022

Conversation

phip1611
Copy link
Member

This implements the ideas discussed in #564.

Checklist

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

@phip1611 phip1611 self-assigned this Nov 13, 2022
@phip1611 phip1611 linked an issue Nov 13, 2022 that may be closed by this pull request
@phip1611 phip1611 force-pushed the workspace-restructure branch from 78609ac to 21209e5 Compare November 13, 2022 10:40
@phip1611 phip1611 force-pushed the workspace-restructure branch 2 times, most recently from dbaf01c to 7c44f0d Compare November 13, 2022 14:00
@phip1611
Copy link
Member Author

phip1611 commented Nov 14, 2022

@phip1611 phip1611 force-pushed the workspace-restructure branch from 7c44f0d to a38454f Compare November 14, 2022 08:58
@GabrielMajeri
Copy link
Collaborator

I do not understand why rust-osdev/uefi-rs/actions/runs/3455556833/jobs/5767663192 fails.

As far as I can tell, the issue is in a doctest we have in the uefi-macros crate. It's meant to check that the #[entry] macro works as advertised, but for some reason, Rust is complaining about the lack of a global allocator in the test binary which gets generated (pretty irrelevant for us, since we don't want to actually run that code).

I'm not sure why it's showing up right now (changes in the latest nightly?), but I've found that it compiles successfully if we remove the #![no_std] attribute and the panic handler.

/// ```no_run
/// #![no_main]
/// #![feature(abi_efiapi)]
/// # // A bit of boilerplate needed to make the example compile in the
/// # // context of `cargo test`.
///
/// use uefi::prelude::*;
///
/// #[entry]
/// fn main(image: Handle, st: SystemTable<Boot>) -> Status {
///     Status::SUCCESS
/// }
/// ```

@nicholasbishop
Copy link
Member

nicholasbishop commented Nov 15, 2022

I do not understand why https://github.com/rust-osdev/uefi-rs/actions/runs/3455556833/jobs/5767663192 fails. I'm seeking your advice @nicholasbishop @GabrielMajeri

Add resolver = 2 to the [workspace] section as described in https://doc.rust-lang.org/cargo/reference/resolver.html#resolver-versions. That should resolve the error since the alloc feature won't be enabled in uefi for that test.

@phip1611
Copy link
Member Author

I do not understand why it builds in the CI but no longer locally..

   Checking uefi-services v0.15.0 (/home/pschuster/dev/other/uefi-rs/uefi-services)
    Checking uefi-test-runner v0.2.0 (/home/pschuster/dev/other/uefi-rs/uefi-test-runner)
    Checking uefi_app v0.1.0 (/home/pschuster/dev/other/uefi-rs/template)
error: language item required, but not found: `eh_personality`
  |
  = note: this can occur when a binary crate with `#![no_std]` is compiled for a target where `eh_personality` is defined in the standard library
  = help: you may be able to compile for a target that doesn't need `eh_personality`, specify a target with `--target` or in `.cargo/config`

error: could not compile `uefi_app` due to previous error
warning: build failed, waiting for other jobs to finish...
error: could not compile `uefi-test-runner` due to previous error

Any idea? cargo clean also doesn't help. I tested multiple nightly versions..

@nicholasbishop
Copy link
Member

Are you running cargo check? That's not something the CI runs. Since there's no longer a top-level package it's now checking the whole workspace instead of of just the uefi package. This was already broken before your change, you just would have had to run cargo check --workspace to see the error. I think it's fine for it to remain broken for this PR, we could look at fixing that command separately.

@nicholasbishop nicholasbishop merged commit c901eda into rust-osdev:main Nov 17, 2022
@phip1611 phip1611 deleted the workspace-restructure branch November 17, 2022 21:02
@phip1611
Copy link
Member Author

Very cool! I'm glad we could merge this.

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.

Better organize Workspace (Directory Structure)
3 participants