Emit Depends and the 8 other dropped control fields

README.md

@@ -65,15 +65,20 @@ are skipped (with a printed note) on systems without them.
 
 `fakeroot dpkg-deb`, `debsigs`, `debsig-verify`, `gpg` (tests), `curl`.
 
+## Control fields
+
+`build` writes `Package`, `Version`, `Architecture`, `Maintainer`, `Section`,
+`Priority`, `Homepage`, `Installed-Size`, `Source`, `Suite`, `Codename`,
+`License`, and `Description` unconditionally, plus these when supplied:
+`Vendor`, `Authors`, `Depends`, `Pre-Depends`, `Recommends`, `Suggests`,
+`Conflicts`, `Replaces`, `Provides`.
+
+The optional fields are omitted entirely when unset or empty, rather than
+emitted with a blank value. Relationship fields are joined with `", "`.
+
 ## Known limitation
 
-The control-file template emits a fixed set of properties: `Package`,
-`Version`, `Architecture`, `Maintainer`, `Section`, `Priority`, `Homepage`,
-`Installed-Size`, `Source`, `Suite`, `Codename`, `License`.  `Depends`,
-`Suggests`, `Recommends`, `Pre-Depends`, `Conflicts`, `Replaces`, `Provides`,
-`Vendor`, and `Authors` are accepted on the CLI and validated by
-`verify content`, but are not yet written to `DEBIAN/control` — to be
-addressed in a follow-up.
+`Origin`, `Label`, and `Breaks` are not modelled and cannot be set.
 
 ## License
 

src/templates.rs

@@ -81,18 +81,57 @@ struct Property<'a> {
     value: String,
 }
 
