rust-osdev / bootimage

Tool to create bootable disk images from a Rust OS kernel.
Apache License 2.0
765 stars 67 forks source link

Add support for workspace subcrate building #53

Closed u1f408 closed 4 years ago

u1f408 commented 4 years ago

This pull request adds the configuration key workspace-subcrate to the package.metadata.bootimage table, which lets the root crate specify a subcrate within it's workspace to build as the kernel executable, instead of the root crate itself.

This arose out of my OS project, where the root crate ("os-core") is platform-agnostic, and there is a subcrate ("os-amd64") that contains the target-specific things. The "os-amd64" crate is the one with the bootsector dependency, and contains the entry_point!() definition.

The idea here is to allow running bootimage from the root of the repository, which is where all the other Cargo operations get run from (cargo fmt, cargo doc, etc), and still have everything work.

As an example, the Cargo manifest of the root crate would have it's metadata set ilke the following:

[package.metadata.bootimage]
workspace-subcrate = "os-amd64"

The "os-amd64" crate (which is expected to be in a direct subdirectory of the root crate) would then have it's manifest specify bootimage options as normal (like setting the default target, QEMU args, etc).

A limitation of using the package.metadata.bootimage manifest table for this is that a top-level pure workspace manifest can not specify a subcrate to use as the kernel executable crate, since we're using a subtable of package to store the configuration (which makes cargo-metadata freak out since there's not actually a valid package definition there). A possible solution to this is to duplicate a lot of the configuration logic and parse a workspace.metadata subtable, however IIRC workspace.metadata is not a standard key so that may cause some other things to go wrong.

A possible addition to this is to walk up the directory hierarchy looking for Cargo manifests (much the same as the locate-cargo-manifest crate does) looking for the highest level crate manifest containing a package.metadata.bootimage table, and treating that as the crate root (and then descending again should the workspace-subcrate key be found), which would allow running bootimage from anywhere within the repository tree and still having everything work - much the same as running bootimage currently from a subdirectory in a single-package repository works currently.

I will add tests for this functionality should the maintainers wish to merge this PR.

phil-opp commented 4 years ago

Thanks for opening this pull request! I'm not sure if it's a good idea to differ from the default cargo behavior. For example, when you set the runner to bootimage runner, you simply type the normal cargo run/cargo xrun without exposing the fact that this is handled by bootimage. So it would be surprising if a different crate is run than cargo would normally run.

Have you tried to set the default-run? According to this stackoverflow answer, it should work to specify the binary that should run when cargo run is invoked from the workspace root. In combination with setting your runner to bootimage runner (as linked above), you should be able to directly run cargo xrun from the workspace root. Does this work for you?

u1f408 commented 4 years ago

My reasoning for this is that bootimage is “the x86_64 build tool”. Setting default-run etc globally for the workspace wouldn’t work for me as very soon in my OS there will be the beginnings of support for another architecture.

I do see your point in that it would be weird for cargo xrun etc using bootimage runner to run something other than what Cargo would usually do on it’s own however.

phil-opp commented 4 years ago

Let me try to clarify the problem I'm seeing: When you're using cargo xrun with bootimage runner, cargo takes care of building the kernel and then invokes bootimage runner with the kernel binary passed as an argument. So this PR does not affect what cargo xrun is running because the crate is built directly by cargo. However, it affects the bootimage run command, because for this command bootimage decides which project to build. So after this PR the two commands (bootimage run and cargo xrun) might now run build different crates.

Another example is the cargo bootimage command that bootimage provides (basically equivalent to bootimage build). When using this command, I would expect that the same crate is built than with cargo build, which would no longer be the case after this PR. So cargo xbuild might build something entirely different than cargo bootimage afterwards, which doesn't seem like a good idea.

My reasoning for this is that bootimage is “the x86_64 build tool”. Setting default-run etc globally for the workspace wouldn’t work for me as very soon in my OS there will be the beginnings of support for another architecture.

So you want to differentiate the architectures by different build commands that are all run from the root workspace? So for example, you plan to use bootimage run to run the OS for the x86_64 architecture and e.g. bootimage-arm run to run it on ARM?

I think in that case it might make sense to create a custom runner tool for your that executes the different build commands in the different subfolders. For example, my-runner --target x86_64 could switch to the x86_64 subdirectory and invoke bootimage run there, while my-runner --target arm would switch to the arm subfolder and run a different run command there. This way, the differences between the build tools are abstracted away.

u1f408 commented 4 years ago

Right, that makes sense. Your point about a custom runner is probably a better idea - I'll close this.