diff --git a/lib/src/git_backend.rs b/lib/src/git_backend.rs index fb21dd6497..ad5380f2ea 100644 --- a/lib/src/git_backend.rs +++ b/lib/src/git_backend.rs @@ -1223,6 +1223,12 @@ impl Backend for GitBackend { // TODO: remove unreachable extras table segments // TODO: pass in keep_newer to "git gc" command run_git_gc(self.git_repo_path()).map_err(|err| BackendError::Other(err.into())) + // TODO: Since "git gc" will move loose refs into packed refs, in-memory + // packed-refs cache should be invalidated here. If mtime accuracy is + // high or "git gc" is slow enough, gix::RefStore can notice the change, + // but it's not always the case. Upgrade gix to 0.63.0 and call + // force_refresh_packed_buffer(). + // https://github.com/Byron/gitoxide/issues/1348 } } diff --git a/lib/tests/test_git_backend.rs b/lib/tests/test_git_backend.rs index 1d072d8d4d..c9f0a7d0a7 100644 --- a/lib/tests/test_git_backend.rs +++ b/lib/tests/test_git_backend.rs @@ -13,9 +13,10 @@ // limitations under the License. use std::collections::HashSet; +use std::path::Path; use std::process::Command; use std::sync::Arc; -use std::time::SystemTime; +use std::time::{Duration, SystemTime}; use jj_lib::backend::CommitId; use jj_lib::git_backend::GitBackend; @@ -30,11 +31,11 @@ fn get_git_backend(repo: &Arc) -> &GitBackend { .unwrap() } -fn get_git_repo(repo: &Arc) -> gix::Repository { - get_git_backend(repo).git_repo() -} - -fn collect_no_gc_refs(git_repo: &gix::Repository) -> HashSet { +fn collect_no_gc_refs(git_repo_path: &Path) -> HashSet { + // Load fresh git repo to isolate from false caching issue. Here we want to + // ensure that the underlying data is correct. We could test the in-memory + // data as well, but we don't have any special handling in our code. + let git_repo = gix::open(git_repo_path).unwrap(); let git_refs = git_repo.references().unwrap(); let no_gc_refs_iter = git_refs.prefixed("refs/jj/keep/").unwrap(); no_gc_refs_iter @@ -53,7 +54,7 @@ fn test_gc() { let settings = testutils::user_settings(); let test_repo = TestRepo::init_with_backend(TestRepoBackend::Git); let repo = test_repo.repo; - let git_repo = get_git_repo(&repo); + let git_repo_path = get_git_backend(&repo).git_repo_path(); let base_index = repo.readonly_index(); // Set up commits: @@ -94,7 +95,7 @@ fn test_gc() { // At first, all commits have no-gc refs assert_eq!( - collect_no_gc_refs(&git_repo), + collect_no_gc_refs(git_repo_path), hashset! { commit_a.id().clone(), commit_b.id().clone(), @@ -113,7 +114,7 @@ fn test_gc() { .gc(base_index.as_index(), SystemTime::UNIX_EPOCH) .unwrap(); assert_eq!( - collect_no_gc_refs(&git_repo), + collect_no_gc_refs(git_repo_path), hashset! { commit_a.id().clone(), commit_b.id().clone(), @@ -126,11 +127,14 @@ fn test_gc() { }, ); + // Don't rely on the exact system time because file modification time might + // have lower precision for example. + let now = || SystemTime::now() + Duration::from_secs(1); + // All reachable: redundant no-gc refs will be removed - let now = SystemTime::now(); - repo.store().gc(repo.index(), now).unwrap(); + repo.store().gc(repo.index(), now()).unwrap(); assert_eq!( - collect_no_gc_refs(&git_repo), + collect_no_gc_refs(git_repo_path), hashset! { commit_d.id().clone(), commit_g.id().clone(), @@ -147,9 +151,9 @@ fn test_gc() { mut_index.add_commit(&commit_e); mut_index.add_commit(&commit_f); mut_index.add_commit(&commit_h); - repo.store().gc(mut_index.as_index(), now).unwrap(); + repo.store().gc(mut_index.as_index(), now()).unwrap(); assert_eq!( - collect_no_gc_refs(&git_repo), + collect_no_gc_refs(git_repo_path), hashset! { commit_d.id().clone(), commit_e.id().clone(), @@ -163,9 +167,9 @@ fn test_gc() { mut_index.add_commit(&commit_b); mut_index.add_commit(&commit_c); mut_index.add_commit(&commit_f); - repo.store().gc(mut_index.as_index(), now).unwrap(); + repo.store().gc(mut_index.as_index(), now()).unwrap(); assert_eq!( - collect_no_gc_refs(&git_repo), + collect_no_gc_refs(git_repo_path), hashset! { commit_c.id().clone(), commit_f.id().clone(), @@ -175,15 +179,15 @@ fn test_gc() { // B|C|F are no longer reachable let mut mut_index = base_index.start_modification(); mut_index.add_commit(&commit_a); - repo.store().gc(mut_index.as_index(), now).unwrap(); + repo.store().gc(mut_index.as_index(), now()).unwrap(); assert_eq!( - collect_no_gc_refs(&git_repo), + collect_no_gc_refs(git_repo_path), hashset! { commit_a.id().clone(), }, ); // All unreachable - repo.store().gc(base_index.as_index(), now).unwrap(); - assert_eq!(collect_no_gc_refs(&git_repo), hashset! {}); + repo.store().gc(base_index.as_index(), now()).unwrap(); + assert_eq!(collect_no_gc_refs(git_repo_path), hashset! {}); }