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<usize>), 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.
2026-09-17 23:34:42 +02:00
parent e7b35f5c37
commit c18f1fe9c2
4 changed files with 192 additions and 67 deletions
@@ -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
@@ -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
    };
@@ -36,8 +36,12 @@ pub struct ChangelogEntry {
    pub closes: Option<String>,
 }

-/// Parse the most recent entry of a Debian changelog file.
-pub fn parse_changelog_entry(path: &Path) -> Result<ChangelogEntry, Box<dyn std::error::Error>> {
+/// 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<usize>,
+) -> Result<Vec<ChangelogEntry>, Box<dyn std::error::Error>> {
    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<ChangelogEntry, Box<dyn std:
            e
        )
    })?;
-    parse_changelog_entry_from_str(&content)
+    parse_changelog_entries_from_str(&content, limit)
 }

-/// Parse the most recent changelog entry from its textual content. `origin`
-/// is used in error messages only.
+/// Parse the most recent entry of a Debian changelog file.
+pub fn parse_changelog_entry(path: &Path) -> Result<ChangelogEntry, Box<dyn std::error::Error>> {
+    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<ChangelogEntry, Box<dyn std::error::Error>> {
+    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<usize>,
+) -> Result<Vec<ChangelogEntry>, Box<dyn std::error::Error>> {
    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<std::str::Lines<'_>>,
+    origin: &str,
+) -> Result<ChangelogEntry, Box<dyn std::error::Error>> {
    // --- 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 <email>  Date`
    let mut body_lines: Vec<String> = Vec::new();
    let mut trailer: Option<String> = 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<String> {
    )
 }

-/// 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<Option<String>, Box<dyn std::error::Error>> {
-    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<Option<String>, Box<dyn std::error::Error>> {
-    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 <pkh@example.com>  Thu, 01 Jan 2026 00:00:00 +0000
+
+pkg (1.4-2) unstable; urgency=medium
+
+ * Revision bump.
+
+ -- Pkh Tester <pkh@example.com>  Wed, 01 Jan 2025 00:00:00 +0000
+
+pkg (1.4-1) unstable; urgency=medium
+
+ * Initial release.
+
+ -- Pkh Tester <pkh@example.com>  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 <pkh@example.com>  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 <pkh@example.com>  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
+        );
+    }
 }
@@ -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::{