build: refuse source build of a binary-only changelog entry
dpkg-source errors with 'building source for a binary-only release' when asked to -b a tree whose newest changelog entry sets binary-only=yes: the source publication is already in the archive and is not being rebuilt. pkh instead built the fresh .dsc and then produced binNMU-style metadata referencing the *previous* version's .dsc and tarballs — behavior dpkg does not have at all. Mirror dpkg: run_source_build now refuses binary-only entries outright, which makes the previous-version references, the binNMU Source field and the Binary-Only-Changes handling in the source pipeline dead code — removed. Binary-only metadata stays in the binary pipeline, where it matches dpkg-genchanges/genbuildinfo (diff_binmu_binary_metadata). New tests: a unit test for the refusal, and a failure-parity differential asserting both dpkg-buildpackage -S and the native pipeline reject the same fixture.
This commit is contained in:
+109
-68
@@ -224,7 +224,8 @@ fn retry_after_revendor(
|
|||||||
/// Run the full native source-build pipeline in `cwd`.
|
/// Run the full native source-build pipeline in `cwd`.
|
||||||
///
|
///
|
||||||
/// Steps (mirroring `dpkg-buildpackage -S -I -i -nc -d`):
|
/// Steps (mirroring `dpkg-buildpackage -S -I -i -nc -d`):
|
||||||
/// 1. sanity checks and metadata resolution (changelog, control),
|
/// 1. sanity checks and metadata resolution (changelog, control); a
|
||||||
|
/// `binary-only=yes` changelog entry is refused, like `dpkg-source -b`,
|
||||||
/// 2. environment setup (`SOURCE_DATE_EPOCH`, `DEB_BUILD_OPTIONS`, arch vars),
|
/// 2. environment setup (`SOURCE_DATE_EPOCH`, `DEB_BUILD_OPTIONS`, arch vars),
|
||||||
/// 3. signing decision (key discovery, UNRELEASED handling),
|
/// 3. signing decision (key discovery, UNRELEASED handling),
|
||||||
/// 4. `dpkg-source --before-build` then `dpkg-source -b`,
|
/// 4. `dpkg-source --before-build` then `dpkg-source -b`,
|
||||||
@@ -281,33 +282,25 @@ pub fn run_source_build(
|
|||||||
let mut entries = crate::debian::changelog::parse_changelog_entries(&changelog_path, Some(2))?;
|
let mut entries = crate::debian::changelog::parse_changelog_entries(&changelog_path, Some(2))?;
|
||||||
let entry = entries.remove(0);
|
let entry = entries.remove(0);
|
||||||
let previous_entry = entries.into_iter().next();
|
let previous_entry = entries.into_iter().next();
|
||||||
|
|
||||||
|
// A binary-only (binNMU) changelog entry is a binary publication whose
|
||||||
|
// source is already in the archive: like dpkg-source, refuse to build
|
||||||
|
// source for it instead of producing binNMU-style source metadata.
|
||||||
|
if entry.binary_only {
|
||||||
|
return Err(
|
||||||
|
"cannot build source for a binary-only publication: the changelog \
|
||||||
|
entry sets binary-only=yes (dpkg-source refuses it too)"
|
||||||
|
.into(),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
let ctrl = ControlInfo::parse(&control_path)?;
|
let ctrl = ControlInfo::parse(&control_path)?;
|
||||||
|
|
||||||
if let Some(u) = &ui {
|
if let Some(u) = &ui {
|
||||||
u.set_build_target(&entry.source, &entry.version.full(), &entry.distribution);
|
u.set_build_target(&entry.source, &entry.version.full(), &entry.distribution);
|
||||||
}
|
}
|
||||||
|
|
||||||
// binNMU builds reference the *previous* (source) version in their
|
let source_display = entry.source.clone();
|
||||||
// artifact metadata, like dpkg-genchanges/genbuildinfo do.
|
|
||||||
let previous_version = if entry.binary_only {
|
|
||||||
previous_entry.as_ref().map(|e| e.version.full())
|
|
||||||
} else {
|
|
||||||
None
|
|
||||||
};
|
|
||||||
let source_display = if entry.binary_only {
|
|
||||||
match previous_version.as_deref() {
|
|
||||||
Some(prev) => format!("{} ({})", entry.source, prev),
|
|
||||||
None => entry.source.clone(),
|
|
||||||
}
|
|
||||||
} else {
|
|
||||||
entry.source.clone()
|
|
||||||
};
|
|
||||||
let binary_only_changes = entry.binary_only.then(|| {
|
|
||||||
format!(
|
|
||||||
"{}\n\n -- {} <{}> {}",
|
|
||||||
entry.changes_field, entry.maintainer_name, entry.maintainer_email, entry.date_raw
|
|
||||||
)
|
|
||||||
});
|
|
||||||
|
|
||||||
let sversion = entry.version.no_epoch();
|
let sversion = entry.version.no_epoch();
|
||||||
let dsc_name = format!("{}_{}.dsc", entry.source, sversion);
|
let dsc_name = format!("{}_{}.dsc", entry.source, sversion);
|
||||||
@@ -431,27 +424,6 @@ pub fn run_source_build(
|
|||||||
.into());
|
.into());
|
||||||
}
|
}
|
||||||
|
|
||||||
// Binary-only uploads redistribute the *previous* source: metadata
|
|
||||||
// references the previous version's .dsc (which must already exist in
|
|
||||||
// the output directory), exactly like dpkg-genchanges/genbuildinfo.
|
|
||||||
let ref_dsc_name = match previous_version.as_deref().filter(|_| entry.binary_only) {
|
|
||||||
Some(prev) => {
|
|
||||||
let prev_version = crate::debian::DebianVersion::parse(prev)?;
|
|
||||||
let name = format!("{}_{}.dsc", entry.source, prev_version.no_epoch());
|
|
||||||
if !parent.join(&name).exists() {
|
|
||||||
return Err(format!(
|
|
||||||
"binary-only build requires the previous source '{} \
|
|
||||||
{}' to exist next to the package",
|
|
||||||
entry.source, prev
|
|
||||||
)
|
|
||||||
.into());
|
|
||||||
}
|
|
||||||
name
|
|
||||||
}
|
|
||||||
None => dsc_name.clone(),
|
|
||||||
};
|
|
||||||
let ref_dsc_path = parent.join(&ref_dsc_name);
|
|
||||||
|
|
||||||
// ------------------------------------------------------------------
|
// ------------------------------------------------------------------
|
||||||
// 6. .buildinfo generation (native dpkg-genbuildinfo equivalent)
|
// 6. .buildinfo generation (native dpkg-genbuildinfo equivalent)
|
||||||
// ------------------------------------------------------------------
|
// ------------------------------------------------------------------
|
||||||
@@ -461,7 +433,7 @@ pub fn run_source_build(
|
|||||||
// What the .buildinfo itself records: like dpkg-genbuildinfo, only the
|
// What the .buildinfo itself records: like dpkg-genbuildinfo, only the
|
||||||
// referenced .dsc — not the tarballs, and never the buildinfo itself.
|
// referenced .dsc — not the tarballs, and never the buildinfo itself.
|
||||||
let mut buildinfo_checksums = FileChecksums::new();
|
let mut buildinfo_checksums = FileChecksums::new();
|
||||||
buildinfo_checksums.add_file_as(&ref_dsc_path, &ref_dsc_name)?;
|
buildinfo_checksums.add_file_as(&dsc_path, &dsc_name)?;
|
||||||
|
|
||||||
let status_path = PathBuf::from("/var/lib/dpkg/status");
|
let status_path = PathBuf::from("/var/lib/dpkg/status");
|
||||||
let bd_fields = [ctrl.source.get("Build-Depends").unwrap_or("")];
|
let bd_fields = [ctrl.source.get("Build-Depends").unwrap_or("")];
|
||||||
@@ -474,7 +446,9 @@ pub fn run_source_build(
|
|||||||
binaries: Vec::new(), // source-only build
|
binaries: Vec::new(), // source-only build
|
||||||
architecture: "source".to_string(),
|
architecture: "source".to_string(),
|
||||||
version: entry.version.full(),
|
version: entry.version.full(),
|
||||||
binary_only_changes: binary_only_changes.clone(),
|
// A binary-only entry never reaches a source build (refused
|
||||||
|
// above), so the buildinfo never carries Binary-Only-Changes.
|
||||||
|
binary_only_changes: None,
|
||||||
build_origin: vendor.clone(),
|
build_origin: vendor.clone(),
|
||||||
build_architecture: arch_vars
|
build_architecture: arch_vars
|
||||||
.get("DEB_BUILD_ARCH")
|
.get("DEB_BUILD_ARCH")
|
||||||
@@ -515,14 +489,14 @@ pub fn run_source_build(
|
|||||||
// Pull the tarball checksums out of the referenced .dsc so they are
|
// Pull the tarball checksums out of the referenced .dsc so they are
|
||||||
// distributed through the .changes like dpkg-genchanges does, in the
|
// distributed through the .changes like dpkg-genchanges does, in the
|
||||||
// order the .dsc itself lists them.
|
// order the .dsc itself lists them.
|
||||||
let dsc_content = std::fs::read_to_string(&ref_dsc_path)
|
let dsc_content = std::fs::read_to_string(&dsc_path)
|
||||||
.map_err(|e| format!("cannot read '{}': {}", ref_dsc_path.display(), e))?;
|
.map_err(|e| format!("cannot read '{}': {}", dsc_path.display(), e))?;
|
||||||
let dsc_para = parse_paragraphs(crate::debian::control::strip_clearsigned_armour(
|
let dsc_para = parse_paragraphs(crate::debian::control::strip_clearsigned_armour(
|
||||||
&dsc_content,
|
&dsc_content,
|
||||||
))
|
))
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.next()
|
.next()
|
||||||
.ok_or_else(|| format!("'{}' is empty", ref_dsc_path.display()))?;
|
.ok_or_else(|| format!("'{}' is empty", dsc_path.display()))?;
|
||||||
|
|
||||||
// Whether the upload redistributes the upstream tarballs (dpkg
|
// Whether the upload redistributes the upstream tarballs (dpkg
|
||||||
// -sa/-si/-sd source styles). Stripping only applies to a split source
|
// -sa/-si/-sd source styles). Stripping only applies to a split source
|
||||||
@@ -542,7 +516,7 @@ pub fn run_source_build(
|
|||||||
continue;
|
continue;
|
||||||
};
|
};
|
||||||
for cl in parse_checksum_field(field, value)
|
for cl in parse_checksum_field(field, value)
|
||||||
.map_err(|e| format!("cannot parse '{}': {e}", ref_dsc_path.display()))?
|
.map_err(|e| format!("cannot parse '{}': {e}", dsc_path.display()))?
|
||||||
{
|
{
|
||||||
if !dsc_files.contains_key(&cl.name) {
|
if !dsc_files.contains_key(&cl.name) {
|
||||||
dsc_file_names.push(cl.name.clone());
|
dsc_file_names.push(cl.name.clone());
|
||||||
@@ -582,7 +556,7 @@ pub fn run_source_build(
|
|||||||
};
|
};
|
||||||
|
|
||||||
for name in &dsc_file_names {
|
for name in &dsc_file_names {
|
||||||
if name == &ref_dsc_name {
|
if name == &dsc_name {
|
||||||
continue; // already computed directly above
|
continue; // already computed directly above
|
||||||
}
|
}
|
||||||
if is_stripped(name) {
|
if is_stripped(name) {
|
||||||
@@ -600,7 +574,7 @@ pub fn run_source_build(
|
|||||||
let partial = dsc_files.get(name).ok_or_else(|| {
|
let partial = dsc_files.get(name).ok_or_else(|| {
|
||||||
format!(
|
format!(
|
||||||
"file '{name}' listed in '{}' has no checksum entry",
|
"file '{name}' listed in '{}' has no checksum entry",
|
||||||
ref_dsc_path.display()
|
dsc_path.display()
|
||||||
)
|
)
|
||||||
})?;
|
})?;
|
||||||
checksums.insert_entry(
|
checksums.insert_entry(
|
||||||
@@ -627,13 +601,9 @@ pub fn run_source_build(
|
|||||||
// dsc and tarballs use the source stanza defaults (not persisted into
|
// dsc and tarballs use the source stanza defaults (not persisted into
|
||||||
// debian/files, matching dpkg).
|
// debian/files, matching dpkg).
|
||||||
let mut changes_files = files_list.clone();
|
let mut changes_files = files_list.clone();
|
||||||
changes_files.add(FilesEntry::new(
|
changes_files.add(FilesEntry::new(&dsc_name, ctrl.section(), ctrl.priority()));
|
||||||
&ref_dsc_name,
|
|
||||||
ctrl.section(),
|
|
||||||
ctrl.priority(),
|
|
||||||
));
|
|
||||||
for name in &dsc_file_names {
|
for name in &dsc_file_names {
|
||||||
if name != &ref_dsc_name && !is_stripped(name) {
|
if name != &dsc_name && !is_stripped(name) {
|
||||||
changes_files.add(FilesEntry::new(name, ctrl.section(), ctrl.priority()));
|
changes_files.add(FilesEntry::new(name, ctrl.section(), ctrl.priority()));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -689,13 +659,9 @@ pub fn run_source_build(
|
|||||||
log::info!("Signing {}", dsc_name);
|
log::info!("Signing {}", dsc_name);
|
||||||
crate::utils::gpg::clearsign_file(&dsc_path, &keyid)?;
|
crate::utils::gpg::clearsign_file(&dsc_path, &keyid)?;
|
||||||
// The freshly built .dsc changed: refresh its digests in both
|
// The freshly built .dsc changed: refresh its digests in both
|
||||||
// checksum sets. For binary-only builds the metadata references the
|
// checksum sets.
|
||||||
// *previous* .dsc (untouched by this build), so there is nothing to
|
buildinfo_checksums.add_file_as(&dsc_path, &dsc_name)?;
|
||||||
// refresh.
|
checksums.add_file_as(&dsc_path, &dsc_name)?;
|
||||||
if !entry.binary_only {
|
|
||||||
buildinfo_checksums.add_file_as(&dsc_path, &dsc_name)?;
|
|
||||||
checksums.add_file_as(&dsc_path, &dsc_name)?;
|
|
||||||
}
|
|
||||||
// Re-render the .buildinfo from its own set (the .dsc only, like
|
// Re-render the .buildinfo from its own set (the .dsc only, like
|
||||||
// dpkg-genbuildinfo): it must not list the tarballs or itself.
|
// dpkg-genbuildinfo): it must not list the tarballs or itself.
|
||||||
buildinfo::save_buildinfo(&buildinfo_path, &render_buildinfo_doc(&buildinfo_checksums))?;
|
buildinfo::save_buildinfo(&buildinfo_path, &render_buildinfo_doc(&buildinfo_checksums))?;
|
||||||
@@ -1051,6 +1017,35 @@ mod tests {
|
|||||||
use super::*;
|
use super::*;
|
||||||
use crate::debian::DebianVersion;
|
use crate::debian::DebianVersion;
|
||||||
|
|
||||||
|
/// A binary-only (binNMU) changelog entry is a binary publication: like
|
||||||
|
/// `dpkg-source -b`, the source build must refuse it outright instead of
|
||||||
|
/// producing binNMU-style source metadata referencing the previous
|
||||||
|
/// version's `.dsc`.
|
||||||
|
#[test]
|
||||||
|
fn source_build_refuses_binary_only_changelog() {
|
||||||
|
let base = tempfile::tempdir().expect("tempdir");
|
||||||
|
let tree = base.path().join("hello-1.0");
|
||||||
|
std::fs::create_dir_all(tree.join("debian")).expect("mkdir tree");
|
||||||
|
std::fs::write(
|
||||||
|
tree.join("debian/changelog"),
|
||||||
|
"hello (1.0-1+b1) unstable; urgency=medium, binary-only=yes\n\n \
|
||||||
|
* Binary-only rebuild.\n\n -- A B <a@b.c> Mon, 01 Jan 2024 00:00:00 +0000\n",
|
||||||
|
)
|
||||||
|
.expect("write changelog");
|
||||||
|
std::fs::write(
|
||||||
|
tree.join("debian/control"),
|
||||||
|
"Source: hello\nMaintainer: A B <a@b.c>\n",
|
||||||
|
)
|
||||||
|
.expect("write control");
|
||||||
|
std::fs::write(tree.join("debian/rules"), "#!/usr/bin/make -f\n").expect("write rules");
|
||||||
|
|
||||||
|
let err = run_source_build(&tree, &SourceBuildOptions::default(), None)
|
||||||
|
.expect_err("binary-only entries must not build a source package");
|
||||||
|
let err = err.to_string();
|
||||||
|
assert!(err.contains("binary-only"), "{err}");
|
||||||
|
assert!(err.contains("source"), "{err}");
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn partial_checksum_defaults() {
|
fn partial_checksum_defaults() {
|
||||||
let p = PartialChecksum::default();
|
let p = PartialChecksum::default();
|
||||||
@@ -1277,6 +1272,8 @@ mod differential_tests {
|
|||||||
extra_source_fields: &'static [(&'static str, &'static str)],
|
extra_source_fields: &'static [(&'static str, &'static str)],
|
||||||
/// Version of the previous changelog entry, when the fixture has one.
|
/// Version of the previous changelog entry, when the fixture has one.
|
||||||
previous_version: Option<&'static str>,
|
previous_version: Option<&'static str>,
|
||||||
|
/// Mark the newest changelog entry `binary-only=yes` (binNMU).
|
||||||
|
binary_only_marker: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl FixtureSpec {
|
impl FixtureSpec {
|
||||||
@@ -1294,6 +1291,7 @@ mod differential_tests {
|
|||||||
patches: &[],
|
patches: &[],
|
||||||
extra_source_fields: &[],
|
extra_source_fields: &[],
|
||||||
previous_version: None,
|
previous_version: None,
|
||||||
|
binary_only_marker: false,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1306,9 +1304,14 @@ mod differential_tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn changelog(&self) -> String {
|
fn changelog(&self) -> String {
|
||||||
|
let params = if self.binary_only_marker {
|
||||||
|
format!("urgency={}, binary-only=yes", self.urgency)
|
||||||
|
} else {
|
||||||
|
format!("urgency={}", self.urgency)
|
||||||
|
};
|
||||||
let mut out = format!(
|
let mut out = format!(
|
||||||
"{} ({}) {}; urgency={}\n\n",
|
"{} ({}) {}; {}\n\n",
|
||||||
self.name, self.version, self.distribution, self.urgency
|
self.name, self.version, self.distribution, params
|
||||||
);
|
);
|
||||||
for line in self.body {
|
for line in self.body {
|
||||||
out.push_str(" * ");
|
out.push_str(" * ");
|
||||||
@@ -2108,8 +2111,10 @@ Provides: virtual-thing (= 2.0), plain-virtual
|
|||||||
spec.body = &["* Binary-only rebuild."];
|
spec.body = &["* Binary-only rebuild."];
|
||||||
spec.previous_version = Some("1.0-1");
|
spec.previous_version = Some("1.0-1");
|
||||||
|
|
||||||
// Binary-only metadata references the previous version's .dsc, which
|
// A binNMU-style version number (+b1) with a previous entry, but
|
||||||
// must already exist next to the package tree.
|
// WITHOUT the binary-only marker: this is a plain source build, and
|
||||||
|
// the sibling previous-version .dsc (as left by an earlier source
|
||||||
|
// build) must not change either side's output.
|
||||||
let base = tempfile::tempdir().expect("tempdir");
|
let base = tempfile::tempdir().expect("tempdir");
|
||||||
let tree = write_fixture(base.path(), &spec);
|
let tree = write_fixture(base.path(), &spec);
|
||||||
let prev_dsc = format!(
|
let prev_dsc = format!(
|
||||||
@@ -2124,6 +2129,42 @@ Provides: virtual-thing (= 2.0), plain-virtual
|
|||||||
differential_on_tree(&tree, &SourceBuildOptions::default());
|
differential_on_tree(&tree, &SourceBuildOptions::default());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Both implementations must refuse a source build of a changelog entry
|
||||||
|
/// marked `binary-only=yes`: dpkg-source errors out, and pkh must refuse
|
||||||
|
/// the same way instead of producing binNMU-style source metadata
|
||||||
|
/// referencing the previous version.
|
||||||
|
#[test]
|
||||||
|
fn diff_source_build_rejects_binary_only_marker() {
|
||||||
|
let mut spec = FixtureSpec::new("pkh-diff-s", "1.0-1+b1", "unstable");
|
||||||
|
spec.body = &["* Binary-only rebuild."];
|
||||||
|
spec.previous_version = Some("1.0-1");
|
||||||
|
spec.binary_only_marker = true;
|
||||||
|
let base = tempfile::tempdir().expect("tempdir");
|
||||||
|
let tree = write_fixture(base.path(), &spec);
|
||||||
|
|
||||||
|
// Golden side: real dpkg refuses.
|
||||||
|
let status = crate::test_support::run_logged(
|
||||||
|
Command::new("dpkg-buildpackage").current_dir(&tree).args([
|
||||||
|
"-S",
|
||||||
|
"-I",
|
||||||
|
"-i",
|
||||||
|
"-nc",
|
||||||
|
"-d",
|
||||||
|
"--no-sign",
|
||||||
|
]),
|
||||||
|
)
|
||||||
|
.expect("run dpkg-buildpackage (is dpkg-dev installed?)");
|
||||||
|
assert!(
|
||||||
|
!status.success(),
|
||||||
|
"dpkg-buildpackage -S must refuse a binary-only changelog entry"
|
||||||
|
);
|
||||||
|
|
||||||
|
// Ours: the native pipeline refuses likewise.
|
||||||
|
let err = run_source_build(&tree, &SourceBuildOptions::default(), None)
|
||||||
|
.expect_err("native source build must refuse a binary-only entry");
|
||||||
|
assert!(err.to_string().contains("binary-only"), "{err}");
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn diff_closes_bugs() {
|
fn diff_closes_bugs() {
|
||||||
let mut spec = FixtureSpec::new("pkh-diff-j", "2.0-1", "unstable");
|
let mut spec = FixtureSpec::new("pkh-diff-j", "2.0-1", "unstable");
|
||||||
|
|||||||
Reference in New Issue
Block a user