fix: Make sure build fails if command returns non-zero code (#1499)

ivoanjo · web-flow · commit c89b6dfb938f · 2026-02-02T17:45:09.000Z
fix: Make sure build fails if command returns non-zero code

**What does this PR do?**

This PR introduces a `wait_for_success` helper and uses it throughout
the builder crate to ensure that the status code for a command is
checked, and the build fails if it is non-zero.

**Motivation:**

I ran into this very sharp edge as I was experimenting with some
build changes in libdatadog: whenever an external command exits
with a non-zero status code, the build would continue, rather
than fail immediately.

This happens because we were not previously checking for the
command status code. This change fixes this.

**Additional Notes:**

N/A

**How to test the change?**

Since the error code only gets triggered when something goes wrong,
you won't see it by default. To test this, I replaced command
invocations (to e.g. `cargo` in `profiling.rs`) with a call to `false`:
this is an actual command that runs successfully, but then returns
a non-zero code and thus can be used to see this change in action.

Make linters happy

Co-authored-by: ivo.anjo &lt;ivo.anjo@datadoghq.com&gt;
diff --git a/builder/src/arch/apple.rs b/builder/src/arch/apple.rs
@@ -1,10 +1,11 @@
 // Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/
 // SPDX-License-Identifier: Apache-2.0
 
+use std::ffi::OsStr;
 use std::os::unix::process::ExitStatusExt;
 use std::process::Command;
 
-use std::ffi::OsStr;
+use crate::utils::wait_for_success;
 
 pub const NATIVE_LIBS: &str =
     " -framework Security -framework CoreFoundation -liconv -lSystem -lresolv -lc -lm -liconv";
@@ -43,13 +44,13 @@ pub fn fix_rpath(lib_path: &str) {
 
 pub fn strip_libraries(lib_path: &str) {
     // objcopy is not available in macos image. Investigate llvm-objcopy
-    let mut strip = Command::new("strip")
+    let strip = Command::new("strip")
         .arg("-S")
         .arg(lib_path.to_owned() + "/libdatadog_profiling.dylib")
         .spawn()
         .expect("Failed to spawn strip");
 
-    strip.wait().expect("Failed to strip library");
+    wait_for_success(strip, "strip");
 }
 
 pub fn add_additional_files(_lib_path: &str, _target_path: &OsStr) {}
diff --git a/builder/src/arch/linux.rs b/builder/src/arch/linux.rs
@@ -1,9 +1,10 @@
 // Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/
 // SPDX-License-Identifier: Apache-2.0
 
+use std::ffi::OsStr;
 use std::process::Command;
 
-use std::ffi::OsStr;
+use crate::utils::wait_for_success;
 
 pub const NATIVE_LIBS: &str = " -ldl -lrt -lpthread -lc -lm -lrt -lpthread -lutil -ldl -lutil";
 pub const PROF_DYNAMIC_LIB: &str = "libdatadog_profiling.so";
@@ -21,50 +22,50 @@ pub const RUSTFLAGS: [&str; 4] = [
 
 pub fn fix_rpath(lib_path: &str) {
     if REMOVE_RPATH {
-        let mut patchelf = Command::new("patchelf")
+        let patchelf = Command::new("patchelf")
             .arg("--remove-rpath")
             .arg(lib_path)
             .spawn()
             .expect("failed to spawn patchelf");
 
-        patchelf.wait().expect("failed to remove rpath");
+        wait_for_success(patchelf, "patchelf");
     }
 }
 
 pub fn strip_libraries(lib_path: &str) {
-    let mut rm_section = Command::new("objcopy")
+    let rm_section = Command::new("objcopy")
         .arg("--remove-section")
         .arg(".llvmbc")
         .arg(lib_path.to_owned() + "/libdatadog_profiling.a")
         .spawn()
         .expect("failed to spawn objcopy");
 
-    rm_section.wait().expect("Failed to remove llvmbc section");
+    wait_for_success(rm_section, "objcopy (remove llvmbc section)");
 
-    let mut create_debug = Command::new("objcopy")
+    let create_debug = Command::new("objcopy")
         .arg("--only-keep-debug")
         .arg(lib_path.to_owned() + "/libdatadog_profiling.so")
         .arg(lib_path.to_owned() + "/libdatadog_profiling.debug")
         .spawn()
         .expect("Failed to spawn objcopy");
 
-    create_debug.wait().expect("Failed to extract debug info");
+    wait_for_success(create_debug, "objcopy (extract debug info)");
 
-    let mut strip = Command::new("strip")
+    let strip = Command::new("strip")
         .arg("-S")
         .arg(lib_path.to_owned() + "/libdatadog_profiling.so")
         .spawn()
         .expect("Failed to spawn strip");
 
-    strip.wait().expect("Failed to strip library");
+    wait_for_success(strip, "strip");
 
-    let mut debug = Command::new("objcopy")
+    let debug = Command::new("objcopy")
         .arg("--add-gnu-debuglink=".to_string() + lib_path + "/libdatadog_profiling.debug")
         .arg(lib_path.to_owned() + "/libdatadog_profiling.so")
         .spawn()
         .expect("Failed to spawn objcopy");
 
-    debug.wait().expect("Failed to set debuglink");
+    wait_for_success(debug, "objcopy (set debuglink)");
 }
 
 pub fn add_additional_files(_lib_path: &str, _target_path: &OsStr) {}
diff --git a/builder/src/arch/musl.rs b/builder/src/arch/musl.rs
@@ -1,9 +1,10 @@
 // Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/
 // SPDX-License-Identifier: Apache-2.0
 
+use std::ffi::OsStr;
 use std::process::Command;
 
-use std::ffi::OsStr;
+use crate::utils::wait_for_success;
 
 pub const NATIVE_LIBS: &str = " -lssp_nonshared -lc";
 pub const PROF_DYNAMIC_LIB: &str = "libdatadog_profiling.so";
@@ -30,39 +31,39 @@ pub fn fix_rpath(lib_path: &str) {
 }
 
 pub fn strip_libraries(lib_path: &str) {
-    let mut rm_section = Command::new("objcopy")
+    let rm_section = Command::new("objcopy")
         .arg("--remove-section")
         .arg(".llvmbc")
         .arg(lib_path.to_owned() + "/libdatadog_profiling.a")
         .spawn()
         .expect("failed to spawn objcopy");
 
-    rm_section.wait().expect("Failed to remove llvmbc section");
+    wait_for_success(rm_section, "objcopy (remove llvmbc section)");
 
-    let mut create_debug = Command::new("objcopy")
+    let create_debug = Command::new("objcopy")
         .arg("--only-keep-debug")
         .arg(lib_path.to_owned() + "/libdatadog_profiling.so")
         .arg(lib_path.to_owned() + "/libdatadog_profiling.debug")
         .spawn()
         .expect("Failed to spawn objcopy");
 
-    create_debug.wait().expect("Failed to extract debug info");
+    wait_for_success(create_debug, "objcopy (extract debug info)");
 
-    let mut strip = Command::new("strip")
+    let strip = Command::new("strip")
         .arg("-s")
         .arg(lib_path.to_owned() + "/libdatadog_profiling.so")
         .spawn()
         .expect("Failed to spawn strip");
 
-    strip.wait().expect("Failed to strip library");
+    wait_for_success(strip, "strip");
 
-    let mut debug = Command::new("objcopy")
+    let debug = Command::new("objcopy")
         .arg("--add-gnu-debuglink=".to_string() + lib_path + "/libdatadog_profiling.debug")
         .arg(lib_path.to_owned() + "/libdatadog_profiling.so")
         .spawn()
         .expect("Failed to spawn objcopy");
 
-    debug.wait().expect("Failed to set debuglink");
+    wait_for_success(debug, "objcopy (set debuglink)");
 }
 
 pub fn add_additional_files(_lib_path: &str, _target_path: &OsStr) {}
diff --git a/builder/src/common.rs b/builder/src/common.rs
@@ -3,7 +3,7 @@
 
 use crate::arch;
 use crate::module::Module;
-use crate::utils::project_root;
+use crate::utils::{project_root, wait_for_success};
 use anyhow::Result;
 use std::fs;
 use std::path::PathBuf;
@@ -18,7 +18,7 @@ pub struct Common {
 
 impl Module for Common {
     fn build(&self) -> Result<()> {
-        let mut cargo = Command::new("cargo")
+        let cargo = Command::new("cargo")
             .env("RUSTFLAGS", arch::RUSTFLAGS.join(" "))
             .current_dir(project_root())
             .args([
@@ -33,7 +33,7 @@ impl Module for Common {
             .spawn()
             .expect("failed to spawn cargo");
 
-        cargo.wait().expect("Cargo failed");
+        wait_for_success(cargo, "Cargo");
         Ok(())
     }
 
diff --git a/builder/src/profiling.rs b/builder/src/profiling.rs
@@ -3,7 +3,7 @@
 
 use crate::arch;
 use crate::module::Module;
-use crate::utils::{file_replace, project_root};
+use crate::utils::{file_replace, project_root, wait_for_success};
 use anyhow::Result;
 use serde::Deserialize;
 use std::ffi::OsStr;
@@ -196,14 +196,14 @@ impl Module for Profiling {
             let mut args = cargo_args.clone();
             args.append(&mut vec!["--crate-type", crate_type]);
 
-            let mut cargo = Command::new("cargo")
+            let cargo = Command::new("cargo")
                 .env("RUSTFLAGS", arch::RUSTFLAGS.join(" "))
                 .current_dir(project_root())
                 .args(args)
                 .spawn()
                 .expect("failed to spawn cargo");
 
-            cargo.wait().expect("Cargo failed");
+            wait_for_success(cargo, "Cargo");
         }
 
         Ok(())
diff --git a/builder/src/utils.rs b/builder/src/utils.rs
@@ -5,6 +5,7 @@ use anyhow::{anyhow, Result};
 use std::fs::{self, OpenOptions};
 use std::io::Write;
 use std::path::{Path, PathBuf};
+use std::process::Child;
 
 pub fn file_replace(file_in: &str, file_out: &str, target: &str, replace: &str) -> Result<()> {
     let content = fs::read_to_string(file_in)?;
@@ -26,3 +27,15 @@ pub fn project_root() -> PathBuf {
         .unwrap()
         .to_path_buf()
 }
+
+/// Waits for a child process to complete and panics if it fails.
+pub fn wait_for_success(mut child: Child, name: &str) {
+    let status = child
+        .wait()
+        .unwrap_or_else(|_| panic!("{name} failed to wait"));
+    assert!(
+        status.success(),
+        "{name} failed with exit code: {:?}",
+        status.code()
+    );
+}
