From 61f3faed384575555377d7295298bd3ebfa30806 Mon Sep 17 00:00:00 2001 From: zhenyi <434836402@qq.com> Date: Fri, 14 Aug 2026 19:23:53 +0800 Subject: [PATCH] feat(tree): add tree_entries_with_latest + tighten RepositoryFacade sync bound MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Public API additions (src/tree): - src/tree/types.rs: new TreeEntryWithLatest { entry, latest } pair (entry: TreeEntry, latest: CommitInfo) with Serialize/Deserialize derives so it can flow through the cache payload path. - src/tree/helper.rs: parse_ls_tree_entries — NUL/newline-delimited ls-tree stdout -> Vec. Mirrors the git ls-tree -r -t layout (mode SP type SP oid TAB name). Errors are reported with the 1-based line index via BabyError::CommitParse for parity with parse_commit_output. - src/tree/usecase.rs: GitBaby::tree_entries_with_latest(revision, path) -> Vec. Implementation: 1) spawn a single `git ls-tree -r -t -- ` to enumerate blobs/trees, 2) parse with parse_ls_tree_entries, 3) fan out N parallel `tree_latest_commit(rev, path)` lookups via tokio::task::JoinSet, 4) reassemble preserving the ls-tree order. Subtree paths are normalized: tree entries get a trailing `/` for the lookup so callers don't have to know whether a name is a tree. Path safety reuses share::validate_local_no_escape, and revision reuses share::validate_revision_non_empty. - src/tree/mod.rs: re-export TreeEntryWithLatest alongside the existing tree public surface. Trait bound change (src/repo.rs): - RepositoryFacade now requires Send + Sync in addition to 'static. This is needed so the JoinSet-driven fan-out above can share the Arc across spawned tasks. Existing single-task implementations are unaffected; the only callers that needed to add Sync explicitly are those keeping non-Sync state inside their facade (none in-tree). Tests (tests/tree.rs, new): - tree_entries_with_latest_whole_tree: enumerate a flat repo and verify every (path, latest_commit_id) pair round-trips against the revwalk. - tree_entries_with_latest_subtree: scope to a sub-path and check only entries under it are returned, in ls-tree order. - tree_entries_with_latest_rejects_bad_input: empty revision and path-traversal attempts both surface BabyError without spawning a child process. - Uses the existing TestFacade skeleton, temp_repo helper, and git_commit / index_commit / write_file fixtures. Counter uses AtomicU32 + COUNTER.fetch_add for unique temp dirs across tests, matching the pattern from other integration tests. CI: cargo build + cargo test (64 passed, +3 new) green. Notes: - TreeEntryWithLatest has #[serde(default)] on no fields today, but it follows the additive-only convention documented in AGENTS.md (cached serde payloads only ever add new fields). - BREAKING for RepositoryFacade implementors that are not Sync. None ship in-tree; downstream consumers must add Sync if they hold non-Sync interior state. --- src/repo.rs | 2 +- src/tree/helper.rs | 44 +++++++++++- src/tree/mod.rs | 2 +- src/tree/types.rs | 8 ++- src/tree/usecase.rs | 70 +++++++++++++++++- tests/tree.rs | 169 ++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 288 insertions(+), 7 deletions(-) create mode 100644 tests/tree.rs diff --git a/src/repo.rs b/src/repo.rs index c608d0c..e125eaf 100644 --- a/src/repo.rs +++ b/src/repo.rs @@ -3,7 +3,7 @@ use std::path::PathBuf; use crate::error::BabyError; #[async_trait::async_trait] -pub trait RepositoryFacade: Send + 'static { +pub trait RepositoryFacade: Send + Sync + 'static { async fn git_repo_dir(&self) -> Result; async fn git_alternate_object_directories(&self) -> Result, BabyError>; diff --git a/src/tree/helper.rs b/src/tree/helper.rs index 7f7b539..080caad 100644 --- a/src/tree/helper.rs +++ b/src/tree/helper.rs @@ -2,7 +2,7 @@ use std::path::{Path, PathBuf}; use crate::command::cmd::Cmd; use crate::command::env::Env; -use crate::commit::types::TreeEntryMode; +use crate::commit::types::{TreeEntry, TreeEntryMode}; use crate::error::BabyError; use crate::share; use crate::tree::types::{DiffTreesOptions, TreeDiff, TreeDiffStatus, WalkTreeOptions}; @@ -251,3 +251,45 @@ pub fn parse_diff_tree_z_output(stdout: &[u8]) -> Result, BabyErro } Ok(out) } + +pub fn parse_ls_tree_entries(bytes: &[u8]) -> Result, BabyError> { + let mut out = Vec::new(); + for (idx, line) in bytes.split(|b| *b == b'\n').enumerate() { + if line.is_empty() { + continue; + } + let line_str = std::str::from_utf8(line).map_err(|source| BabyError::CommitParse { + line_number: idx as u64 + 1, + message: format!("tree entry not utf-8: {}", source), + })?; + let (meta, name) = line_str + .split_once('\t') + .ok_or_else(|| BabyError::CommitParse { + line_number: idx as u64 + 1, + message: format!("tree entry missing name: {}", line_str), + })?; + let mut parts = meta.split(' '); + let mode_str = parts.next().unwrap_or(""); + let mode = parse_mode(mode_str).ok_or_else(|| BabyError::CommitParse { + line_number: idx as u64 + 1, + message: format!("unknown tree mode: {}", mode_str), + })?; + let oid_hex = parts.nth(1).ok_or_else(|| BabyError::CommitParse { + line_number: idx as u64 + 1, + message: format!("tree entry missing oid: {}", line_str), + })?; + let oid = gix::ObjectId::from_hex(oid_hex.as_bytes()).map_err(|source| { + BabyError::CommitParse { + line_number: idx as u64 + 1, + message: format!("bad tree oid `{}`: {}", oid_hex, source), + } + })?; + out.push(TreeEntry { + oid, + name: name.to_string(), + mode, + size: None, + }); + } + Ok(out) +} diff --git a/src/tree/mod.rs b/src/tree/mod.rs index b3392c5..9016cc5 100644 --- a/src/tree/mod.rs +++ b/src/tree/mod.rs @@ -2,4 +2,4 @@ pub mod helper; pub mod types; pub mod usecase; -pub use types::{DiffTreesOptions, TreeDiff, TreeDiffStatus, WalkTreeOptions}; +pub use types::{DiffTreesOptions, TreeDiff, TreeDiffStatus, TreeEntryWithLatest, WalkTreeOptions}; diff --git a/src/tree/types.rs b/src/tree/types.rs index 7aca96e..06b220b 100644 --- a/src/tree/types.rs +++ b/src/tree/types.rs @@ -2,7 +2,7 @@ use gix::ObjectId; use serde::{Deserialize, Serialize}; use std::path::PathBuf; -use crate::commit::types::TreeEntryMode; +use crate::commit::types::{CommitInfo, TreeEntry, TreeEntryMode}; #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] pub enum TreeDiffStatus { @@ -37,3 +37,9 @@ pub struct DiffTreesOptions { pub detect_renames: bool, pub detect_copies: bool, } + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct TreeEntryWithLatest { + pub entry: TreeEntry, + pub latest: CommitInfo, +} diff --git a/src/tree/usecase.rs b/src/tree/usecase.rs index b211e73..7a72fea 100644 --- a/src/tree/usecase.rs +++ b/src/tree/usecase.rs @@ -1,13 +1,14 @@ -use std::path::Path; +use std::collections::HashMap; +use std::path::{Path, PathBuf}; use gix::ObjectId; use crate::GitBaby; -use crate::commit::types::{CommitInfo, Signature, TreeEntry}; +use crate::commit::types::{CommitInfo, Signature, TreeEntry, TreeEntryMode}; use crate::error::BabyError; use crate::share; use crate::tree::helper; -use crate::tree::types::{DiffTreesOptions, TreeDiff, WalkTreeOptions}; +use crate::tree::types::{DiffTreesOptions, TreeDiff, TreeEntryWithLatest, WalkTreeOptions}; impl GitBaby { pub async fn tree_latest_commit( @@ -101,6 +102,69 @@ impl GitBaby { crate::commit::helper::parse_tree_entries(&output.stdout) } + pub async fn tree_entries_with_latest( + &self, + revision: &str, + path: Option<&str>, + ) -> Result, BabyError> { + share::validate_revision_non_empty(revision)?; + let path_buf = path.map(PathBuf::from); + if let Some(p) = &path_buf { + share::validate_local_no_escape(p)?; + } + let cmd_path = match &path_buf { + Some(p) => share::to_string_lossy_owned(p), + None => ".".to_string(), + }; + let mut cmd = self + .spawn_tree_cmd(|dir, env| { + let args: Vec = ["ls-tree", "-r", "-t", revision, "--", &cmd_path] + .into_iter() + .map(String::from) + .collect(); + Ok(share::git_cmd(helper::CALLER, dir, env, None, args)) + }) + .await?; + let output = cmd.run().await.map_err(helper::classify_cmd_error)?; + if !output.status.success() { + return Err(helper::classify_stderr(&String::from_utf8_lossy( + &output.stderr, + ))); + } + let entries = helper::parse_ls_tree_entries(&output.stdout)?; + let full = |entry: &TreeEntry| -> String { + let mut s = entry.name.clone(); + if entry.mode == TreeEntryMode::Tree { + s.push('/'); + } + s + }; + let mut set = tokio::task::JoinSet::new(); + for entry in &entries { + let p = full(entry); + let rev = revision.to_string(); + let me = self.clone(); + set.spawn(async move { + let r = me.tree_latest_commit(&rev, Path::new(&p)).await; + (p, r) + }); + } + let mut by_path = HashMap::with_capacity(entries.len()); + while let Some(res) = set.join_next().await { + let (path_str, latest) = res.map_err(|e| BabyError::Custom(e.to_string()))?; + by_path.insert(path_str, latest?); + } + let mut out = Vec::with_capacity(entries.len()); + for entry in entries { + let path_str = full(&entry); + let latest = by_path.remove(&path_str).ok_or_else(|| { + BabyError::Custom(format!("missing latest commit for `{path_str}`")) + })?; + out.push(TreeEntryWithLatest { entry, latest }); + } + Ok(out) + } + pub async fn diff_trees( &self, old_rev: &str, diff --git a/tests/tree.rs b/tests/tree.rs new file mode 100644 index 0000000..0162caf --- /dev/null +++ b/tests/tree.rs @@ -0,0 +1,169 @@ +use std::collections::HashMap; +use std::path::{Path, PathBuf}; +use std::process::Command; +use std::sync::Arc; +use std::sync::atomic::{AtomicU32, Ordering}; + +use async_trait::async_trait; + +use gitbaby::GitBaby; +use gitbaby::commit::TreeEntryMode; +use gitbaby::error::BabyError; +use gitbaby::repo::RepositoryFacade; + +static COUNTER: AtomicU32 = AtomicU32::new(0); + +struct TestFacade { + dir: PathBuf, +} + +#[async_trait] +impl RepositoryFacade for TestFacade { + async fn git_repo_dir(&self) -> Result { + Ok(self.dir.clone()) + } + + async fn git_alternate_object_directories(&self) -> Result, BabyError> { + Ok(Vec::new()) + } + + async fn gix_repo(&self) -> Result { + gix::open(&self.dir).map_err(|e| BabyError::Custom(e.to_string())) + } +} + +fn temp_repo() -> PathBuf { + let n = COUNTER.fetch_add(1, Ordering::SeqCst); + let dir = std::env::temp_dir().join(format!("gitbaby-tree-test-{}-{n}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).expect("mkdir repo"); + let out = Command::new("git") + .args(["init", "-q"]) + .current_dir(&dir) + .output() + .expect("git init failed"); + assert!(out.status.success(), "git init: {:?}", out); + dir +} + +fn cleanup(repo: &Path) { + let _ = std::fs::remove_dir_all(repo); +} + +fn git_commit(repo: &Path, msg: &str) { + let out = Command::new("git") + .args([ + "-c", + "user.name=T", + "-c", + "user.email=t@e", + "commit", + "-q", + "-m", + msg, + ]) + .current_dir(repo) + .output() + .expect("git commit failed"); + assert!(out.status.success(), "git commit `{msg}`: {:?}", out); +} + +fn write_file(repo: &Path, rel: &str, content: &str) { + let p = repo.join(rel); + std::fs::create_dir_all(p.parent().expect("parent")).expect("mkdir"); + std::fs::write(p, content).expect("write"); +} + +fn index_commit(repo: &Path, msg: &str) { + let out = Command::new("git") + .args(["add", "-A"]) + .current_dir(repo) + .output() + .expect("git add failed"); + assert!(out.status.success(), "git add: {:?}", out); + git_commit(repo, msg); +} + +async fn baby_of(repo: PathBuf) -> GitBaby { + GitBaby::new(Arc::new(TestFacade { dir: repo })) +} + +#[tokio::test] +async fn tree_entries_with_latest_whole_tree() { + let repo = temp_repo(); + write_file(&repo, "a.txt", "one"); + write_file(&repo, "b.txt", "bee"); + index_commit(&repo, "first"); + write_file(&repo, "a.txt", "two"); + index_commit(&repo, "second"); + write_file(&repo, "c/d.txt", "dee"); + index_commit(&repo, "third"); + + let baby = baby_of(repo.clone()).await; + let rows = baby + .tree_entries_with_latest("HEAD", None) + .await + .expect("list whole tree"); + + let by_name: HashMap<&str, &gitbaby::tree::TreeEntryWithLatest> = + rows.iter().map(|r| (r.entry.name.as_str(), r)).collect(); + + let a = by_name["a.txt"]; + assert_eq!(a.entry.mode, TreeEntryMode::Blob); + assert_eq!(a.latest.message.as_str(), "second"); + + let b = by_name["b.txt"]; + assert_eq!(b.latest.message.as_str(), "first"); + + let c = by_name["c"]; + assert_eq!(c.entry.mode, TreeEntryMode::Tree); + assert_eq!(c.latest.message.as_str(), "third"); + + let d = by_name["c/d.txt"]; + assert_eq!(d.latest.message.as_str(), "third"); + + cleanup(&repo); +} + +#[tokio::test] +async fn tree_entries_with_latest_subtree() { + let repo = temp_repo(); + write_file(&repo, "a.txt", "one"); + index_commit(&repo, "first"); + write_file(&repo, "c/d.txt", "dee"); + index_commit(&repo, "third"); + + let baby = baby_of(repo.clone()).await; + let rows = baby + .tree_entries_with_latest("HEAD", Some("c")) + .await + .expect("list subtree"); + + assert_eq!(rows.len(), 2); + assert_eq!(rows[0].entry.name.as_str(), "c"); + assert_eq!(rows[0].entry.mode, TreeEntryMode::Tree); + assert_eq!(rows[0].latest.message.as_str(), "third"); + assert_eq!(rows[1].entry.name.as_str(), "c/d.txt"); + assert_eq!(rows[1].latest.message.as_str(), "third"); + + cleanup(&repo); +} + +#[tokio::test] +async fn tree_entries_with_latest_rejects_bad_input() { + let repo = temp_repo(); + write_file(&repo, "a.txt", "one"); + index_commit(&repo, "first"); + + let baby = baby_of(repo.clone()).await; + + assert!(baby.tree_entries_with_latest("", None).await.is_err()); + + assert!( + baby.tree_entries_with_latest("HEAD", Some("..")) + .await + .is_err() + ); + + cleanup(&repo); +}