From 4ab41e691a235e6e4993094d15655aba70505b22 Mon Sep 17 00:00:00 2001 From: Valentin Haudiquet Date: Thu, 17 Sep 2026 23:45:15 +0200 Subject: [PATCH] build: only redistribute the orig tarball on new upstream (-si) dpkg-genchanges includes the upstream tarballs in the .changes only when the upload brings a new upstream: no previous changelog entry, a changed upstream version or a renamed source. On a plain revision bump the tarball already sits in the archive, and dpkg strips it (and its .asc) from the distribution set. pkh's native source pipeline listed every .dsc-referenced tarball unconditionally, making every upload re-ship the orig. Implement the dpkg source styles as --orig auto|always|never (auto being the -si default; always/never are -sa/-sd), stripping the tarballs out of the changes, buildinfo-free checksum set and artifact list like dpkg, with the explicit 'never' ignored for native packages. Comparison uses the epoch-less upstream version, exactly like dpkg's version(). Differential tests against real dpkg cover revision bumps, new upstream versions and both forced styles. --- src/build/changes.rs | 133 +++++++++++++++++++++++++++++++ src/build/mod.rs | 185 +++++++++++++++++++++++++++++++++++-------- src/main.rs | 20 ++++- src/new/questions.rs | 6 +- 4 files changed, 308 insertions(+), 36 deletions(-) diff --git a/src/build/changes.rs b/src/build/changes.rs index 84f0015..52e37c2 100644 --- a/src/build/changes.rs +++ b/src/build/changes.rs @@ -4,10 +4,58 @@ use std::path::Path; +use super::OrigSourceMode; +use crate::debian::changelog::ChangelogEntry; use crate::debian::checksums::FileChecksums; use crate::debian::control::{Paragraph, write_paragraph}; use crate::debian::files::FilesList; +/// Compression suffixes dpkg recognizes on source tarballs. +const TARBALL_COMPRESSIONS: &[&str] = &[".gz", ".bz2", ".xz", ".lzma", ".zst"]; + +/// Whether this `.dsc`-listed file is an upstream orig tarball +/// (`*.orig.tar.` or a component tarball `*.orig-.tar.`), +/// mirroring dpkg-genchanges' strip pattern `\.orig(-.+)?\.tar\.$ext`. +pub fn is_orig_tarball(name: &str) -> bool { + TARBALL_COMPRESSIONS.iter().any(|ext| { + name.strip_suffix(ext) + .and_then(|s| s.strip_suffix(".tar")) + .is_some_and(|stem| stem.ends_with(".orig") || stem.contains(".orig-")) + }) +} + +/// Whether this `.dsc`-listed file is the Debian part of the source package +/// (`*.debian.tar.` for the 3.0 formats, `*.diff.` for 1.0). +pub fn is_debian_tarball_or_diff(name: &str) -> bool { + TARBALL_COMPRESSIONS.iter().any(|ext| { + name.ends_with(&format!(".debian.tar{ext}")) || name.ends_with(&format!(".diff{ext}")) + }) +} + +/// Whether the upload redistributes the upstream tarballs, mirroring the +/// dpkg-genchanges source styles: `Always`/`Never` are the forced +/// `-sa`/`-sd`, while `Auto` is the default `-si` — include them only when +/// there is no previous changelog entry (first upload) or the source name or +/// upstream version changed since it. Like dpkg, the comparison uses the +/// epoch-less upstream version: a plain revision bump reuses the tarball +/// already in the archive. +pub fn include_orig_tarball( + mode: OrigSourceMode, + current: &ChangelogEntry, + previous: Option<&ChangelogEntry>, +) -> bool { + match mode { + OrigSourceMode::Always => true, + OrigSourceMode::Never => false, + OrigSourceMode::Auto => match previous { + None => true, + Some(prev) => { + prev.source != current.source || prev.version.upstream != current.version.upstream + } + }, + } +} + /// Everything needed to render a `.changes` file. #[derive(Debug, Clone)] pub struct ChangesInput { @@ -159,6 +207,91 @@ pub fn save_changes(path: &Path, paragraph: &Paragraph) -> Result<(), Box ChangelogEntry { + crate::debian::changelog::parse_changelog_entries_from_str( + &format!( + "{src} ({ver}) unstable; urgency=medium\n\n * x\n\n \ + -- A B Thu, 01 Jan 2026 00:00:00 +0000\n" + ), + Some(1), + ) + .unwrap() + .remove(0) + } + + #[test] + fn orig_inclusion_matrix() { + let cur = entry("pkg", "1.4-2"); + let prev_same_upstream = entry("pkg", "1.4-1"); + let prev_new_upstream = entry("pkg", "2.0-1"); + let prev_renamed = entry("renamed", "1.4-1"); + + // -sa / -sd force the outcome. + assert!(include_orig_tarball( + OrigSourceMode::Always, + &cur, + Some(&prev_same_upstream) + )); + assert!(!include_orig_tarball( + OrigSourceMode::Never, + &cur, + Some(&prev_new_upstream) + )); + + // -si: first upload includes; a revision bump excludes; a new + // upstream version or a renamed source includes. + assert!(include_orig_tarball(OrigSourceMode::Auto, &cur, None)); + assert!(!include_orig_tarball( + OrigSourceMode::Auto, + &cur, + Some(&prev_same_upstream) + )); + assert!(include_orig_tarball( + OrigSourceMode::Auto, + &cur, + Some(&prev_new_upstream) + )); + assert!(include_orig_tarball( + OrigSourceMode::Auto, + &cur, + Some(&prev_renamed) + )); + + // The epoch is not part of the comparison, like dpkg's version(). + let cur_epoch = entry("pkg", "2:1.4-2"); + assert!(!include_orig_tarball( + OrigSourceMode::Auto, + &cur_epoch, + Some(&prev_same_upstream) + )); + } + #[test] fn description_formatting() { assert_eq!( diff --git a/src/build/mod.rs b/src/build/mod.rs index dd3a088..8847f45 100644 --- a/src/build/mod.rs +++ b/src/build/mod.rs @@ -27,6 +27,20 @@ use crate::debian::{ use crate::ui::deb::DebUi; use crate::ui::logfmt::{DpkgSourceClassifier, GenericClassifier}; +/// Whether the upload distributes the upstream orig tarballs (`--orig`), +/// mirroring the `dpkg-genchanges` source styles. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub enum OrigSourceMode { + /// Include them only when the upstream version changed since the + /// previous changelog entry (`-si`, dpkg's default). + #[default] + Auto, + /// Always include them (`-sa`). + Always, + /// Never include them (`-sd`). + Never, +} + /// Options for a native source-package build. #[derive(Debug, Clone, Default)] pub struct SourceBuildOptions { @@ -39,6 +53,10 @@ pub struct SourceBuildOptions { /// build (`-D`). `dpkg-buildpackage` skips `dpkg-checkbuilddeps` /// entirely for source-only builds unless forced. pub force_dep_check: bool, + /// Whether the upload distributes the upstream orig tarballs + /// (`--orig`). Defaults to [`OrigSourceMode::Auto`], like + /// `dpkg-genchanges`' `-si`. + pub orig_source: OrigSourceMode, } /// Artifacts produced by a successful source build. @@ -70,6 +88,7 @@ pub struct SourceBuildOutput { /// exactly once. pub fn build_source_package( cwd: Option<&Path>, + opts: SourceBuildOptions, ui: Option>, ) -> Result<(), Box> { // Default to the process's current working directory, resolved to an @@ -81,10 +100,10 @@ pub fn build_source_package( None => std::env::current_dir() .map_err(|e| format!("cannot determine the current working directory: {e}"))?, }; - let output = match run_source_build(&cwd, &SourceBuildOptions::default(), ui.clone()) { + let output = match run_source_build(&cwd, &opts, ui.clone()) { Ok(output) => output, Err(e) if e.downcast_ref::().is_some() => { - return retry_after_revendor(&cwd, ui, e); + return retry_after_revendor(&cwd, ui, opts, e); } Err(e) => { if let Some(u) = &ui { @@ -133,6 +152,7 @@ pub fn build_source_package( fn retry_after_revendor( cwd: &Path, ui: Option>, + opts: SourceBuildOptions, original: Box, ) -> Result<(), Box> { let interactive = std::io::stdin().is_terminal() && std::io::stdout().is_terminal(); @@ -197,8 +217,8 @@ fn retry_after_revendor( } crate::new::orig::create_vendor_component(cwd, &entry.source, uversion)?; - // 3. One retry. - run_source_build(cwd, &SourceBuildOptions::default(), ui).map(|_| ()) + // 3. One retry, with the options of the original attempt. + run_source_build(cwd, &opts, ui).map(|_| ()) } /// Run the full native source-build pipeline in `cwd`. @@ -498,6 +518,13 @@ pub fn run_source_build( .next() .ok_or_else(|| format!("'{}' is empty", ref_dsc_path.display()))?; + // Whether the upload redistributes the upstream tarballs (dpkg + // -sa/-si/-sd source styles). Stripping only applies to a split source + // package — a native one has no orig tarball, and an explicit `never` + // for one is only a warning, like dpkg-genchanges with -sd. + let include_orig = + changes::include_orig_tarball(opts.orig_source, &entry, previous_entry.as_ref()); + let mut tarball_paths = Vec::new(); let mut dsc_file_names: Vec = Vec::new(); let mut dsc_files: HashMap = HashMap::new(); @@ -523,10 +550,38 @@ pub fn run_source_build( slot.size = Some(cl.size); } } + + let has_debian_part = dsc_file_names + .iter() + .any(|n| changes::is_debian_tarball_or_diff(n)); + let strip_origs = !include_orig && has_debian_part; + if opts.orig_source == OrigSourceMode::Never && !has_debian_part { + log::warn!("ignoring --orig never for a native Debian package"); + } + if let Some(u) = &ui { + u.progress_message(if strip_origs { + "Not including original source code in upload" + } else { + "Including full source code in upload" + }); + } + // Stripped orig tarballs (and their detached .asc signatures) are not + // distributed at all: not hashed, not required on disk, like + // dpkg-genchanges. + let is_stripped = |name: &str| { + strip_origs + && (changes::is_orig_tarball(name) + || (name.ends_with(".asc") + && changes::is_orig_tarball(name.strip_suffix(".asc").unwrap_or(name)))) + }; + for name in &dsc_file_names { if name == &ref_dsc_name { continue; // already computed directly above } + if is_stripped(name) { + continue; + } let path = parent.join(name); if !path.exists() { return Err(format!( @@ -572,7 +627,7 @@ pub fn run_source_build( ctrl.priority(), )); for name in &dsc_file_names { - if name != &ref_dsc_name { + if name != &ref_dsc_name && !is_stripped(name) { changes_files.add(FilesEntry::new(name, ctrl.section(), ctrl.priority())); } } @@ -1209,8 +1264,8 @@ mod differential_tests { body: &'static [&'static str], patches: &'static [&'static str], extra_source_fields: &'static [(&'static str, &'static str)], - /// Two-entry changelog (previous entry) for binNMU cases. - with_previous_entry: bool, + /// Version of the previous changelog entry, when the fixture has one. + previous_version: Option<&'static str>, } impl FixtureSpec { @@ -1227,7 +1282,7 @@ mod differential_tests { body: &["* Something changed."], patches: &[], extra_source_fields: &[], - with_previous_entry: false, + previous_version: None, } } @@ -1250,13 +1305,7 @@ mod differential_tests { out.push('\n'); } out.push_str(&format!("\n -- {MAINTAINER} {DATE}\n")); - if self.with_previous_entry { - let prev = match self.version.split_once(':') { - Some((_, r)) => r.to_string(), - None => self.version.to_string(), - }; - // Turn "1.0-1+b1" into "1.0-1" for the previous entry. - let prev = prev.rsplit_once('+').map(|(p, _)| p).unwrap_or(&prev); + if let Some(prev) = self.previous_version { out.push_str(&format!( "\n{p} ({pv}) {d}; urgency={u}\n\n * Initial release.\n\n -- {MAINTAINER} {DATE}\n", p = self.name, @@ -1382,16 +1431,12 @@ mod differential_tests { ); } - fn run_dpkg(tree: &Path) { + fn run_dpkg(tree: &Path, source_style: &[&str]) { let status = crate::test_support::run_logged( - Command::new("dpkg-buildpackage").current_dir(tree).args([ - "-S", - "-I", - "-i", - "-nc", - "-d", - "--no-sign", - ]), + Command::new("dpkg-buildpackage") + .current_dir(tree) + .args(["-S", "-I", "-i", "-nc", "-d", "--no-sign"]) + .args(source_style), ) .expect("failed to run dpkg-buildpackage (is dpkg-dev installed?)"); assert!(status.success(), "dpkg-buildpackage failed"); @@ -1480,8 +1525,10 @@ mod differential_tests { } } - /// Build `src_tree` with both implementations and compare all artifacts. - fn differential_on_tree(src_tree: &Path) { + /// Build `src_tree` with both implementations and compare all artifacts, + /// the native side running with `opts` (dpkg receives the matching + /// source style so both sides make the same orig-tarball decision). + fn differential_on_tree(src_tree: &Path, opts: &SourceBuildOptions) { let src_parent = src_tree.parent().expect("tree has a parent directory"); let tree_name = src_tree.file_name().expect("tree has a name").to_owned(); @@ -1508,9 +1555,13 @@ mod differential_tests { let golden_tree = golden_root.join(&tree_name); let ours_tree = ours_root.join(&tree_name); - run_dpkg(&golden_tree); - run_source_build(&ours_tree, &SourceBuildOptions::default(), None) - .expect("native source pipeline failed"); + let source_style: &[&str] = match opts.orig_source { + OrigSourceMode::Auto => &[], + OrigSourceMode::Always => &["-sa"], + OrigSourceMode::Never => &["-sd"], + }; + run_dpkg(&golden_tree, source_style); + run_source_build(&ours_tree, opts, None).expect("native source pipeline failed"); let entry = crate::debian::parse_changelog_entry(&ours_tree.join("debian/changelog")).unwrap(); @@ -1540,7 +1591,7 @@ mod differential_tests { fn differential_case(spec: &FixtureSpec) { let base = tempfile::tempdir().expect("tempdir"); let tree = write_fixture(base.path(), spec); - differential_on_tree(&tree); + differential_on_tree(&tree, &SourceBuildOptions::default()); } /// Differential check of [`crate::debian::arch::arch_env`] against real @@ -1986,7 +2037,7 @@ Provides: virtual-thing (= 2.0), plain-virtual fn diff_binmu_binary_only() { let mut spec = FixtureSpec::new("pkh-diff-i", "1.0-1+b1", "unstable"); spec.body = &["* Binary-only rebuild."]; - spec.with_previous_entry = true; + spec.previous_version = Some("1.0-1"); // Binary-only metadata references the previous version's .dsc, which // must already exist next to the package tree. @@ -2001,7 +2052,7 @@ Provides: virtual-thing (= 2.0), plain-virtual prev_dsc, ) .expect("write previous dsc"); - differential_on_tree(&tree); + differential_on_tree(&tree, &SourceBuildOptions::default()); } #[test] @@ -2029,6 +2080,74 @@ Provides: virtual-thing (= 2.0), plain-virtual differential_case(&spec); } + /// The default dpkg-genchanges source style (-si): a revision bump + /// within the same upstream version must NOT redistribute the orig + /// tarball in the `.changes`. + #[test] + fn diff_quilt_revision_bump_excludes_orig() { + let mut spec = FixtureSpec::new("pkh-diff-n", "1.4-2", "unstable"); + spec.format = "3.0 (quilt)"; + spec.patches = &[ + "--- a/hello.txt\n+++ b/hello.txt\n@@ -1 +1 @@\n-upstream content v1\n+patched by debian\n", + ]; + spec.previous_version = Some("1.4-1"); + differential_case(&spec); + } + + /// Conversely, a new upstream version must redistribute the orig + /// tarball, even though a previous entry exists. + #[test] + fn diff_quilt_new_upstream_includes_orig() { + let mut spec = FixtureSpec::new("pkh-diff-o", "2.0-1", "unstable"); + spec.format = "3.0 (quilt)"; + spec.patches = &[ + "--- a/hello.txt\n+++ b/hello.txt\n@@ -1 +1 @@\n-upstream content v1\n+patched by debian\n", + ]; + spec.previous_version = Some("1.4-2"); + differential_case(&spec); + } + + /// `--orig always` (-sa) forces the tarball into a same-upstream + /// revision bump upload. + #[test] + fn diff_orig_always_forces_inclusion() { + let mut spec = FixtureSpec::new("pkh-diff-p", "1.4-2", "unstable"); + spec.format = "3.0 (quilt)"; + spec.patches = &[ + "--- a/hello.txt\n+++ b/hello.txt\n@@ -1 +1 @@\n-upstream content v1\n+patched by debian\n", + ]; + spec.previous_version = Some("1.4-1"); + let base = tempfile::tempdir().expect("tempdir"); + let tree = write_fixture(base.path(), &spec); + differential_on_tree( + &tree, + &SourceBuildOptions { + orig_source: OrigSourceMode::Always, + ..Default::default() + }, + ); + } + + /// `--orig never` (-sd) forces the tarball out of a new-upstream upload. + #[test] + fn diff_orig_never_forces_exclusion() { + let mut spec = FixtureSpec::new("pkh-diff-q", "2.0-1", "unstable"); + spec.format = "3.0 (quilt)"; + spec.patches = &[ + "--- a/hello.txt\n+++ b/hello.txt\n@@ -1 +1 @@\n-upstream content v1\n+patched by debian\n", + ]; + spec.previous_version = Some("1.4-2"); + let base = tempfile::tempdir().expect("tempdir"); + let tree = write_fixture(base.path(), &spec); + differential_on_tree( + &tree, + &SourceBuildOptions { + orig_source: OrigSourceMode::Never, + ..Default::default() + }, + ); + } + /// Differential check of a single real archive package: pull it with /// pkh's own [`crate::pull`] (archive download mode) from `dist` /// (optionally `series`), then compare artifacts produced by real @@ -2072,7 +2191,7 @@ Provides: virtual-thing (= 2.0), plain-virtual dist, series.unwrap_or("latest") ); - differential_on_tree(&tree); + differential_on_tree(&tree, &SourceBuildOptions::default()); } #[test] diff --git a/src/main.rs b/src/main.rs index e42d2ff..c7005d5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -184,7 +184,10 @@ fn main() { .subcommand( Command::new("build") .about("Build the source package (into a .dsc)") - .arg(arg!(--verbose "Show raw tool output instead of the live build view").required(false)), + .arg(arg!(--verbose "Show raw tool output instead of the live build view").required(false)) + .arg(arg!(--orig "Original source tarball in the upload [auto, always, never] (default: auto)").required(false) + .long_help("Whether the upload distributes the original source tarball(s), like dpkg-genchanges' -sa/-si/-sd source styles.\nauto: include them only when the upstream version changed since the previous changelog entry (the default);\nalways: force inclusion, even for a revision bump of the same upstream version;\nnever: never include them, even for a new upstream version.\nAn explicit value is ignored with a warning for native packages (they have no separate orig tarball).") + .value_parser(["auto", "always", "never"])), ) .subcommand( Command::new("put") @@ -495,7 +498,20 @@ fn main() { Some(std::sync::Arc::new(pkh::ui::deb::DebUi::new(&multi))) }; - if let Err(e) = pkh::build::build_source_package(Some(&cwd), ui) { + let orig_source = match sub_matches.get_one::("orig").map(String::as_str) { + Some("always") => pkh::build::OrigSourceMode::Always, + Some("never") => pkh::build::OrigSourceMode::Never, + _ => pkh::build::OrigSourceMode::Auto, + }; + + if let Err(e) = pkh::build::build_source_package( + Some(&cwd), + pkh::build::SourceBuildOptions { + orig_source, + ..Default::default() + }, + ui, + ) { error!("{}", e); // Unmet build dependencies/conflicts exit with status 3, // like dpkg-buildpackage does. diff --git a/src/new/questions.rs b/src/new/questions.rs index ebc1d36..ee0d6bd 100644 --- a/src/new/questions.rs +++ b/src/new/questions.rs @@ -598,7 +598,11 @@ pub async fn offer_verification( } let ui = Some(std::sync::Arc::new(crate::ui::deb::DebUi::new(multi))); - if let Err(e) = crate::build::build_source_package(Some(&tree), ui) { + if let Err(e) = crate::build::build_source_package( + Some(&tree), + crate::build::SourceBuildOptions::default(), + ui, + ) { log::error!("Verification source build failed: {e}"); log::info!( "The scaffolded tree is intact. Inspect it, then retry with \