Skip to content

Commit

Permalink
chore(x86_64): change build script to invoke cargo
Browse files Browse the repository at this point in the history
The `mnemos-x86_64-bootloader` image is built by a separate
crate `mnemos-x86_64-bootloader`, which includes a build script that
runs the post-build workflow to take the `bootloader_api`-compatible
kernel binary and turn it into a bootable disk image that links with the
actual bootloader. At present, we use a Cargo artifact dependency to
build the actual kernel and expose it to the bootloader image build
script. This is the ideal solution for this, since it allows the
dependency between the build script and the kernel binary to be
expressed as a normal Cargo dependency.

However, using an artifact dep on a binary that's built for a different
target than the crate with the artifact dep is sadly broken: see
rust-lang/cargo#12358. We've somehow managed to find a way to avoid this
issue in the past, but updating to a recent nightly seems to have
brought it back (see #322). Therefore, this PR changes the build script
to instead shell out to a new `cargo` invocation that builds the kernel.
This is a bit unfortunate, since it means that the build output from
building the actual kernel isn't exposed to the user as nicely, and
there's probably less build caching. However, it works for now.

I've modified the `just build-x86` Just recipe to invoke `cargo check`
for the actual x86_64 kernel binary before building the bootable image,
to help ensure that the cargo build output is made visible to the user.

A nicer long-term solution would be to switch from a build script to
using a cargo `xtask`-like builder, that's invoked as a cargo `runner`
for that target, with the path to the builder binary passed in on the
CLI. This is what [mycelium does][1]. That approach is nicer because it
allows the kernel to be built using a normal cargo invocation that's
actually visible to the user, instead of secretly in a build script.

[1]: https://github.com/hawkw/mycelium/blob/e51eb8aa98e7609490fa674f408db32fd51caa70/.cargo/config.toml#L1-L2
  • Loading branch information
hawkw committed Aug 11, 2024
1 parent a6ba1c3 commit 96dc2e5
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 17 deletions.
14 changes: 13 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 7 additions & 1 deletion justfile
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ _d1_pkg := "mnemos-d1"

_espbuddy_pkg := "mnemos-esp32c3-buddy"

_x86_pkg := "mnemos-x86_64-bootloader"
_x86_pkg := "mnemos-x86_64-core"
_x86_bootloader_pkg := "mnemos-x86_64-bootloader"

_mn_pkg := "manganese"
Expand Down Expand Up @@ -169,6 +169,12 @@ flash-c3 board *espflash-args: (_get-cargo-command "espflash" "cargo-espflash")
# build a bootable x86_64 disk image, using rust-osdev/bootloader.
build-x86 *args='':
# run `cargo check` first because the actual binary will be built in a build
# script that that eats compiler output :(
{{ _cargo }} check --package {{ _x86_pkg }} \
--bin bootloader \
--target x86_64-unknown-none \
--features bootloader_api {{ args }}
{{ _cargo }} build --package {{ _x86_bootloader_pkg }} {{ args }}
# run an x86_64 MnemOS image in QEMU
Expand Down
10 changes: 2 additions & 8 deletions platforms/x86_64/bootloader/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,6 @@ ovmf-prebuilt = "0.1.0-alpha.1"
clap = { version = "4", features = ["derive", "env"] }

[build-dependencies]
anyhow = "1"
bootloader = "0.11"

# the actual MnemOS kernel binary
[build-dependencies.mnemos]
package = "mnemos-x86_64-core"
path = "../core"
artifact = "bin:bootloader"
target = "x86_64-unknown-none"
features = ["bootloader_api"]
escargot = "0.5"
56 changes: 49 additions & 7 deletions platforms/x86_64/bootloader/build.rs
Original file line number Diff line number Diff line change
@@ -1,24 +1,66 @@
use anyhow::Context;
use std::path::PathBuf;

fn main() {
fn main() -> anyhow::Result<()> {
const PKG_NAME: &'static str = "mnemos-x86_64-core";

Check failure on line 5 in platforms/x86_64/bootloader/build.rs

View workflow job for this annotation

GitHub Actions / just clippy

error: constants have by default a `'static` lifetime --> platforms/x86_64/bootloader/build.rs:5:22 | 5 | const PKG_NAME: &'static str = "mnemos-x86_64-core"; | -^^^^^^^---- help: consider removing `'static`: `&str` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes = note: `-D clippy::redundant-static-lifetimes` implied by `-D warnings`
const BIN_NAME: &'static str = "bootloader";

Check failure on line 6 in platforms/x86_64/bootloader/build.rs

View workflow job for this annotation

GitHub Actions / just clippy

error: constants have by default a `'static` lifetime --> platforms/x86_64/bootloader/build.rs:6:22 | 6 | const BIN_NAME: &'static str = "bootloader"; | -^^^^^^^---- help: consider removing `'static`: `&str` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes
const TARGET_TRIPLE: &'static str = "x86_64-unknown-none";

Check failure on line 7 in platforms/x86_64/bootloader/build.rs

View workflow job for this annotation

GitHub Actions / just clippy

error: constants have by default a `'static` lifetime --> platforms/x86_64/bootloader/build.rs:7:27 | 7 | const TARGET_TRIPLE: &'static str = "x86_64-unknown-none"; | -^^^^^^^---- help: consider removing `'static`: `&str` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes
const ENV_OUT_DIR: &'static str = "OUT_DIR";

Check failure on line 8 in platforms/x86_64/bootloader/build.rs

View workflow job for this annotation

GitHub Actions / just clippy

error: constants have by default a `'static` lifetime --> platforms/x86_64/bootloader/build.rs:8:25 | 8 | const ENV_OUT_DIR: &'static str = "OUT_DIR"; | -^^^^^^^---- help: consider removing `'static`: `&str` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes
const ENV_PROFILE: &'static str = "PROFILE";

Check failure on line 9 in platforms/x86_64/bootloader/build.rs

View workflow job for this annotation

GitHub Actions / just clippy

error: constants have by default a `'static` lifetime --> platforms/x86_64/bootloader/build.rs:9:25 | 9 | const ENV_PROFILE: &'static str = "PROFILE"; | -^^^^^^^---- help: consider removing `'static`: `&str` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes

// set by cargo, build scripts should use this directory for output files
let out_dir = PathBuf::from(std::env::var_os("OUT_DIR").unwrap());
// set by cargo's artifact dependency feature, see
// https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#artifact-dependencies
let kernel = PathBuf::from(std::env::var_os("CARGO_BIN_FILE_MNEMOS_bootloader").unwrap());
let out_dir = PathBuf::from(
std::env::var_os(ENV_OUT_DIR)
.with_context(|| format!("missing {ENV_OUT_DIR} environment variable!"))?,
);
let release = match std::env::var_os(ENV_PROFILE) {
Some(x) if x == "release" => true,
Some(x) if x == "debug" => false,
x => {
println!("cargo:warning={ENV_PROFILE} env var either unset or weird: {x:?}");
false
}
};

// XXX(eliza): it's sad that this way of building the binary by just
// shelling out to a child `cargo` invocation will eat things like the
// compiler output. but, the alternative approach where the same cargo
// invocation can build everything would be to use artifact deps, which are
// unfortunately broken due to this Cargo bug:
// https://github.com/rust-lang/cargo/issues/12358
//
// If upstream PR https://github.com/rust-lang/cargo/pull/13207 ever merges,
// we should revisit this approach and see if we can switch back to artifact
// deps...
let mut build = escargot::CargoBuild::new()
.package(PKG_NAME)
.bin(BIN_NAME)
.target(TARGET_TRIPLE)
.target_dir(&out_dir)
.features("bootloader_api");
if release {
build = build.release();
}

let cargo_output = build
.run()
.context("failed to execute cargo build command")?;

let kernel = cargo_output.path();

let uefi_path = out_dir.join("mnemos-x86_64-uefi.img");
bootloader::UefiBoot::new(&kernel)
bootloader::UefiBoot::new(kernel)
.create_disk_image(&uefi_path)
.unwrap();

// create a BIOS disk image
let bios_path = out_dir.join("mnemos-x86_64-bios.img");
bootloader::BiosBoot::new(&kernel)
bootloader::BiosBoot::new(kernel)
.create_disk_image(&bios_path)
.unwrap();

// pass the disk image paths as env variables to the `main.rs`
println!("cargo:rustc-env=UEFI_PATH={}", uefi_path.display());
println!("cargo:rustc-env=BIOS_PATH={}", bios_path.display());
Ok(())
}

0 comments on commit 96dc2e5

Please sign in to comment.