backend: replace git_repo() by as_any()

This has several advantages:

 * Makes it possible to downcast to non-Git custom backends (might be
   useful at Google, but we haven't needed it yet)

 * Lets us access more specific functionality on the `GitBackend`,
   making it possible to access the `git2::Repository` without
   creating a copy of it.

 * Removes the dependency on Git from the backend
This commit is contained in:
Martin von Zweigbergk 2023-05-12 06:05:32 -07:00 committed by Martin von Zweigbergk
parent 8d56b199bc
commit e7419e76a1
11 changed files with 124 additions and 63 deletions

View file

@ -12,10 +12,10 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use std::any::Any;
use std::io::Read;
use std::path::Path;
use git2::Repository;
use jujutsu::cli_util::{CliRunner, CommandError, CommandHelper};
use jujutsu::ui::Ui;
use jujutsu_lib::backend::{
@ -89,6 +89,10 @@ impl JitBackend {
}
impl Backend for JitBackend {
fn as_any(&self) -> &dyn Any {
self
}
fn name(&self) -> &str {
"jit"
}
@ -101,10 +105,6 @@ impl Backend for JitBackend {
self.inner.change_id_length()
}
fn git_repo(&self) -> Option<Repository> {
self.inner.git_repo()
}
fn read_file(&self, path: &RepoPath, id: &FileId) -> BackendResult<Box<dyn Read>> {
self.inner.read_file(path, id)
}

View file

@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use std::any::Any;
use std::collections::BTreeMap;
use std::fmt::{Debug, Error, Formatter};
use std::io::Read;
@ -369,6 +370,8 @@ pub fn make_root_commit(root_change_id: ChangeId, empty_tree_id: TreeId) -> Comm
}
pub trait Backend: Send + Sync + Debug {
fn as_any(&self) -> &dyn Any;
/// A unique name that identifies this backend. Written to
/// `.jj/repo/store/backend` when the repo is created.
fn name(&self) -> &str;
@ -379,8 +382,6 @@ pub trait Backend: Send + Sync + Debug {
/// The length of change IDs in bytes.
fn change_id_length(&self) -> usize;
fn git_repo(&self) -> Option<git2::Repository>;
fn read_file(&self, path: &RepoPath, id: &FileId) -> BackendResult<Box<dyn Read>>;
fn write_file(&self, path: &RepoPath, contents: &mut dyn Read) -> BackendResult<FileId>;

View file

@ -12,11 +12,12 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use std::any::Any;
use std::fmt::{Debug, Error, Formatter};
use std::fs::File;
use std::io::{Cursor, Read, Write};
use std::path::Path;
use std::sync::{Arc, Mutex};
use std::sync::{Arc, Mutex, MutexGuard};
use git2::Oid;
use itertools::Itertools;
@ -92,6 +93,15 @@ impl GitBackend {
let extra_metadata_store = TableStore::load(store_path.join("extra"), HASH_LENGTH);
GitBackend::new(repo, extra_metadata_store)
}
pub fn git_repo(&self) -> MutexGuard<'_, git2::Repository> {
self.repo.lock().unwrap()
}
pub fn git_repo_clone(&self) -> git2::Repository {
let path = self.repo.lock().unwrap().path().to_owned();
git2::Repository::open(path).unwrap()
}
}
fn signature_from_git(signature: git2::Signature) -> Signature {
@ -187,6 +197,10 @@ impl Debug for GitBackend {
}
impl Backend for GitBackend {
fn as_any(&self) -> &dyn Any {
self
}
fn name(&self) -> &str {
"git"
}
@ -199,11 +213,6 @@ impl Backend for GitBackend {
CHANGE_ID_LENGTH
}
fn git_repo(&self) -> Option<git2::Repository> {
let path = self.repo.lock().unwrap().path().to_owned();
Some(git2::Repository::open(path).unwrap())
}
fn read_file(&self, _path: &RepoPath, id: &FileId) -> BackendResult<Box<dyn Read>> {
let git_blob_id = validate_git_object_id(id)?;
let locked_repo = self.repo.lock().unwrap();
@ -824,7 +833,6 @@ mod tests {
let commit_id = store.write_commit(&commit).unwrap();
let git_refs = store
.git_repo()
.unwrap()
.references_glob("refs/jj/keep/*")
.unwrap()
.map(|git_ref| git_ref.unwrap().target().unwrap())

View file

@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use std::any::Any;
use std::fmt::Debug;
use std::fs;
use std::fs::File;
@ -125,6 +126,10 @@ impl LocalBackend {
}
impl Backend for LocalBackend {
fn as_any(&self) -> &dyn Any {
self
}
fn name(&self) -> &str {
"local"
}
@ -137,10 +142,6 @@ impl Backend for LocalBackend {
CHANGE_ID_LENGTH
}
fn git_repo(&self) -> Option<git2::Repository> {
None
}
fn read_file(&self, _path: &RepoPath, id: &FileId) -> BackendResult<Box<dyn Read>> {
let path = self.file_path(id);
let file = File::open(path).map_err(|err| map_not_found_err(err, id))?;

View file

@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use std::any::Any;
use std::collections::HashMap;
use std::io::Read;
use std::sync::{Arc, RwLock};
@ -43,6 +44,10 @@ impl Store {
})
}
pub fn backend_impl(&self) -> &dyn Any {
self.backend.as_any()
}
pub fn commit_id_length(&self) -> usize {
self.backend.commit_id_length()
}
@ -51,10 +56,6 @@ impl Store {
self.backend.change_id_length()
}
pub fn git_repo(&self) -> Option<git2::Repository> {
self.backend.git_repo()
}
pub fn empty_tree_id(&self) -> &TreeId {
self.backend.empty_tree_id()
}

View file

@ -58,13 +58,21 @@ fn git_id(commit: &Commit) -> Oid {
Oid::from_bytes(commit.id().as_bytes()).unwrap()
}
fn get_git_repo(repo: &Arc<ReadonlyRepo>) -> git2::Repository {
repo.store()
.backend_impl()
.downcast_ref::<GitBackend>()
.unwrap()
.git_repo_clone()
}
#[test]
fn test_import_refs() {
let settings = testutils::user_settings();
let git_settings = GitSettings::default();
let test_repo = TestRepo::init(true);
let repo = &test_repo.repo;
let git_repo = repo.store().git_repo().unwrap();
let git_repo = get_git_repo(repo);
let commit1 = empty_git_commit(&git_repo, "refs/heads/main", &[]);
git_ref(&git_repo, "refs/remotes/origin/main", commit1.id());
@ -79,7 +87,7 @@ fn test_import_refs() {
git_repo.set_head("refs/heads/main").unwrap();
let git_repo = repo.store().git_repo().unwrap();
let git_repo = get_git_repo(repo);
let mut tx = repo.start_transaction(&settings, "test");
git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap();
tx.mut_repo().rebase_descendants(&settings).unwrap();
@ -170,7 +178,7 @@ fn test_import_refs_reimport() {
let git_settings = GitSettings::default();
let test_workspace = TestRepo::init(true);
let repo = &test_workspace.repo;
let git_repo = repo.store().git_repo().unwrap();
let git_repo = get_git_repo(repo);
let commit1 = empty_git_commit(&git_repo, "refs/heads/main", &[]);
git_ref(&git_repo, "refs/remotes/origin/main", commit1.id());
@ -266,7 +274,7 @@ fn test_import_refs_reimport_head_removed() {
let git_settings = GitSettings::default();
let test_repo = TestRepo::init(true);
let repo = &test_repo.repo;
let git_repo = repo.store().git_repo().unwrap();
let git_repo = get_git_repo(repo);
let commit = empty_git_commit(&git_repo, "refs/heads/main", &[]);
let mut tx = repo.start_transaction(&settings, "test");
@ -291,7 +299,7 @@ fn test_import_refs_reimport_git_head_counts() {
let git_settings = GitSettings::default();
let test_repo = TestRepo::init(true);
let repo = &test_repo.repo;
let git_repo = repo.store().git_repo().unwrap();
let git_repo = get_git_repo(repo);
let commit = empty_git_commit(&git_repo, "refs/heads/main", &[]);
git_repo.set_head_detached(commit.id()).unwrap();
@ -319,7 +327,7 @@ fn test_import_refs_reimport_git_head_without_ref() {
let git_settings = GitSettings::default();
let test_repo = TestRepo::init(true);
let repo = &test_repo.repo;
let git_repo = repo.store().git_repo().unwrap();
let git_repo = get_git_repo(repo);
// First, HEAD points to commit1.
let mut tx = repo.start_transaction(&settings, "test");
@ -353,7 +361,7 @@ fn test_import_refs_reimport_git_head_with_moved_ref() {
let git_settings = GitSettings::default();
let test_repo = TestRepo::init(true);
let repo = &test_repo.repo;
let git_repo = repo.store().git_repo().unwrap();
let git_repo = get_git_repo(repo);
// First, both HEAD and main point to commit1.
let mut tx = repo.start_transaction(&settings, "test");
@ -390,7 +398,7 @@ fn test_import_refs_reimport_git_head_with_fixed_ref() {
let git_settings = GitSettings::default();
let test_repo = TestRepo::init(true);
let repo = &test_repo.repo;
let git_repo = repo.store().git_repo().unwrap();
let git_repo = get_git_repo(repo);
// First, both HEAD and main point to commit1.
let mut tx = repo.start_transaction(&settings, "test");
@ -425,7 +433,7 @@ fn test_import_refs_reimport_all_from_root_removed() {
let git_settings = GitSettings::default();
let test_repo = TestRepo::init(true);
let repo = &test_repo.repo;
let git_repo = repo.store().git_repo().unwrap();
let git_repo = get_git_repo(repo);
let commit = empty_git_commit(&git_repo, "refs/heads/main", &[]);
let mut tx = repo.start_transaction(&settings, "test");
@ -451,7 +459,7 @@ fn test_import_some_refs() {
let git_settings = GitSettings::default();
let test_workspace = TestRepo::init(true);
let repo = &test_workspace.repo;
let git_repo = repo.store().git_repo().unwrap();
let git_repo = get_git_repo(repo);
let commit_main = empty_git_commit(&git_repo, "refs/remotes/origin/main", &[]);
let commit_feat1 = empty_git_commit(&git_repo, "refs/remotes/origin/feature1", &[&commit_main]);
@ -1475,7 +1483,7 @@ fn test_push_updates_success() {
let settings = testutils::user_settings();
let temp_dir = testutils::new_temp_dir();
let setup = set_up_push_repos(&settings, &temp_dir);
let clone_repo = setup.jj_repo.store().git_repo().unwrap();
let clone_repo = get_git_repo(&setup.jj_repo);
let result = git::push_updates(
&clone_repo,
"origin",
@ -1512,14 +1520,14 @@ fn test_push_updates_deletion() {
let settings = testutils::user_settings();
let temp_dir = testutils::new_temp_dir();
let setup = set_up_push_repos(&settings, &temp_dir);
let clone_repo = setup.jj_repo.store().git_repo().unwrap();
let clone_repo = get_git_repo(&setup.jj_repo);
let source_repo = git2::Repository::open(&setup.source_repo_dir).unwrap();
// Test the setup
assert!(source_repo.find_reference("refs/heads/main").is_ok());
let result = git::push_updates(
&setup.jj_repo.store().git_repo().unwrap(),
&get_git_repo(&setup.jj_repo),
"origin",
&[GitRefUpdate {
qualified_name: "refs/heads/main".to_string(),
@ -1546,7 +1554,7 @@ fn test_push_updates_mixed_deletion_and_addition() {
let settings = testutils::user_settings();
let temp_dir = testutils::new_temp_dir();
let setup = set_up_push_repos(&settings, &temp_dir);
let clone_repo = setup.jj_repo.store().git_repo().unwrap();
let clone_repo = get_git_repo(&setup.jj_repo);
let result = git::push_updates(
&clone_repo,
"origin",
@ -1587,7 +1595,7 @@ fn test_push_updates_not_fast_forward() {
let new_commit = write_random_commit(tx.mut_repo(), &settings);
setup.jj_repo = tx.commit();
let result = git::push_updates(
&setup.jj_repo.store().git_repo().unwrap(),
&get_git_repo(&setup.jj_repo),
"origin",
&[GitRefUpdate {
qualified_name: "refs/heads/main".to_string(),
@ -1608,7 +1616,7 @@ fn test_push_updates_not_fast_forward_with_force() {
let new_commit = write_random_commit(tx.mut_repo(), &settings);
setup.jj_repo = tx.commit();
let result = git::push_updates(
&setup.jj_repo.store().git_repo().unwrap(),
&get_git_repo(&setup.jj_repo),
"origin",
&[GitRefUpdate {
qualified_name: "refs/heads/main".to_string(),
@ -1634,7 +1642,7 @@ fn test_push_updates_no_such_remote() {
let temp_dir = testutils::new_temp_dir();
let setup = set_up_push_repos(&settings, &temp_dir);
let result = git::push_updates(
&setup.jj_repo.store().git_repo().unwrap(),
&get_git_repo(&setup.jj_repo),
"invalid-remote",
&[GitRefUpdate {
qualified_name: "refs/heads/main".to_string(),
@ -1652,7 +1660,7 @@ fn test_push_updates_invalid_remote() {
let temp_dir = testutils::new_temp_dir();
let setup = set_up_push_repos(&settings, &temp_dir);
let result = git::push_updates(
&setup.jj_repo.store().git_repo().unwrap(),
&get_git_repo(&setup.jj_repo),
"http://invalid-remote",
&[GitRefUpdate {
qualified_name: "refs/heads/main".to_string(),

View file

@ -14,6 +14,7 @@
use std::path::{Path, PathBuf};
use jujutsu_lib::git_backend::GitBackend;
use jujutsu_lib::op_store::WorkspaceId;
use jujutsu_lib::repo::Repo;
use jujutsu_lib::settings::UserSettings;
@ -33,7 +34,11 @@ fn test_init_local() {
let temp_dir = testutils::new_temp_dir();
let (canonical, uncanonical) = canonicalize(temp_dir.path());
let (workspace, repo) = Workspace::init_local(&settings, &uncanonical).unwrap();
assert!(repo.store().git_repo().is_none());
assert!(repo
.store()
.backend_impl()
.downcast_ref::<GitBackend>()
.is_none());
assert_eq!(repo.repo_path(), &canonical.join(".jj").join("repo"));
assert_eq!(workspace.workspace_root(), &canonical);
@ -48,7 +53,11 @@ fn test_init_internal_git() {
let temp_dir = testutils::new_temp_dir();
let (canonical, uncanonical) = canonicalize(temp_dir.path());
let (workspace, repo) = Workspace::init_internal_git(&settings, &uncanonical).unwrap();
assert!(repo.store().git_repo().is_some());
assert!(repo
.store()
.backend_impl()
.downcast_ref::<GitBackend>()
.is_some());
assert_eq!(repo.repo_path(), &canonical.join(".jj").join("repo"));
assert_eq!(workspace.workspace_root(), &canonical);
@ -67,7 +76,12 @@ fn test_init_external_git() {
std::fs::create_dir(uncanonical.join("jj")).unwrap();
let (workspace, repo) =
Workspace::init_external_git(&settings, &uncanonical.join("jj"), &git_repo_path).unwrap();
assert!(repo.store().git_repo().is_some());
assert!(repo
.store()
.backend_impl()
.downcast_ref::<GitBackend>()
.is_some());
assert_eq!(
repo.repo_path(),
&canonical.join("jj").join(".jj").join("repo")

View file

@ -19,6 +19,7 @@ use itertools::Itertools;
use jujutsu_lib::backend::{ChangeId, CommitId, MillisSinceEpoch, ObjectId, Signature, Timestamp};
use jujutsu_lib::commit::Commit;
use jujutsu_lib::git;
use jujutsu_lib::git_backend::GitBackend;
use jujutsu_lib::index::{HexPrefix, PrefixResolution};
use jujutsu_lib::op_store::{RefTarget, WorkspaceId};
use jujutsu_lib::repo::Repo;
@ -203,7 +204,12 @@ fn test_resolve_symbol_change_id(readonly: bool) {
let test_repo = TestRepo::init(true);
let repo = &test_repo.repo;
let git_repo = repo.store().git_repo().unwrap();
let git_repo = repo
.store()
.backend_impl()
.downcast_ref::<GitBackend>()
.unwrap()
.git_repo_clone();
// Add some commits that will end up having change ids with common prefixes
let empty_tree_id = git_repo.treebuilder(None).unwrap().write().unwrap();
let git_author = git2::Signature::new(

View file

@ -32,6 +32,7 @@ use itertools::Itertools;
use jujutsu_lib::backend::{BackendError, ChangeId, CommitId, ObjectId, TreeId};
use jujutsu_lib::commit::Commit;
use jujutsu_lib::git::{GitExportError, GitImportError};
use jujutsu_lib::git_backend::GitBackend;
use jujutsu_lib::gitignore::GitIgnoreFile;
use jujutsu_lib::hex_util::to_reverse_hex;
use jujutsu_lib::id_prefix::IdPrefixContext;
@ -558,6 +559,10 @@ impl ReadonlyUserRepo {
id_prefix_context: OnceCell::new(),
}
}
pub fn git_backend(&self) -> Option<&GitBackend> {
self.repo.store().backend_impl().downcast_ref()
}
}
// Provides utilities for writing a command that works on a workspace (like most
@ -601,10 +606,9 @@ impl WorkspaceCommandHelper {
let loaded_at_head = &global_args.at_operation == "@";
let may_update_working_copy = loaded_at_head && !global_args.ignore_working_copy;
let mut working_copy_shared_with_git = false;
let maybe_git_repo = repo.store().git_repo();
if let Some(git_workdir) = maybe_git_repo
.as_ref()
.and_then(|git_repo| git_repo.workdir())
let maybe_git_backend = repo.store().backend_impl().downcast_ref::<GitBackend>();
if let Some(git_workdir) = maybe_git_backend
.and_then(|git_backend| git_backend.git_repo().workdir().map(ToOwned::to_owned))
.and_then(|workdir| workdir.canonicalize().ok())
{
working_copy_shared_with_git = git_workdir == workspace.workspace_root().as_path();
@ -623,6 +627,10 @@ impl WorkspaceCommandHelper {
})
}
pub fn git_backend(&self) -> Option<&GitBackend> {
self.user_repo.git_backend()
}
pub fn check_working_copy_writable(&self) -> Result<(), CommandError> {
if self.may_update_working_copy {
Ok(())
@ -644,8 +652,8 @@ impl WorkspaceCommandHelper {
pub fn snapshot(&mut self, ui: &mut Ui) -> Result<(), CommandError> {
if self.may_update_working_copy {
if self.working_copy_shared_with_git {
let maybe_git_repo = self.repo().store().git_repo();
self.import_git_refs_and_head(ui, maybe_git_repo.as_ref().unwrap())?;
let git_repo = self.git_backend().unwrap().git_repo_clone();
self.import_git_refs_and_head(ui, &git_repo)?;
}
self.snapshot_working_copy(ui)?;
}
@ -704,7 +712,12 @@ impl WorkspaceCommandHelper {
}
fn export_head_to_git(&self, mut_repo: &mut MutableRepo) -> Result<(), CommandError> {
let git_repo = mut_repo.store().git_repo().unwrap();
let git_repo = mut_repo
.store()
.backend_impl()
.downcast_ref::<GitBackend>()
.unwrap()
.git_repo_clone();
let current_git_head_ref = git_repo.find_reference("HEAD").unwrap();
let current_git_commit_id = current_git_head_ref
.peel_to_commit()
@ -807,8 +820,8 @@ impl WorkspaceCommandHelper {
}
pub fn git_config(&self) -> Result<git2::Config, git2::Error> {
if let Some(git_repo) = self.repo().store().git_repo() {
git_repo.config()
if let Some(git_backend) = self.git_backend() {
git_backend.git_repo().config()
} else {
git2::Config::open_default()
}
@ -836,9 +849,11 @@ impl WorkspaceCommandHelper {
{
git_ignores = git_ignores.chain_with_file("", excludes_file_path);
}
if let Some(git_repo) = self.repo().store().git_repo() {
git_ignores =
git_ignores.chain_with_file("", git_repo.path().join("info").join("exclude"));
if let Some(git_backend) = self.git_backend() {
git_ignores = git_ignores.chain_with_file(
"",
git_backend.git_repo().path().join("info").join("exclude"),
);
}
git_ignores
}
@ -1081,8 +1096,8 @@ impl WorkspaceCommandHelper {
}
if self.working_copy_shared_with_git {
let git_repo = self.user_repo.repo.store().git_repo().unwrap();
let failed_branches = git::export_refs(mut_repo, &git_repo)?;
let failed_branches =
git::export_refs(mut_repo, &self.user_repo.git_backend().unwrap().git_repo())?;
print_failed_git_export(ui, &failed_branches)?;
}
@ -1147,8 +1162,8 @@ impl WorkspaceCommandHelper {
}
if self.working_copy_shared_with_git {
self.export_head_to_git(mut_repo)?;
let git_repo = self.repo().store().git_repo().unwrap();
let failed_branches = git::export_refs(mut_repo, &git_repo)?;
let failed_branches =
git::export_refs(mut_repo, &self.git_backend().unwrap().git_repo())?;
print_failed_git_export(ui, &failed_branches)?;
}
let maybe_old_commit = tx

View file

@ -10,6 +10,7 @@ use clap::{ArgGroup, Subcommand};
use itertools::Itertools;
use jujutsu_lib::backend::ObjectId;
use jujutsu_lib::git::{self, GitFetchError, GitPushError, GitRefUpdate};
use jujutsu_lib::git_backend::GitBackend;
use jujutsu_lib::op_store::{BranchTarget, RefTarget};
use jujutsu_lib::refs::{classify_branch_push_action, BranchPushAction, BranchPushUpdate};
use jujutsu_lib::repo::Repo;
@ -149,9 +150,9 @@ pub struct GitImportArgs {}
pub struct GitExportArgs {}
fn get_git_repo(store: &Store) -> Result<git2::Repository, CommandError> {
match store.git_repo() {
match store.backend_impl().downcast_ref::<GitBackend>() {
None => Err(user_error("The repo is not backed by a git repo")),
Some(git_repo) => Ok(git_repo),
Some(git_backend) => Ok(git_backend.git_repo_clone()),
}
}

View file

@ -34,6 +34,7 @@ use jujutsu_lib::backend::{CommitId, ObjectId, TreeValue};
use jujutsu_lib::commit::Commit;
use jujutsu_lib::dag_walk::topo_order_reverse;
use jujutsu_lib::default_index_store::{DefaultIndexStore, ReadonlyIndexWrapper};
use jujutsu_lib::git_backend::GitBackend;
use jujutsu_lib::matchers::EverythingMatcher;
use jujutsu_lib::op_store::{RefTarget, WorkspaceId};
use jujutsu_lib::repo::{ReadonlyRepo, Repo};
@ -1057,7 +1058,12 @@ fn cmd_init(ui: &mut Ui, command: &CommandHelper, args: &InitArgs) -> Result<(),
}
let (workspace, repo) =
Workspace::init_external_git(command.settings(), &wc_path, &git_store_path)?;
let git_repo = repo.store().git_repo().unwrap();
let git_repo = repo
.store()
.backend_impl()
.downcast_ref::<GitBackend>()
.unwrap()
.git_repo_clone();
let mut workspace_command = command.for_loaded_repo(ui, workspace, repo)?;
workspace_command.snapshot(ui)?;
if workspace_command.working_copy_shared_with_git() {