chore: move to One Big Cargo Workspace #219
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is needed when building new versions of `llvm-tools`.
This commit changes the Cargo workspace setup to put all crates in One
Big Workspace, rather than having separate workspaces for some targets.
We now use the `per-package-target` unstable cargo feature to build
different crates for different targets. This means that `cargo` commands
in the root workspace now work without requiring the user to `cd` into a
particular directory to build a platform target --- for example, I can
now run:
```console
# in the repo root directory
$ cargo build -p mnemos-d1 --bin mq-pro
```
and build a MnemOS binary for the MQ Pro, without having to `cd` into
the MQ Pro directory.
One issue is that `cargo build --workspace` (and `check --workspace`,
etc) still does not work correctly, due to some [weird][1] [issues][2]
with feature unification which I don't entirely understand. However, as
a workaround, I've changed the workspace Cargo.toml to add a
[`default-members` field][3], so that running `cargo build` or `cargo
check` _without_ `--workspace` will build the subset of crates in the
`default-members` key. This way, `cargo {build, check, etc}` in the repo
root will do something reasonable by default, and the actual platform
targets can be built/checked with `cargo $WHATEVER --package $CRATE`.
IMO, this is still substantially nicer than having a bunch of separate
workspaces.
[1]: ia0/data-encoding#47
[2]: bincode-org/bincode#556
[3]: https://doc.rust-lang.org/cargo/reference/workspaces.html#the-default-members-field
??????? ??? ????????? ???
jamesmunns
approved these changes
Aug 9, 2023
There was a problem hiding this comment.
Overall LGTM, it would be nice to document somewhere the nightly cargo trickery we are using and why, and link to the related tracking issues so we know when we can get rid of them, but if it still works and produces valid binaries for the various targets, no objections here.
jamesmunns
reviewed
Aug 9, 2023
Mostly, we are just using features that haven't yet been stabilized. These aren't workarounds for bugs, per se. I'm happy to add links to the tracking issues for those features. |
|
@jamesmunns 069c07a adds comments explaining the various unstable features & stuff. |
hawkw
added a commit
that referenced
this pull request
Aug 9, 2023
Depends on #219. This branch adds an initial implementation of an x86_64 platform target for mnemOS, using the [`mycelium` x86 HAL][hal]. Currently, I haven't implemented drivers for the UART, keyboard, or emb-display service (using the framebuffer as the display), so we don't have SerMux, Forth, trace-proto, or anything else interesting. However, what we *do* have is a basic bootable image with a timer, allocator, and rudimentary kernel run loop, and --- as you can see here --- it works:  I've also added new `just` commands for building x86_64 images and running them in QEMU for development. [hal]: https://github.com/hawkw/mycelium/tree/main/hal-x86_64
jamesmunns
added
the
area: tools & build
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit changes the Cargo workspace setup to put all crates in One
Big Workspace, rather than having separate workspaces for some targets.
We now use the
per-package-targetunstable cargo feature to builddifferent crates for different targets. This means that
cargocommandsin the root workspace now work without requiring the user to
cdinto aparticular directory to build a platform target --- for example, I can
now run:
# in the repo root directory $ cargo build -p mnemos-d1 --bin mq-proand build a MnemOS binary for the MQ Pro, without having to
cdintothe MQ Pro directory.
This is also necessary in order to make the
x86_64build process addedin PR #216 work, since it relies on cargo artifact dependencies, which
appear not to work across workspaces.
One issue is that
cargo build --workspace(andcheck --workspace,etc) still does not work correctly, due to some weird issues
with feature unification which I don't entirely understand. However, as
a workaround, I've changed the workspace Cargo.toml to add a
default-membersfield, so that runningcargo buildorcargo checkwithout
--workspacewill build the subset of crates in thedefault-memberskey. This way,cargo {build, check, etc}in the reporoot will do something reasonable by default, and the actual platform
targets can be built/checked with
cargo $WHATEVER --package $CRATE.IMO, this is still substantially nicer than having a bunch of separate
workspaces.