diff --git a/src/api_clients.rs b/src/api_clients.rs index 1a1edfb..15d746d 100644 --- a/src/api_clients.rs +++ b/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")?; diff --git a/src/changes.rs b/src/changes.rs index 8c86560..2c75b80 100644 --- a/src/changes.rs +++ b/src/changes.rs @@ -25,14 +25,40 @@ use crate::remote::Remote; pub struct RepoChangeset { pub name: String, pub remote: Remote, - pub base_commit: String, + pub base_commit: Option, pub head_commit: String, pub changes: Vec, } +impl RepoChangeset { + pub fn new( + name: String, + remote: crate::Remote, + base_commit: Option, + head_commit: String, + ) -> RepoChangeset { + Self { + name, + remote, + base_commit, + head_commit, + changes: Vec::new(), + } + } +} + impl RepoChangeset { pub async fn analyze_commits(mut self) -> anyhow::Result { - 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); diff --git a/src/main.rs b/src/main.rs index 9102f8f..01e44fa 100644 --- a/src/main.rs +++ b/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]) -> 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())?;