From 351487b9f5db57b0f3fdc8569cd0f369060683df Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Tue, 9 Jan 2024 17:14:30 +0900 Subject: [PATCH] backend: pass Index and keep_newer timestamp parameters to gc() GitBackend::gc() will need to check if a commit is reachable from any historical operations. This could be calculated from the view and commit objects, but the Index will do a better job. --- cli/examples/custom-backend/main.rs | 6 ++++-- cli/src/commands/util.rs | 2 +- lib/src/backend.rs | 9 +++++++-- lib/src/git_backend.rs | 5 ++++- lib/src/local_backend.rs | 4 +++- lib/src/store.rs | 6 ++++-- lib/testutils/src/test_backend.rs | 4 +++- 7 files changed, 26 insertions(+), 10 deletions(-) diff --git a/cli/examples/custom-backend/main.rs b/cli/examples/custom-backend/main.rs index d5c3271e5..08145d69b 100644 --- a/cli/examples/custom-backend/main.rs +++ b/cli/examples/custom-backend/main.rs @@ -15,6 +15,7 @@ use std::any::Any; use std::io::Read; use std::path::Path; +use std::time::SystemTime; use async_trait::async_trait; use jj_cli::cli_util::{CliRunner, CommandError, CommandHelper}; @@ -24,6 +25,7 @@ use jj_lib::backend::{ Conflict, ConflictId, FileId, SigningFn, SymlinkId, Tree, TreeId, }; use jj_lib::git_backend::GitBackend; +use jj_lib::index::Index; use jj_lib::repo::StoreFactories; use jj_lib::repo_path::RepoPath; use jj_lib::settings::UserSettings; @@ -171,7 +173,7 @@ impl Backend for JitBackend { self.inner.write_commit(contents, sign_with) } - fn gc(&self) -> BackendResult<()> { - self.inner.gc() + fn gc(&self, index: &dyn Index, keep_newer: SystemTime) -> BackendResult<()> { + self.inner.gc(index, keep_newer) } } diff --git a/cli/src/commands/util.rs b/cli/src/commands/util.rs index eab86471f..4c0d9c7cf 100644 --- a/cli/src/commands/util.rs +++ b/cli/src/commands/util.rs @@ -136,7 +136,7 @@ fn cmd_util_gc( let repo = workspace_command.repo(); repo.op_store() .gc(slice::from_ref(repo.op_id()), keep_newer)?; - repo.store().gc()?; + repo.store().gc(repo.index(), keep_newer)?; Ok(()) } diff --git a/lib/src/backend.rs b/lib/src/backend.rs index fdb3360f5..ca5c0c1f1 100644 --- a/lib/src/backend.rs +++ b/lib/src/backend.rs @@ -19,12 +19,14 @@ use std::collections::BTreeMap; use std::fmt::Debug; use std::io::Read; use std::result::Result; +use std::time::SystemTime; use std::vec::Vec; use async_trait::async_trait; use thiserror::Error; use crate::content_hash::ContentHash; +use crate::index::Index; use crate::merge::Merge; use crate::object_id::{id_type, ObjectId}; use crate::repo_path::{RepoPath, RepoPathComponent, RepoPathComponentBuf}; @@ -450,6 +452,9 @@ pub trait Backend: Send + Sync + Debug { ) -> BackendResult<(CommitId, Commit)>; /// Perform garbage collection. - // TODO: pass in the set of commits to keep here - fn gc(&self) -> BackendResult<()>; + /// + /// All commits found in the `index` won't be removed. In addition to that, + /// objects created after `keep_newer` will be preserved. This mitigates a + /// risk of deleting new commits created concurrently by another process. + fn gc(&self, index: &dyn Index, keep_newer: SystemTime) -> BackendResult<()>; } diff --git a/lib/src/git_backend.rs b/lib/src/git_backend.rs index 12d4bc5ae..d5abb3d65 100644 --- a/lib/src/git_backend.rs +++ b/lib/src/git_backend.rs @@ -21,6 +21,7 @@ use std::io::{Cursor, Read}; use std::path::{Path, PathBuf}; use std::process::{Command, ExitStatus}; use std::sync::{Arc, Mutex, MutexGuard}; +use std::time::SystemTime; use std::{fs, io, str}; use async_trait::async_trait; @@ -37,6 +38,7 @@ use crate::backend::{ TreeValue, }; use crate::file_util::{IoResultExt as _, PathError}; +use crate::index::Index; use crate::lock::FileLock; use crate::merge::{Merge, MergeBuilder}; use crate::object_id::ObjectId; @@ -1081,7 +1083,8 @@ impl Backend for GitBackend { Ok((id, contents)) } - fn gc(&self) -> BackendResult<()> { + fn gc(&self, _index: &dyn Index, _keep_newer: SystemTime) -> BackendResult<()> { + // TODO: pass in keep_newer to "git gc" command run_git_gc(self.git_repo_path()).map_err(|err| BackendError::Other(err.into())) } } diff --git a/lib/src/local_backend.rs b/lib/src/local_backend.rs index db736ba21..52555d45f 100644 --- a/lib/src/local_backend.rs +++ b/lib/src/local_backend.rs @@ -20,6 +20,7 @@ use std::fs; use std::fs::File; use std::io::{Read, Write}; use std::path::{Path, PathBuf}; +use std::time::SystemTime; use async_trait::async_trait; use blake2::{Blake2b512, Digest}; @@ -33,6 +34,7 @@ use crate::backend::{ }; use crate::content_hash::blake2b_hash; use crate::file_util::persist_content_addressed_temp_file; +use crate::index::Index; use crate::merge::MergeBuilder; use crate::object_id::ObjectId; use crate::repo_path::{RepoPath, RepoPathComponentBuf}; @@ -299,7 +301,7 @@ impl Backend for LocalBackend { Ok((id, commit)) } - fn gc(&self) -> BackendResult<()> { + fn gc(&self, _index: &dyn Index, _keep_newer: SystemTime) -> BackendResult<()> { Ok(()) } } diff --git a/lib/src/store.rs b/lib/src/store.rs index 04b2858a1..2b55e0abc 100644 --- a/lib/src/store.rs +++ b/lib/src/store.rs @@ -19,6 +19,7 @@ use std::collections::HashMap; use std::fmt::{Debug, Formatter}; use std::io::Read; use std::sync::{Arc, RwLock}; +use std::time::SystemTime; use pollster::FutureExt; @@ -27,6 +28,7 @@ use crate::backend::{ SymlinkId, TreeId, }; use crate::commit::Commit; +use crate::index::Index; use crate::merge::{Merge, MergedTreeValue}; use crate::merged_tree::MergedTree; use crate::repo_path::{RepoPath, RepoPathBuf}; @@ -266,7 +268,7 @@ impl Store { TreeBuilder::new(self.clone(), base_tree_id) } - pub fn gc(&self) -> BackendResult<()> { - self.backend.gc() + pub fn gc(&self, index: &dyn Index, keep_newer: SystemTime) -> BackendResult<()> { + self.backend.gc(index, keep_newer) } } diff --git a/lib/testutils/src/test_backend.rs b/lib/testutils/src/test_backend.rs index 29f8dc30d..91ece531d 100644 --- a/lib/testutils/src/test_backend.rs +++ b/lib/testutils/src/test_backend.rs @@ -18,12 +18,14 @@ use std::fmt::{Debug, Error, Formatter}; use std::io::{Cursor, Read}; use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex, MutexGuard, OnceLock}; +use std::time::SystemTime; use async_trait::async_trait; use jj_lib::backend::{ make_root_commit, Backend, BackendError, BackendResult, ChangeId, Commit, CommitId, Conflict, ConflictId, FileId, SecureSig, SigningFn, SymlinkId, Tree, TreeId, }; +use jj_lib::index::Index; use jj_lib::object_id::ObjectId; use jj_lib::repo_path::{RepoPath, RepoPathBuf}; @@ -298,7 +300,7 @@ impl Backend for TestBackend { Ok((id, contents)) } - fn gc(&self) -> BackendResult<()> { + fn gc(&self, _index: &dyn Index, _keep_newer: SystemTime) -> BackendResult<()> { Ok(()) } }