Skip to content
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

chore(x86_64): change build script to invoke cargo #326

Merged
merged 2 commits into from
Aug 11, 2024
Merged

Conversation

hawkw
Copy link
Contributor

@hawkw hawkw commented Aug 11, 2024

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. 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. What I've written here is just a minimum viable solution, and I'd like to change it to use the cargo runner approach later.

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
@hawkw hawkw enabled auto-merge (squash) August 11, 2024 18:10
@hawkw hawkw merged commit 1bd7037 into main Aug 11, 2024
10 checks passed
@hawkw hawkw deleted the eliza/cargo-shellout branch August 11, 2024 18:32
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.

1 participant