Properly parse PRs which introduce new images.yaml files, don't multi…

src/api_clients.rs

@@ -160,7 +160,7 @@ impl Client for RealClient {
             .await
             .context("failed to get pr commits")?
             .last()
-            .ok_or_else(|| anyhow!("PR contains no commits?"))?
+            .ok_or_else(|| anyhow!("PR {owner}/{repo}/pull/{pr_number} contains no commits?"))?
             .sha
             .clone())
     }
@@ -172,7 +172,8 @@ impl Client for RealClient {
             .octocrab
             .pulls(owner, repo)
             .pr_commits(pr_number)
-            .page(250u32)
+            .per_page(250)
+            .page(1u32)
             .send()
             .await
             .context("failed to get pr commits")?;

src/changes.rs

@@ -25,14 +25,40 @@ use crate::remote::Remote;
 pub struct RepoChangeset<C: Client> {
     pub name: String,
     pub remote: Remote<C>,
-    pub base_commit: String,
+    pub base_commit: Option<String>,
     pub head_commit: String,
     pub changes: Vec<Changeset>,
 }
 
+impl<C: Client> RepoChangeset<C> {
+    pub fn new(
+        name: String,
+        remote: crate::Remote<C>,
+        base_commit: Option<String>,
+        head_commit: String,
+    ) -> RepoChangeset<C> {
+        Self {
+            name,
+            remote,
+            base_commit,
+            head_commit,
+            changes: Vec::new(),
+        }
+    }
+}
+
 impl<C: Client + Sync + Send + 'static> RepoChangeset<C> {
     pub async fn analyze_commits(mut self) -> anyhow::Result<Self> {
-        let compare_commits = self.remote.compare(&self.base_commit, &self.head_commit).await?;
+        // if this is a newly introduced source, compare the commit to itself
+        if self.base_commit.is_none() {
+            self.base_commit = Some(self.head_commit.clone());
+            self.head_commit += "^1";
+        }
+
+        let compare_commits = self
+            .remote
+            .compare(self.base_commit.as_deref().unwrap_or(""), &self.head_commit)
+            .await?;
 
         let mut join_set = JoinSet::new();
         let remote = Arc::new(self.remote);

src/main.rs

@@ -21,6 +21,7 @@ mod helm_config;
 mod remote;
 mod repo;
 
+use std::collections::HashMap;
 use std::fs::File;
 use std::io::Write;
 use std::sync::LazyLock;
@@ -32,7 +33,7 @@ use changes::RepoChangeset;
 use clap::builder::styling::Style;
 use clap::builder::NonEmptyStringValueParser;
 use clap::{Parser, Subcommand};
-use git2::Repository;
+use git2::{Oid, Repository};
 use helm_config::ImageRefs;
 use remote::Remote;
 use tokio::task::JoinSet;
@@ -110,7 +111,7 @@ async fn main() -> Result<(), anyhow::Error> {
             let repo = RepoChangeset {
                 name: remote.repository.clone(),
                 remote,
-                base_commit: cli.base,
+                base_commit: Some(cli.base),
                 head_commit: cli.head,
                 changes: Vec::new(),
             };
@@ -157,32 +158,52 @@ fn find_values_yaml(
 
     for diff_delta in diff_tree.deltas() {
         let new_file = diff_delta.new_file();
-        if !new_file.exists() {
-            continue;
-        }
-
         let path = new_file.path().ok_or_else(|| anyhow!("failed to get file path"))?;
         if !path.ends_with("images.yaml") {
             continue;
         }
 
+        let new_image_refs = ImageRefs::parse(&repo, &new_file).context("while parsing new file")?;
+
         let old_file = diff_delta.old_file();
-        if !old_file.exists() {
-            continue;
+        let mut old_image_refs = ImageRefs {
+            container_images: HashMap::new(),
+        };
+        // only zeros means the file was newly created and there is no old file to parse
+        if old_file.id() != Oid::from_str("0000000000000000000000000000000000000000")? {
+            old_image_refs = ImageRefs::parse(&repo, &old_file).context("while parsing old file")?;
         }
 
-        let new_image_refs = ImageRefs::parse(&repo, &new_file).context("while parsing new file")?;
-        let old_image_refs = ImageRefs::parse(&repo, &old_file).context("while parsing old file")?;
         for (name, image) in &new_image_refs.container_images {
-            for source in &image.sources {
-                for container_image_source in &old_image_refs.container_images[name].sources {
-                    changes.push(RepoChangeset {
-                        name: name.clone(),
-                        remote: remote::Remote::parse(&source.repo)?,
-                        base_commit: source.commit.clone(),
-                        head_commit: container_image_source.commit.clone(),
-                        changes: Vec::new(),
-                    });
+            for new_source in &image.sources {
+                // Is this a new container image?
+                if !old_image_refs.container_images.contains_key(name) {
+                    changes.push(RepoChangeset::new(
+                        name.clone(),
+                        remote::Remote::parse(&new_source.repo)?,
+                        None,
+                        new_source.commit.clone(),
+                    ));
+                    continue;
+                }
+
+                for old_source in &old_image_refs.container_images[name].sources {
+                    // Did we previously have this source?
+                    if new_source.repo == old_source.repo {
+                        changes.push(RepoChangeset::new(
+                            name.clone(),
+                            remote::Remote::parse(&new_source.repo)?,
+                            Some(old_source.commit.clone()),
+                            new_source.commit.clone(),
+                        ));
+                    } else {
+                        changes.push(RepoChangeset::new(
+                            name.clone(),
+                            remote::Remote::parse(&new_source.repo)?,
+                            None,
+                            new_source.commit.clone(),
+                        ));
+                    }
                 }
             }
         }
@@ -207,7 +228,10 @@ fn print_changes(repo_changeset: &[RepoChangeset<RealClient>]) -> Result<(), any
     for change in repo_changeset {
         println_or_redirect(format!(
             "Name {} from {} moved from {} to {}",
-            change.name, change.remote.original, change.base_commit, change.head_commit,
+            change.name,
+            change.remote.original,
+            change.base_commit.as_deref().unwrap_or(""),
+            change.head_commit,
         ))?;
         println_or_redirect("| Commit link | Pull Request link | Approvals | Reviewer's verdict |".to_string())?;
         println_or_redirect("|-------------|-------------------|-----------|--------------------|".to_string())?;