From 3cf102d0d9e4fa69087bb31fb6335ba26d905104 Mon Sep 17 00:00:00 2001 From: Tom Kirchner Date: Fri, 5 Mar 2021 10:50:45 -0800 Subject: [PATCH 1/2] os: run static and non-static Rust builds in parallel There's a long tail of crate builds for each that can be mitigated by running them in parallel, saving a fair amount of time. --- packages/os/os.spec | 50 ++++++++++++++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 14 deletions(-) diff --git a/packages/os/os.spec b/packages/os/os.spec index ed2f8f42214..06ad2566727 100644 --- a/packages/os/os.spec +++ b/packages/os/os.spec @@ -205,6 +205,35 @@ Requires: %{_cross_os}apiserver = %{version}-%{release} %build mkdir bin + +# We want to build some components statically: +# * apiclient, because it needs to run from containers that don't have the same libraries available. +# * migrations, because they need to run after a system update where available libraries can change. +# +# Most of our components don't need to be static, though. This means we run cargo once for static +# and once for non-static. There's a long tail of crate builds for each of these that can be +# mitigated by running them in parallel, saving a fair amount of time. To do this, we kick off the +# static build in the background, run the non-static (main) build in the foreground, and then wait +# for the static build and print its output afterward. A failure of either will stop the build. + +# For static builds, first we find the migrations in the source tree. We assume the directory name +# is the same as the crate name. +migrations=() +for migration in $(find %{_builddir}/sources/api/migration/migrations/* -mindepth 1 -maxdepth 1 -type d); do + migrations+=("-p $(basename ${migration})") +done +# Store the output so we can print it after waiting for the backgrounded job. +static_output="$(mktemp)" +# Build static binaries in the background. +%cargo_build_static --manifest-path %{_builddir}/sources/Cargo.toml \ + -p apiclient \ + ${migrations[*]} \ + >> ${static_output} 2>&1 & +# Save the PID so we can wait for it later. +static_pid="$!" + +# Run non-static builds in the foreground. +echo "** Output from non-static builds:" %cargo_build --manifest-path %{_builddir}/sources/Cargo.toml \ -p apiserver \ -p early-boot-config \ @@ -236,20 +265,13 @@ mkdir bin %endif %{nil} -# Next, build components that should be static. -# * apiclient, because it needs to run from containers that don't have the same libraries available. -# * migrations, because they need to run after a system update where available libraries can change. - -# First we find the migrations in the source tree. We assume the directory name is the same as the crate name. -migrations=() -for migration in $(find %{_builddir}/sources/api/migration/migrations/* -mindepth 1 -maxdepth 1 -type d); do - migrations+=("-p $(basename ${migration})") -done -# Build static binaries. -%cargo_build_static --manifest-path %{_builddir}/sources/Cargo.toml \ - -p apiclient \ - ${migrations[*]} \ - %{nil} +# Wait for static builds from the background, if they're not already done. +set +e; wait "${static_pid}"; static_rc="${?}"; set -e +echo -e "\n** Output from static builds:" +cat "${static_output}" +if [ "${static_rc}" -ne 0 ]; then + exit "${static_rc}" +fi %install install -d %{buildroot}%{_cross_bindir} From a7f68d70792b748a73d77ae09fba6395c9c00ee0 Mon Sep 17 00:00:00 2001 From: Tom Kirchner Date: Fri, 5 Mar 2021 14:53:43 -0800 Subject: [PATCH 2/2] models build.rs: remove early exit for target toolchain This is a partial revert of 79465cd6d3f1e6f121dd15915a0346fee2bbe9ac; the nicer structure for main() is kept, but the actual early-exit is removed. We don't actually need the early-exit if it's safe to run this build.rs in parallel, which it is since 3943a32212a424c6dc0c0c97e3f15b0f627e937c introduced safe link swapping. The other action it takes, README generation, is skipped during production builds, and would be obvious if it went wrong during local builds due to version controlled changes, though it should also be safe there as long as our READMEs don't balloon and become unsafe to write in parallel. The reason for removing this is that it effectively worked by chance, because we were running non-static builds before static builds. We'd like to run both sets of builds in parallel to save a significant amount of time. Static builds, as run in os.spec, only run build.rs once, with a vendor of "bottlerocket". This means build.rs was skipped, and unless non-static builds won the race and ran their build.rs before static builds got to that crate, the link wouldn't be created and the build would fail. --- sources/api/storewolf/Cargo.toml | 4 ++-- sources/models/build.rs | 10 ++-------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/sources/api/storewolf/Cargo.toml b/sources/api/storewolf/Cargo.toml index f2c17ccbc0a..3844002349f 100644 --- a/sources/api/storewolf/Cargo.toml +++ b/sources/api/storewolf/Cargo.toml @@ -26,8 +26,8 @@ merge-toml = { path = "merge-toml" } # We have a models build-dep because we read default settings from the models # directory and need its build.rs to run first; we also reflect the dependency # with cargo:rerun-if-changed statements in our build.rs. The models build.rs -# runs twice, once for the above dependency and once for this build-dependency; -# there's a check in models build.rs to skip running the second time. +# runs twice, once for the above dependency and once for this build-dependency, +# so it's important that it remains reentrant. models = { path = "../../models" } snafu = "0.6" toml = "0.5" diff --git a/sources/models/build.rs b/sources/models/build.rs index 9c9557ba9e4..98594666010 100644 --- a/sources/models/build.rs +++ b/sources/models/build.rs @@ -17,18 +17,12 @@ const MOD_LINK: &str = "src/variant/mod.rs"; const VARIANT_ENV: &str = "VARIANT"; fn main() { - // Tell cargo when we have to rerun, regardless of early-exit below. + // Tell cargo when we have to rerun; we always want variant links to be correct, especially + // after changing the variant we're building for. println!("cargo:rerun-if-env-changed={}", VARIANT_ENV); println!("cargo:rerun-if-changed={}", VARIANT_LINK); println!("cargo:rerun-if-changed={}", MOD_LINK); - // This build.rs runs once as a build-dependency of storewolf, and again as a (regular) - // dependency of storewolf. There's no reason to do this work twice. - if env::var("CARGO_CFG_TARGET_VENDOR").unwrap_or_else(|_| String::new()) == "bottlerocket" { - println!("cargo:warning=Already ran model build.rs for host, skipping for target"); - process::exit(0); - } - generate_readme(); link_current_variant(); }