From 3fbf275749200208ee02d35de29ec71667058afd Mon Sep 17 00:00:00 2001 From: Friel Date: Mon, 22 Sep 2025 01:05:29 -0700 Subject: [PATCH] Add computed filter resolver support Add plumbing for computed filters Introduce a ComputedFilter hook on transactions so a `:computed` filter can be resolved per commit, cached, and have its metadata recorded for reverse lookups. Teach the history helpers to consult that metadata when finding original commits or unapplied filters, returning clear errors when the resolver or its records are missing. Extend the parser and pretty-printer to understand `:computed` (with optional arguments), and cover the resolver lifecycle, caching, and failure modes with new tests that rely on a tempfile-backed repository. --- Cargo.lock | 1 + josh-core/Cargo.toml | 3 + josh-core/src/cache.rs | 126 +++++++++++++++- josh-core/src/filter/mod.rs | 266 ++++++++++++++++++++++++++++++++++ josh-core/src/filter/parse.rs | 2 + josh-core/src/history.rs | 61 +++++++- 6 files changed, 456 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d8380555..cdaee3bb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1932,6 +1932,7 @@ dependencies = [ "serde_yaml", "sled", "strfmt", + "tempfile", "tracing", ] diff --git a/josh-core/Cargo.toml b/josh-core/Cargo.toml index c6d856a6..290ff542 100644 --- a/josh-core/Cargo.toml +++ b/josh-core/Cargo.toml @@ -33,3 +33,6 @@ serde_json = { workspace = true } serde_yaml = { workspace = true } sled = "0.34.7" tracing = { workspace = true } + +[dev-dependencies] +tempfile = "3.10.1" diff --git a/josh-core/src/cache.rs b/josh-core/src/cache.rs index 5995a965..1f279690 100644 --- a/josh-core/src/cache.rs +++ b/josh-core/src/cache.rs @@ -63,12 +63,40 @@ struct Transaction2 { missing: Vec<(filter::Filter, git2::Oid)>, misses: usize, walks: usize, + resolved_filters: HashMap<(git2::Oid, git2::Oid), filter::Filter>, } pub struct Transaction { t2: std::cell::RefCell, repo: git2::Repository, ref_prefix: String, + computed: Option>, +} + +pub trait ComputedFilter { + fn forward_filter_for_commit( + &self, + repo: &git2::Repository, + commit_oid: git2::Oid, + args: &[String], + ) -> JoshResult; + + fn record_filtered_commit( + &self, + original_commit_oid: git2::Oid, + filtered_commit_oid: git2::Oid, + filter: filter::Filter, + ) -> JoshResult<()>; + + fn lookup_commit_filter( + &self, + filtered_commit_oid: git2::Oid, + ) -> JoshResult>; + + fn lookup_filtered_base_commit( + &self, + filtered_commit_oid: git2::Oid, + ) -> JoshResult>; } impl Transaction { @@ -86,6 +114,16 @@ impl Transaction { )) } + pub fn open_with_computed( + path: &std::path::Path, + ref_prefix: Option<&str>, + resolver: Option>, + ) -> JoshResult { + let mut tx = Transaction::open(path, ref_prefix)?; + tx.computed = resolver; + Ok(tx) + } + pub fn open_from_env(load_cache: bool) -> JoshResult { let repo = git2::Repository::open_from_env()?; let path = repo.path().to_owned(); @@ -139,14 +177,18 @@ impl Transaction { missing: vec![], misses: 0, walks: 0, + resolved_filters: HashMap::new(), }), repo, ref_prefix: ref_prefix.unwrap_or("").to_string(), + computed: None, } } pub fn try_clone(&self) -> JoshResult { - Transaction::open(self.repo.path(), Some(&self.ref_prefix)) + let mut tx = Transaction::open(self.repo.path(), Some(&self.ref_prefix))?; + tx.computed = self.computed.clone(); + Ok(tx) } pub fn repo(&self) -> &git2::Repository { @@ -171,6 +213,88 @@ impl Transaction { self.t2.borrow_mut().walks -= 1; } + pub fn set_computed_resolver( + &mut self, + resolver: Option>, + ) { + self.computed = resolver; + } + + fn require_computed(&self) -> JoshResult<&(dyn ComputedFilter + Send + Sync)> { + self.computed.as_deref().ok_or_else(|| { + josh_error("computed filter resolver is not installed on this Transaction") + }) + } + + pub fn forward_filter_for_commit( + &self, + base_filter: filter::Filter, + commit: &git2::Commit<'_>, + ) -> JoshResult { + if let Some(args) = filter::computed_args(base_filter) { + let key = (base_filter.id(), commit.id()); + if let Some(resolved) = self.t2.borrow().resolved_filters.get(&key) { + return Ok(*resolved); + } + + let resolver = self.require_computed()?; + let resolved = resolver.forward_filter_for_commit(self.repo(), commit.id(), &args)?; + if filter::is_computed(resolved) { + return Err(josh_error( + "computed filter resolver returned another computed filter", + )); + } + self.t2.borrow_mut().resolved_filters.insert(key, resolved); + Ok(resolved) + } else { + Ok(base_filter) + } + } + + pub fn record_computed_commit( + &self, + original_commit_oid: git2::Oid, + filtered_commit_oid: git2::Oid, + resolved_filter: filter::Filter, + ) -> JoshResult<()> { + if filter::is_computed(resolved_filter) { + return Err(josh_error( + "resolved computed filter must not remain computed", + )); + } + + if let Some(resolver) = self.computed.as_ref() { + resolver.record_filtered_commit( + original_commit_oid, + filtered_commit_oid, + resolved_filter, + )?; + } + Ok(()) + } + + pub fn lookup_commit_filter( + &self, + filtered_commit_oid: git2::Oid, + ) -> JoshResult> { + if let Some(resolver) = self.computed.as_ref() { + resolver.lookup_commit_filter(filtered_commit_oid) + } else { + Ok(None) + } + } + + pub fn lookup_filtered_base_commit( + &self, + filtered_commit_oid: git2::Oid, + ) -> JoshResult> { + if let Some(resolver) = self.computed.as_ref() { + resolver.lookup_filtered_base_commit(filtered_commit_oid) + } else { + Ok(None) + } + } + pub fn insert_apply(&self, filter: filter::Filter, from: git2::Oid, to: git2::Oid) { let mut t2 = self.t2.borrow_mut(); t2.apply_map diff --git a/josh-core/src/filter/mod.rs b/josh-core/src/filter/mod.rs index a6423be1..7e7c0733 100644 --- a/josh-core/src/filter/mod.rs +++ b/josh-core/src/filter/mod.rs @@ -100,6 +100,18 @@ fn to_op(filter: Filter) -> Op { .clone() } +pub fn is_computed(filter: Filter) -> bool { + matches!(to_op(filter), Op::Computed(_)) +} + +pub fn computed_args(filter: Filter) -> Option> { + if let Op::Computed(args) = to_op(filter) { + Some(args) + } else { + None + } +} + #[derive(Hash, Clone, Debug, PartialEq, PartialOrd, Eq, Ord)] enum LazyRef { Resolved(git2::Oid), @@ -156,6 +168,8 @@ enum Op { Subdir(std::path::PathBuf), Workspace(std::path::PathBuf), + Computed(Vec), + Glob(String), Message(String), @@ -260,6 +274,7 @@ fn nesting2(op: &Op) -> usize { Op::Compose(filters) => 1 + filters.iter().map(|f| nesting(*f)).fold(0, |a, b| a.max(b)), Op::Exclude(filter) => 1 + nesting(*filter), Op::Workspace(_) => usize::MAX / 2, // divide by 2 to make sure there is enough headroom to avoid overflows + Op::Computed(_) => usize::MAX / 2, Op::Chain(a, b) => 1 + nesting(*a).max(nesting(*b)), Op::Subtract(a, b) => 1 + nesting(*a).max(nesting(*b)), Op::Rev(filters) => { @@ -306,6 +321,7 @@ fn lazy_refs2(op: &Op) -> Vec { av.append(&mut lazy_refs(*b)); av } + Op::Computed(_) => vec![], Op::Rev(filters) => lazy_refs2(&Op::Join(filters.clone())), Op::Join(filters) => { let mut lr = lazy_refs2(&Op::Compose(filters.values().copied().collect())); @@ -449,6 +465,14 @@ fn spec2(op: &Op) -> String { Op::Workspace(path) => { format!(":workspace={}", parse::quote_if(&path.to_string_lossy())) } + Op::Computed(args) => { + let encoded: Vec = args.iter().map(|a| parse::quote(a)).collect(); + if encoded.is_empty() { + ":computed".to_string() + } else { + format!(":computed={}", encoded.join(";")) + } + } Op::RegexReplace(replacements) => { let v = replacements .iter() @@ -898,6 +922,16 @@ fn apply_to_commit2( )) .transpose(); } + Op::Computed(_) => { + let resolved = transaction.forward_filter_for_commit(filter, commit)?; + if let Some(result) = apply_to_commit2(&to_op(resolved), commit, transaction)? { + transaction.insert(filter, commit.id(), result, true); + transaction.record_computed_commit(commit.id(), result, resolved)?; + return Ok(Some(result)); + } else { + return Ok(None); + } + } Op::Fold => { let filtered_parent_ids = commit .parents() @@ -1057,6 +1091,8 @@ fn apply2<'a>( Op::Invert => tree::invert_paths(transaction, "", tree), + Op::Computed(_) => Err(josh_error("computed filter requires commit context")), + Op::Workspace(path) => { let wsj_file = to_filter(Op::File(Path::new("workspace.josh").to_owned())); let base = to_filter(Op::Subdir(path.to_owned())); @@ -1320,7 +1356,237 @@ pub fn is_linear(filter: Filter) -> bool { #[cfg(test)] mod tests { use super::*; + use crate::cache::{self, ComputedFilter as ComputedFilterTrait}; + use crate::history; + use git2::Repository; + use std::collections::HashMap; + use std::path::Path; use std::path::PathBuf; + use std::sync::{Arc, Mutex}; + use tempfile::TempDir; + + #[derive(Default)] + struct TestResolver { + forward_calls: Mutex>, + records: Mutex>, + } + + impl ComputedFilterTrait for TestResolver { + fn forward_filter_for_commit( + &self, + _repo: &Repository, + commit_oid: git2::Oid, + _args: &[String], + ) -> JoshResult { + let mut calls = self.forward_calls.lock().unwrap(); + calls.push(commit_oid); + Ok(parse(":prefix=resolved")?) + } + + fn record_filtered_commit( + &self, + original_commit_oid: git2::Oid, + filtered_commit_oid: git2::Oid, + filter: Filter, + ) -> JoshResult<()> { + self.records + .lock() + .unwrap() + .insert(filtered_commit_oid, (original_commit_oid, filter)); + Ok(()) + } + + fn lookup_commit_filter( + &self, + filtered_commit_oid: git2::Oid, + ) -> JoshResult> { + Ok(self + .records + .lock() + .unwrap() + .get(&filtered_commit_oid) + .map(|(_, filter)| *filter)) + } + + fn lookup_filtered_base_commit( + &self, + new_filtered_commit_oid: git2::Oid, + ) -> JoshResult> { + Ok(self + .records + .lock() + .unwrap() + .get(&new_filtered_commit_oid) + .map(|(original, _)| *original)) + } + } + + fn init_repo() -> (TempDir, Repository, git2::Oid) { + let tmp = TempDir::new().unwrap(); + let repo = Repository::init(tmp.path()).unwrap(); + + std::fs::write(tmp.path().join("file.txt"), "hello").unwrap(); + let mut index = repo.index().unwrap(); + index.add_path(Path::new("file.txt")).unwrap(); + index.write().unwrap(); + let tree_id = index.write_tree().unwrap(); + let sig = git2::Signature::now("test", "test@example.com").unwrap(); + let commit_id = { + let tree = repo.find_tree(tree_id).unwrap(); + repo.commit(Some("HEAD"), &sig, &sig, "initial", &tree, &[]) + .unwrap() + }; + + (tmp, repo, commit_id) + } + + #[test] + fn computed_filter_records_metadata() { + let (_tmp, repo, commit_id) = init_repo(); + let commit = repo.find_commit(commit_id).unwrap(); + + let resolver = Arc::new(TestResolver::default()); + cache::load(repo.path()).unwrap(); + let tx = cache::Transaction::open_with_computed(repo.path(), None, Some(resolver.clone())) + .unwrap(); + + let filter = parse(":computed=\"demo\"").unwrap(); + let result_oid = apply_to_commit(filter, &commit, &tx).unwrap(); + assert_ne!(result_oid, git2::Oid::zero()); + + let calls = resolver.forward_calls.lock().unwrap().clone(); + assert_eq!(calls, vec![commit_id]); + + let resolved_filter = resolver + .records + .lock() + .unwrap() + .get(&result_oid) + .map(|(_, f)| *f) + .unwrap(); + assert_eq!(spec(resolved_filter), ":prefix=resolved"); + + let lookup_filter = tx.lookup_commit_filter(result_oid).unwrap().unwrap(); + assert_eq!(spec(lookup_filter), ":prefix=resolved"); + + let base = tx.lookup_filtered_base_commit(result_oid).unwrap().unwrap(); + assert_eq!(base, commit_id); + } + + #[test] + fn computed_filter_caches_resolved_filters() { + let (_tmp, repo, commit_id) = init_repo(); + let commit = repo.find_commit(commit_id).unwrap(); + + let resolver = Arc::new(TestResolver::default()); + cache::load(repo.path()).unwrap(); + let tx = cache::Transaction::open_with_computed(repo.path(), None, Some(resolver.clone())) + .unwrap(); + + let filter = parse(":computed=\"demo\"").unwrap(); + let result_first = apply_to_commit(filter, &commit, &tx).unwrap(); + let result_second = apply_to_commit(filter, &commit, &tx).unwrap(); + assert_eq!(result_first, result_second); + + let calls = resolver.forward_calls.lock().unwrap().clone(); + assert_eq!(calls, vec![commit_id]); + } + + #[test] + fn computed_filter_unapply_requires_metadata() { + let (_tmp, repo, commit_id) = init_repo(); + let commit = repo.find_commit(commit_id).unwrap(); + + let resolver = Arc::new(TestResolver::default()); + cache::load(repo.path()).unwrap(); + let tx = cache::Transaction::open_with_computed(repo.path(), None, Some(resolver.clone())) + .unwrap(); + + let filter = parse(":computed=\"demo\"").unwrap(); + let filtered_oid = apply_to_commit(filter, &commit, &tx).unwrap(); + assert_ne!(filtered_oid, git2::Oid::zero()); + + let mut change_ids = None; + let unapplied = history::unapply_filter( + &tx, + filter, + commit_id, + git2::Oid::zero(), + filtered_oid, + false, + None, + &mut change_ids, + ) + .unwrap(); + assert_eq!(unapplied, commit_id); + + resolver.records.lock().unwrap().remove(&filtered_oid); + + let tx2 = cache::Transaction::open_with_computed(repo.path(), None, Some(resolver.clone())) + .unwrap(); + let err = history::unapply_filter( + &tx2, + filter, + commit_id, + git2::Oid::zero(), + filtered_oid, + false, + None, + &mut None, + ) + .unwrap_err(); + assert!( + err.to_string() + .contains("missing computed filter metadata for filtered commit"), + "unexpected error: {err:?}" + ); + } + + #[test] + fn computed_filter_requires_resolver() { + let (_tmp, repo, commit_id) = init_repo(); + let commit = repo.find_commit(commit_id).unwrap(); + + cache::load(repo.path()).unwrap(); + let tx = cache::Transaction::open(repo.path(), None).unwrap(); + + let filter = parse(":computed=\"demo\"").unwrap(); + let err = apply_to_commit(filter, &commit, &tx).unwrap_err(); + assert!( + err.to_string() + .contains("computed filter resolver is not installed on this Transaction"), + "unexpected error: {err:?}" + ); + } + + #[test] + fn computed_filter_find_original_uses_metadata() { + let (_tmp, repo, commit_id) = init_repo(); + let commit = repo.find_commit(commit_id).unwrap(); + + let resolver = Arc::new(TestResolver::default()); + cache::load(repo.path()).unwrap(); + let tx = cache::Transaction::open_with_computed(repo.path(), None, Some(resolver.clone())) + .unwrap(); + + let filter = parse(":computed=\"demo\"").unwrap(); + let filtered_oid = apply_to_commit(filter, &commit, &tx).unwrap(); + assert_ne!(filtered_oid, git2::Oid::zero()); + + let original = history::find_original(&tx, filter, commit_id, filtered_oid, false).unwrap(); + assert_eq!(original, commit_id); + + resolver.records.lock().unwrap().remove(&filtered_oid); + + let tx2 = cache::Transaction::open_with_computed(repo.path(), None, Some(resolver.clone())) + .unwrap(); + let err = history::find_original(&tx2, filter, commit_id, filtered_oid, false).unwrap_err(); + assert!( + err.to_string() + .contains("missing computed filter metadata for filtered commit"), + "unexpected error: {err:?}" + ); + } #[test] fn src_path_test() { diff --git a/josh-core/src/filter/parse.rs b/josh-core/src/filter/parse.rs index cc9b62fe..56f967ae 100644 --- a/josh-core/src/filter/parse.rs +++ b/josh-core/src/filter/parse.rs @@ -40,6 +40,8 @@ fn make_op(args: &[&str]) -> JoshResult { ["INDEX"] => Ok(Op::Index), ["INVERT"] => Ok(Op::Invert), ["FOLD"] => Ok(Op::Fold), + ["computed"] => Ok(Op::Computed(vec![])), + ["computed", rest @ ..] => Ok(Op::Computed(rest.iter().map(|s| s.to_string()).collect())), _ => Err(josh_error( formatdoc!( r#" diff --git a/josh-core/src/history.rs b/josh-core/src/history.rs index 1e47d9d3..3d54512d 100644 --- a/josh-core/src/history.rs +++ b/josh-core/src/history.rs @@ -99,6 +99,19 @@ fn find_unapply_base( return Ok(*original); } + if filter::is_computed(filter) { + if let Some(original) = transaction.lookup_filtered_base_commit(filtered)? { + filtered_to_original.insert(filtered, original); + tracing::info!("Found via computed metadata {}, {}", filtered, original); + return Ok(original); + } else { + return Err(josh_error(&format!( + "missing computed filter metadata for filtered commit {}", + filtered + ))); + } + } + let contained_in_commit = transaction.repo().find_commit(contained_in)?; let oid = filter::apply_to_commit(filter, &contained_in_commit, transaction)?; if oid != git2::Oid::zero() { @@ -138,6 +151,16 @@ pub fn find_original( if filter == filter::nop() { return Ok(filtered); } + if filter::is_computed(filter) { + return transaction + .lookup_filtered_base_commit(filtered)? + .ok_or_else(|| { + josh_error(&format!( + "missing computed filter metadata for filtered commit {}", + filtered + )) + }); + } let mut walk = transaction.repo().revwalk()?; walk.set_sorting(git2::Sort::TOPOLOGICAL)?; if linear { @@ -290,6 +313,18 @@ fn find_new_branch_base( contained_in: git2::Oid, filtered: git2::Oid, ) -> JoshResult { + if filter::is_computed(filter) { + if let Some(base) = transaction.lookup_filtered_base_commit(filtered)? { + filtered_to_original.insert(filtered, base); + return Ok(filtered); + } else { + return Err(josh_error(&format!( + "missing computed filter metadata for filtered commit {}", + filtered + ))); + } + } + let walk = { let mut walk = transaction.repo().revwalk()?; walk.set_sorting(git2::Sort::TOPOLOGICAL)?; @@ -408,6 +443,25 @@ pub fn unapply_filter( tracing::info!("walk commit: {:?}", rev); let module_commit = transaction.repo().find_commit(rev)?; + let commit_filter = if filter::is_computed(filter) { + let resolved = transaction + .lookup_commit_filter(module_commit.id())? + .ok_or_else(|| { + josh_error(&format!( + "missing computed filter metadata for filtered commit {}", + module_commit.id() + )) + })?; + if filter::is_computed(resolved) { + return Err(josh_error( + "computed resolver returned nested computed filter during unapply", + )); + } + resolved + } else { + filter + }; + if filtered_to_original.contains_key(&module_commit.id()) { continue; } @@ -430,7 +484,7 @@ pub fn unapply_filter( find_unapply_base( transaction, &mut filtered_to_original, - filter, + commit_filter, original_target, *filtered_parent_id, ) @@ -478,7 +532,10 @@ pub fn unapply_filter( original_parents .iter() .map(|commit| -> JoshResult<_> { - Ok(filter::unapply(transaction, filter, tree.clone(), commit.tree()?)?.id()) + Ok( + filter::unapply(transaction, commit_filter, tree.clone(), commit.tree()?)? + .id(), + ) }) .collect() };