-
Notifications
You must be signed in to change notification settings - Fork 516
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
Combine first-party Rust builds #428
Conversation
Signed-off-by: iliana destroyer of worlds <[email protected]>
@@ -19,7 +19,7 @@ maplit = "1.0.1" | |||
olpc-cjson = { path = "../updater/olpc-cjson" } | |||
pem = "0.6.0" | |||
rayon = "1.2" | |||
reqwest = "0.9.20" | |||
reqwest = { version = "0.9.20", features = ["rustls-tls"], default-features = false } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this related to the other changes in the PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this failed when I was using --bins
, but it's not wrong for us to be using rustls here when we're using it everywhere else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤘
Co-Authored-By: Tom Kirchner <[email protected]>
Issue #, if available: fixes #291
Description of changes:
Combines all of our first-party Rust builds into a single RPM.
Running
cargo install
in a loop recompiles every dependency every time becausecargo install
is not the right tool for this use case. Instead, we're building all the binary targets inworkspaces/
at the same time, so we can compile each dependency once.This lists all the crate names we want to build. Other options included
--bins
(which does not work with--exclude
) or--all --exclude
, but since you need to copy the binary in%install
anyway it seemed reasonable to list all the crates to build.This also makes
sd_notify
the default feature because cargo has strange behavior when trying to enable feature flags while building a workspace, and the discussion appears to be gridlocked between "fix it" and "don't break people", so I kind of just want to steer clear of that entirely.Timing tests:
Tests performed on an m5.metal, ymmv (but this should be a clear enough improvement to everyone).
Fresh compile (run
cargo make clean && cargo make fetch
, then timecargo make world
):Incremental apiserver build (run
touch workspaces/api/apiserver/src/bin/apiserver.rs
, then timecargo make world
):Launch tests: An AMI built with these changes boots and joins my cluster.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.