From 47b462ad617514d9fed7639c04aba29e161eb699 Mon Sep 17 00:00:00 2001
From: Valentin Haudiquet <valentin.haudiquet@canonical.com>
Date: Thu, 17 Sep 2026 19:05:15 +0200
Subject: [PATCH] build: merge inherited DEB_BUILD_OPTIONS instead of
 overwriting it

The source-build pipeline exported its computed DEB_BUILD_OPTIONS
verbatim, silently dropping options the user set in the environment
(e.g. terse) where dpkg-buildpackage prepends the inherited value.
Options are now merged inherited-first through a shared helper, with
whitespace normalized.
---
 src/build/env.rs | 83 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 80 insertions(+), 3 deletions(-)

diff --git a/src/build/env.rs b/src/build/env.rs
index 3abb9ec..90ccfea 100644
--- a/src/build/env.rs
+++ b/src/build/env.rs
@@ -12,12 +12,36 @@ pub fn num_parallel() -> usize {
         .unwrap_or(1)
 }
 
+/// Merge an inherited `DEB_BUILD_OPTIONS` value with options pkh computes
+/// itself.
+///
+/// `dpkg-buildpackage` prepends the environment's `DEB_BUILD_OPTIONS` to the
+/// options it derives (`parallel=N`, ...), so caller-set options such as
+/// `terse` or `nocheck` survive alongside pkh's own. The result is therefore
+/// the inherited options followed by `computed`, space-separated; each side is
+/// trimmed and its internal whitespace runs collapsed. An unset or blank
+/// inherited value yields just `computed`.
+pub fn merge_deb_build_options(inherited: Option<&str>, computed: &str) -> String {
+    let computed = normalize_build_options(computed);
+    match inherited.map(normalize_build_options) {
+        Some(inherited) if !inherited.is_empty() => format!("{} {}", inherited, computed),
+        _ => computed,
+    }
+}
+
+/// Trim and collapse internal whitespace in a `DEB_BUILD_OPTIONS` fragment.
+fn normalize_build_options(options: &str) -> String {
+    options.split_whitespace().collect::<Vec<_>>().join(" ")
+}
+
 /// Compute the environment variables exported before running any build step.
 ///
 /// Mirrors dpkg behavior:
 /// - `SOURCE_DATE_EPOCH` from the changelog entry timestamp
 ///   (<https://reproducible-builds.org/specs/source-date-epoch/>),
-/// - `DEB_BUILD_OPTIONS=parallel=N` (auto-detected job count),
+/// - `DEB_BUILD_OPTIONS`: any value inherited from the invoking environment
+///   (dpkg-buildpackage prepends it) followed by `parallel=N` (auto-detected
+///   job count),
 /// - `DEB_BUILD_PROFILES` when non-default profiles are requested.
 ///
 /// The locale is pinned to `C` (`LC_ALL`, which takes precedence over any
@@ -38,7 +62,10 @@ pub fn build_env(
     );
     env.insert(
         "DEB_BUILD_OPTIONS".to_string(),
-        format!("parallel={}", parallel),
+        merge_deb_build_options(
+            std::env::var("DEB_BUILD_OPTIONS").ok().as_deref(),
+            &format!("parallel={}", parallel),
+        ),
     );
     if !build_profiles.is_empty() {
         env.insert("DEB_BUILD_PROFILES".to_string(), build_profiles.join(","));
@@ -287,13 +314,63 @@ mod tests {
         assert_eq!(env.get("LANG").unwrap(), "C");
         assert_eq!(env.get("LC_ALL").unwrap(), "C");
         assert_eq!(env.get("SOURCE_DATE_EPOCH").unwrap(), "1787392800");
-        assert_eq!(env.get("DEB_BUILD_OPTIONS").unwrap(), "parallel=16");
+        // Reading the var is race-free; the expected value goes through the
+        // same merge so the assertion holds whatever the ambient environment
+        // carries.
+        let expected = merge_deb_build_options(
+            std::env::var("DEB_BUILD_OPTIONS").ok().as_deref(),
+            "parallel=16",
+        );
+        assert_eq!(env.get("DEB_BUILD_OPTIONS").unwrap(), &expected);
         assert!(!env.contains_key("DEB_BUILD_PROFILES"));
 
         let env = build_env(1, 4, &["nodoc".to_string(), "cross".to_string()]);
         assert_eq!(env.get("DEB_BUILD_PROFILES").unwrap(), "nodoc,cross");
     }
 
+    /// dpkg-buildpackage prepends the inherited `DEB_BUILD_OPTIONS`, so
+    /// user-set options survive alongside the computed ones.
+    #[test]
+    fn merge_prepends_inherited_options() {
+        assert_eq!(
+            merge_deb_build_options(Some("terse"), "parallel=16"),
+            "terse parallel=16"
+        );
+        assert_eq!(
+            merge_deb_build_options(Some("nocheck terse"), "parallel=4"),
+            "nocheck terse parallel=4"
+        );
+    }
+
+    /// An unset, empty or blank inherited value yields just the computed
+    /// options.
+    #[test]
+    fn merge_skips_empty_inherited() {
+        assert_eq!(merge_deb_build_options(None, "parallel=8"), "parallel=8");
+        assert_eq!(
+            merge_deb_build_options(Some(""), "parallel=8"),
+            "parallel=8"
+        );
+        assert_eq!(
+            merge_deb_build_options(Some("   "), "parallel=8"),
+            "parallel=8"
+        );
+    }
+
+    /// Both sides are trimmed and internal whitespace runs collapsed: no
+    /// leading/trailing space, no double spaces in the merged result.
+    #[test]
+    fn merge_normalizes_whitespace() {
+        assert_eq!(
+            merge_deb_build_options(Some("  terse  "), "parallel=2"),
+            "terse parallel=2"
+        );
+        assert_eq!(
+            merge_deb_build_options(Some("nocheck\t terse"), "parallel=2"),
+            "nocheck terse parallel=2"
+        );
+    }
+
     #[test]
     fn vendor_defaults() {
         assert!(default_build_profiles("debian").is_empty());