diff --git a/README.md b/README.md index 7171011..774ca37 100644 --- a/README.md +++ b/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 diff --git a/src/templates.rs b/src/templates.rs index 3f39da1..f15708e 100644 --- a/src/templates.rs +++ b/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>, 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>, name: &'static str, value: &Option>) { + let Some(items) = value else { + return; + }; + + let joined = items + .iter() + .map(|s| s.trim()) + .filter(|s| !s.is_empty()) + .collect::>() + .join(", "); + + if !joined.is_empty() { + props.push(Property { + name, + value: joined, + }); + } +} + pub fn format_control_file(input: &DebianControl) -> Result { let mut env = Environment::new(); env.add_template("control", DEBIAN_CONTROL_FILE_TEMPLATE)?; let tmpl = env.get_template("control")?; - let properties: Vec = 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 = vec![ Property { name: "Package", value: input.package_name.clone(), @@ -109,6 +148,12 @@ pub fn format_control_file(input: &DebianControl) -> Result { 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 { 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 { 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 { }) .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 ".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 { + 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 = rendered + .split(',') + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) + .collect(); + + assert_eq!(parsed, vec!["libssl3", "libgmp10", "tzdata"]); + } +}