Skip to content

Commit 8de9b1b

Browse files
committed
devtool: Add flag to build to allow compiling arbitrary revisions
Passing `--rev` to `tools/devtool build` will do a checkout of the specified revision into a temporary directory, compile firecracker into it, and then copy over the final binaries into build/$revision. This has advantages over our current approach for compiling A/B revisions, as by only copying the final binaries into build/$revision we save a lot of bandwidth when uploading artifacts for transfer between different buildkite steps (as we no longer include gigabytes of compilation artifacts). This now means that all revisions are compiled in the same docker container, however due to the rust-toolchain.toml this should not have an impact on cross-toolchain testability, and has the advantage that the cargo cache is shared (meaning we're hitting crates.io for downloads less). Signed-off-by: Patrick Roy <[email protected]>
1 parent 5122c4d commit 8de9b1b

File tree

5 files changed

+34
-14
lines changed

5 files changed

+34
-14
lines changed

.buildkite/common.py

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -180,16 +180,7 @@ def random_str(k: int):
180180

181181
def ab_revision_build(revision):
182182
"""Generate steps for building an A/B-test revision"""
183-
# Copied from framework/ab_test. Double dollar signs needed for Buildkite (otherwise it will try to interpolate itself)
184-
return [
185-
f"commitish={revision}",
186-
f"if ! git cat-file -t $$commitish; then commitish=origin/{revision}; fi",
187-
"branch_name=tmp-$$commitish",
188-
"git branch $$branch_name $$commitish",
189-
f"git clone -b $$branch_name . build/{revision}",
190-
f"cd build/{revision} && ./tools/devtool -y build --release && cd -",
191-
"git branch -D $$branch_name",
192-
]
183+
return [f"./tools/devtool -y build --rev {revision} --release"]
193184

194185

195186
def shared_build():

.buildkite/pipeline_perf.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,9 @@
9898
pytest_opts = ""
9999
if REVISION_A:
100100
devtool_opts += " --ab"
101-
pytest_opts = f"{ab_opts} run build/{REVISION_A}/build/cargo_target/$(uname -m)-unknown-linux-musl/release build/{REVISION_B}/build/cargo_target/$(uname -m)-unknown-linux-musl/release --test {test_path}"
101+
pytest_opts = (
102+
f"{ab_opts} run build/{REVISION_A}/ build/{REVISION_B} --test {test_path}"
103+
)
102104
else:
103105
# Passing `-m ''` below instructs pytest to collect tests regardless of
104106
# their markers (e.g. it will collect both tests marked as nonci, and

tests/framework/ab_test.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,14 +101,15 @@ def git_ab_test(
101101
return result_a, result_b, comparison
102102

103103

104+
DEFAULT_A_DIRECTORY = FC_WORKSPACE_DIR / "build" / "main"
104105
DEFAULT_B_DIRECTORY = FC_WORKSPACE_DIR / "build" / "cargo_target" / DEFAULT_TARGET_DIR
105106

106107

107108
def binary_ab_test(
108109
test_runner: Callable[[Path, bool], T],
109110
comparator: Callable[[T, T], U] = default_comparator,
110111
*,
111-
a_directory: Path,
112+
a_directory: Path = DEFAULT_A_DIRECTORY,
112113
b_directory: Path = DEFAULT_B_DIRECTORY,
113114
):
114115
"""

tools/ab_test.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ def load_data_series(report_path: Path, tag=None, *, reemit: bool = False):
155155

156156
def collect_data(binary_dir: Path, tests: list[str]):
157157
"""Executes the specified test using the provided firecracker binaries"""
158-
# Example binary_dir: ../build/main/build/cargo_target/x86_64-unknown-linux-musl/release
158+
binary_dir = binary_dir.resolve()
159159

160160
print(f"Collecting samples with {binary_dir}")
161161
subprocess.run(

tools/devtool

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -446,12 +446,14 @@ cmd_build() {
446446
profile="debug"
447447
libc="musl"
448448

449+
449450
# Parse any command line args.
450451
while [ $# -gt 0 ]; do
451452
case "$1" in
452453
"-h"|"--help") { cmd_help; exit 1; } ;;
453454
"--debug") { profile="debug"; } ;;
454455
"--release") { profile="release"; } ;;
456+
"--rev") { shift; revision=$1; } ;;
455457
"--ssh-keys")
456458
shift
457459
[[ -z "$1" ]] && \
@@ -489,16 +491,40 @@ cmd_build() {
489491
extra_args="--volume $host_pub_key_path:$PUB_KEY_PATH:z \
490492
--volume $host_priv_key_path:$PRIV_KEY_PATH:z"
491493

494+
workdir="$CTR_FC_ROOT_DIR"
495+
if [ ! -z "$revision" ]; then
496+
commitish="$revision"
497+
if ! git cat-file -t "$commitish"; then commitish=origin/"$revision"; fi
498+
branch_name=tmp-$commitish
499+
500+
tmp_dir=$(mktemp -d)
501+
502+
git branch $branch_name $commitish
503+
git clone -b $branch_name . $tmp_dir
504+
pushd $tmp_dir
505+
workdir=$tmp_dir
506+
extra_args="$extra_args --volume $tmp_dir:$tmp_dir:z"
507+
fi
508+
492509
# Run the cargo build process inside the container.
493510
# We don't need any special privileges for the build phase, so we run the
494511
# container as the current user/group.
495512
run_devctr \
496513
--user "$(id -u):$(id -g)" \
497-
--workdir "$CTR_FC_ROOT_DIR" \
514+
--workdir "$workdir" \
498515
${extra_args} \
499516
-- \
500517
./tools/release.sh --libc $libc --profile $profile
501518
ret=$?
519+
520+
if [ ! -z "$revision" ]; then
521+
popd
522+
git branch -D $branch_name
523+
mkdir -p build/"$revision"
524+
cp $tmp_dir/build/cargo_target/$(uname -m)-unknown-linux-$libc/$profile/* build/"$revision"
525+
rm -rf $tmp_dir
526+
fi
527+
502528
return $ret
503529
}
504530

0 commit comments

Comments
 (0)