From 96dc2e53ffb8d7a3d8d1943cfb3dc6105649ccfa Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Sun, 11 Aug 2024 10:58:36 -0700 Subject: [PATCH] chore(x86_64): change build script to invoke cargo 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 --- Cargo.lock | 14 ++++++- justfile | 8 +++- platforms/x86_64/bootloader/Cargo.toml | 10 +---- platforms/x86_64/bootloader/build.rs | 56 ++++++++++++++++++++++---- 4 files changed, 71 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ce497e85..2d6ed5d9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2250,6 +2250,18 @@ dependencies = [ "libc", ] +[[package]] +name = "escargot" +version = "0.5.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c000f23e9d459aef148b7267e02b03b94a0aaacf4ec64c65612f67e02f525fb6" +dependencies = [ + "log", + "once_cell", + "serde", + "serde_json", +] + [[package]] name = "esp-alloc" version = "0.3.0" @@ -5212,7 +5224,7 @@ dependencies = [ "anyhow", "bootloader", "clap 4.4.0", - "mnemos-x86_64-core", + "escargot", "ovmf-prebuilt", ] diff --git a/justfile b/justfile index 388d7338..25babfb6 100644 --- a/justfile +++ b/justfile @@ -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" @@ -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 diff --git a/platforms/x86_64/bootloader/Cargo.toml b/platforms/x86_64/bootloader/Cargo.toml index 8aabdb87..5eaf8c38 100644 --- a/platforms/x86_64/bootloader/Cargo.toml +++ b/platforms/x86_64/bootloader/Cargo.toml @@ -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" diff --git a/platforms/x86_64/bootloader/build.rs b/platforms/x86_64/bootloader/build.rs index 9d9605dc..f7933184 100644 --- a/platforms/x86_64/bootloader/build.rs +++ b/platforms/x86_64/bootloader/build.rs @@ -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"; + const BIN_NAME: &'static str = "bootloader"; + const TARGET_TRIPLE: &'static str = "x86_64-unknown-none"; + const ENV_OUT_DIR: &'static str = "OUT_DIR"; + const ENV_PROFILE: &'static str = "PROFILE"; + // 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(()) }