From c18f1fe9c247665cf8aa2cf09b43fb063e29b3ed Mon Sep 17 00:00:00 2001 From: Valentin Haudiquet Date: Thu, 17 Sep 2026 23:34:26 +0200 Subject: [PATCH] changelog: parse entries with a shared limit-based helper Replace parse_previous_version/parse_previous_version_from_str with parse_changelog_entries(path, limit: Option), parsing up to the given number of entries (None: the whole file) newest-first through the same strict entry parser instead of a header-only scan. The single-entry helpers stay as thin wrappers, and callers needing the previous entry now get its full source name and version, not just the raw string. --- src/build/binary.rs | 35 +++---- src/build/mod.rs | 9 +- src/debian/changelog.rs | 211 ++++++++++++++++++++++++++++++++-------- src/debian/mod.rs | 4 +- 4 files changed, 192 insertions(+), 67 deletions(-) diff --git a/src/build/binary.rs b/src/build/binary.rs index 064b8e2..46e2cdc 100644 --- a/src/build/binary.rs +++ b/src/build/binary.rs @@ -14,9 +14,7 @@ use std::path::{Path, PathBuf}; use std::sync::Arc; use crate::context::Context; -use crate::debian::{ - ChecksumEntry, ControlInfo, FileChecksums, FilesList, parse_changelog_entry_from_str, -}; +use crate::debian::{ChecksumEntry, ControlInfo, FileChecksums, FilesList}; use super::parse_checksum_field; @@ -70,7 +68,10 @@ pub fn generate_binary_metadata( // Metadata sources inside the context // ------------------------------------------------------------------ let changelog_content = ctx.read_file(&package_dir.join("debian/changelog"))?; - let entry = parse_changelog_entry_from_str(&changelog_content)?; + let mut entries = + crate::debian::changelog::parse_changelog_entries_from_str(&changelog_content, Some(2))?; + let entry = entries.remove(0); + let previous_entry = entries.into_iter().next(); let control_content = ctx.read_file(&package_dir.join("debian/control"))?; let control = ControlInfo::parse_content(&control_content)?; @@ -147,22 +148,13 @@ pub fn generate_binary_metadata( // a changelog that cannot yield it is a hard error, like in the // source-build path. Reuse the changelog read above instead of // reading the file a second time. - let changelog_path = package_dir.join("debian/changelog"); - let prev = crate::debian::changelog::parse_previous_version_from_str(&changelog_content) - .map_err(|e| { - format!( - "cannot parse the previous version from '{}': {e}", - changelog_path.display() - ) - })?; - if let Some(prev) = prev { - source_display = format!("{} ({})", entry.source, prev); + if let Some(prev) = &previous_entry { + source_display = format!("{} ({})", entry.source, prev.version.full()); binary_only_changes = Some(format!( "{}\n\n -- {} <{}> {}", entry.changes_field, entry.maintainer_name, entry.maintainer_email, entry.date_raw )); - let prev_version = crate::debian::DebianVersion::parse(&prev)?; - let dsc_name = format!("{}_{}.dsc", entry.source, prev_version.no_epoch()); + let dsc_name = format!("{}_{}.dsc", entry.source, prev.version.no_epoch()); let dsc_path = upload_dir.join(&dsc_name); if ctx.exists(&dsc_path)? { include_dsc_artifacts(ctx, upload_dir, &dsc_name, &mut checksums)?; @@ -628,10 +620,10 @@ Files: } /// A binary-only (binNMU) build whose changelog cannot yield the - /// previous version (malformed second header, unbalanced parenthesis) - /// must fail the metadata generation with a diagnostic naming the - /// changelog, instead of silently emitting a plain `Source:` `.changes` - /// with no `Binary-Only-Changes` and no redistributed previous `.dsc`. + /// previous entry (malformed second header, unbalanced parenthesis) must + /// fail the metadata generation with a diagnostic naming the problem, + /// instead of silently emitting a plain `Source:` `.changes` with no + /// `Binary-Only-Changes` and no redistributed previous `.dsc`. #[test] fn binary_only_prev_version_parse_failure_errors_instead_of_wrong_metadata() { let changelog = "\ @@ -683,9 +675,8 @@ Description: test package let err = generate_binary_metadata(&ctx, &tree, base.path(), &opts) .expect_err("binary-only build with an unparseable changelog must fail"); let err = err.to_string(); - assert!(err.contains("debian/changelog"), "{err}"); - assert!(err.contains("previous version"), "{err}"); assert!(err.contains("unbalanced parenthesis"), "{err}"); + assert!(err.contains("1.0-1 unstable"), "{err}"); } /// An unreadable `debian/files` (e.g. permissions) must fail the diff --git a/src/build/mod.rs b/src/build/mod.rs index 5c6aba4..dd3a088 100644 --- a/src/build/mod.rs +++ b/src/build/mod.rs @@ -255,7 +255,12 @@ pub fn run_source_build( // ------------------------------------------------------------------ // 2. Metadata resolution // ------------------------------------------------------------------ - let entry = crate::debian::parse_changelog_entry(&changelog_path)?; + // The current entry plus the one below it: the previous entry drives + // both the binNMU metadata references and the orig-tarball inclusion + // decision. + let mut entries = crate::debian::changelog::parse_changelog_entries(&changelog_path, Some(2))?; + let entry = entries.remove(0); + let previous_entry = entries.into_iter().next(); let ctrl = ControlInfo::parse(&control_path)?; if let Some(u) = &ui { @@ -265,7 +270,7 @@ pub fn run_source_build( // binNMU builds reference the *previous* (source) version in their // artifact metadata, like dpkg-genchanges/genbuildinfo do. let previous_version = if entry.binary_only { - crate::debian::changelog::parse_previous_version(&changelog_path)? + previous_entry.as_ref().map(|e| e.version.full()) } else { None }; diff --git a/src/debian/changelog.rs b/src/debian/changelog.rs index 0859621..5efff47 100644 --- a/src/debian/changelog.rs +++ b/src/debian/changelog.rs @@ -36,8 +36,12 @@ pub struct ChangelogEntry { pub closes: Option, } -/// Parse the most recent entry of a Debian changelog file. -pub fn parse_changelog_entry(path: &Path) -> Result> { +/// Parse up to `limit` entries of a Debian changelog file, newest first +/// (`None` parses the whole file). +pub fn parse_changelog_entries( + path: &Path, + limit: Option, +) -> Result, Box> { let content = std::fs::read_to_string(path).map_err(|e| { format!( "failed to read changelog '{}': {}. Make sure you are running \ @@ -46,17 +50,60 @@ pub fn parse_changelog_entry(path: &Path) -> Result Result> { + parse_changelog_entries(path, Some(1)).map(|mut entries| entries.remove(0)) +} + +/// Parse the most recent changelog entry from its textual content. pub fn parse_changelog_entry_from_str( content: &str, ) -> Result> { + parse_changelog_entries_from_str(content, Some(1)).map(|mut entries| entries.remove(0)) +} + +/// Parse changelog entries from their textual content, newest first. +/// +/// `limit` bounds the number of parsed entries (`None` parses the whole +/// file). Content below the last entry that is not another entry header +/// (e.g. an older changelog kept in a non-Debian format) is ignored. +pub fn parse_changelog_entries_from_str( + content: &str, + limit: Option, +) -> Result, Box> { let origin = "changelog"; let mut lines = content.lines().peekable(); + let mut entries = Vec::new(); + loop { + if limit.is_some_and(|n| entries.len() >= n) { + break; + } + // Blank separators between entries. + while lines.peek().is_some_and(|l| l.trim().is_empty()) { + lines.next(); + } + let Some(next) = lines.peek() else { + break; + }; + if !entries.is_empty() && !looks_like_header(next.trim_end()) { + break; + } + entries.push(parse_one_entry(&mut lines, origin)?); + } + Ok(entries) +} +/// Parse one entry: header line, body, maintainer trailer. Parsing stops +/// without consuming the first line that is a trailer terminator, an emacs +/// local-variables block, or the next entry's header — the stream can then +/// be resumed for the following entry. +fn parse_one_entry( + lines: &mut std::iter::Peekable>, + origin: &str, +) -> Result> { // --- Header line: `package (version) distributions; urgency=medium[, key=value]` let header = loop { match lines.next() { @@ -104,19 +151,24 @@ pub fn parse_changelog_entry_from_str( // --- Body until trailer line ` -- Name Date` let mut body_lines: Vec = Vec::new(); let mut trailer: Option = None; - for line in lines { + loop { + let Some(line) = lines.peek().copied() else { + break; + }; let line = line.trim_end(); if line.starts_with(" -- ") { - trailer = Some(line.to_string()); + trailer = lines.next().map(|l| l.trim_end().to_string()); break; } - // Stop at an emacs local-variables block or a new entry header. + // Stop at an emacs local-variables block or a new entry header + // (both peeked, not consumed). if line.starts_with("Local variables:") { break; } if !line.trim().is_empty() && looks_like_header(line) && !body_lines.is_empty() { break; } + lines.next(); // Blank lines become "." like dpkg does for the Changes field. if line.trim().is_empty() { body_lines.push(".".to_string()); @@ -211,39 +263,6 @@ fn find_closes(body_lines: &[String]) -> Option { ) } -/// Return the version of the *previous* changelog entry (the second header -/// in the file), or `None` when only one entry exists. -pub fn parse_previous_version(path: &Path) -> Result, Box> { - let content = std::fs::read_to_string(path) - .map_err(|e| format!("failed to read changelog '{}': {}", path.display(), e))?; - parse_previous_version_from_str(&content) -} - -/// Return the version of the *previous* changelog entry from the textual -/// content of a changelog file. -pub fn parse_previous_version_from_str( - content: &str, -) -> Result, Box> { - let mut seen_first = false; - for line in content.lines() { - let line = line.trim_end(); - if looks_like_header(line) { - if !seen_first { - seen_first = true; - continue; - } - let open = line - .find('(') - .ok_or_else(|| format!("invalid changelog header: {line}"))?; - let close = line[open..] - .find(')') - .ok_or_else(|| format!("unbalanced parenthesis in changelog header '{line}'"))?; - return Ok(Some(line[open + 1..open + close].to_string())); - } - } - Ok(None) -} - /// Heuristic check for a changelog entry header line /// (`name (version) dist; urgency=...`). fn looks_like_header(line: &str) -> bool { @@ -309,4 +328,114 @@ pkg (1.0-1+b1) unstable; urgency=medium, binary-only=yes assert!(entry.binary_only); assert_eq!(entry.version.full(), "1.0-1+b1"); } + + const THREE_ENTRIES: &str = "\ +pkg (2.0-1) unstable; urgency=low + + * New upstream release. + + -- Pkh Tester Thu, 01 Jan 2026 00:00:00 +0000 + +pkg (1.4-2) unstable; urgency=medium + + * Revision bump. + + -- Pkh Tester Wed, 01 Jan 2025 00:00:00 +0000 + +pkg (1.4-1) unstable; urgency=medium + + * Initial release. + + -- Pkh Tester Sat, 01 Mar 2025 00:00:00 +0000 +"; + + #[test] + fn entries_parse_newest_first_with_limits() { + // Whole file. + let all = parse_changelog_entries_from_str(THREE_ENTRIES, None).unwrap(); + assert_eq!(all.len(), 3); + assert_eq!(all[0].version.full(), "2.0-1"); + assert_eq!(all[1].version.full(), "1.4-2"); + assert_eq!(all[2].version.full(), "1.4-1"); + + // Bounded limits. + assert_eq!( + parse_changelog_entries_from_str(THREE_ENTRIES, Some(1)) + .unwrap() + .len(), + 1 + ); + let two = parse_changelog_entries_from_str(THREE_ENTRIES, Some(2)).unwrap(); + assert_eq!(two.len(), 2); + assert_eq!(two[0].version.full(), "2.0-1"); + assert_eq!(two[1].version.full(), "1.4-2"); + + // A limit beyond the entry count yields everything. + assert_eq!( + parse_changelog_entries_from_str(THREE_ENTRIES, Some(10)) + .unwrap() + .len(), + 3 + ); + + // The single-entry helpers agree with a limit of 1. + let one = parse_changelog_entries_from_str(THREE_ENTRIES, Some(1)).unwrap(); + let via_helper = parse_changelog_entry_from_str(THREE_ENTRIES).unwrap(); + assert_eq!(one[0].version.full(), via_helper.version.full()); + assert_eq!(one[0].source, via_helper.source); + } + + #[test] + fn entries_ignore_trailing_foreign_content() { + let content = "\ +pkg (1.0) unstable; urgency=medium + + * Something. + + -- Pkh Tester Thu, 01 Jan 2026 00:00:00 +0000 + +older changelog kept in an ad-hoc format: +version 0.9 - some text, not a Debian entry +version 0.8 - more text +"; + let entries = parse_changelog_entries_from_str(content, None).unwrap(); + assert_eq!(entries.len(), 1); + assert_eq!(entries[0].version.full(), "1.0"); + } + + #[test] + fn entries_parse_body_of_later_entries() { + let entries = parse_changelog_entries_from_str(THREE_ENTRIES, Some(2)).unwrap(); + // The second entry's body and trailer are fully parsed, not merely + // its header line. + assert_eq!( + entries[1].changes_field, + "\npkg (1.4-2) unstable; urgency=medium\n.\n * Revision bump." + ); + assert_eq!(entries[1].maintainer_email, "pkh@example.com"); + assert_eq!(entries[1].urgency, "medium"); + } + + #[test] + fn entries_reject_malformed_later_entry() { + let content = "\ +pkg (1.0) unstable; urgency=medium + + * Something. + + -- Pkh Tester Thu, 01 Jan 2026 00:00:00 +0000 + +pkg (0.9) unstable; urgency=medium + + * No trailer below. +"; + assert!(parse_changelog_entries_from_str(content, None).is_err()); + // Not parsed when not requested. + assert_eq!( + parse_changelog_entries_from_str(content, Some(1)) + .unwrap() + .len(), + 1 + ); + } } diff --git a/src/debian/mod.rs b/src/debian/mod.rs index 6da6e49..5ba7422 100644 --- a/src/debian/mod.rs +++ b/src/debian/mod.rs @@ -20,8 +20,8 @@ pub mod files; pub mod version; pub use changelog::{ - ChangelogEntry, parse_changelog_entry, parse_changelog_entry_from_str, - parse_previous_version_from_str, + ChangelogEntry, parse_changelog_entries, parse_changelog_entries_from_str, + parse_changelog_entry, parse_changelog_entry_from_str, }; pub use checksums::{ChecksumKind, Entry as ChecksumEntry, FileChecksums}; pub use control::{