diff --git a/Cargo.lock b/Cargo.lock index ed1f183..6ad8a94 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8143,6 +8143,7 @@ dependencies = [ "gix-worktree", "gix-worktree-state", "ignore", + "log", "nostr", "serde", "serde_json", diff --git a/crates/signed_git/Cargo.toml b/crates/signed_git/Cargo.toml index 37eb6c5..75f0dfa 100644 --- a/crates/signed_git/Cargo.toml +++ b/crates/signed_git/Cargo.toml @@ -10,10 +10,11 @@ signed_core = { path = "../signed_core" } nostr.workspace = true serde.workspace = true serde_json.workspace = true +log.workspace = true +anyhow.workspace = true gix = { workspace = true, features = ["revision", "blob-diff"] } gix-worktree = "0.57" gix-worktree-state = "0.35" -anyhow.workspace = true diffy = "0.5" ignore = "0.4" diff --git a/crates/signed_git/src/cache.rs b/crates/signed_git/src/cache.rs index d55ddea..dfb40fa 100644 --- a/crates/signed_git/src/cache.rs +++ b/crates/signed_git/src/cache.rs @@ -43,7 +43,9 @@ impl GitCache { let path = self.repo_path(addr); if let Some(repo) = self.open(addr)? { - repo.fetch().ok(); + if let Err(error) = repo.fetch() { + log::warn!("failed to refresh the cached repository: {error:#}"); + } return Ok(repo); } diff --git a/crates/signed_git/src/diff.rs b/crates/signed_git/src/diff.rs index c1ad2e9..1c315f2 100644 --- a/crates/signed_git/src/diff.rs +++ b/crates/signed_git/src/diff.rs @@ -110,6 +110,74 @@ impl CommitDiff { } } +pub(crate) struct HunkBuilder { + old_start: u32, + old_lines: u32, + new_start: u32, + new_lines: u32, + old: u32, + new: u32, + lines: Vec, + insertions: usize, + deletions: usize, +} + +impl HunkBuilder { + /// Starts a hunk spanning the given old and new ranges. + pub(crate) fn new(old_start: u32, old_lines: u32, new_start: u32, new_lines: u32) -> Self { + Self { + old_start, + old_lines, + new_start, + new_lines, + old: old_start, + new: new_start, + lines: Vec::new(), + insertions: 0, + deletions: 0, + } + } + + /// Appends a line, numbering it and counting insertions and deletions. + pub(crate) fn push(&mut self, kind: DiffLineKind, text: impl Into) { + let (old_no, new_no) = match kind { + DiffLineKind::Context => { + let numbers = (Some(self.old), Some(self.new)); + self.old += 1; + self.new += 1; + numbers + } + DiffLineKind::Addition => { + self.insertions += 1; + let number = Some(self.new); + self.new += 1; + (None, number) + } + DiffLineKind::Deletion => { + self.deletions += 1; + let number = Some(self.old); + self.old += 1; + (number, None) + } + }; + + self.lines + .push(DiffLine::new(kind, old_no, new_no, text.into())); + } + + /// Finishes the hunk and returns it with its insertion and deletion counts. + pub(crate) fn finish(self) -> (DiffHunk, usize, usize) { + let hunk = DiffHunk::new( + self.old_start, + self.old_lines, + self.new_start, + self.new_lines, + self.lines, + ); + (hunk, self.insertions, self.deletions) + } +} + pub(crate) struct HunkCollector<'a> { hunks: &'a mut Vec, insertions: &'a mut usize, @@ -140,43 +208,26 @@ impl ConsumeHunk for HunkCollector<'_> { header: HunkHeader, lines: &[(GixLineKind, &[u8])], ) -> std::io::Result<()> { - let mut old_ln = header.before_hunk_start; - let mut new_ln = header.after_hunk_start; - let mut out = Vec::with_capacity(lines.len()); - - for (kind, content) in lines { - let text = String::from_utf8_lossy(content).into_owned(); - let line = match kind { - GixLineKind::Context => { - let line = - DiffLine::new(DiffLineKind::Context, Some(old_ln), Some(new_ln), text); - old_ln += 1; - new_ln += 1; - line - } - GixLineKind::Remove => { - *self.deletions += 1; - let line = DiffLine::new(DiffLineKind::Deletion, Some(old_ln), None, text); - old_ln += 1; - line - } - GixLineKind::Add => { - *self.insertions += 1; - let line = DiffLine::new(DiffLineKind::Addition, None, Some(new_ln), text); - new_ln += 1; - line - } - }; - out.push(line); - } - - self.hunks.push(DiffHunk::new( + let mut builder = HunkBuilder::new( header.before_hunk_start, header.before_hunk_len, header.after_hunk_start, header.after_hunk_len, - out, - )); + ); + + for (kind, content) in lines { + let kind = match kind { + GixLineKind::Context => DiffLineKind::Context, + GixLineKind::Remove => DiffLineKind::Deletion, + GixLineKind::Add => DiffLineKind::Addition, + }; + builder.push(kind, String::from_utf8_lossy(content).into_owned()); + } + + let (hunk, insertions, deletions) = builder.finish(); + *self.insertions += insertions; + *self.deletions += deletions; + self.hunks.push(hunk); Ok(()) } diff --git a/crates/signed_git/src/patch.rs b/crates/signed_git/src/patch.rs index b07931b..b9883a1 100644 --- a/crates/signed_git/src/patch.rs +++ b/crates/signed_git/src/patch.rs @@ -2,7 +2,7 @@ use anyhow::Result; use diffy::patch_set::{FileOperation, FilePatch, ParseOptions, PatchSet}; use diffy::{Hunk, Line}; -use crate::diff::{CommitDiff, DiffHunk, DiffLine, DiffLineKind, DiffStatus, FileDiff}; +use crate::diff::{CommitDiff, DiffHunk, DiffLineKind, DiffStatus, FileDiff, HunkBuilder}; use crate::history::FileCommit; pub struct PatchParser; @@ -169,17 +169,9 @@ impl PatchParser { if let Some(text) = patch.as_text() { for hunk in text.hunks() { - let hunk = Self::hunk_diff(hunk); - insertions += hunk - .lines - .iter() - .filter(|line| line.kind == DiffLineKind::Addition) - .count(); - deletions += hunk - .lines - .iter() - .filter(|line| line.kind == DiffLineKind::Deletion) - .count(); + let (hunk, hunk_insertions, hunk_deletions) = Self::hunk_diff(hunk); + insertions += hunk_insertions; + deletions += hunk_deletions; hunks.push(hunk); } } @@ -195,14 +187,17 @@ impl PatchParser { ) } - /// Converts a diffy hunk into a `DiffHunk` with per-line old and new numbers. - fn hunk_diff(hunk: &Hunk<'_, str>) -> DiffHunk { + /// Converts a diffy hunk into a `DiffHunk` with its insertion and deletion counts. + fn hunk_diff(hunk: &Hunk<'_, str>) -> (DiffHunk, usize, usize) { let old_range = hunk.old_range(); let new_range = hunk.new_range(); - let mut old = old_range.start() as u32; - let mut new = new_range.start() as u32; - let mut lines = Vec::with_capacity(hunk.lines().len()); + let mut builder = HunkBuilder::new( + old_range.start() as u32, + old_range.len() as u32, + new_range.start() as u32, + new_range.len() as u32, + ); for line in hunk.lines() { let (kind, text) = match line { @@ -211,41 +206,12 @@ impl PatchParser { Line::Insert(text) => (DiffLineKind::Addition, *text), }; - let (old_no, new_no) = match kind { - DiffLineKind::Context => { - let numbers = (Some(old), Some(new)); - old += 1; - new += 1; - numbers - } - DiffLineKind::Addition => { - let number = Some(new); - new += 1; - (None, number) - } - DiffLineKind::Deletion => { - let number = Some(old); - old += 1; - (number, None) - } - }; - - let text = text - .strip_suffix('\n') - .unwrap_or(text) - .strip_suffix('\r') - .unwrap_or(text); - - lines.push(DiffLine::new(kind, old_no, new_no, text.to_owned())); + let text = text.strip_suffix('\n').unwrap_or(text); + let text = text.strip_suffix('\r').unwrap_or(text); + builder.push(kind, text.to_owned()); } - DiffHunk::new( - old_range.start() as u32, - old_range.len() as u32, - new_range.start() as u32, - new_range.len() as u32, - lines, - ) + builder.finish() } } diff --git a/crates/signed_git/src/repo.rs b/crates/signed_git/src/repo.rs index c3f2e1e..64282ad 100644 --- a/crates/signed_git/src/repo.rs +++ b/crates/signed_git/src/repo.rs @@ -140,7 +140,9 @@ impl Repo { for url in clone_urls { match Self::clone_from(url.as_ref(), path) { Ok(repo) => { - repo.fetch().ok(); + if let Err(error) = repo.fetch() { + log::warn!("failed to fetch NIP-34 refs after cloning: {error:#}"); + } return Ok(repo); } Err(error) => last_error = Some(error), @@ -871,9 +873,9 @@ impl Repo { readme_path, readme, self.current_branch(), - self.head_commit().unwrap_or(None), - self.branches().unwrap_or_default(), - self.tags().unwrap_or_default(), + self.head_commit()?, + self.branches()?, + self.tags()?, )) } diff --git a/crates/signed_git/src/scan.rs b/crates/signed_git/src/scan.rs index 97281fd..4b77127 100644 --- a/crates/signed_git/src/scan.rs +++ b/crates/signed_git/src/scan.rs @@ -1,4 +1,5 @@ use std::path::{Path, PathBuf}; +use std::sync::{Arc, Mutex}; use ignore::WalkBuilder; @@ -20,39 +21,58 @@ impl LocalRepo { } } -/// Finds the outermost git repositories under `root`, up to `SCAN_MAX_DEPTH` deep. +/// Finds git repositories under `root` up to `SCAN_MAX_DEPTH` deep, without descending into them. pub fn find_git_repos(root: &Path) -> Vec { let Ok(root) = root.canonicalize() else { return Vec::new(); }; + let found = Arc::new(Mutex::new(Vec::::new())); + let walker = WalkBuilder::new(&root) .max_depth(Some(SCAN_MAX_DEPTH)) .require_git(false) + .filter_entry({ + let found = found.clone(); + move |entry| { + let Some(kind) = entry.file_type() else { + return true; + }; + if !kind.is_dir() { + return true; + } + + let dir = entry.path(); + let Ok(repo) = gix::discover(dir) else { + return true; + }; + + let Some(workdir) = repo.workdir() else { + return false; + }; + + if workdir != dir { + return true; + } + + match found.lock() { + Ok(mut repos) => repos.push(dir.to_path_buf()), + Err(poisoned) => poisoned.into_inner().push(dir.to_path_buf()), + } + false + } + }) .build(); - let mut repos: Vec = walker - .flatten() - .filter(|entry| entry.file_type().is_some_and(|kind| kind.is_dir())) - .map(ignore::DirEntry::into_path) - .filter_map(|dir| { - let repo = gix::discover(&dir).ok()?; - let workdir = repo.workdir()?.to_path_buf(); - workdir.starts_with(&root).then_some(workdir) - }) - .collect(); + for _ in walker {} + let mut repos = match found.lock() { + Ok(mut repos) => std::mem::take(&mut *repos), + Err(poisoned) => std::mem::take(&mut *poisoned.into_inner()), + }; repos.sort(); - repos.dedup(); - let mut roots: Vec = Vec::with_capacity(repos.len()); - for repo in repos { - if !roots.iter().any(|kept| repo.starts_with(kept)) { - roots.push(repo); - } - } - - roots + repos .into_iter() .map(|path| { let nip34 = Repo::open(&path).ok().and_then(|repo| repo.nip34_binding()); diff --git a/crates/signed_git/src/tests.rs b/crates/signed_git/src/tests.rs index f0c1b3d..62a31cf 100644 --- a/crates/signed_git/src/tests.rs +++ b/crates/signed_git/src/tests.rs @@ -450,6 +450,34 @@ fn find_git_repos_reports_the_outermost_repository() { assert!(repos[0].nip34.is_none()); } +#[test] +fn find_git_repos_skips_repositories_nested_in_repositories() { + let root = tempfile::tempdir().expect("tempdir"); + let outer = root.path().join("outer"); + std::fs::create_dir_all(&outer).expect("mkdir"); + git_run(&outer, &["init", "-q"]); + + let nested = outer.join("nested"); + std::fs::create_dir_all(&nested).expect("mkdir"); + git_run(&nested, &["init", "-q"]); + + let repos = find_git_repos(root.path()); + + assert_eq!(repos.len(), 1); + assert_eq!(repos[0].path, outer.canonicalize().expect("canonical")); +} + +#[test] +fn find_git_repos_ignores_a_repository_above_the_scan_root() { + let outer = tempfile::tempdir().expect("tempdir"); + git_run(outer.path(), &["init", "-q"]); + + let root = outer.path().join("scan"); + std::fs::create_dir_all(&root).expect("mkdir"); + + assert!(find_git_repos(&root).is_empty()); +} + /// Creates an empty bare server repository for push tests. fn bare_server(base: &Path, owner: &str, name: &str) -> PathBuf { let repo = base.join(owner).join(format!("{name}.git"));