-// NOTE: parity with OCaml — the original `format_control_file` only emits the
-// 12 hardcoded fields below.  Depends / Suggests / Recommends / Pre-Depends /
-// Conflicts / Replaces / Provides / Vendor / Authors are stored on the input
-// struct (and validated by the CLI) but are NOT written to the control file.
-// This appears to be a latent bug in the OCaml original; preserving behavior
-// for now, to be revisited as a separate change once parity is established.
+/// Appends an optional single-valued field, skipping it when blank.
+///
+/// `Vendor` and `Authors` have no meaningful empty form — emitting a bare
+/// `Vendor:` would be a syntactically odd (and useless) control stanza — so a
+/// blank value means "caller did not set this" and the field is omitted.
+fn push_scalar(props: &mut Vec<Property<'static>>, name: &'static str, value: &str) {
+    if !value.trim().is_empty() {
+        props.push(Property {
+            name,
+            value: value.to_string(),
+        });
+    }
+}
+
+/// Appends a relationship field (`Depends`, `Conflicts`, …) as a
+/// comma-separated list, skipping it when absent or empty.
+///
+/// Entries are trimmed and blanks dropped so that sloppy caller input (a
+/// trailing comma, an empty element) cannot produce a malformed
+/// `Depends: foo, ` line. The `", "` separator round-trips through
+/// `content_verifier`, which splits on `,` and trims.
+fn push_list(props: &mut Vec<Property<'static>>, name: &'static str, value: &Option<Vec<String>>) {
+    let Some(items) = value else {
+        return;
+    };
+
+    let joined = items
+        .iter()
+        .map(|s| s.trim())
+        .filter(|s| !s.is_empty())
+        .collect::<Vec<_>>()
+        .join(", ");
+
+    if !joined.is_empty() {
+        props.push(Property {
+            name,
+            value: joined,
+        });
+    }
+}
+
 pub fn format_control_file(input: &DebianControl) -> Result<String> {
     let mut env = Environment::new();
     env.add_template("control", DEBIAN_CONTROL_FILE_TEMPLATE)?;
     let tmpl = env.get_template("control")?;
 
-    let properties: Vec<Property> = vec![
+    // The twelve fields that were always emitted stay unconditional, blank or
+    // not, so packages built before this change keep byte-identical stanzas
+    // (`Installed-Size:` and `Source:` are routinely empty in practice).
+    // Everything added here is emitted only when the caller supplied it.
+    let mut properties: Vec<Property> = vec![
         Property {
             name: "Package",
             value: input.package_name.clone(),
@@ -109,6 +148,12 @@ pub fn format_control_file(input: &DebianControl) -> Result<String> {
             name: "Maintainer",
             value: input.package_maintainer.clone(),
         },
+    ];
+
+    push_scalar(&mut properties, "Vendor", &input.vendor);
+    push_scalar(&mut properties, "Authors", &input.package_authors);
+
+    properties.extend([
         Property {
             name: "Section",
             value: input.package_section.clone(),
@@ -125,6 +170,18 @@ pub fn format_control_file(input: &DebianControl) -> Result<String> {
             name: "Installed-Size",
             value: input.package_installed_size.clone(),
         },
+    ]);
+
+    // Relationship fields follow Installed-Size, per Debian convention.
+    push_list(&mut properties, "Depends", &input.depends);
+    push_list(&mut properties, "Pre-Depends", &input.pre_depends);
+    push_list(&mut properties, "Recommends", &input.recommended_depends);
+    push_list(&mut properties, "Suggests", &input.suggested_depends);
+    push_list(&mut properties, "Conflicts", &input.conflicts);
+    push_list(&mut properties, "Replaces", &input.replaces);
+    push_list(&mut properties, "Provides", &input.provides);
+
+    properties.extend([
         Property {
             name: "Source",
             value: input.package_source.clone(),
@@ -141,7 +198,7 @@ pub fn format_control_file(input: &DebianControl) -> Result<String> {
             name: "License",
             value: input.license.clone(),
         },
-    ];
+    ]);
 
     tmpl.render(context! {
         description => &input.package_description,
@@ -151,3 +208,188 @@ pub fn format_control_file(input: &DebianControl) -> Result<String> {
     })
     .context("rendering control template")
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// A control input with every optional field unset, so each test can set
+    /// just the one it cares about.
+    fn bare() -> DebianControl {
+        DebianControl {
+            package_name: "mina-daemon".into(),
+            version: "1.2.3".into(),
+            vendor: String::new(),
+            package_authors: String::new(),
+            package_maintainer: "O(1)Labs <build@o1labs.org>".into(),
+            package_description: "Mina daemon".into(),
+            package_section: "base".into(),
+            package_priority: "optional".into(),
+            package_homepage: "https://minaprotocol.com/".into(),
+            package_installed_size: String::new(),
+            package_source: String::new(),
+            architecture: "amd64".into(),
+            suite: "stable".into(),
+            codename: "bookworm".into(),
+            depends: None,
+            suggested_depends: None,
+            recommended_depends: None,
+            pre_depends: None,
+            conflicts: None,
+            replaces: None,
+            provides: None,
+            license: "Apache-2.0".into(),
+            githash: "abc1234".into(),
+            buildurl: "https://ci.example/1".into(),
+        }
+    }
+
+    fn field(out: &str, name: &str) -> Option<String> {
+        out.lines()
+            .find(|l| l.starts_with(&format!("{}: ", name)) || l.trim_end() == format!("{}:", name))
+            .map(|l| l[name.len() + 1..].trim().to_string())
+    }
+
+    #[test]
+    fn emits_depends_when_set() {
+        let mut input = bare();
+        input.depends = Some(vec!["libssl3".into(), "libgmp10".into()]);
+
+        let out = format_control_file(&input).unwrap();
+
+        assert_eq!(field(&out, "Depends").as_deref(), Some("libssl3, libgmp10"));
+    }
+
+    #[test]
+    fn emits_every_relationship_field() {
+        let mut input = bare();
+        input.depends = Some(vec!["a".into()]);
+        input.pre_depends = Some(vec!["b".into()]);
+        input.recommended_depends = Some(vec!["c".into()]);
+        input.suggested_depends = Some(vec!["d".into()]);
+        input.conflicts = Some(vec!["e".into()]);
+        input.replaces = Some(vec!["f".into()]);
+        input.provides = Some(vec!["g".into()]);
+        input.vendor = "O(1)Labs".into();
+        input.package_authors = "Mina".into();
+
+        let out = format_control_file(&input).unwrap();
+
+        for (name, expected) in [
+            ("Depends", "a"),
+            ("Pre-Depends", "b"),
+            ("Recommends", "c"),
+            ("Suggests", "d"),
+            ("Conflicts", "e"),
+            ("Replaces", "f"),
+            ("Provides", "g"),
+            ("Vendor", "O(1)Labs"),
+            ("Authors", "Mina"),
+        ] {
+            assert_eq!(
+                field(&out, name).as_deref(),
+                Some(expected),
+                "{} wrong in:\n{}",
+                name,
+                out
+            );
+        }
+    }
+
+    #[test]
+    fn omits_unset_optional_fields() {
+        let out = format_control_file(&bare()).unwrap();
+
+        for name in [
+            "Depends",
+            "Pre-Depends",
+            "Recommends",
+            "Suggests",
+            "Conflicts",
+            "Replaces",
+            "Provides",
+            "Vendor",
+            "Authors",
+        ] {
+            assert!(
+                !out.contains(&format!("{}:", name)),
+                "{} should be absent from:\n{}",
+                name,
+                out
+            );
+        }
+    }
+
+    /// An empty list is "caller set it to nothing", which must not produce a
+    /// bare `Depends:` — dpkg rejects a relationship field with no value.
+    #[test]
+    fn omits_empty_and_blank_only_lists() {
+        let mut input = bare();
+        input.depends = Some(vec![]);
+        input.conflicts = Some(vec!["".into(), "   ".into()]);
+
+        let out = format_control_file(&input).unwrap();
+
+        assert!(!out.contains("Depends:"), "got:\n{}", out);
+        assert!(!out.contains("Conflicts:"), "got:\n{}", out);
+    }
+
+    #[test]
+    fn trims_entries_and_drops_blanks() {
+        let mut input = bare();
+        input.depends = Some(vec!["  libssl3 ".into(), "".into(), "libgmp10".into()]);
+
+        let out = format_control_file(&input).unwrap();
+
+        assert_eq!(field(&out, "Depends").as_deref(), Some("libssl3, libgmp10"));
+    }
+
+    /// The twelve originally-emitted fields must keep appearing even when
+    /// blank, so existing packages keep byte-identical stanzas.
+    #[test]
+    fn always_emits_the_original_fields() {
+        let out = format_control_file(&bare()).unwrap();
+
+        for name in [
+            "Package",
+            "Version",
+            "Architecture",
+            "Maintainer",
+            "Section",
+            "Priority",
+            "Homepage",
+            "Installed-Size",
+            "Source",
+            "Suite",
+            "Codename",
+            "License",
+        ] {
+            assert!(
+                out.contains(&format!("{}:", name)),
+                "{} missing from:\n{}",
+                name,
+                out
+            );
+        }
+    }
+
+    /// The separator this writes must survive the reader in `content_verifier`,
+    /// which splits on `,` and trims — otherwise `build` and `verify content`
+    /// disagree about the same package.
+    #[test]
+    fn list_separator_round_trips_through_the_verifier_split() {
+        let mut input = bare();
+        input.depends = Some(vec!["libssl3".into(), "libgmp10".into(), "tzdata".into()]);
+
+        let out = format_control_file(&input).unwrap();
+        let rendered = field(&out, "Depends").unwrap();
+
+        let parsed: Vec<String> = rendered
+            .split(',')
+            .map(|s| s.trim().to_string())
+            .filter(|s| !s.is_empty())
+            .collect();
+
+        assert_eq!(parsed, vec!["libssl3", "libgmp10", "tzdata"]);
+    }
+}